Merge pull request k8snetworkplumbingwg#307 from thom311/th/send-na-e…

…addrnotavail-retry

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

pkg/sriov/sriov.go

@@ -16,6 +16,7 @@ type pciUtils interface {
 	GetVFLinkNamesFromVFID(pfName string, vfID int) ([]string, error)
 	GetPciAddress(ifName string, vf int) (string, error)
 	EnableArpAndNdiscNotify(ifName string) error
+	EnableOptimisticDad(ifName string) error
 }
 
 type pciUtilsImpl struct{}
@@ -36,6 +37,10 @@ func (p *pciUtilsImpl) EnableArpAndNdiscNotify(ifName string) error {
 	return utils.EnableArpAndNdiscNotify(ifName)
 }
 
+func (p *pciUtilsImpl) EnableOptimisticDad(ifName string) error {
+	return utils.EnableOptimisticDad(ifName)
+}
+
 // Manager provides interface invoke sriov nic related operations
 type Manager interface {
 	SetupVF(conf *sriovtypes.NetConf, podifName string, netns ns.NetNS) error
@@ -147,8 +152,12 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
 			}
 		}
 
-		// 8. Bring IF up in Pod netns
-		logging.Debug("8. Bring IF up in Pod netns",
+		logging.Debug("8. Enable Optimistic DAD for IPv6 addresses", "func", "SetupVF",
+			"linkObj", linkObj)
+		_ = s.utils.EnableOptimisticDad(podifName)
+
+		// 9. Bring IF up in Pod netns
+		logging.Debug("9. Bring IF up in Pod netns",
 			"func", "SetupVF",
 			"linkObj", linkObj)
 		if err := s.nLink.LinkSetUp(linkObj); err != nil {

pkg/sriov/sriov_test.go

@@ -70,6 +70,7 @@ var _ = Describe("Sriov", func() {
 			mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil)
 			mocked.On("LinkSetUp", fakeLink).Return(nil)
 			mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)
+			mockedPciUtils.On("EnableOptimisticDad", mock.AnythingOfType("string")).Return(nil)
 			sm := sriovManager{nLink: mocked, utils: mockedPciUtils}
 			err = sm.SetupVF(netconf, podifName, targetNetNS)
 			Expect(err).NotTo(HaveOccurred())
@@ -121,6 +122,7 @@ var _ = Describe("Sriov", func() {
 			mocked.On("LinkSetNsFd", net2Link, mock.AnythingOfType("int")).Return(nil)
 			mocked.On("LinkSetUp", net2Link).Return(nil)
 			mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)
+			mockedPciUtils.On("EnableOptimisticDad", mock.AnythingOfType("string")).Return(nil)
 			sm := sriovManager{nLink: mocked, utils: mockedPciUtils}
 			err = sm.SetupVF(netconf, podifName, targetNetNS)
 			Expect(err).NotTo(HaveOccurred())
@@ -174,6 +176,7 @@ var _ = Describe("Sriov", func() {
 			mocked.On("LinkSetNsFd", net2Link, mock.AnythingOfType("int")).Return(nil)
 			mocked.On("LinkSetUp", net2Link).Return(nil)
 			mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)
+			mockedPciUtils.On("EnableOptimisticDad", mock.AnythingOfType("string")).Return(nil)
 			sm := sriovManager{nLink: mocked, utils: mockedPciUtils}
 			err = sm.SetupVF(netconf, podifName, targetNetNS)
 			Expect(err).NotTo(HaveOccurred())
@@ -228,6 +231,7 @@ var _ = Describe("Sriov", func() {
 			mocked.On("LinkSetNsFd", net2Link, mock.AnythingOfType("int")).Return(nil)
 			mocked.On("LinkSetUp", net2Link).Return(nil)
 			mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)
+			mockedPciUtils.On("EnableOptimisticDad", mock.AnythingOfType("string")).Return(nil)
 			sm := sriovManager{nLink: mocked, utils: mockedPciUtils}
 			err = sm.SetupVF(netconf, podifName, targetNetNS)
 			Expect(err).NotTo(HaveOccurred())

pkg/utils/packet.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"net"
 	"syscall"
@@ -164,7 +165,11 @@ func SendUnsolicitedNeighborAdvertisement(srcIP net.IP, linkObj netlink.Link) er
 	return nil
 }
 
-// AnnounceIPs sends either a GARP or Unsolicited NA depending on the IP address type (IPv4 vs. IPv6 respectively) configured on the interface.
+// AnnounceIPs sends IPv4 GARP and IPv6 Unsolicited NA for the addresses on the
+// interfaces.  If ifName is not found or has no MAC address, an error is
+// returned. If sending of announcements fail, the returned error is a combined
+// errors.Join() of each failed announcement. Despite such errors, remaining
+// addresses are still attempted to be announced.
 func AnnounceIPs(ifName string, ipConfigs []*current.IPConfig) error {
 	// Retrieve the interface name in the container.
 	linkObj, err := netLinkLib.LinkByName(ifName)
@@ -175,26 +180,27 @@ func AnnounceIPs(ifName string, ipConfigs []*current.IPConfig) error {
 		return fmt.Errorf("invalid Ethernet MAC address: %q", linkObj.Attrs().HardwareAddr)
 	}
 
+	var errResult error
+
 	// For all the IP addresses assigned by IPAM, we will send either a GARP (IPv4) or Unsolicited NA (IPv6).
 	for _, ipc := range ipConfigs {
-		var err error
-		if IsIPv6(ipc.Address.IP) {
+		if IsIPv4(ipc.Address.IP) {
+			err := SendGratuitousArp(ipc.Address.IP, linkObj)
+			if err != nil {
+				errResult = errors.Join(errResult, fmt.Errorf("failed to send GARP message for ip %s on interface %q: %v", ipc.Address.IP.String(), ifName, err))
+			}
+		} else if IsIPv6(ipc.Address.IP) {
 			/* As per RFC 4861, sending unsolicited neighbor advertisements should be considered as a performance
 			* optimization. It does not reliably update caches in all nodes. The Neighbor Unreachability Detection
 			* algorithm is more reliable although it may take slightly longer to update.
 			 */
-			err = SendUnsolicitedNeighborAdvertisement(ipc.Address.IP, linkObj)
-		} else if IsIPv4(ipc.Address.IP) {
-			err = SendGratuitousArp(ipc.Address.IP, linkObj)
-		} else {
-			return fmt.Errorf("the IP %s on interface %q is neither IPv4 or IPv6", ipc.Address.IP.String(), ifName)
-		}
-
-		if err != nil {
-			return fmt.Errorf("failed to send GARP/NA message for ip %s on interface %q: %v", ipc.Address.IP.String(), ifName, err)
+			err := SendUnsolicitedNeighborAdvertisement(ipc.Address.IP, linkObj)
+			if err != nil {
+				errResult = errors.Join(errResult, fmt.Errorf("failed to send NA message for ip %s on interface %q: %v", ipc.Address.IP.String(), ifName, err))
+			}
 		}
 	}
-	return nil
+	return errResult
 }
 
 // Blocking wait for interface ifName to have carrier (!NO_CARRIER flag).

pkg/utils/utils.go

@@ -51,6 +51,16 @@ func EnableArpAndNdiscNotify(ifName string) error {
 	return nil
 }
 
+// EnableOptimisticDad enables IPv6 /proc/sys/net/ipv6/conf/$ifName/optimistic_dad
+func EnableOptimisticDad(ifName string) error {
+	path := filepath.Join(SysV6NdiscNotify, ifName, "optimistic_dad")
+	err := os.WriteFile(path, []byte("1"), os.ModeAppend)
+	if err != nil {
+		return fmt.Errorf("failed to write optimistic_dad=1 for interface %s: %v", ifName, err)
+	}
+	return nil
+}
+
 // GetSriovNumVfs takes in a PF name(ifName) as string and returns number of VF configured as int
 func GetSriovNumVfs(ifName string) (int, error) {
 	var vfTotal int