Set MAC address after renaming the interface

Setting the MAC address at the end of SetupVF reduces
the chances of race conditions with tools that set MAC
address asynchronously (i.e. iavf).

Signed-off-by: Andrea Panattoni <[email protected]>

pkg/sriov/sriov.go

@@ -2,6 +2,7 @@ package sriov
 
 import (
 	"fmt"
+
 	"github.com/containernetworking/plugins/pkg/ns"
 
 	sriovtypes "github.com/k8snetworkplumbingwg/sriov-cni/pkg/types"
@@ -65,6 +66,9 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
 		return fmt.Errorf("error getting VF netdevice with name %s", linkName)
 	}
 
+	// Save the original effective MAC address before overriding it
+	conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String()
+
 	// tempName used as intermediary name to avoid name conflicts
 	tempName := fmt.Sprintf("%s%d", "temp_", linkObj.Attrs().Index)
 
@@ -78,31 +82,29 @@ func (s *sriovManager) SetupVF(conf *sriovtypes.NetConf, podifName string, netns
 		return fmt.Errorf("error setting temp IF name %s for %s", tempName, linkName)
 	}
 
-	// Save the original effective MAC address before overriding it
-	conf.OrigVfState.EffectiveMAC = linkObj.Attrs().HardwareAddr.String()
-	// 3. Set MAC address
-	if conf.MAC != "" {
-		err = utils.SetVFEffectiveMAC(s.nLink, tempName, conf.MAC)
-		if err != nil {
-			return fmt.Errorf("failed to set netlink MAC address to %s: %v", conf.MAC, err)
-		}
-	}
-
-	// 4. Change netns
+	// 3. Change netns
 	if err := s.nLink.LinkSetNsFd(linkObj, int(netns.Fd())); err != nil {
 		return fmt.Errorf("failed to move IF %s to netns: %q", tempName, err)
 	}
 
 	if err := netns.Do(func(_ ns.NetNS) error {
-		// 5. Set Pod IF name
+		// 4. Set Pod IF name
 		if err := s.nLink.LinkSetName(linkObj, podifName); err != nil {
 			return fmt.Errorf("error setting container interface name %s for %s", linkName, tempName)
 		}
 
-		// 6. Enable IPv4 ARP notify and IPv6 Network Discovery notify
+		// 5. Enable IPv4 ARP notify and IPv6 Network Discovery notify
 		// Error is ignored here because enabling this feature is only a performance enhancement.
 		_ = s.utils.EnableArpAndNdiscNotify(podifName)
 
+		// 6. Set MAC address
+		if conf.MAC != "" {
+			err = utils.SetVFEffectiveMAC(s.nLink, podifName, conf.MAC)
+			if err != nil {
+				return fmt.Errorf("failed to set netlink MAC address to %s: %v", conf.MAC, err)
+			}
+		}
+
 		// 7. Bring IF up in Pod netns
 		if err := s.nLink.LinkSetUp(linkObj); err != nil {
 			return fmt.Errorf("error bringing interface up in container ns: %q", err)

pkg/sriov/sriov_test.go

@@ -1,9 +1,10 @@
 package sriov
 
 import (
-	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils"
 	"net"
 
+	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/utils"
+
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containernetworking/plugins/pkg/testutils"
 	"github.com/k8snetworkplumbingwg/sriov-cni/pkg/sriov/mocks"
@@ -100,17 +101,17 @@ var _ = Describe("Sriov", func() {
 				HardwareAddr: fakeMac,
 			}}
 
-			tempLink := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{
+			net1Link := &utils.FakeLink{LinkAttrs: netlink.LinkAttrs{
 				Index:        1000,
-				Name:         "temp_1000",
+				Name:         "net1",
 				HardwareAddr: expMac,
 			}}
 
 			mocked.On("LinkByName", "enp175s6").Return(fakeLink, nil)
-			mocked.On("LinkByName", "temp_1000").Return(tempLink, nil)
+			mocked.On("LinkByName", "net1").Return(net1Link, nil)
 			mocked.On("LinkSetDown", fakeLink).Return(nil)
 			mocked.On("LinkSetName", fakeLink, mock.Anything).Return(nil)
-			mocked.On("LinkSetHardwareAddr", tempLink, expMac).Return(nil)
+			mocked.On("LinkSetHardwareAddr", net1Link, expMac).Return(nil)
 			mocked.On("LinkSetNsFd", fakeLink, mock.AnythingOfType("int")).Return(nil)
 			mocked.On("LinkSetUp", fakeLink).Return(nil)
 			mockedPciUtils.On("EnableArpAndNdiscNotify", mock.AnythingOfType("string")).Return(nil)