Retry Sendto() for IPv6 NA for tentative IPv6 addresses and EADDRNOTAVAIL

[thom311]

When kernel initially adds an IPv6 address, it is still tentative and
pending DuplicateAddressDetection. Trying to use the address results in
EADDRNOTAVAIL error.

Detect that error, and retry for up to 3 seconds to send the message.

Unfortunately, DAD can easily take more than a second, so this change
delays the completion of the CNI binary quite a bit.

---

@SchSeba Follow-up for #306

This seems to happen with Openshift 4.17, which makes me wonder whether previous versions had opportunisticoptimistic DAD enabled. If so, why was it disabled?

[coveralls]

Pull Request Test Coverage Report for Build 10466886239

Details

• 6 of 29 (20.69%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.2%) to 41.19%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/sriov/sriov.go	6	9	66.67%
pkg/utils/utils.go	0	7	0.0%
pkg/utils/packet.go	0	13	0.0%

Totals
Change from base Build 10353172804:	-0.2%
Covered Lines:	540
Relevant Lines:	1311

---

💛 - Coveralls

[adrianchiris]

delaying CNI call for a single interface by up to 3 seconds for an optimization mechanism is very problematic in my view.
moreover if the first(ipv6) ip consumed these 3 seconds, then other IP6s will not have neighbour advertisment.

having opportunistic DAD enabled seems like the correct approach.
how do you enable that in the system ?

from cni POV we dont expect addresses to be duplicate (unless IPAM plugin is doing something wrong).

[thom311]

delaying CNI call for a single interface by up to 3 seconds f

In practice, it seems to be more like 1.8 seconds. The 3 seconds is a maximum (which the patch maybe should reduce to 2 seconds). Still, it's long.

delaying CNI call for a single interface by up to 3 seconds for an optimization mechanism is very problematic in my view.

Note that the IPv6 addresses are not really usable on the POD anyway. It also means, to start the pod with non-complete IP configuration (e.g. they cannot bind to the IPv6 address). So delaying the startup may be desirable also for those reasons. However, to have this wait implemented via AnnounceIPs() is non-obvious.

moreover if the first(ipv6) ip consumed these 3 seconds, then other IP6s will not have neighbour advertisment.

I don't understand this. If the first IPv6 was delayed by the full 3 seconds, then indeed, subsequent addresses won't be delayed/retried. But they are still tried at least once.

having opportunistic DAD enabled seems like the correct approach.
how do you enable that in the system ?

Sorry, I misspoke earlier: s/opportunistic/optimistic/

I agree. Something like echo 1 > /proc/sys/net/ipv6/conf/eth0/optimistic_dad

[adrianchiris]

I don't understand this. If the first IPv6 was delayed by the full 3 seconds, then indeed, subsequent addresses won't be delayed/retried. But they are still tried at least once.

yes, i re-read the code. you are correct.

[thom311]

@adrianchiris branch completely reworked. Now, optimistic_dad is set.

This is a far-reaching change, I am not sure I understand all implications. It does however seem the right thing to me, also because it seems bad to hand control over to the container with IPv6 addresses that are not ready yet.

The retry is gone because it should no longer be necessary. It would however not hurt, because the retry was only done when it was determined to be necessary.

WDYT?

[wizhaoredhat]

I really like the opportunistic DAD approach! It looks much better now. Also IPAM should really never give a duplicate IPv6 address when setup properly. As Adrian said, duplicate IPv6 addresses shouldn't occur.

[SchSeba]

but we still need to remember the case where a user wants a static IP address on a pod and he restart the pod.

that is a valid use-case

[thom311]

but we still need to remember the case where a user wants a static IP address on a pod and he restart the pod.

I don't follow. Could you please elaborate?

[SchSeba]

LGTM

[adrianchiris]

overall LGTM, once comments addressed we can merge.