Skip to content

Commit

Permalink
Avoid using iface.Name from spec during configuration
Browse files Browse the repository at this point in the history 
This ensures proper handling when the interface name changed for some reason.
  • Loading branch information
@ykulazhenkov
ykulazhenkov committed Nov 20, 2024
1 parent 5fce462 commit 4930922
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 24 deletions.
49 changes: 26 additions & 23 deletions pkg/host/internal/sriov/sriov.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func (s *sriov) DiscoverSriovDevices(storeManager store.ManagerInterface) ([]sri
return pfList, nil
}

func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
func (s *sriov) configSriovPFDevice(ifaceName string, iface *sriovnetworkv1.Interface) error {
log.Log.V(2).Info("configSriovPFDevice(): configure PF sriov device",
"device", iface.PciAddress)
totalVfs := s.dputilsLib.GetSriovVFcapacity(iface.PciAddress)
Expand All @@ -320,7 +320,7 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
log.Log.Error(err, "configSriovPFDevice(): fail to set NumVfs for device", "device", iface.PciAddress)
return err
}
if err := s.configureHWOptionsForSwitchdev(iface); err != nil {
if err := s.configureHWOptionsForSwitchdev(ifaceName, iface); err != nil {
return err
}
// remove all UDEV rules for the PF before adding new rules to
Expand All @@ -330,7 +330,7 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
log.Log.Error(err, "configSriovPFDevice(): fail to remove udev rules", "device", iface.PciAddress)
return err
}
err := s.addUdevRules(iface)
err := s.addUdevRules(ifaceName, iface)
if err != nil {
log.Log.Error(err, "configSriovPFDevice(): fail to add udev rules", "device", iface.PciAddress)
return err
Expand All @@ -340,7 +340,7 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
log.Log.Error(err, "configSriovPFDevice(): fail to set NumVfs for device", "device", iface.PciAddress)
return err
}
if err := s.addVfRepresentorUdevRule(iface); err != nil {
if err := s.addVfRepresentorUdevRule(ifaceName, iface); err != nil {
log.Log.Error(err, "configSriovPFDevice(): fail to add VR representor udev rule", "device", iface.PciAddress)
return err
}
Expand All @@ -355,14 +355,14 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error {
return nil
}

func (s *sriov) configureHWOptionsForSwitchdev(iface *sriovnetworkv1.Interface) error {
func (s *sriov) configureHWOptionsForSwitchdev(ifaceName string, iface *sriovnetworkv1.Interface) error {
log.Log.V(2).Info("configureHWOptionsForSwitchdev(): configure HW options for device",
"device", iface.PciAddress)
if sriovnetworkv1.GetEswitchModeFromSpec(iface) != sriovnetworkv1.ESwithModeSwitchDev {
// 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 {
if err := s.networkHelper.EnableHwTcOffload(ifaceName); err != nil {
return err
}
desiredFlowSteeringMode := "smfs"
Expand Down Expand Up @@ -428,20 +428,23 @@ func (s *sriov) checkExternallyManagedPF(iface *sriovnetworkv1.Interface) error
return nil
}

func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
func (s *sriov) configSriovVFDevices(ifaceName string, iface *sriovnetworkv1.Interface) error {
log.Log.V(2).Info("configSriovVFDevices(): configure PF sriov device",
"device", iface.PciAddress)
if iface.NumVfs > 0 {
vfAddrs, err := s.dputilsLib.GetVFList(iface.PciAddress)
if err != nil {
log.Log.Error(err, "configSriovVFDevices(): unable to parse VFs for device", "device", iface.PciAddress)
}
pfLink, err := s.netlinkLib.LinkByName(iface.Name)
pfLink, err := s.netlinkLib.LinkByName(ifaceName)
if err != nil {
log.Log.Error(err, "configSriovVFDevices(): unable to get PF link for device", "device", iface)
log.Log.Error(err, "configSriovVFDevices(): unable to get PF link for device", "device", iface.PciAddress)
return err
}

linkType := iface.LinkType
if linkType == "" {
linkType = s.encapTypeToLinkType(pfLink.Attrs().EncapType)
}
for _, addr := range vfAddrs {
hasDriver, _ := s.kernelHelper.HasDriver(addr)
if !hasDriver {
Expand Down Expand Up @@ -476,10 +479,6 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
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
Expand Down Expand Up @@ -558,8 +557,12 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, skipVFConfiguration bool) error {
log.Log.V(2).Info("configSriovDevice(): configure sriov device",
"device", iface.PciAddress, "config", iface, "skipVFConfiguration", skipVFConfiguration)
ifaceName := s.networkHelper.TryGetInterfaceName(iface.PciAddress)
if ifaceName == "" {
return fmt.Errorf("failed to get network interface name for device")
}
if !iface.ExternallyManaged {
if err := s.configSriovPFDevice(iface); err != nil {
if err := s.configSriovPFDevice(ifaceName, iface); err != nil {
return err
}
}
Expand All @@ -580,11 +583,11 @@ func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, skipVFConfigu
return err
}
}
if err := s.configSriovVFDevices(iface); err != nil {
if err := s.configSriovVFDevices(ifaceName, iface); err != nil {
return err
}
// Set PF link up
pfLink, err := s.netlinkLib.LinkByName(iface.Name)
pfLink, err := s.netlinkLib.LinkByName(ifaceName)
if err != nil {
return err
}
Expand Down Expand Up @@ -935,14 +938,14 @@ func (s *sriov) encapTypeToLinkType(encapType string) string {
// create required udev rules for PF:
// * rule to disable NetworkManager for VFs - for all modes
// * rule to keep PF name after switching to switchdev mode - only for switchdev mode
func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error {
func (s *sriov) addUdevRules(ifaceName string, iface *sriovnetworkv1.Interface) error {
log.Log.V(2).Info("addUdevRules(): add udev rules for device",
"device", iface.PciAddress)
if err := s.udevHelper.AddDisableNMUdevRule(iface.PciAddress); err != nil {
return err
}
if sriovnetworkv1.GetEswitchModeFromSpec(iface) == sriovnetworkv1.ESwithModeSwitchDev {
if err := s.udevHelper.AddPersistPFNameUdevRule(iface.PciAddress, iface.Name); err != nil {
if err := s.udevHelper.AddPersistPFNameUdevRule(iface.PciAddress, ifaceName); err != nil {
return err
}
}
Expand All @@ -953,19 +956,19 @@ func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error {
// this rule relies on phys_port_name and phys_switch_id parameter which
// 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 {
func (s *sriov) addVfRepresentorUdevRule(ifaceName string, iface *sriovnetworkv1.Interface) error {
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
}
Expand Down
9 changes: 8 additions & 1 deletion pkg/host/internal/sriov/sriov_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,10 @@ var _ = Describe("SRIOV", func() {
hostMock.EXPECT().RemovePersistPFNameUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemoveVfRepresentorUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().AddDisableNMUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)
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)
netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").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)
Expand Down Expand Up @@ -285,6 +286,7 @@ var _ = Describe("SRIOV", func() {
hostMock.EXPECT().RemovePersistPFNameUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().RemoveVfRepresentorUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().AddDisableNMUdevRule("0000:d8:00.0").Return(nil)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)
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)
Expand Down Expand Up @@ -338,6 +340,7 @@ var _ = Describe("SRIOV", func() {
hostMock.EXPECT().AddPersistPFNameUdevRule("0000:d8:00.0", "enp216s0f0np0").Return(nil)
hostMock.EXPECT().EnableHwTcOffload("enp216s0f0np0").Return(nil)
hostMock.EXPECT().GetDevlinkDeviceParam("0000:d8:00.0", "flow_steering_mode").Return("", syscall.EINVAL)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)
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)
Expand Down Expand Up @@ -407,6 +410,7 @@ var _ = Describe("SRIOV", func() {
hostMock.EXPECT().AddPersistPFNameUdevRule("0000:d8:00.0", "enp216s0f0np0").Return(nil)
hostMock.EXPECT().EnableHwTcOffload("enp216s0f0np0").Return(nil)
hostMock.EXPECT().GetDevlinkDeviceParam("0000:d8:00.0", "flow_steering_mode").Return("", syscall.EINVAL)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)
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)
Expand Down Expand Up @@ -461,6 +465,7 @@ var _ = Describe("SRIOV", func() {

It("externally managed - wrong VF count", func() {
dputilsLibMock.EXPECT().GetVFconfigured("0000:d8:00.0").Return(0)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)

Expect(s.ConfigSriovInterfaces(storeManagerMode,
[]sriovnetworkv1.Interface{{
Expand Down Expand Up @@ -488,6 +493,7 @@ var _ = Describe("SRIOV", func() {
nil)

hostMock.EXPECT().GetNetdevMTU("0000:d8:00.0")
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)
Expect(s.ConfigSriovInterfaces(storeManagerMode,
[]sriovnetworkv1.Interface{{
Name: "enp216s0f0np0",
Expand Down Expand Up @@ -577,6 +583,7 @@ var _ = Describe("SRIOV", func() {
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)
hostMock.EXPECT().Unbind("0000:d8:00.3").Return(nil)
hostMock.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0").Times(1)

storeManagerMode.EXPECT().SaveLastPfAppliedStatus(gomock.Any()).Return(nil)

Expand Down

0 comments on commit 4930922

Please sign in to comment.