diff --git a/api/v1/helper.go b/api/v1/helper.go index 9183030c5..53398af98 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -218,7 +218,13 @@ func GetVfDeviceID(deviceID string) string { } func IsSwitchdevModeSpec(spec SriovNetworkNodeStateSpec) bool { - for _, iface := range spec.Interfaces { + return ContainsSwitchdevInterface(spec.Interfaces) +} + +// ContainsSwitchdevInterface returns true if provided interface list contains interface +// with switchdev configuration +func ContainsSwitchdevInterface(interfaces []Interface) bool { + for _, iface := range interfaces { if iface.EswitchMode == ESwithModeSwitchDev { return true } diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index 78e463684..8dc071f97 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -99,6 +99,8 @@ const ( UdevDisableNM = "/bindata/scripts/udev-find-sriov-pf.sh" UdevRepName = "/bindata/scripts/switchdev-vf-link-name.sh" // nolint:goconst + PFNameUdevRule = `SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", KERNELS=="%s", NAME="%s"` + // nolint:goconst NMUdevRule = `SUBSYSTEM=="net", ` + `ACTION=="add|change|move", ` + `ATTRS{device}=="%s", ` + diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 853c5e954..4d8db7bf6 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -39,18 +39,32 @@ func (m *MockHostHelpersInterface) EXPECT() *MockHostHelpersInterfaceMockRecorde return m.recorder } -// AddUdevRule mocks base method. -func (m *MockHostHelpersInterface) AddUdevRule(pfPciAddress string) error { +// AddDisableNMUdevRule mocks base method. +func (m *MockHostHelpersInterface) AddDisableNMUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddUdevRule", pfPciAddress) + ret := m.ctrl.Call(m, "AddDisableNMUdevRule", pfPciAddress) ret0, _ := ret[0].(error) return ret0 } -// AddUdevRule indicates an expected call of AddUdevRule. -func (mr *MockHostHelpersInterfaceMockRecorder) AddUdevRule(pfPciAddress interface{}) *gomock.Call { +// AddDisableNMUdevRule indicates an expected call of AddDisableNMUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) AddDisableNMUdevRule(pfPciAddress interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).AddUdevRule), pfPciAddress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddDisableNMUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).AddDisableNMUdevRule), pfPciAddress) +} + +// AddPersistPFNameUdevRule mocks base method. +func (m *MockHostHelpersInterface) AddPersistPFNameUdevRule(pfPciAddress, pfName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddPersistPFNameUdevRule", pfPciAddress, pfName) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddPersistPFNameUdevRule indicates an expected call of AddPersistPFNameUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) AddPersistPFNameUdevRule(pfPciAddress, pfName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPersistPFNameUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).AddPersistPFNameUdevRule), pfPciAddress, pfName) } // AddVfRepresentorUdevRule mocks base method. @@ -712,6 +726,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) LoadPfsStatus(pciAddress interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadPfsStatus", reflect.TypeOf((*MockHostHelpersInterface)(nil).LoadPfsStatus), pciAddress) } +// LoadUdevRules mocks base method. +func (m *MockHostHelpersInterface) LoadUdevRules() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LoadUdevRules") + ret0, _ := ret[0].(error) + return ret0 +} + +// LoadUdevRules indicates an expected call of LoadUdevRules. +func (mr *MockHostHelpersInterfaceMockRecorder) LoadUdevRules() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadUdevRules", reflect.TypeOf((*MockHostHelpersInterface)(nil).LoadUdevRules)) +} + // MlxConfigFW mocks base method. func (m *MockHostHelpersInterface) MlxConfigFW(attributesToChange map[string]mlxutils.MlxNic) error { m.ctrl.T.Helper() @@ -858,18 +886,32 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ReloadDriver(driver interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadDriver", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReloadDriver), driver) } -// RemoveUdevRule mocks base method. -func (m *MockHostHelpersInterface) RemoveUdevRule(pfPciAddress string) error { +// RemoveDisableNMUdevRule mocks base method. +func (m *MockHostHelpersInterface) RemoveDisableNMUdevRule(pfPciAddress string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveDisableNMUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveDisableNMUdevRule indicates an expected call of RemoveDisableNMUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) RemoveDisableNMUdevRule(pfPciAddress interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDisableNMUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveDisableNMUdevRule), pfPciAddress) +} + +// RemovePersistPFNameUdevRule mocks base method. +func (m *MockHostHelpersInterface) RemovePersistPFNameUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RemoveUdevRule", pfPciAddress) + ret := m.ctrl.Call(m, "RemovePersistPFNameUdevRule", pfPciAddress) ret0, _ := ret[0].(error) return ret0 } -// RemoveUdevRule indicates an expected call of RemoveUdevRule. -func (mr *MockHostHelpersInterfaceMockRecorder) RemoveUdevRule(pfPciAddress interface{}) *gomock.Call { +// RemovePersistPFNameUdevRule indicates an expected call of RemovePersistPFNameUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) RemovePersistPFNameUdevRule(pfPciAddress interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveUdevRule), pfPciAddress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemovePersistPFNameUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemovePersistPFNameUdevRule), pfPciAddress) } // RemoveVfRepresentorUdevRule mocks base method. diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index a09231e14..65d18e09d 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -315,18 +315,25 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error { if err := s.configureHWOptionsForSwitchdev(iface); err != nil { return err } + // remove all UDEV rules for the PF before adding new rules to + // make sure that rules are always in a consistent state, e.g. there is no + // switchdev-related rules for PF in legacy mode + if err := s.removeUdevRules(iface.PciAddress); err != nil { + log.Log.Error(err, "configSriovPFDevice(): fail to remove udev rules", "device", iface.PciAddress) + return err + } err := s.addUdevRules(iface) if err != nil { - log.Log.Error(err, "configSriovPFDevice(): fail to set add udev rules", "device", iface.PciAddress) + log.Log.Error(err, "configSriovPFDevice(): fail to add udev rules", "device", iface.PciAddress) return err } err = s.createVFs(iface) if err != nil { log.Log.Error(err, "configSriovPFDevice(): fail to set NumVfs for device", "device", iface.PciAddress) - errRemove := s.removeUdevRules(iface.PciAddress) - if errRemove != nil { - log.Log.Error(errRemove, "configSriovPFDevice(): fail to remove udev rule", "device", iface.PciAddress) - } + return err + } + if err := s.addVfRepresentorUdevRule(iface); err != nil { + log.Log.Error(err, "configSriovPFDevice(): fail to add VR representor udev rule", "device", iface.PciAddress) return err } // set PF mtu @@ -596,6 +603,14 @@ func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface, log.Log.Error(err, "cannot configure sriov interfaces") return fmt.Errorf("cannot configure sriov interfaces") } + if sriovnetworkv1.ContainsSwitchdevInterface(interfaces) && len(toBeConfigured) > 0 { + // for switchdev devices we create udev rule that renames VF representors + // after VFs are created. Reload rules to update interfaces + if err := s.udevHelper.LoadUdevRules(); err != nil { + log.Log.Error(err, "cannot reload udev rules") + return fmt.Errorf("failed to reload udev rules: %v", err) + } + } if vars.ParallelNicConfig { err = s.resetSriovInterfacesInParallel(storeManager, toBeResetted) @@ -890,25 +905,38 @@ func (s *sriov) encapTypeToLinkType(encapType string) string { // create required udev rules for PF: // * rule to disable NetworkManager for VFs - for all modes -// * rule to rename VF representors - only for switchdev mode +// * rule to keep PF name after switching to switchdev mode - only for switchdev mode func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error { log.Log.V(2).Info("addUdevRules(): add udev rules for device", "device", iface.PciAddress) - if err := s.udevHelper.AddUdevRule(iface.PciAddress); err != nil { + 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 { + return err + } + } + return nil +} + +// add switchdev-specific udev rule that renames representors. +// 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 { if sriovnetworkv1.GetEswitchModeFromSpec(iface) == sriovnetworkv1.ESwithModeSwitchDev { portName, err := s.networkHelper.GetPhysPortName(iface.Name) if err != nil { - return err + 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) if err != nil { - return err - } - if err := s.udevHelper.AddVfRepresentorUdevRule(iface.PciAddress, iface.Name, switchID, portName); err != nil { - return err + 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 nil } @@ -917,10 +945,13 @@ func (s *sriov) addUdevRules(iface *sriovnetworkv1.Interface) error { func (s *sriov) removeUdevRules(pciAddress string) error { log.Log.V(2).Info("removeUdevRules(): remove udev rules for device", "device", pciAddress) - if err := s.udevHelper.RemoveUdevRule(pciAddress); err != nil { + if err := s.udevHelper.RemoveDisableNMUdevRule(pciAddress); err != nil { + return err + } + if err := s.udevHelper.RemoveVfRepresentorUdevRule(pciAddress); err != nil { return err } - return s.udevHelper.RemoveVfRepresentorUdevRule(pciAddress) + return s.udevHelper.RemovePersistPFNameUdevRule(pciAddress) } // create VFs on the PF diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go index 020a930cb..210b62237 100644 --- a/pkg/host/internal/sriov/sriov_test.go +++ b/pkg/host/internal/sriov/sriov_test.go @@ -114,7 +114,10 @@ var _ = Describe("SRIOV", func() { dputilsLibMock.EXPECT().GetVFconfigured("0000:d8:00.0").Return(0) netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(&netlink.DevlinkDevice{ Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}}, nil) - hostMock.EXPECT().AddUdevRule("0000:d8:00.0").Return(nil) + 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().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) @@ -179,7 +182,10 @@ var _ = Describe("SRIOV", func() { dputilsLibMock.EXPECT().GetVFconfigured("0000:d8:00.0").Return(0) netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return(&netlink.DevlinkDevice{ Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}}, nil) - hostMock.EXPECT().AddUdevRule("0000:d8:00.0").Return(nil) + 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().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) @@ -227,7 +233,11 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().IsKernelLockdownMode().Return(false) dputilsLibMock.EXPECT().GetSriovVFcapacity("0000:d8:00.0").Return(1) dputilsLibMock.EXPECT().GetVFconfigured("0000:d8:00.0").Return(0) - hostMock.EXPECT().AddUdevRule("0000:d8:00.0").Return(nil) + 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().AddDisableNMUdevRule("0000:d8:00.0").Return(nil) + 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) dputilsLibMock.EXPECT().GetVFList("0000:d8:00.0").Return([]string{"0000:d8:00.2"}, nil).Times(2) @@ -257,6 +267,7 @@ var _ = Describe("SRIOV", func() { hostMock.EXPECT().GetPhysSwitchID("enp216s0f0np0").Return("7cfe90ff2cc0", nil) hostMock.EXPECT().AddVfRepresentorUdevRule("0000:d8:00.0", "enp216s0f0np0", "7cfe90ff2cc0", "p0").Return(nil) hostMock.EXPECT().CreateVDPADevice("0000:d8:00.2", "vhost_vdpa") + hostMock.EXPECT().LoadUdevRules().Return(nil) storeManagerMode.EXPECT().SaveLastPfAppliedStatus(gomock.Any()).Return(nil) @@ -345,7 +356,8 @@ var _ = Describe("SRIOV", func() { netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return( &netlink.DevlinkDevice{Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}}, nil) - hostMock.EXPECT().RemoveUdevRule("0000:d8:00.0").Return(nil) + 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().SetNetdevMTU("0000:d8:00.0", 1500).Return(nil) @@ -391,7 +403,10 @@ var _ = Describe("SRIOV", func() { netlinkLibMock.EXPECT().DevLinkGetDeviceByName("pci", "0000:d8:00.0").Return( &netlink.DevlinkDevice{Attrs: netlink.DevlinkDevAttrs{Eswitch: netlink.DevlinkDevEswitchAttr{Mode: "legacy"}}}, nil) - hostMock.EXPECT().AddUdevRule("0000:d8:00.0").Return(nil) + 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().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) hostMock.EXPECT().Unbind("0000:d8:00.3").Return(nil) diff --git a/pkg/host/internal/udev/udev.go b/pkg/host/internal/udev/udev.go index 13602ae8e..3f828bb70 100644 --- a/pkg/host/internal/udev/udev.go +++ b/pkg/host/internal/udev/udev.go @@ -69,19 +69,32 @@ func (u *udev) PrepareVFRepUdevRule() error { return nil } -// AddUdevRule adds a udev rule that disables network-manager for VFs on the concrete PF -func (u *udev) AddUdevRule(pfPciAddress string) error { - log.Log.V(2).Info("AddUdevRule()", "device", pfPciAddress) +// AddDisableNMUdevRule adds udev rule that disables NetworkManager for VFs on the concrete PF: +func (u *udev) AddDisableNMUdevRule(pfPciAddress string) error { + log.Log.V(2).Info("AddDisableNMUdevRule()", "device", pfPciAddress) udevRuleContent := fmt.Sprintf(consts.NMUdevRule, strings.Join(vars.SupportedVfIds, "|"), pfPciAddress) return u.addUdevRule(pfPciAddress, "10-nm-disable", udevRuleContent) } -// RemoveUdevRule removes a udev rule that disables network-manager for VFs on the concrete PF -func (u *udev) RemoveUdevRule(pfPciAddress string) error { - log.Log.V(2).Info("RemoveUdevRule()", "device", pfPciAddress) +// RemoveDisableNMUdevRule removes udev rule that disables NetworkManager for VFs on the concrete PF +func (u *udev) RemoveDisableNMUdevRule(pfPciAddress string) error { + log.Log.V(2).Info("RemoveDisableNMUdevRule()", "device", pfPciAddress) return u.removeUdevRule(pfPciAddress, "10-nm-disable") } +// AddPersistPFNameUdevRule add udev rule that preserves PF name after switching to switchdev mode +func (u *udev) AddPersistPFNameUdevRule(pfPciAddress, pfName string) error { + log.Log.V(2).Info("AddPersistPFNameUdevRule()", "device", pfPciAddress) + udevRuleContent := fmt.Sprintf(consts.PFNameUdevRule, pfPciAddress, pfName) + return u.addUdevRule(pfPciAddress, "10-pf-name", udevRuleContent) +} + +// RemovePersistPFNameUdevRule removes udev rule that preserves PF name after switching to switchdev mode +func (u *udev) RemovePersistPFNameUdevRule(pfPciAddress string) error { + log.Log.V(2).Info("RemovePersistPFNameUdevRule()", "device", pfPciAddress) + return u.removeUdevRule(pfPciAddress, "10-pf-name") +} + // AddVfRepresentorUdevRule adds udev rule that renames VF representors on the concrete PF func (u *udev) AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort string) error { log.Log.V(2).Info("AddVfRepresentorUdevRule()", @@ -96,6 +109,23 @@ func (u *udev) RemoveVfRepresentorUdevRule(pfPciAddress string) error { return u.removeUdevRule(pfPciAddress, "20-switchdev") } +// LoadUdevRules triggers udev rules for network subsystem +func (u *udev) LoadUdevRules() error { + log.Log.V(2).Info("LoadUdevRules()") + udevAdmTool := "udevadm" + _, stderr, err := u.utilsHelper.RunCommand(udevAdmTool, "control", "--reload-rules") + if err != nil { + log.Log.Error(err, "LoadUdevRules(): failed to reload rules", "error", stderr) + return err + } + _, stderr, err = u.utilsHelper.RunCommand(udevAdmTool, "trigger", "--action", "add", "--attr-match", "subsystem=net") + if err != nil { + log.Log.Error(err, "LoadUdevRules(): failed to trigger rules", "error", stderr) + return err + } + return nil +} + func (u *udev) addUdevRule(pfPciAddress, ruleName, ruleContent string) error { log.Log.V(2).Info("addUdevRule()", "device", pfPciAddress, "rule", ruleName) rulePath := u.getRuleFolderPath() diff --git a/pkg/host/internal/udev/udev_test.go b/pkg/host/internal/udev/udev_test.go index e002aee4e..4a2e17e7e 100644 --- a/pkg/host/internal/udev/udev_test.go +++ b/pkg/host/internal/udev/udev_test.go @@ -1,19 +1,24 @@ package udev import ( + "fmt" "os" "path/filepath" + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + utilsMockPkg "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils/mock" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/fakefilesystem" "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/helpers" ) const ( + testExpectedPFUdevRule = `SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", KERNELS=="0000:d8:00.0", NAME="enp129"` testExpectedNMUdevRule = `SUBSYSTEM=="net", ACTION=="add|change|move", ` + `ATTRS{device}=="0x1017|0x1018", ` + `IMPORT{program}="/etc/udev/disable-nm-sriov.sh $env{INTERFACE} 0000:d8:00.0"` @@ -25,15 +30,26 @@ const ( var _ = Describe("UDEV", func() { var ( - s types.UdevInterface + s types.UdevInterface + testCtrl *gomock.Controller + utilsMock *utilsMockPkg.MockCmdInterface + testError = fmt.Errorf("test") ) + BeforeEach(func() { - s = New(nil) + testCtrl = gomock.NewController(GinkgoT()) + utilsMock = utilsMockPkg.NewMockCmdInterface(testCtrl) + s = New(utilsMock) }) - Context("AddUdevRule", func() { + + AfterEach(func() { + testCtrl.Finish() + }) + + Context("AddDisableNMUdevRule", func() { It("Created", func() { helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{}) - Expect(s.AddUdevRule("0000:d8:00.0")).To(BeNil()) + Expect(s.AddDisableNMUdevRule("0000:d8:00.0")).To(BeNil()) helpers.GinkgoAssertFileContentsEquals( "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules", testExpectedNMUdevRule) @@ -45,13 +61,13 @@ var _ = Describe("UDEV", func() { "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules": []byte("something"), }, }) - Expect(s.AddUdevRule("0000:d8:00.0")).To(BeNil()) + Expect(s.AddDisableNMUdevRule("0000:d8:00.0")).To(BeNil()) helpers.GinkgoAssertFileContentsEquals( "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules", testExpectedNMUdevRule) }) }) - Context("RemoveUdevRule", func() { + Context("RemoveDisableNMUdevRule", func() { It("Exist", func() { helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ Dirs: []string{"/etc/udev/rules.d"}, @@ -59,7 +75,7 @@ var _ = Describe("UDEV", func() { "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules": []byte(testExpectedNMUdevRule), }, }) - Expect(s.RemoveUdevRule("0000:d8:00.0")).To(BeNil()) + Expect(s.RemoveDisableNMUdevRule("0000:d8:00.0")).To(BeNil()) _, err := os.Stat(filepath.Join(vars.FilesystemRoot, "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules")) Expect(os.IsNotExist(err)).To(BeTrue()) @@ -68,7 +84,49 @@ var _ = Describe("UDEV", func() { helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ Dirs: []string{"/etc/udev/rules.d"}, }) - Expect(s.RemoveUdevRule("0000:d8:00.0")).To(BeNil()) + Expect(s.RemoveDisableNMUdevRule("0000:d8:00.0")).To(BeNil()) + }) + }) + Context("AddPersistPFNameUdevRule", func() { + It("Created", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{}) + Expect(s.AddPersistPFNameUdevRule("0000:d8:00.0", "enp129")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/10-pf-name-0000:d8:00.0.rules", + testExpectedPFUdevRule) + + }) + It("Overwrite", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "etc/udev/rules.d/10-pf-name-0000:d8:00.0.rules": []byte("something"), + }, + }) + Expect(s.AddPersistPFNameUdevRule("0000:d8:00.0", "enp129")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/10-pf-name-0000:d8:00.0.rules", + testExpectedPFUdevRule) + }) + }) + Context("RemovePersistPFNameUdevRule", func() { + It("Exist", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "/etc/udev/rules.d/10-pf-name-0000:d8:00.0.rules": []byte(testExpectedPFUdevRule), + }, + }) + Expect(s.RemovePersistPFNameUdevRule("0000:d8:00.0")).To(BeNil()) + _, err := os.Stat(filepath.Join(vars.FilesystemRoot, + "/etc/udev/rules.d/10-pf-name-0000:d8:00.0.rules")) + Expect(os.IsNotExist(err)).To(BeTrue()) + }) + It("Not found", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + }) + Expect(s.RemovePersistPFNameUdevRule("0000:d8:00.0")).To(BeNil()) }) }) Context("AddVfRepresentorUdevRule", func() { @@ -136,4 +194,20 @@ var _ = Describe("UDEV", func() { Expect(s.PrepareVFRepUdevRule()).NotTo(BeNil()) }) }) + Context("LoadUdevRules", func() { + It("Succeed", func() { + utilsMock.EXPECT().RunCommand("udevadm", "control", "--reload-rules").Return("", "", nil) + utilsMock.EXPECT().RunCommand("udevadm", "trigger", "--action", "add", "--attr-match", "subsystem=net").Return("", "", nil) + Expect(s.LoadUdevRules()).NotTo(HaveOccurred()) + }) + It("Failed to reload rules", func() { + utilsMock.EXPECT().RunCommand("udevadm", "control", "--reload-rules").Return("", "", testError) + Expect(s.LoadUdevRules()).To(MatchError(testError)) + }) + It("Failed to trigger rules", func() { + utilsMock.EXPECT().RunCommand("udevadm", "control", "--reload-rules").Return("", "", nil) + utilsMock.EXPECT().RunCommand("udevadm", "trigger", "--action", "add", "--attr-match", "subsystem=net").Return("", "", testError) + Expect(s.LoadUdevRules()).To(MatchError(testError)) + }) + }) }) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 188c2fb27..4ea2f0e8b 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -38,18 +38,32 @@ func (m *MockHostManagerInterface) EXPECT() *MockHostManagerInterfaceMockRecorde return m.recorder } -// AddUdevRule mocks base method. -func (m *MockHostManagerInterface) AddUdevRule(pfPciAddress string) error { +// AddDisableNMUdevRule mocks base method. +func (m *MockHostManagerInterface) AddDisableNMUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AddUdevRule", pfPciAddress) + ret := m.ctrl.Call(m, "AddDisableNMUdevRule", pfPciAddress) ret0, _ := ret[0].(error) return ret0 } -// AddUdevRule indicates an expected call of AddUdevRule. -func (mr *MockHostManagerInterfaceMockRecorder) AddUdevRule(pfPciAddress interface{}) *gomock.Call { +// AddDisableNMUdevRule indicates an expected call of AddDisableNMUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) AddDisableNMUdevRule(pfPciAddress interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).AddUdevRule), pfPciAddress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddDisableNMUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).AddDisableNMUdevRule), pfPciAddress) +} + +// AddPersistPFNameUdevRule mocks base method. +func (m *MockHostManagerInterface) AddPersistPFNameUdevRule(pfPciAddress, pfName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddPersistPFNameUdevRule", pfPciAddress, pfName) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddPersistPFNameUdevRule indicates an expected call of AddPersistPFNameUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) AddPersistPFNameUdevRule(pfPciAddress, pfName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddPersistPFNameUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).AddPersistPFNameUdevRule), pfPciAddress, pfName) } // AddVfRepresentorUdevRule mocks base method. @@ -620,6 +634,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) LoadKernelModule(name interface{ return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadKernelModule", reflect.TypeOf((*MockHostManagerInterface)(nil).LoadKernelModule), varargs...) } +// LoadUdevRules mocks base method. +func (m *MockHostManagerInterface) LoadUdevRules() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LoadUdevRules") + ret0, _ := ret[0].(error) + return ret0 +} + +// LoadUdevRules indicates an expected call of LoadUdevRules. +func (mr *MockHostManagerInterfaceMockRecorder) LoadUdevRules() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LoadUdevRules", reflect.TypeOf((*MockHostManagerInterface)(nil).LoadUdevRules)) +} + // PrepareNMUdevRule mocks base method. func (m *MockHostManagerInterface) PrepareNMUdevRule(supportedVfIds []string) error { m.ctrl.T.Helper() @@ -736,18 +764,32 @@ func (mr *MockHostManagerInterfaceMockRecorder) ReloadDriver(driver interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadDriver", reflect.TypeOf((*MockHostManagerInterface)(nil).ReloadDriver), driver) } -// RemoveUdevRule mocks base method. -func (m *MockHostManagerInterface) RemoveUdevRule(pfPciAddress string) error { +// RemoveDisableNMUdevRule mocks base method. +func (m *MockHostManagerInterface) RemoveDisableNMUdevRule(pfPciAddress string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveDisableNMUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveDisableNMUdevRule indicates an expected call of RemoveDisableNMUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) RemoveDisableNMUdevRule(pfPciAddress interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDisableNMUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveDisableNMUdevRule), pfPciAddress) +} + +// RemovePersistPFNameUdevRule mocks base method. +func (m *MockHostManagerInterface) RemovePersistPFNameUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "RemoveUdevRule", pfPciAddress) + ret := m.ctrl.Call(m, "RemovePersistPFNameUdevRule", pfPciAddress) ret0, _ := ret[0].(error) return ret0 } -// RemoveUdevRule indicates an expected call of RemoveUdevRule. -func (mr *MockHostManagerInterfaceMockRecorder) RemoveUdevRule(pfPciAddress interface{}) *gomock.Call { +// RemovePersistPFNameUdevRule indicates an expected call of RemovePersistPFNameUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) RemovePersistPFNameUdevRule(pfPciAddress interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveUdevRule), pfPciAddress) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemovePersistPFNameUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemovePersistPFNameUdevRule), pfPciAddress) } // RemoveVfRepresentorUdevRule mocks base method. diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 05fc6a6fb..48d47b424 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -165,14 +165,20 @@ type UdevInterface interface { PrepareNMUdevRule(supportedVfIds []string) error // PrepareVFRepUdevRule creates a script which helps to configure representor name for the VF PrepareVFRepUdevRule() error - // AddUdevRule adds a udev rule that disables network-manager for VFs on the concrete PF - AddUdevRule(pfPciAddress string) error - // RemoveUdevRule removes a udev rule that disables network-manager for VFs on the concrete PF - RemoveUdevRule(pfPciAddress string) error + // AddDisableNMUdevRule adds udev rule that disables NetworkManager for VFs on the concrete PF: + AddDisableNMUdevRule(pfPciAddress string) error + // RemoveDisableNMUdevRule removes udev rule that disables NetworkManager for VFs on the concrete PF + RemoveDisableNMUdevRule(pfPciAddress string) error + // AddPersistPFNameUdevRule add udev rule that preserves PF name after switching to switchdev mode + AddPersistPFNameUdevRule(pfPciAddress, pfName string) error + // RemovePersistPFNameUdevRule removes udev rule that preserves PF name after switching to switchdev mode + RemovePersistPFNameUdevRule(pfPciAddress string) error // AddVfRepresentorUdevRule adds udev rule that renames VF representors on the concrete PF AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort string) error // RemoveVfRepresentorUdevRule removes udev rule that renames VF representors on the concrete PF RemoveVfRepresentorUdevRule(pfPciAddress string) error + // LoadUdevRules triggers udev rules for network subsystem + LoadUdevRules() error } type VdpaInterface interface {