icmpv6: stop Duplicate Address Detection of an address that is removed

[adamgeorge309]

An address removed from an interface while its Duplicate Address Detection (DAD) is running keeps its DAD timer, and DAD then completes for an address the interface no longer holds: a debug build stops on ASSERT(k != -1) in Ipv6InterfaceData::permanentlyAssign(), a release build writes one byte to addresses[-1]. This pull request stops the DAD of an address wherever Ipv6NeighbourDiscovery removes one.

Closes #1253

The problem

An Internet Protocol version 6 (IPv6) Router Advertisement (RA) with two autonomous prefixes that are new to the host, arriving after its link-local DAD has completed:

Time (s)	Event in the host
3.097290857161	RA, first prefix aaaa:0:65::/64: aaaa:0:65:0:8aa:ff:fe00:2 assigned tentative, its DAD starts
3.097290857161	second prefix aaaa:2::/64: every address set tentative, DAD restarts on fe80::8aa:ff:fe00:2, aaaa:0:65:0:8aa:ff:fe00:2 recorded as the old care-of address
4.394825462416	link-local DAD completes; makeTentativeAddressPermanent() removes aaaa:0:65:0:8aa:ff:fe00:2
4.534878067138	"DAD completed for address aaaa:0:65:0:8aa:ff:fe00:2": permanentlyAssign() with findAddress() = -1

This is master at seed-set = 0 with the network of tests/module/IPv6_DAD_removed_address.test.

The commits

1. icmpv6: refactor: move the Duplicate Address Detection stop into cancelDad(): the loop that dadHasFailed() ran inline becomes a method the other removal sites can call. No behavior change.
2. icmpv6: fix: stop Duplicate Address Detection of an address that is removed: the three other places that remove an address call it. Only the old care-of address in makeTentativeAddressPermanent() has a known trigger on master; an address whose prefix is advertised with a Valid Lifetime of zero, and the care-of address removed on returning home, can be tentative with a DAD running in the same way.

Architectural surface

One new protected virtual method, Ipv6NeighbourDiscovery::cancelDad(). No packet, parameter or signal declaration changes. In a run that takes this path, the removed address no longer completes DAD: dadCompleted has one emission less, and with sendGratuitousNa = true one unsolicited Neighbor Advertisement for that address is no longer sent.

Verification

tests/module/IPv6_DAD_removed_address.test reproduces the table above. On master its %not-contains for the "DAD completed" line of the removed address fails (the line is in the output, at 4.534878067138 s); with the fix it passes:

cd tests/module && inet_run_module_tests -m release --no-build -l ERROR -f IPv6_DAD_removed
# PASS

The memory error, with the test's test.ned and omnetpp.ini extracted into a directory (the ned-path line removed):

valgrind --error-limit=no opp_run -l <inet>/src/INET -u Cmdenv -f omnetpp.ini --seed-set=0 -n .:<inet>/src
# master:   Invalid write of size 1 ... permanentlyAssign ... (Ipv6InterfaceData.cc:394); ERROR SUMMARY: 1 errors
# this PR:  ERROR SUMMARY: 0 errors

In a debug build, with the test's network at seed-set = 0, commit 1 (no fix yet) stops with ASSERT: Condition 'k != -1' does not hold in function 'permanentlyAssign' at inet/networklayer/ipv6/Ipv6InterfaceData.cc:393 ... at t=4.534878067138s; with this pull request the run completes. The 39 module tests that inet_run_module_tests -m debug --no-build -l ERROR -f 'IPv6|DAD|MIPv6|PMIPv6|NUD|ND_' selects all pass in debug, including IPv6_packet_too_big and MIPv6_tcp_handover, which fail on master in release only, and the nd protocol suite gives the same 22 PASS, 15 FAIL in debug as in release.

Gates: check-commits.sh origin/master..HEAD and check-classification.sh origin/master..HEAD pass (notes: subjects of 75 and 76 characters). On src/inet/networklayer/icmpv6, check-architecture.sh, check-interfaces.sh and check-source-seals.sh pass; check-naming.sh reports the two hits it reports on master, in Icmpv6Header.msg and Mldv2Message.msg, which this pull request does not touch.

Suites, release build, results diffed by test name against unmodified master. The fingerprint, module and ipv6 runs were made on master 49e1fa0 and on both commits. The branch was then rebased onto master 4eb3bb4, which changes nothing under src/, tests/module/ or tests/fingerprint/; the nd and mld protocol suites, new in 4eb3bb4, were run on 4eb3bb4 and on commit 2 only.

cd tests/fingerprint && ./fingerprinttest -s -F tyf
cd tests/module && inet_run_module_tests -m release --no-build -l ERROR
cd tests/protocol/<suite> && inet_run_protocol_tests -m release

Suite	master	commit 1	commit 2
fingerprints (-F tyf)	1711 PASS, 63 ERROR	1711 PASS, 63 ERROR	1711 PASS, 63 ERROR
module	346: 300 PASS, 46 FAIL	346: 300 PASS, 46 FAIL	347: 301 PASS, 46 FAIL
protocol, ipv6	27: 26 PASS, 1 FAIL	27: 26 PASS, 1 FAIL	27: 26 PASS, 1 FAIL
protocol, nd	37: 22 PASS, 15 FAIL	not run	37: 22 PASS, 15 FAIL
protocol, mld	47: 21 PASS, 26 FAIL	not run	47: 21 PASS, 26 FAIL

The failing and erroring tests are the same by name in every column. The 63 fingerprint errors are configurations of optional features this build does not enable (VoIPStream, TcpLwip, the OpenSceneGraph (OSG) visualizer, the Z3 gate scheduler) and ethernet-nonstandardspeed.ini at 5 Gbps half duplex. The 46 module failures are 34 tcp_* tests, ConvolutionalCoder12/34, EtherHost_lifecycle, ExternalProcess_3, Ieee80211BitDomain/SymbolDomain, Ieee8021d-Rstp/Stp, IPv6_packet_too_big, MIPv6_tcp_handover, PacketGate_1 and UDPSocket_1. The ipv6 protocol failure is Rfc8200OverlappingFragments; the nd and mld failures are the gaps their suites record on master (one mld test is marked as an expected failure). The one module test added is IPv6_DAD_removed_address. No fingerprint moves.

Not addressed here

• The address of the first prefix is still lost. The else branch of processRaPrefixInfoForAddrAutoConf() treats any second new prefix as a Mobile IPv6 handover and removes the first address as the old care-of address, which Request for Comments (RFC) 4862 (IPv6 Stateless Address Autoconfiguration) Section 5.5.3 does not ask for. Limiting that branch to a real handover is a separate change; ipv6: verify a care-of address before registering it with the home agent #1141 (DAD for the care-of address formed at a handover) edits the same branch.
• Ipv6InterfaceData also removes addresses whose valid lifetime has expired, outside Ipv6NeighbourDiscovery; that removal does not stop a running DAD either.
• With three new prefixes in one RA the else branch runs twice, two link-local DADs complete, and each starts Router Discovery; valgrind then reports the second Router Discovery entry freed twice at shutdown (the same network with a third prefix aaaa:1::/64, seed-set = 1). That is a separate Router Discovery defect, not filed yet.

---

[devin-ai-integration]

🔍 Devin Review: 2 flags

Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)