Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid using iface.Name from spec during configuration. #807

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions pkg/host/internal/sriov/sriov.go
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,17 +428,17 @@ 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
}

Expand Down Expand Up @@ -478,7 +478,7 @@ func (s *sriov) configSriovVFDevices(iface *sriovnetworkv1.Interface) error {
// if nothing is specified in the SriovNodePolicy
linkType := iface.LinkType
if linkType == "" {
linkType = s.GetLinkType(iface.Name)
linkType = s.GetLinkType(ifaceName)
}
if strings.EqualFold(linkType, consts.LinkTypeIB) {
if err := s.infinibandHelper.ConfigureVfGUID(addr, iface.PciAddress, vfID, pfLink); err != nil {
Expand Down Expand Up @@ -558,8 +558,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 +584,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 +939,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 +957,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
7 changes: 7 additions & 0 deletions pkg/host/internal/sriov/sriov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,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", "0000:d8:00.3"}, nil)
pfLinkMock := netlinkMockPkg.NewMockLink(testCtrl)
netlinkLibMock.EXPECT().LinkByName("enp216s0f0np0").Return(pfLinkMock, nil).Times(3)
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
Loading