[gty111]

Overview

Avoid NIXL EC producer control-port collisions when multiple instances or DP replicas share a host, while retaining explicit endpoint configuration.

Claims

• Allocate an OS-assigned port by default and advertise the actual bound port through ec_transfer_params.peer_port.
• Keep VLLM_EC_SIDE_CHANNEL_HOST and VLLM_EC_SIDE_CHANNEL_PORT overrides. A nonzero port uses base_port + data_parallel_index; 0 remains automatic.
• Preserve ec_transfer_config.engine_id in the NIXL agent name and append the DP index to distinguish replicas.

Validation

Transport and configuration validation

.venv/bin/python -m pytest tests/v1/ec_connector/unit/test_{control,data,session,scheduler_nixl_ctor,scheduler_nixl_producer,scheduler_nixl_consumer}.py -q

112 passed, including real ZMQ request/reply and NIXL peer registration. The updated tests reproduce ignored endpoint overrides and engine IDs before the fix. GPU serving was not rerun for this revision.

Details

Bind port 0 directly on the serving socket and read LAST_ENDPOINT, avoiding a separate port-reservation race. The host defaults to get_ip() (honoring VLLM_HOST_IP); set VLLM_EC_SIDE_CHANNEL_HOST=localhost for loopback-only binding. Wildcard addresses are rejected because peers need a reachable address.

Fixed-port conflicts fail startup rather than silently choosing another port. Independent instances using fixed ports must use non-overlapping ranges; consumers use the advertised endpoint without matching port configuration.

No equivalent open fix was found in the EC control-port search.

---

Pull Request Checklist

• I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).
• AI assistance was used during the creation of this PR.
• Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
• Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.
• Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
• Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

[mergify]

Documentation preview: https://vllm--61764.org.readthedocs.build/en/61764/

[mergify]

Documentation preview: https://vllm--61764.org.readthedocs.build/en/61764/

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[omerpaz95]

Thanks @gty111 , that's a real bug.
Some concerns about the implementation:

1. Ports can no longer be pinned. Port 0 is unconditional, so we can no longer write firewall rules or NetworkPolicies for the control plane.
   My suggestion: Keep env var VLLM_EC_SIDE_CHANNEL_PORT, add + data_parallel_index like the KV NIXL connector (nixl/base_scheduler.py) and the P2P offload tier (kv_offload/tiering/p2p/manager.py), and treat 0 as an opt-in "OS-assigned":

host = envs.VLLM_EC_SIDE_CHANNEL_HOST
base = envs.VLLM_EC_SIDE_CHANNEL_PORT
transport = ZmqServerTransport(host, base + dp_index if base else 0)
self._peer_port = transport.port # always announce the bound port

2. The default bind becomes network-reachable. Without the host env var, main binds localhost. This PR binds whatever get_ip() finds. If detection fails, get_ip() returns 0.0.0.0: the socket binds every interface, and consumers are told to dial 0.0.0.0.
   I suggest to keep VLLM_EC_SIDE_CHANNEL_HOST with its localhost default. If auto-detection is wanted, make it opt-in.

3. Removing env vars that already exist in released versions of vLLM breaks existing deployments: They may fail or crash. I think it's best (As a practice) to deprecate in one release and then remove. But if you follow the suggestions in 1, 2, this won't be necessary.

4. ec_transfer_config.engine_id is now ignored by this connector, even if the user sets it.

5. (From Claude code review): Tests don't cover the identity fix. test_producers_allocate_distinct_ports_across_dp_and_instances passes without the agent-name change, since OS-assigned ports always differ. The parametrized dp_index test asserts nothing about dp_index.
   → Assert distinct agent names across DP ranks, or register two producers with one consumer. Add tests that a non-zero base binds base + dp_index, and that 0 announces the bound port rather than 0.

[omerpaz95]

LGTM. Thanks @gty111

[mergify]

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @gty111.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

[gty111]

/ci run

[github-actions]

✅ Triggered Buildkite CI #99036 for commit 55e65c0a4041.

ℹ️ This PR is 1 commit behind upstream main. Merged upstream main into this branch as 55e65c0a4041 before starting CI. Pull the branch before pushing more changes.