introduce a recheck for the VF interface name

It's possible to have a race in the VFIsReady function. vf netdevice can have a default eth0 device name and be the time we call the netlink syscall to get the device information eth0 can be a different device. this cause duplicate mac allocation on vf admin mac address Signed-off-by: Sebastian Sch <sebassch@gmail.com>
k8snetworkplumbingwg · Jun 21, 2024 · ad0b2f7 · ad0b2f7
1 parent ddb57f3
commit ad0b2f7
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 6 deletions.
diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go
@@ -176,8 +176,16 @@ func (s *sriov) VFIsReady(pciAddr string) (netlink.Link, error) {
 		vfLink, err = s.netlinkLib.LinkByName(vfName)
 		if err != nil {
 			log.Log.Error(err, "VFIsReady(): unable to get VF link", "device", pciAddr)
+			return false, nil
 		}
-		return err == nil, nil
+
+		// try to get the interface name again and check if the name changed
+		vfName = s.networkHelper.TryGetInterfaceName(pciAddr)
+		if vfName != vfLink.Attrs().Name {
+			log.Log.Error(err, "VFIsReady(): VF changed name for device", "device", pciAddr, "previewsName", vfLink.Attrs().Name, "currentName", vfName)
+		}
+
+		return vfName == vfLink.Attrs().Name, nil
 	})
 	if err != nil {
 		return vfLink, err

diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go
@@ -131,10 +131,10 @@ var _ = Describe("SRIOV", func() {
 			hostMock.EXPECT().UnbindDriverIfNeeded("0000:d8:00.2", true).Return(nil)
 			hostMock.EXPECT().BindDefaultDriver("0000:d8:00.2").Return(nil)
 			hostMock.EXPECT().SetNetdevMTU("0000:d8:00.2", 2000).Return(nil)
-			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("enp216s0f0_0")
+			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("enp216s0f0_0").Times(2)
 			vf0LinkMock := netlinkMockPkg.NewMockLink(testCtrl)
 			vf0Mac, _ := net.ParseMAC("02:42:19:51:2f:af")
-			vf0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{HardwareAddr: vf0Mac})
+			vf0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Name: "enp216s0f0_0", HardwareAddr: vf0Mac}).AnyTimes()
 			netlinkLibMock.EXPECT().LinkByName("enp216s0f0_0").Return(vf0LinkMock, nil)
 			netlinkLibMock.EXPECT().LinkSetVfHardwareAddr(vf0LinkMock, 0, vf0Mac).Return(nil)
 
@@ -255,11 +255,11 @@ var _ = Describe("SRIOV", func() {
 			hostMock.EXPECT().UnbindDriverIfNeeded("0000:d8:00.2", true).Return(nil)
 			hostMock.EXPECT().BindDefaultDriver("0000:d8:00.2").Return(nil)
 			hostMock.EXPECT().SetNetdevMTU("0000:d8:00.2", 2000).Return(nil)
-			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("enp216s0f0_0")
+			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("enp216s0f0_0").AnyTimes()
 			vf0LinkMock := netlinkMockPkg.NewMockLink(testCtrl)
 			vf0Mac, _ := net.ParseMAC("02:42:19:51:2f:af")
-			vf0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{HardwareAddr: vf0Mac})
-			netlinkLibMock.EXPECT().LinkByName("enp216s0f0_0").Return(vf0LinkMock, nil)
+			vf0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Name: "enp216s0f0_0", HardwareAddr: vf0Mac}).Times(3)
+			netlinkLibMock.EXPECT().LinkByName("enp216s0f0_0").Return(vf0LinkMock, nil).AnyTimes()
 			netlinkLibMock.EXPECT().LinkSetVfHardwareAddr(vf0LinkMock, 0, vf0Mac).Return(nil)
 			hostMock.EXPECT().GetPhysPortName("enp216s0f0np0").Return("p0", nil)
 			hostMock.EXPECT().GetPhysSwitchID("enp216s0f0np0").Return("7cfe90ff2cc0", nil)
@@ -433,4 +433,22 @@ var _ = Describe("SRIOV", func() {
 			helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "2")
 		})
 	})
+
+	Context("VfIsReady", func() {
+		It("Should retry if the interface name change", func() {
+			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("eth0").Times(1)
+			hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.2").Return("enp216s0f0_0").Times(3)
+			vf0LinkMock := netlinkMockPkg.NewMockLink(testCtrl)
+			vf0Mac, _ := net.ParseMAC("02:42:19:51:2f:af")
+			vf0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Name: "enp216s0f0_0", HardwareAddr: vf0Mac}).Times(3)
+			netlinkLibMock.EXPECT().LinkByName("enp216s0f0_0").Return(vf0LinkMock, nil)
+			eth0LinkMock := netlinkMockPkg.NewMockLink(testCtrl)
+			eth0Mac, _ := net.ParseMAC("00:00:00:00:00:00")
+			eth0LinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Name: "eth0", HardwareAddr: eth0Mac}).Times(3)
+			netlinkLibMock.EXPECT().LinkByName("eth0").Return(eth0LinkMock, nil).Times(1)
+			vfLink, err := s.VFIsReady("0000:d8:00.2")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(vfLink.Attrs().HardwareAddr).To(Equal(vf0Mac))
+		})
+	})
 })