ipv6: delay the first Duplicate Address Detection probe - #1183
Draft
adamgeorge309 wants to merge 1 commit into
Draft
ipv6: delay the first Duplicate Address Detection probe#1183adamgeorge309 wants to merge 1 commit into
adamgeorge309 wants to merge 1 commit into
Conversation
A node sent the first Neighbor Solicitation of Duplicate Address Detection
in the same event in which it formed the tentative address, and added the
random delay to the timeout instead. In examples/ipv6/nclients, config ETH,
cli[0] formed its link-local address at 0.922007 s and its probe was on the
wire at the same instant; the check then ran for 1.944669 s rather than the
one second of RetransTimer, because the draw had gone into the timeout.
RFC 4862 Section 5.4.2 orders the two:
Before sending a Neighbor Solicitation, an interface MUST join the
all-nodes multicast address and the solicited-node multicast address
of the tentative address.
If the Neighbor Solicitation is going to be the first message sent
from an interface after interface (re)initialization, the node SHOULD
delay joining the solicited-node multicast address by a random delay
between 0 and MAX_RTR_SOLICITATION_DELAY as specified in [RFC4861].
This serves to alleviate congestion when many nodes start up on the
link at the same time, such as after a power failure, and may help to
avoid race conditions when more than one node is trying to solicit
for the same address at the same time.
The join must precede the solicitation, so delaying the join delays the
solicitation with it. The delay belongs before the probe, not after it.
Schedule the timer with the random delay alone and leave the sending to
processDadTimeout(), which already sends a solicitation and rearms the
timer whenever fewer than DupAddrDetectTransmits have gone out. This also
repairs the spacing of the probes: the first two used to be
RetransTimer + delay apart while every later pair was RetransTimer apart,
against the same section's requirement that the solicitations be "each
separated by RetransTimer milliseconds".
Only the probe moves. The random draw is the same one, so the check still
ends where it did: cli[0] starts Duplicate Address Detection at 0.922007 s
and completes it at 2.866676 s exactly as before, but the probe is now at
1.866676 s, and completion is RetransTimer after the probe rather than
RetransTimer plus a random delay after the address was formed. The node is
already listening: the tentative address is on the interface before the
delay starts, which is what Section 5.4.2 asks for when it requires an
interface to "receive and process datagrams sent to the all-nodes multicast
address or solicited-node multicast address of the tentative address during
the delay period".
This commit knowingly leaves two tests failing, and does not repair them.
Moving every node's first probe by up to one second re-phases a wireless Mobile
IPv6 handover: the mobile node catches a later Router Advertisement after
re-associating, its care-of address is formed about 5.4 s later, and home
registration moves from 66.707 s to 72.043 s. Both Mobile IPv6 tests in
tests/protocol/ipv6 were tuned to the old timeline.
Mipv6Registration passes again once its three tuned numbers move by the same
amount -- the step-0 deadline 72 s to 78 s, the correspondent node's ping start
71 s to 77 s, and sim-time-limit 85 s to 91 s. Mipv6Interface does not: with the
new phase its Care-of Test reply arrives 1.9 ms before the tester finishes
matching step 2, because the care-of round trip goes direct while the home round
trip goes through the home agent, and the test's ordered step 2 and step 3 assume
the other order. Repairing that means changing what the test asserts, which
belongs with its author, so neither test is touched here.
The re-recorded fingerprints follow from the moved solicitations, which every
node in an IPv6 network sends at startup: 45 rows, 36 in examples.csv and 9 in
mipv6-refactoring.csv. 't' moves because the probes and their replies happen at
new times; 'p' and 'N' with the modules and nodes those events run in; 'l', 'b'
and 'D' because the solicitations enter the hash at different points; 'x' with
the event sequence. Graphical ('tyf') fingerprints were excluded from the run
and are unchanged.
The three mipv6-refactoring.csv rows that already fail on master --
examples/ipv6/mipv6 Handover and RouteOptimizationTwoCNs, and
examples/ipv6/mipv6roaming Roaming -- keep their recorded values. Their probes
move too, but those values are stale on master already, and re-recording them
here would hide that rather than fix it. The examples.csv rows of the same three
configurations pass on master, so they are re-recorded like the rest.
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
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 node sent the first Neighbor Solicitation of Duplicate Address Detection in
the same event in which it formed the tentative address, and added the random
delay to the timeout instead. RFC 4862 Section 5.4.2 puts the delay before the
probe, not after it.
Closes #1179
Please read The open question below before merging. This branch leaves two
Mobile IPv6 protocol tests failing, and how to resolve that is a question for
their author.
The problem
RFC 4862 Section 5.4.2 requires an interface to join the solicited-node
multicast address of the tentative address before it sends a Neighbor
Solicitation for it, and asks the node to delay that join by a random time of up
to MAX_RTR_SOLICITATION_DELAY, one second:
The join has to precede the solicitation, so delaying the join delays the
solicitation with it.
initiateDad()kept the random draw but added it to thetimeout: the probe went out immediately and the node then listened for between
one and two seconds, instead of waiting up to one second and then listening for
RetransTimer. Its comment names the right section and the right reason; onlythe code disagrees.
Two things are wrong, and neither makes detection less safe -- a node that
probes early and listens longer covers the same window. They are conformance and
timing defects.
instant probe at the same instant, which is the congestion and the race the
delay exists to avoid, and a capture taken from INET does not look like one
from a real host during boot.
RetransTimerplus thedraw apart while every later pair is
RetransTimerapart, against the samesection's "each separated by RetransTimer milliseconds".
In
examples/ipv6/nclients, configETH, on master:cli[0]forms its tentative link-local addresscli[0]transmits its first Neighbor Solicitationcli[0]completes Duplicate Address DetectionThe check ran for 1.944669 s rather than the 1 s of
RetransTimer.The fix
Schedule the timer with the random delay alone and leave the sending to
processDadTimeout(), which already sends a solicitation and rearms the timerwhenever fewer than
DupAddrDetectTransmitshave gone out. That also repairs theprobe spacing.
Only the probe moves. The random draw is the same one, so the check still ends
where it did:
cli[0]starts at 0.922007 s and completes at 2.866676 s exactlyas before, but the probe is now at 1.866676 s, and completion is
RetransTimerafter the probe rather than
RetransTimerplus a random delay after the addresswas formed. The node is listening throughout, because the tentative address is on
the interface before the delay starts -- which is what the same section asks for
when it requires an interface to "receive and process datagrams sent to the
all-nodes multicast address or solicited-node multicast address of the tentative
address during the delay period".
The open question
Moving every node's first probe by up to one second re-phases a wireless Mobile
IPv6 handover: after re-associating, the mobile node catches a later Router
Advertisement, and its care-of address is formed about 5.4 s later than on
master. Home registration moves from 66.707 s to 72.043 s.
That breaks both Mobile IPv6 tests in
tests/protocol/ipv6, which were tuned tomaster's timeline:
Mipv6Registration.testpasses again after moving the three tuned numbers bythe same amount the handover moved: the step-0 deadline 72 s to 78 s, the
correspondent node's ping start 71 s to 77 s, and
sim-time-limit85 s to 91 s.Mipv6Interface.teststill fails after that, and the reason is not a tuningnumber. With the new phase the Care-of Test reply arrives at 77.002006 s,
1.9 ms before the tester finishes matching its step 2 at 77.003946 s, so it
falls outside step 3's window. The care-of round trip goes direct while the
home round trip goes through the home agent, so the reply can come back before
the Home Test Init is even sent; the test's split of the exchange into an
ordered step 2 and step 3 assumes the other order.
Neither test is touched by this branch. Retuning them, merging their steps 2 and
3 into one unordered group, or holding this fix are all reasonable, and the
choice belongs with the tests' author.
Verification
The fingerprint suite is run whole -- all 1774 rows of every
.csv, not anIPv6-filtered subset -- because this change moves rows whose configuration names
do not contain the string
ipv6:examples/ospfv3,examples/bgpv4,examples/manetrouting/dymo,gpsranddsdv,examples/rip/simpletestandexamples/inet/hierarchical99.Unmodified master gives 1774 rows with 3 failures and 62 errors, and so does
this branch. The 62 errors are disabled optional features --
VoIPStream,TcpLwip,VoipStreamSender-- and have nothing to do with IPv6. The 3 failuresare
examples/ipv6/mipv6HandoverandRouteOptimizationTwoCNsandexamples/ipv6/mipv6roamingRoaming, all~tNlbrows ofmipv6-refactoring.csv. Their recorded values are stale on master already, sothey keep them here; re-recording them would absorb that staleness rather than
fix it.
Baselines re-recorded: 45 rows, 36 in
examples.csvand 9 inmipv6-refactoring.csv, covering every example with IPv6 nodes -- every node inan IPv6 network sends these solicitations at startup. 't' moves because the
probes and their replies happen at new times; 'p' and 'N' with the modules and
nodes those events run in; 'l', 'b' and 'D' because the solicitations enter the
hash at different points; 'x' with the event sequence. Graphical ('tyf')
fingerprints were excluded from every run and are unchanged.
Landing order against #1181
#1181, the pull request for #1178, re-records 20 of the same fingerprint rows as this branch -- 16 in
examples.csvand 4 inmipv6-refactoring.csv, inexamples/ipv6/nclients,examples/bgpv4over IPv6,examples/inet/hierarchical99IPv6,examples/manetrouting/gpsrandexamples/rip/simpletest. Both branches recordthose values against plain master, because each has to stand on its own, so the
two conflict textually in
tests/fingerprint/examples.csvandtests/fingerprint/mipv6-refactoring.csv, and whichever lands second needsthose 20 rows re-recorded on top of the first. Ping me and I will do it for
whichever order you prefer.
The source changes do not conflict: they touch disjoint parts of
Ipv6NeighbourDiscovery.cc.Module tests: 42 tests, 40 pass.
MIPv6_tcp_handoverandIPv6_packet_too_bigfail identically on unmodified master.
Protocol tests,
tests/protocol/ipv6: 29 tests, 19 pass and 8 expected failures.Mipv6RegistrationandMipv6Interfaceare unexpected failures, for the reasonset out under The open question. Master gives 21 pass and the same 8 expected
failures.
tests/fingerprint/store.json, the separate expectation storeopp_replreads,is deliberately left alone. It is already out of step with the
.csvbaselinesfor these rows on master: for
examples/ipv6/nclientsETHat the samedirectory, ini file, configuration, run and time limit it holds
tplxc5d4-8871whereexamples.csvholds5ceb-d724, and forexamples/inet/udpclientserverudp_OK_ipv6it holds8fe6-9756whereexamples.csvholdsd73a-ba68. That drift predates this branch and, as3a9cc27 records, no single source commit causes it; it belongs in a
baseline-only commit of its own.
check-commits.shpasses.check-architecture.shandcheck-naming.shreportonly candidates already present on master, in
src/inet/applications/sctpapp,src/inet/applications/rtpappandimages/misc.Architectural surface
No contract, packet content, configuration surface or feature descriptor
changes, and no sealed path is touched. No new
AV-*orNV-*row is needed.Not addressed here
startAddressProbe()on thetopic/gy/mipv6-ha-dadbranch repeats the samescheduling that this branch fixes, and will need the same change when that branch
is rebased.
Two further Neighbour Discovery conformance defects are fixed separately, in
#1178 (the initial Router Advertisement clamp) and #1180 (address lifetimes on a
repeated prefix).