-
Notifications
You must be signed in to change notification settings - Fork 433
[WIP]TCP transport support ipv6 #1067
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: LCAIZJ <[email protected]>
Summary of ChangesHello @LCAIZJ, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a host resolution error encountered with TCP transport by introducing comprehensive IPv6 support. The changes modify the TCP transport layer to correctly handle both IPv4 and IPv6 addresses for listening and connecting, enhancing network compatibility and resolving issues related to host not found errors in modern network environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds IPv6 support to the TCP transport layer. The changes correctly modify the server-side acceptor to create a dual-stack (IPv4/IPv6) socket and update the client-side resolver to handle both IPv4 and IPv6 addresses. My review includes a suggestion to improve error handling and robustness in the acceptor setup logic.
| TcpContext(short port) : acceptor(io_context) { | ||
| asio::ip::tcp::endpoint endpoint(asio::ip::tcp::v6(), port); | ||
|
|
||
| acceptor.open(endpoint.protocol()); | ||
| acceptor.set_option(asio::ip::v6_only(false)); | ||
| acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true)); | ||
| acceptor.bind(endpoint); | ||
| acceptor.listen(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The operations on the asio::ip::tcp::acceptor (open, set_option, bind, listen) can throw asio::system_error on failure (e.g., if the port is already in use). Since this is in a constructor, an unhandled exception will prevent the object from being fully constructed and will likely crash the application. It's better to wrap these operations in a try-catch block to log the specific error before re-throwing it, which improves debuggability. Additionally, it's good practice to specify a backlog size for listen.
TcpContext(short port) : acceptor(io_context) {
try {
asio::ip::tcp::endpoint endpoint(asio::ip::tcp::v6(), port);
acceptor.open(endpoint.protocol());
acceptor.set_option(asio::ip::v6_only(false));
acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true));
acceptor.bind(endpoint);
acceptor.listen(asio::socket_base::max_listen_connections);
} catch (const asio::system_error& e) {
LOG(ERROR) << "Failed to setup TCP acceptor: " << e.what();
throw;
}
}
@LCAIZJ Could you resolve this compiling issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds IPv6 support to the TCP transport layer to resolve connection issues when using IPv6 addresses. The changes enable the TCP transport to accept both IPv4 and IPv6 connections using dual-stack sockets.
- Modified the
TcpContextconstructor to use IPv6 endpoints with dual-stack support (v6_only=false) - Updated the resolver to automatically determine the protocol (IPv4 or IPv6) based on the provided hostname/IP address
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| asio::ip::tcp::endpoint endpoint(asio::ip::tcp::v6(), port); | ||
|
|
||
| acceptor.open(endpoint.protocol()); | ||
| acceptor.set_option(asio::ip::v6_only(false)); | ||
| acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true)); | ||
| acceptor.bind(endpoint); | ||
| acceptor.listen(); |
Copilot
AI
Nov 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Setting v6_only(false) enables dual-stack mode, allowing the IPv6 socket to accept both IPv4 and IPv6 connections. However, on some systems (particularly older Linux kernels or Windows systems), this option might not be supported or may have different default behaviors. Consider documenting this requirement or adding a fallback mechanism to create an IPv4-only acceptor if IPv6 with dual-stack fails.
| asio::ip::tcp::endpoint endpoint(asio::ip::tcp::v6(), port); | |
| acceptor.open(endpoint.protocol()); | |
| acceptor.set_option(asio::ip::v6_only(false)); | |
| acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true)); | |
| acceptor.bind(endpoint); | |
| acceptor.listen(); | |
| asio::ip::tcp::endpoint endpoint_v6(asio::ip::tcp::v6(), port); | |
| asio::ip::tcp::endpoint endpoint_v4(asio::ip::tcp::v4(), port); | |
| bool dual_stack_ok = true; | |
| try { | |
| acceptor.open(endpoint_v6.protocol()); | |
| acceptor.set_option(asio::ip::v6_only(false)); | |
| acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true)); | |
| acceptor.bind(endpoint_v6); | |
| acceptor.listen(); | |
| } catch (const std::exception& e) { | |
| LOG(WARNING) << "Dual-stack (IPv6+IPv4) socket setup failed: " << e.what() | |
| << ". Falling back to IPv4-only acceptor."; | |
| dual_stack_ok = false; | |
| } | |
| if (!dual_stack_ok) { | |
| acceptor.close(); | |
| acceptor.open(endpoint_v4.protocol()); | |
| acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true)); | |
| acceptor.bind(endpoint_v4); | |
| acceptor.listen(); | |
| } |
Description
vllm-project/vllm-ascend#4103 use tcp transport with error
E20251112 17:02:55.861567 33158 tcp_transport.cpp:480] TcpTransport::startTransfer encountered an ASIO exception. Slice details - source_addr: 0x12d11ac40000, length: 32768, opcode: 0, target_id: 1. Exception: resolve: Host not found (authoritative)