route: support v4-mapped IPv6 nexthops on V6 routes - #1198
Open
fdomain wants to merge 1 commit into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesV4-mapped IPv6 gateway support
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
route_linux.go (1)
993-1001: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated v4-mapped conform logic between single-Gw and multipath nh.Gw paths.
The exact same conform condition/comment is repeated for
route.Gw(Lines 993-1001) andnh.Gw(Lines 1036-1044). Extracting a small helper would keep the two paths from silently diverging if this logic is tweaked later.♻️ Proposed refactor
+// conformV4MappedGatewayFamily conforms a v4-mapped IPv6 gateway +// (::ffff:a.b.c.d, 16 bytes) to FAMILY_V6 when the caller explicitly +// requested a V6 route via route.Family. This is opt-in rather than +// silently reinterpreting an ambiguous address; a 4-byte IPv4 gateway +// is left to error downstream. +func conformV4MappedGatewayFamily(routeFamily int, gw net.IP, gwFamily int) int { + if routeFamily == FAMILY_V6 && gwFamily == FAMILY_V4 && len(gw) == net.IPv6len { + return FAMILY_V6 + } + return gwFamily +} + if route.Gw != nil { gwFamily := nl.GetIPFamily(route.Gw) - if route.Family == FAMILY_V6 && gwFamily == FAMILY_V4 && len(route.Gw) == net.IPv6len { - gwFamily = FAMILY_V6 - } + gwFamily = conformV4MappedGatewayFamily(route.Family, route.Gw, gwFamily) ... if nh.Gw != nil { gwFamily := nl.GetIPFamily(nh.Gw) - if route.Family == FAMILY_V6 && gwFamily == FAMILY_V4 && len(nh.Gw) == net.IPv6len { - gwFamily = FAMILY_V6 - } + gwFamily = conformV4MappedGatewayFamily(route.Family, nh.Gw, gwFamily)Also applies to: 1036-1044
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@route_linux.go` around lines 993 - 1001, The v4-mapped IPv6 gateway conforming logic is duplicated in both the single-gateway and multipath nexthop paths, which risks the two branches drifting apart. Extract the shared FAMILY_V6/FAMILY_V4 + net.IPv6len check into a small helper and call it from both the route.Gw handling and the nh.Gw handling so the behavior stays identical in both places.route_test.go (1)
2842-2890: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the multipath (
nh.Gw) conform path.All four new tests cover only
route.Gw. The PR/AI summary states the v4-mapped conform is applied toroute.MultiPathnexthops as well (route_linux.go Lines 1036-1044), but that duplicated logic is currently untested — a regression there (e.g. someone updates one conform block but not the other) would go unnoticed.Consider adding a case using
Route{Family: FAMILY_V6, MultiPath: []*NexthopInfo{{Gw: gw, LinkIndex: ...}}}and asserting the nestedRTA_MULTIPATHattribute's childRTA_GATEWAYis 16 bytes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@route_test.go` around lines 2842 - 2890, The current tests only cover the direct Route.Gw path, so the multipath nexthop branch in prepareRouteReq remains untested. Add a test in TestPrepareRouteReqV4MappedV6Gateway (or a sibling test) that builds a Route with Family set to FAMILY_V6 and MultiPath containing a NexthopInfo with Gw set to the v4-mapped IP, then verify the nested RTA_MULTIPATH child RTA_GATEWAY is encoded as 16 bytes. Use the existing prepareRouteReq, Route, and NexthopInfo symbols to locate the multipath conform logic and ensure it matches the direct gateway behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@route_linux.go`:
- Around line 993-1001: The v4-mapped IPv6 gateway conforming logic is
duplicated in both the single-gateway and multipath nexthop paths, which risks
the two branches drifting apart. Extract the shared FAMILY_V6/FAMILY_V4 +
net.IPv6len check into a small helper and call it from both the route.Gw
handling and the nh.Gw handling so the behavior stays identical in both places.
In `@route_test.go`:
- Around line 2842-2890: The current tests only cover the direct Route.Gw path,
so the multipath nexthop branch in prepareRouteReq remains untested. Add a test
in TestPrepareRouteReqV4MappedV6Gateway (or a sibling test) that builds a Route
with Family set to FAMILY_V6 and MultiPath containing a NexthopInfo with Gw set
to the v4-mapped IP, then verify the nested RTA_MULTIPATH child RTA_GATEWAY is
encoded as 16 bytes. Use the existing prepareRouteReq, Route, and NexthopInfo
symbols to locate the multipath conform logic and ensure it matches the direct
gateway behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 418d0262-d92f-4524-87f4-e070abc8c347
📒 Files selected for processing (2)
route_linux.goroute_test.go
A v4-mapped IPv6 address (::ffff:a.b.c.d) is byte-identical to its IPv4
form as a net.IP, so nl.GetIPFamily reports FAMILY_V4 for it. When such
an address was used as a route's gateway on a V6 route it was encoded as
a 4-byte AF_INET nexthop, and combined with a V6 destination it failed
outright with "gateway... not the same IP family".
Resolve this locally in prepareRouteReq rather than changing
GetIPFamily (which has no route context and must keep reporting V4 for
the common net.ParseIP("1.2.3.4") case used by many other callers):
- Honor route.Family, which was previously ignored on the add path, so
a gateway-only route (no destination to pin the family) can still be
requested as V6.
- When the caller explicitly requests a V6 route via route.Family, let a
16-byte v4-mapped gateway (route.Gw and multipath nexthops) conform to
it and be encoded as a 16-byte AF_INET6 nexthop.
Support is opt-in via route.Family so existing callers see no change: a
v4-mapped gateway without route.Family set is still treated as V4 and
rejected on a V6 route, exactly as before. The conform is also gated on
a 16-byte slice so that an explicit 4-byte IPv4 gateway still errors
rather than being silently reinterpreted; a 4-byte slice is the only
unambiguous "I meant IPv4" signal available at the net.IP level.
Scope is limited to the gateway/nexthop, the real-world case (verified
against the kernel: `ip -6 route` reports the nexthop as ::ffff:a.b.c.d).
v4-mapped addresses are a socket-API representation construct and are
not routable as IPv6 destinations, so Dst and Src are left unchanged.
Fixes vishvananda#1114
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fdomain
force-pushed
the
route-v4mapped-v6-gateway
branch
from
July 7, 2026 07:01
b275745 to
03feb9f
Compare
Author
|
Hi @vishvananda @aboch , This PR is ready for review, but the build is failing due to a seemingly unrelated issue. The same failure is visible on the following PR. Thanks in advance for taking a look at my change whenever you have time! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is an alternative proposal to fix #1114. The initial plan was to migrate to netip #1181, but considering the size of the change, we believe it would be more acceptable to make minor fixes to handle IPv4-mapped IPv6 address as gw.
Change:
A v4-mapped IPv6 address (::ffff:a.b.c.d) is byte-identical to its IPv4 form as a net.IP, so
nl.GetIPFamilyreports FAMILY_V4 for it. When such an address was used as a route's gateway on a V6 route it was encoded as a 4-byte AF_INET nexthop, and combined with a V6 destination it failed outright with "gateway... not the same IP family".Resolve this locally in
prepareRouteReqrather than changingGetIPFamily(which has no route context and must keep reporting V4 for the common net.ParseIP("1.2.3.4") case used by many other callers):Support is opt-in via route.Family so existing callers see no change: a v4-mapped gateway without route.Family set is still treated as V4 and rejected on a V6 route, exactly as before. The conform is also gated on a 16-byte slice so that an explicit 4-byte IPv4 gateway still errors rather than being silently reinterpreted; a 4-byte slice is the only unambiguous "I meant IPv4" signal available at the net.IP level.
Scope is limited to the gateway/nexthop, the real-world case (verified against the kernel:
ip -6 routereports the nexthop as ::ffff:a.b.c.d). v4-mapped addresses are a socket-API representation construct and are not routable as IPv6 destinations, so Dst and Src are left unchanged.Fixes #1114
Summary by CodeRabbit