ipv6: address a pinned unicast datagram from the Neighbour Cache - #1152
Open
adamgeorge309 wants to merge 1 commit into
Open
ipv6: address a pinned unicast datagram from the Neighbour Cache#1152adamgeorge309 wants to merge 1 commit into
adamgeorge309 wants to merge 1 commit into
Conversation
A locally-originated unicast datagram that carries a pinned output interface went to the link layer with dest = FF-FF-FF-FF-FF-FF. Every Neighbour Discovery message pins its output interface, so a solicited Neighbour Advertisement and a Neighbour Unreachability Detection probe both left the node as an Ethernet broadcast, although the neighbour was in the Neighbour Cache. Every node on the link then received them, and a router forwarded the advertisement and answered it with a spurious ICMPv6 Redirect. RFC 4861 Section 7.2.4 requires the solicited advertisement to be unicast to the soliciting node, and Section 5.2 says where the link-layer address comes from: "Once the IP address of the next-hop node is known, the sender examines the Neighbor Cache for link-layer information about that neighbor." The multicast half of the same branch was corrected in 3be7618; the unicast half was left on the broadcast address. Pinning an output interface makes the Ipv6 module skip next-hop determination, so it has no next-hop address to look up. Neighbour Discovery therefore states the next hop it already knows -- an ND message is by definition addressed to a neighbour on the link it goes out on -- and datagramLocalOut() takes the link-layer address from the cache when a next hop is stated. Resolving the destination instead would be wrong. A pinned interface does not imply an on-link destination: PIM-SM pins one for the unicast it sends toward the rendezvous point, and a mobile node keeps a cache entry for its home agent after it has roamed, so the entry addresses a node that is no longer on the link. Both keep the previous behaviour, because neither states a next hop. Not repaired here: an advertisement answering a unicast solicitation that carried no Source Link-Layer Address option still goes out as a broadcast, because the responder has no cache entry to use. RFC 4861 Section 7.2.4 says such a node has to resolve the neighbour first, and the TODO in sendSolicitedNa() records that this is not implemented. The 14 fingerprints below are the IPv6 configurations that perform address resolution. Their Neighbour Discovery frames are now addressed to one neighbour instead of to the whole link, so the switch forwards them to one port and the nodes that no longer receive them lose their reception events; tplx, ~tNl, ~tND and ~tNlb move accordingly. tyf was excluded from the run and is carried over unchanged. No configuration without IPv6 address resolution moves.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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.
A locally-originated IPv6 unicast datagram that carries a pinned output interface went to the link
layer with
dest = FF-FF-FF-FF-FF-FF. Every Neighbour Discovery (ND) message pins its outputinterface, so a solicited Neighbour Advertisement (NA) and a Neighbour Unreachability Detection (NUD)
probe both left the node as an Ethernet broadcast, although the neighbour was in the Neighbour Cache.
This addresses them from the cache instead.
Closes #1151
One commit, and it carries its own baseline update as
change-a-baseline.md requires.
The problem
Ipv6::datagramLocalOut()took thedestIE != nullptrbranch — the pinned-interface branch — andused
MacAddress::BROADCAST_ADDRESSfor every non-multicast destination. The multicast half of thesame
ifwas corrected in 3be7618; the unicast half was not.RFC 4861 Section 5.2 says where the link-layer address comes from:
Measured on a switched IPv6 link (one
EthernetSwitch, oneRouter6, threeStandardHost6,Ipv6FlatNetworkConfigurator, one host sending UDP to another), 120 s:FF-FF-FF-FF-FF-FFFF-FF-FF-FF-FF-FFCorrelating the two headers shows the NUD probe, which is a unicast Neighbour Solicitation:
The fix
Pinning an output interface makes the
Ipv6module skip next-hop determination, so at that point ithas no next-hop address to look up. Two halves, neither of which changes anything on its own:
Ipv6NeighbourDiscovery::sendPacketToIpv6Module()states the next hop it already knows, in theNextHopAddressReqtag that GPSR and the netfilter hooks already use. An ND message is bydefinition addressed to a neighbour on the link it goes out on, so its next hop is its destination.
Ipv6::datagramLocalOut()takes the link-layer address from the Neighbour Cache when a next hop isstated, and keeps the previous behaviour when none is.
Why resolving the destination instead would be wrong
A pinned output interface does not imply an on-link destination, so looking the destination up is
not equivalent. Routing this branch through
resolveMACAddressAndSendPacket()— the obvious fix —breaks 13 module tests:
(
PimSm.cc:1692). Address resolution then fails and ND emits an ICMPv6 Destination Unreachablethat
PimSplittercannot handle:check_and_cast(): Cannot cast (inet::Indication*)ICMPv6-error to type 'inet::Packet *'.MIPv6_route_optimization.test), andresolveMACAddressAndSendPacket()additionally applies amobile-node guard that drops Home-Address-sourced datagrams, which the pinned path had never been
subject to.
Discovery on loopback.
Looking the destination up in the cache without resolving is not safe either: a mobile node keeps its
cache entry for the home agent after it has roamed, so a Binding Update pinned to the same interface
would be unicast to a node that is no longer on the link.
MIPv6_return_home_route_optimization.testcatches exactly that — the home agent never receives the Binding Update, and the mobile node
retransmits with a doubling timeout.
Requiring the sender to state the next hop avoids all three: PIM-SM and MIPv6 do not state one, so
their behaviour is unchanged.
Verification
The baseline was recorded on unmodified
origin/masterbefore any edit, and every suite was rerunagainst it.
tests/module/IPv6_nd_unicast_mac.test: a router and three hosts on oneswitched Ethernet link, 120 s so that Neighbour Unreachability Detection probes as well. An
IPv6-only Ethernet link carries multicast and unicast frames only, so the test asserts that no
frame is addressed to the broadcast address. It passes with this change and fails on unmodified
master.
-f '.*([Ii][Pp][Vv]6|MLD|ospfv3).*'): baseline 61 total, 59 PASS, 2 FAIL; after62 total (the new test), 60 PASS, 2 FAIL. The per-test diff is empty apart from the added test.
MIPv6_tcp_handover.testandIPv6_packet_too_big.testfail identically on unmodified master, sothey are pre-existing and out of scope.
./fingerprinttest -s -F tyfover the IPv6 entries ofexamples.csvand all ofmipv6-refactoring.csv): baseline 97/97 PASS; 14 entries moved; 97/97 PASS again afterre-recording.
Baseline updates
The 14 entries below are the IPv6 configurations that perform address resolution. Their Neighbour
Discovery frames are now addressed to one neighbour instead of to the whole link, so the switch
forwards them to one port and the nodes that no longer receive them lose their reception events.
Only the trajectory and content ingredients move —
tplx,~tNl,~tND,~tNlb. No configurationwithout IPv6 address resolution moves.
tests/fingerprint/examples.csv:/examples/bgpv4/BgpIpv6Basic/ -c General/examples/bgpv4/BgpAndOspfv3/ -c General/examples/bgpv4/BgpDiamondFailover6/ -c General/examples/bgpv4/BgpWithdrawal6/ -c Generaland-c Restart/examples/ipv6/nclients/ -c ETH/examples/ipv6/mipv6/ -c Handoverand-c RouteOptimizationTwoCNs/examples/ipv6/mipv6roaming/ -c Roaming/examples/ipv6/pmipv6/ -c General/examples/manetrouting/gpsr/ -c IPv6,-c MultiIPv6and-c DynamicIPv6/examples/rip/simpletest/ -c IPv6tests/fingerprint/mipv6-refactoring.csv: the~tNlbvalues of the four configurations it shareswith the list above.
tyfwas excluded from the run, as the harness itself recommends, so those values are carried overuntouched and are not claimed to be verified.
Not addressed here
still goes out as a broadcast, because the responder has no cache entry to use. RFC 4861
Section 7.2.4 says such a node has to resolve the neighbour first; the TODO in
sendSolicitedNa()records that this is not implemented. Six such frames remain in
examples/manetrouting/gpsr -c IPv6, down from 158.