Fix dual-stack socket handling: IPV6_V6ONLY, IPv4-first, is_port_available all-family check - #20643
Conversation
Summary of ChangesHello, 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 significantly enhances the network handling capabilities, particularly for IPv6, by refining socket binding logic and ensuring more reliable port allocation. The changes aim to improve the stability and predictability of network operations within the system, complemented by new tests to validate the behavior of key utility functions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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.
Code Review
This pull request introduces several improvements to IPv6 network handling and related socket utilities. The changes include refactoring address info deduplication for better readability, correctly setting the IPV6_V6ONLY socket option for IPv6 sockets, refining exception handling during port binding, and making get_open_port more reliable by disabling address reuse. Additionally, new tests are added for port reservation and URL normalization, which enhance test coverage. The changes are logical and well-implemented. I've found a minor issue in one of the new tests regarding an incorrect mock patch target and have provided a suggestion to fix it.
|
/tag-and-rerun-ci |
|
/rerun-ut test_socket_utils.py |
|
❌ No This file may not be a registered CUDA CI test. |
IPV6_V6ONLY, IPv4-first ordering, is_port_available correctness
IPV6_V6ONLY, IPv4-first ordering, is_port_available correctnessIPV6_V6ONLY, IPv4-first, is_port_available all-family check
|
/rerun-ut test/registered/disaggregation/test_disaggregation_basic.py |
|
❌ Ambiguous filename
Please provide the full path, e.g. |
|
✅ Triggered |
|
/rerun-ut test_disaggregation_dp_attention.py |
|
✅ Triggered |
…vailable` all-family check (sgl-project#20643)
…vailable` all-family check (sgl-project#20643)
…vailable` all-family check (sgl-project#20643)
Changes
IPV6_V6ONLY=1on IPv6 sockets intry_bind_socket— isolate IPv4/IPv6 to eliminate platform-dependent dual-stack behavioris_port_availableto check all address families withSO_REUSEADDR— previously only checked first-success family, missing conflicts on the other family underIPV6_V6ONLY_get_addrinfos_for_bindIPv4-first for consistent default across platformsget_open_portto useis_port_available(all-family) forSGLANG_PORTpath; use defaultreuse_addr=Truefor ephemeral path to match server bind behaviorNits
_get_addrinfos_for_binddedup from set-comprehension side-effect hack to explicit loopOverflowErrorcatch fromis_port_availabledown totry_bind_socketreserve_port,release_port,normalize_base_url, port-occupied detectionTest plan
/rerun-stage stage-a-cpu-only