From c5a086f636060601d3d68e454c7527b4894245cb Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Wed, 20 Mar 2024 20:49:10 +0200 Subject: [PATCH] Change UDEV rules for switchdev mode The operator will create two rules specific for PFs in swithcdev mode: - rule to persist PF name after switching to switchdev mode - rule to rename VF representors First rule can be applied before PF is switched to switchdev mode. Second rule can be applied only when the PF is already in switchdev mode(phys_port_name and phys_switch_id) may not be available when PF is in legacy mode. reload of UDEV rules is required to apply VF representor rule for already created VFs. Signed-off-by: Yury Kulazhenkov --- api/v1/helper.go | 8 ++- pkg/consts/constants.go | 2 + pkg/helper/mock/mock_helper.go | 66 ++++++++++++++++---- pkg/host/internal/sriov/sriov.go | 48 +++++++++++--- pkg/host/internal/sriov/sriov_test.go | 13 ++-- pkg/host/internal/udev/udev.go | 41 ++++++++++-- pkg/host/internal/udev/udev_test.go | 90 ++++++++++++++++++++++++--- pkg/host/mock/mock_host.go | 66 ++++++++++++++++---- pkg/host/types/interfaces.go | 14 +++-- 9 files changed, 290 insertions(+), 58 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 9183030c59..53398af980 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 78e4636842..8dc071f974 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 853c5e9545..4d8db7bf68 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 a09231e14a..02ce58522f 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -317,7 +317,7 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error { } 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) @@ -329,6 +329,10 @@ func (s *sriov) configSriovPFDevice(iface *sriovnetworkv1.Interface) error { } 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 if iface.Mtu > 0 && iface.Mtu > s.networkHelper.GetNetdevMTU(iface.PciAddress) { err = s.networkHelper.SetNetdevMTU(iface.PciAddress, iface.Mtu) @@ -596,6 +600,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 +902,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 +942,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 020a930cb6..0db424b833 100644 --- a/pkg/host/internal/sriov/sriov_test.go +++ b/pkg/host/internal/sriov/sriov_test.go @@ -114,7 +114,7 @@ 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().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 +179,7 @@ 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().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 +227,8 @@ 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().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 +258,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 +347,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 +394,7 @@ 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().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 13602ae8ef..774b46fd35 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,22 @@ 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 { + udevAdmTool := "udevadm" + _, _, err := u.utilsHelper.RunCommand(udevAdmTool, "control", "--reload-rules") + if err != nil { + log.Log.Error(err, "LoadUdevRules(): failed to reload rules") + return err + } + _, _, err = u.utilsHelper.RunCommand(udevAdmTool, "--action", "add", "--attr-match", "subsystem=net") + if err != nil { + log.Log.Error(err, "LoadUdevRules(): failed to trigger rules") + 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 e002aee4ee..36691d4251 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", "--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", "--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 188c2fb274..4ea2f0e8bf 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 05fc6a6fb0..48d47b4245 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 {