Repository navigation
Fix ipv4 forwarding when both v4 and v6 ports are listening - #41672
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It changes localhost forwarding behavior and netlink parsing in core networking paths, which warrants final human review despite the added test coverage.
Review effort: Lite
Findings: None
What changed in this PR
This PR addresses a WSL2 NAT localhost relay gap where dual-stack (IPv6 socket with IPV6_V6ONLY=0) listeners were only resulting in IPv6 Windows forwarders, breaking IPv4 localhost forwarding. It also tightens netlink attribute typing/parsing so attribute access returns correctly typed pointers and uses correct payload-size checks.
Changes:
- Extend the sock_diag-based listener scan to detect dual-stack IPv6 wildcard listeners and synthesize a corresponding IPv4 wildcard socket entry when
INET_DIAG_SKV6ONLY == 0. - Fix netlink attribute parsing to use typed attributes (
in_addr/in6_addr) and validate attribute payload length withRTA_PAYLOAD(). - Update/expand NAT localhost relay tests to cover dual-stack and IPv6-only scenarios.
| File | Description |
|---|---|
| test/windows/NetworkTests.cpp | Adds dual-stack and IPv6-only NAT localhost relay coverage; refactors relay traffic validation to reuse a single guest listener. |
| src/linux/netlinkutil/RoutingTable.cpp | Switches route address attribute reads to typed netlink attributes keyed off rtm_family. |
| src/linux/netlinkutil/NetlinkMessage.hxx | Adds inet_diag_msg attribute traversal and corrects attribute size checks to use payload size. |
| src/linux/netlinkutil/Interface.cpp | Fixes address enumeration to use typed netlink attributes based on ifa_family. |
| src/linux/init/localhost.cpp | Detects dual-stack IPv6 wildcard listeners via INET_DIAG_SKV6ONLY and adds an IPv4 forwarder candidate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keith Horton (keith-horton)
left a comment
Keith Horton (keith-horton)
left a comment
There was a problem hiding this comment.
Thanks for fixing these!
|
Ben Hillis (@benhillis) , some of this fixes the WSL Relay interaction. The changes looks good to me. |
|
Ben Hillis (@benhillis) , Catalin is out for a while. Can he be removed from one of the required reviewers? |
He's not a required reviewer. The change can be merged now that it's approved by wsl-reviewers |
Thanks! Sorry - I saw him listed as a 'pending review' and thought policy might now require him. |
Summary of the Pull Request
The NAT localhost scanner ignores dual-stack listeners and only creates IPV6 forwarders in Windows.
This PR checks the
INET_DIAG_SKV6ONLYfield of the v6 port. And adds a v4 windows forwarder when it's dual stack.This PR also fixes the misuse of
NetlinkMessage.Attributeswhere theTMessagewas specified asconst void*instead of the actual message type. Causing wrong return types and wrong size checks.PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Add / update tests:
NetworkTests::NetworkTests::NatLocalhostRelayDualStack
NetworkTests::NetworkTests::NatLocalhostRelayIpv6Only
NetworkTests::NetworkTests::NatLocalhostRelay
NetworkTests::NetworkTests::NatLocalhostRelayNoIpv6