ipv6: answer every Router Solicitation, not only the first one - #1139
Open
adamgeorge309 wants to merge 1 commit into
Open
ipv6: answer every Router Solicitation, not only the first one#1139adamgeorge309 wants to merge 1 commit into
adamgeorge309 wants to merge 1 commit into
Conversation
An advertising interface answered the first Router Solicitation it received
and then silently discarded every later one until its next periodic Router
Advertisement was due. With the RFC 4861 default MinRtrAdvInterval and
MaxRtrAdvInterval (200s and 600s, which are also INET's NED defaults) that
window is far longer than a host's Router Discovery budget of
MAX_RTR_SOLICITATIONS * RTR_SOLICITATION_INTERVAL, about 9s, so a host that
solicited second gave up with "No RA messages were received. Assume no routers
are on-link", never autoconfigured a global address, and stayed unreachable for
the rest of the run, with no error and a zero exit code.
processRsPacket() decides whether to schedule a solicited advertisement by
comparing the computed send time against advIfEntry->nextScheduledRATime,
which per RFC 4861 Section 6.2.6 must be the time the *next* multicast Router
Advertisement is scheduled to be sent:
- Upon receipt of a Router Solicitation, compute a random delay within
the range 0 through MAX_RA_DELAY_TIME. If the computed value
corresponds to a time later than the time the next multicast Router
Advertisement is scheduled to be sent, ignore the random delay and
send the advertisement at the already-scheduled time.
Scheduling a solicited advertisement correctly sets nextScheduledRATime to its
send time, because it then is the next multicast advertisement to go out. But
sendSolicitedRa() never moved the field on once that advertisement had been
sent, so it kept pointing at an advertisement already in the past. Every later
solicitation computed a send time in the future, found it "later than" a past
timestamp, and fell into the ignore branch -- where nothing then served the
solicitation, although Section 6.2.6 also states that "a router sends
advertisements in response to valid solicitations received on an advertising
interface".
Point nextScheduledRATime back at the pending periodic advertisement once the
solicited one has been sent. Also log the ignore path, which is now only taken
when a future advertisement really will serve the solicitation.
The comparison itself was made dimensionally correct only recently, in
632e460 ("IPv6: three Neighbour Discovery conformance fixes"); before that
it compared a duration against an absolute time, which was wrong but almost
always true, so the ignore branch was effectively unreachable and the stale
field did no harm.
Adds tests/module/IPv6_RA_repeated_solicitations.test, which shuts a host down
and restarts it so that it has to redo Router Discovery while no periodic
advertisement is due. The test fails on unmodified master.
Fingerprints re-recorded for the IPv6 examples that have two or more soliciting
hosts on one link: ipv6/mipv6 (Handover), ipv6/mld (MldV2Ssm), ospfv3/small,
ospfv3/multiple_areas_FINAL (all three configurations) and inet/hierarchical99
(IPv6). Every ingredient of those fingerprints moves for one reason: the router
now emits Router Advertisements it used to discard. That adds events (t), in
modules that previously saw none at that point (p, N), carrying Router
Advertisement payloads that were not in the hashed stream before (l, D, b), and
everything downstream on those nodes shifts (x). The graphical (tyf)
ingredients were excluded from the run and are left untouched. Verified stable
across two independent runs.
IPv6 examples with only one soliciting host per router interface are unaffected
and were not re-recorded: ipv6/nclients (each client has its own point-to-point
link) and inet/udpclientserver IPv6 configurations all still pass.
ipv6/mipv6roaming (Roaming, ~tNlb) fails both before and after this change with
the same actual value 7ed8-bee3, so it is pre-existing and deliberately not
re-recorded here.
![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.
An advertising IPv6 interface answered the first Router Solicitation (RS) it received and then
silently discarded every later one until its next periodic Router Advertisement (RA) was due. With
INET's own NED defaults for the periodic interval — 200 s / 600 s, which are also RFC 4861's
MinRtrAdvInterval/MaxRtrAdvInterval— that window is far longer than a host's RouterDiscovery budget, so a host that solicited second never autoconfigured a global address and stayed
unreachable for the rest of the run, with no error and a zero exit code.
Closes #1138
The problem
processRsPacket()compares the computed send time of a solicited advertisement againstadvIfEntry->nextScheduledRATime, which per RFC 4861 Section 6.2.6 is the time the nextmulticast Router Advertisement is scheduled to be sent. Scheduling a solicited advertisement
correctly sets the field to its own send time, because it then is the next multicast advertisement
to go out — but
sendSolicitedRa()never moved the field on once that advertisement had beensent. It kept pointing at an advertisement already in the past, so every later solicitation
computed a send time in the future, found it "later than" a past timestamp, and fell into the
branch that discards the solicitation.
Measured on the added module test, with
minIntervalBetweenRAs= 200 s andmaxIntervalBetweenRAs= 600 s.hostBis shut down at t = 1 s and restarted at t = 20 s, so itloses its addresses and has to redo Router Discovery at a time when no periodic advertisement is
due:
hostB's solicitation 1 reachesRRsends the solicited Router AdvertisementhostBassigns its new address oneth0RRhostB:No RA messages were receivedhostBsends 20 UDP packets tosrvsrvreceives 0srvreceives 20The defect
sendSolicitedRa()now pointsnextScheduledRATimeback at the pending periodic advertisementonce the solicited one has been sent.
createRaTimer()is the only place anAdvIfEntryiscreated and it always schedules
raTimeoutMsg, andsendPeriodicRa()reschedules it beforereturning, so the timer is always pending at a future time whenever the entry exists.
The
elsebranch is also given a log line. It is now taken only when a future advertisementreally will serve the solicitation, which is the case RFC 4861 Section 6.2.6 describes.
Verification
tests/module/IPv6_RA_repeated_solicitations.testis new. It shuts a host down and restarts it sothat it has to redo Router Discovery while no periodic advertisement is due. It fails on
unmodified master (
No RA messages were received,srvreceives 0 packets) and passes here.All 39
IPv6*andMIPv6*module tests pass.Fingerprints re-recorded for the IPv6 examples that have two or more soliciting hosts on one link:
ipv6/mipv6(Handover),ipv6/mld(MldV2Ssm),ospfv3/small,ospfv3/multiple_areas_FINAL(allthree configurations) and
inet/hierarchical99(IPv6). Every ingredient of those fingerprintsmoves for one reason: the router now emits Router Advertisements it used to discard. That adds
events (
t), in modules that previously saw none at that point (p,N), carrying RouterAdvertisement payloads that were not in the hashed stream before (
l,D,b), and everythingdownstream on those nodes shifts (
x). The graphical (tyf) ingredients were excluded from therun and are left untouched. Verified stable across two independent runs.
IPv6 examples with only one soliciting host per router interface are unaffected and were not
re-recorded:
ipv6/nclients(each client has its own point-to-point link) and theinet/udpclientserverIPv6 configurations all still pass.ipv6/mipv6roaming(Roaming,~tNlb) fails both before and after this change with the sameactual value
7ed8-bee3, so it is pre-existing and deliberately not re-recorded here. Confirmedby reverting only the
Ipv6NeighbourDiscovery.cchunk, rebuilding, and rerunning that row.Not addressed here
The host protocol constants
IPv6_MAX_RTR_SOLICITATIONS,IPv6_RTR_SOLICITATION_INTERVALandIPv6_MAX_RTR_SOLICITATION_DELAYremain unreachable from an ini file — their setters have nocallers in
src/. Recorded in the issue.