From cda5cfbb1c7b17a0b628502b0c90ef0f1b9b5420 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Thu, 14 Nov 2024 19:57:18 +0200 Subject: [PATCH] Avoid using iface.Name from spec during configuration. This ensures proper handling when the interface name changed for some reason. --- pkg/host/internal/sriov/sriov.go | 51 +++++++++++++++++++-------- pkg/host/internal/sriov/sriov_test.go | 17 ++++++--- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 3e5989bae..30e104044 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -362,7 +362,12 @@ func (s *sriov) configureHWOptionsForSwitchdev(iface *sriovnetworkv1.Interface) // we need to configure HW options only for PFs for which switchdev is a target mode return nil } - if err := s.networkHelper.EnableHwTcOffload(iface.Name); err != nil { + ifaceName := s.networkHelper.TryGetInterfaceName(iface.PciAddress) + if ifaceName == "" { + log.Log.Error(nil, "configureHWOptionsForSwitchdev(): WARNING: can't get interface namde for device, skip switchdev HW options configuration") + return nil + } + if err := s.networkHelper.EnableHwTcOffload(ifaceName); err != nil { return err } desiredFlowSteeringMode := "smfs" @@ -436,12 +441,22 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error { if err != nil { log.Log.Error(err, "configSriovVFDevices(): unable to parse VFs for device", "device", iface.PciAddress) } - pfLink, err := s.netlinkLib.LinkByName(iface.Name) + ifIndex, err := s.networkHelper.GetInterfaceIndex(iface.PciAddress) + if err != nil { + log.Log.Error(err, "configSriovVFDevices(): unable to get device index", "device", iface) + return err + } + pfLink, err := s.netlinkLib.LinkByIndex(ifIndex) if err != nil { log.Log.Error(err, "configSriovVFDevices(): unable to get PF link for device", "device", iface) return err } - + // LinkType is an optional field. Let's fallback to current link type + // if nothing is specified in the SriovNodePolicy + linkType := iface.LinkType + if linkType == "" { + linkType = s.encapTypeToLinkType(pfLink.Attrs().EncapType) + } for _, addr := range vfAddrs { hasDriver, _ := s.kernelHelper.HasDriver(addr) if !hasDriver { @@ -474,12 +489,6 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error { // for userspace drivers like vfio we configure the vf mac using the kernel nic mac address // before we switch to the userspace driver if yes, d := s.kernelHelper.HasDriver(addr); yes && !sriovnetworkv1.StringInArray(d, vars.DpdkDrivers) { - // LinkType is an optional field. Let's fallback to current link type - // if nothing is specified in the SriovNodePolicy - linkType := iface.LinkType - if linkType == "" { - linkType = s.GetLinkType(iface.Name) - } if strings.EqualFold(linkType, consts.LinkTypeIB) { if err := s.infinibandHelper.ConfigureVfGUID(addr, iface.PciAddress, vfID, pfLink); err != nil { return err @@ -583,8 +592,12 @@ func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, skipVFConfigu if err := s.configSriovVFDevices(iface); err != nil { return err } + ifIndex, err := s.networkHelper.GetInterfaceIndex(iface.PciAddress) + if err != nil { + return err + } // Set PF link up - pfLink, err := s.netlinkLib.LinkByName(iface.Name) + pfLink, err := s.netlinkLib.LinkByIndex(ifIndex) if err != nil { return err } @@ -942,7 +955,12 @@ func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error { return err } if sriovnetworkv1.GetEswitchModeFromSpec(iface) == sriovnetworkv1.ESwithModeSwitchDev { - if err := s.udevHelper.AddPersistPFNameUdevRule(iface.PciAddress, iface.Name); err != nil { + ifaceName := s.networkHelper.TryGetInterfaceName(iface.PciAddress) + if ifaceName == "" { + log.Log.Error(nil, "addUdevRules(): WARNING: can't get interface namde for device, skip creation of persist PF name UDEV rule") + return nil + } + if err := s.udevHelper.AddPersistPFNameUdevRule(iface.PciAddress, ifaceName); err != nil { return err } } @@ -954,18 +972,23 @@ func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error { // on old kernels can be read only after switching PF to switchdev mode. // if PF doesn't expose phys_port_name and phys_switch_id, then rule creation will be skipped func (s *sriov) addVfRepresentorUdevRule(iface *sriovnetworkv1.Interface) error { + ifaceName := s.networkHelper.TryGetInterfaceName(iface.PciAddress) + if ifaceName == "" { + log.Log.Error(nil, "addVfRepresentorUdevRule(): WARNING: can't get interface namde for device, skip creation of UDEV rule") + return nil + } if sriovnetworkv1.GetEswitchModeFromSpec(iface) == sriovnetworkv1.ESwithModeSwitchDev { - portName, err := s.networkHelper.GetPhysPortName(iface.Name) + portName, err := s.networkHelper.GetPhysPortName(ifaceName) if err != nil { log.Log.Error(err, "addVfRepresentorUdevRule(): WARNING: can't read phys_port_name for device, skip creation of UDEV rule") return nil } - switchID, err := s.networkHelper.GetPhysSwitchID(iface.Name) + switchID, err := s.networkHelper.GetPhysSwitchID(ifaceName) if err != nil { log.Log.Error(err, "addVfRepresentorUdevRule(): WARNING: can't read phys_switch_id for device, skip creation of UDEV rule") return nil } - return s.udevHelper.AddVfRepresentorUdevRule(iface.PciAddress, iface.Name, switchID, portName) + return s.udevHelper.AddVfRepresentorUdevRule(iface.PciAddress, ifaceName, switchID, portName) } return nil } diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go index 319bacf54..39f996b87 100644 --- a/pkg/host/internal/sriov/sriov_test.go +++ b/pkg/host/internal/sriov/sriov_test.go @@ -218,7 +218,9 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().AddDisableNMUdevRule("0000:d8:00.0").Return(nil) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2", "0000:d8:00.3"}, nil) pfLinkMock := netlinkMockPkg.NewMockLink(testCtrl) - netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").Return(pfLinkMock, nil).Times(3) + hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0") + hostMock.EXPECT().GetInterfaceIndex("0000:d8:00.0").Return(10, nil).Times(2) + netlinkLibMock.EXPECT().LinkByIndex(10).Return(pfLinkMock, nil).Times(2) pfLinkMock.EXPECT().Attrs().Return(&netlink.LinkAttrs{Flags: 0, EncapType: "ether"}) netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false) netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil) @@ -287,7 +289,9 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().AddDisableNMUdevRule("0000:d8:00.0").Return(nil) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2"}, nil) pfLinkMock := netlinkMockPkg.NewMockLink(testCtrl) - netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").Return(pfLinkMock, nil).Times(2) + hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0") + hostMock.EXPECT().GetInterfaceIndex("0000:d8:00.0").Return(10, nil).Times(2) + netlinkLibMock.EXPECT().LinkByIndex(10).Return(pfLinkMock, nil).Times(2) netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false) netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil) @@ -340,7 +344,9 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().GetDevlinkDeviceParam("0000:d8:00.0", "flow_steering_mode").Return("", syscall.EINVAL) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2"}, nil).Times(2) pfLinkMock := netlinkMockPkg.NewMockLink(testCtrl) - netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").Return(pfLinkMock, nil).Times(2) + hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(3) + hostMock.EXPECT().GetInterfaceIndex("0000:d8:00.0").Return(10, nil).Times(2) + netlinkLibMock.EXPECT().LinkByIndex(10).Return(pfLinkMock, nil).Times(2) netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false) netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil) netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(&netlink.DevlinkDevice{ @@ -409,7 +415,9 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().GetDevlinkDeviceParam("0000:d8:00.0", "flow_steering_mode").Return("", syscall.EINVAL) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2"}, nil).Times(1) pfLinkMock := netlinkMockPkg.NewMockLink(testCtrl) - netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").Return(pfLinkMock, nil).Times(2) + hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(3) + hostMock.EXPECT().GetInterfaceIndex("0000:d8:00.0").Return(10, nil).Times(2) + netlinkLibMock.EXPECT().LinkByIndex(10).Return(pfLinkMock, nil).Times(2) netlinkLibMock.EXPECT().IsLinkAdminStateUp(pfLinkMock).Return(false) netlinkLibMock.EXPECT().LinkSetUp(pfLinkMock).Return(nil) netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(&netlink.DevlinkDevice{ @@ -573,6 +581,7 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().RemoveDisableNMUdevRule("0000:d8:00.0").Return(nil) hostMock.EXPECT().RemovePersistPFNameUdevRule("0000:d8:00.0").Return(nil) hostMock.EXPECT().RemoveVfRepresentorUdevRule("0000:d8:00.0").Return(nil) + hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1) hostMock.EXPECT().AddDisableNMUdevRule("0000:d8:00.0").Return(nil) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2", "0000:d8:00.3"}, nil) hostMock.EXPECT().Unbind("0000:d8:00.2").Return(nil)