Skip to content

Conversation

@Evgueni-Petrov-aka-espetrov
Copy link

@Evgueni-Petrov-aka-espetrov Evgueni-Petrov-aka-espetrov commented Jan 22, 2026

Use ipv6 address familty, if address family for host:port resolves to ipv6 only
This PR fixes #10877 introduced by #9646

Summary by CodeRabbit

  • New Features
    • Server now supports both IPv4 and IPv6 addresses for host configuration, with automatic detection based on the provided host.

✏️ Tip: You can customize this high-level summary in your review settings.

feel free to improve :-)

Signed-off-by: Evgueni Petrov <evgueni.s.petrov@gmail.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

The socket address family selection in the server initialization was changed from hard-coded IPv4 (AF_INET) to dynamic selection: AF_INET6 if the host contains a colon (IPv6 format), otherwise AF_INET.

Changes

Cohort / File(s) Summary
Socket address family selection
tensorrt_llm/commands/serve.py
Modified socket creation to dynamically select address family based on host format instead of hard-coded IPv4 selection

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description is minimal and lacks required sections from the repository template, including Test Coverage and PR Checklist confirmation. Expand the description to include Test Coverage section detailing test cases that validate IPv6 support, and complete the PR Checklist confirming adherence to coding guidelines and test coverage requirements.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: restoring IPv6 support by dynamically selecting the address family based on the host string.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tensorrt_llm/commands/serve.py (1)

1-1: Add NVIDIA copyright header for latest modification year.

This file should include the standard NVIDIA header with the year of the latest meaningful modification. As per coding guidelines, please add the required header at the top of the file.

🧹 Nitpick comments (1)
tensorrt_llm/commands/serve.py (1)

189-190: Consider using socket.getaddrinfo() for more robust address family resolution.

The colon-based heuristic won't handle IPv6-only DNS hostnames (e.g., ipv6-host.example.com that resolve only to IPv6 addresses), which would be incorrectly classified as IPv4 and cause bind failures. Consider using socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket.SOCK_STREAM) to properly resolve the address family, or implement a fallback retry mechanism that attempts AF_INET6 if the initial bind with AF_INET fails.

@svc-trtllm-gh-bot svc-trtllm-gh-bot added the Community want to contribute PRs initiated from Community label Jan 22, 2026
check for ipv6 with socket.getaddrinfo

Signed-off-by: Evgueni Petrov <evgueni.s.petrov@gmail.com>
@Evgueni-Petrov-aka-espetrov Evgueni-Petrov-aka-espetrov changed the title [#10877][fix] idea of fix for ipv6 support [#10877][fix] restore ipv6 support in serve.py Jan 22, 2026
@yuxianq yuxianq requested a review from JunyiXu-nv January 23, 2026 01:57
Copy link
Collaborator

@JunyiXu-nv JunyiXu-nv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! LGTM

@JunyiXu-nv
Copy link
Collaborator

/bot run

@tensorrt-cicd
Copy link
Collaborator

PR_Github #33269 [ run ] triggered by Bot. Commit: fc09904

@tensorrt-cicd
Copy link
Collaborator

PR_Github #33269 [ run ] completed with state DISABLED
CI server is currently disabled for scheduled maintenance. Estimated completion time: 8 PM PST on 1/22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community want to contribute PRs initiated from Community

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: tensorrt_llm/commands/serve.py seems to hard-code ipv4 support

4 participants