From 0bdf464a07cac23c79ef3a2e2986bc97a8ea1e29 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 | 59 +++++++++++++----- pkg/host/internal/sriov/sriov_test.go | 25 ++++++-- pkg/host/internal/udev/udev.go | 42 +++++++++++-- pkg/host/internal/udev/udev_test.go | 90 ++++++++++++++++++++++++--- pkg/host/mock/mock_host.go | 66 ++++++++++++++++---- pkg/host/types/interfaces.go | 14 +++-- 9 files changed, 310 insertions(+), 62 deletions(-) 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 {