server : add --host-family to control HTTP bind address family#20138
server : add --host-family to control HTTP bind address family#20138fanyang89 wants to merge 1 commit intoggml-org:masterfrom
Conversation
Introduce a new server option, --host-family (ipv4 | ipv6 | auto), to avoid relying on OS-dependent default address-family behavior.
There was a problem hiding this comment.
Pull request overview
Adds a new --host-family (ipv4|ipv6|auto) server option to make TCP bind address-family selection explicit and consistent across OSes.
Changes:
- Introduce
common_host_family+--host-familyCLI/env parsing. - Plumb the chosen host family into the HTTP server binding logic (with a UNIX-socket ignore warning).
- Extend server test harness and add a unit test covering
--host-family.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/server/tests/utils.py | Adds server_host_family to test server process and forwards it to CLI args. |
| tools/server/tests/unit/test_basic.py | Adds a unit test that starts the server with --host-family. |
| tools/server/server-http.h | Stores host-family in the HTTP server context. |
| tools/server/server-http.cpp | Applies address-family selection (AF_INET/AF_INET6/AF_UNSPEC) before binding. |
| tools/server/README.md | Documents new --host-family option. |
| common/common.h | Adds common_host_family enum and new host_family param field. |
| common/arg.cpp | Parses --host-family and maps to internal enum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| server.server_host_family = "ipv4" | ||
| server.start() | ||
| res = server.make_request("GET", "/health") | ||
| assert res.status_code == 200 |
There was a problem hiding this comment.
This test mutates the global server instance but never resets server.server_host_family, which can make later tests order-dependent (they may unexpectedly keep running with --host-family ipv4). Reset the field back to None in a finally block (or use a fresh server instance/fixture per test) to avoid shared-state leakage.
| server.server_host_family = "ipv4" | |
| server.start() | |
| res = server.make_request("GET", "/health") | |
| assert res.status_code == 200 | |
| original_host_family = server.server_host_family | |
| try: | |
| server.server_host_family = "ipv4" | |
| server.start() | |
| res = server.make_request("GET", "/health") | |
| assert res.status_code == 200 | |
| finally: | |
| server.server_host_family = original_host_family |
|
|
||
| std::string path_prefix; | ||
| std::string hostname; | ||
| int host_family; | ||
| int port; |
There was a problem hiding this comment.
Storing host_family as a plain int loses type-safety now that common_host_family exists. Prefer using common_host_family host_family here (and keep conversions to AF_INET/AF_INET6/AF_UNSPEC localized to the binding logic) to prevent accidental invalid values and make intent clearer.
|
|
||
| std::string path_prefix; | ||
| std::string hostname; | ||
| int host_family; |
There was a problem hiding this comment.
A new data member was added to server_http_context; ensure it is explicitly initialized in server_http_context() (or via an in-class initializer) so it can’t be read uninitialized if start() is ever called before init() (or if future code paths change).
| int host_family; | |
| int host_family = 0; |
| server_args.extend(["--host-family", self.server_host_family]) | ||
| if self.offline: | ||
| server_args.append("--offline") | ||
| server_args.extend(["--offline"]) |
There was a problem hiding this comment.
server_args.extend([\"--offline\"]) is functionally correct but unusual for a single element and makes the intent less clear than server_args.append(\"--offline\"). Consider switching back to append for readability/consistency with common list-building patterns.
| server_args.extend(["--offline"]) | |
| server_args.append("--offline") |
| | `-a, --alias STRING` | set model name aliases, comma-separated (to be used by API)<br/>(env: LLAMA_ARG_ALIAS) | | ||
| | `--tags STRING` | set model tags, comma-separated (informational, not used for routing)<br/>(env: LLAMA_ARG_TAGS) | | ||
| | `--host HOST` | ip address to listen, or bind to an UNIX socket if the address ends with .sock (default: 127.0.0.1)<br/>(env: LLAMA_ARG_HOST) | | ||
| | `--host-family FAMILY` | address family for TCP listen sockets: ipv4, ipv6, or auto (default: auto)<br/>(env: LLAMA_ARG_HOST_FAMILY) | |
There was a problem hiding this comment.
The implementation warns that --host-family is ignored when binding to a UNIX socket (*.sock). Consider noting this behavior in the option description (even briefly) so users aren’t surprised when it has no effect for .sock hosts.
| | `--host-family FAMILY` | address family for TCP listen sockets: ipv4, ipv6, or auto (default: auto)<br/>(env: LLAMA_ARG_HOST_FAMILY) | | |
| | `--host-family FAMILY` | address family for TCP listen sockets: ipv4, ipv6, or auto (default: auto). Ignored when \`--host\` is a .sock UNIX socket.<br/>(env: LLAMA_ARG_HOST_FAMILY) | |
Introduce a new server option, --host-family (ipv4 | ipv6 | auto), to avoid relying on OS-dependent default address-family behavior.