diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index e7255368d..d3ddd3f6d 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -92,7 +92,18 @@ const ( UdevRulesFolder = UdevFolder + "/rules.d" HostUdevRulesFolder = Host + UdevRulesFolder UdevDisableNM = "/bindata/scripts/udev-find-sriov-pf.sh" - NMUdevRule = "SUBSYSTEM==\"net\", ACTION==\"add|change|move\", ATTRS{device}==\"%s\", IMPORT{program}=\"/etc/udev/disable-nm-sriov.sh $env{INTERFACE} %s\"" + // nolint:goconst + NMUdevRule = `SUBSYSTEM=="net", ` + + `ACTION=="add|change|move", ` + + `ATTRS{device}=="%s", ` + + `IMPORT{program}="/etc/udev/disable-nm-sriov.sh $env{INTERFACE} %s"` + // nolint:goconst + SwitchdevUdevRule = `SUBSYSTEM=="net", ` + + `ACTION=="add|move", ` + + `ATTRS{phys_switch_id}=="%s", ` + + `ATTR{phys_port_name}=="pf%svf*", ` + + `IMPORT{program}="/etc/udev/switchdev-vf-link-name.sh $attr{phys_port_name}", ` + + `NAME="%s_$env{NUMBER}"` KernelArgPciRealloc = "pci=realloc" KernelArgIntelIommu = "intel_iommu=on" diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index b77f46720..a8e417e49 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -54,6 +54,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) AddUdevRule(pfPciAddress interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).AddUdevRule), pfPciAddress) } +// AddVfRepresentorUdevRule mocks base method. +func (m *MockHostHelpersInterface) AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddVfRepresentorUdevRule", pfPciAddress, pfName, pfSwitchID, pfSwitchPort) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddVfRepresentorUdevRule indicates an expected call of AddVfRepresentorUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddVfRepresentorUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).AddVfRepresentorUdevRule), pfPciAddress, pfName, pfSwitchID, pfSwitchPort) +} + // BindDefaultDriver mocks base method. func (m *MockHostHelpersInterface) BindDefaultDriver(pciAddr string) error { m.ctrl.T.Helper() @@ -851,6 +865,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) RemoveUdevRule(pfPciAddress inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveUdevRule), pfPciAddress) } +// RemoveVfRepresentorUdevRule mocks base method. +func (m *MockHostHelpersInterface) RemoveVfRepresentorUdevRule(pfPciAddress string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveVfRepresentorUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveVfRepresentorUdevRule indicates an expected call of RemoveVfRepresentorUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) RemoveVfRepresentorUdevRule(pfPciAddress interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveVfRepresentorUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveVfRepresentorUdevRule), pfPciAddress) +} + // ResetSriovDevice mocks base method. func (m *MockHostHelpersInterface) ResetSriovDevice(ifaceStatus v1.InterfaceExt) error { m.ctrl.T.Helper() diff --git a/pkg/host/internal/udev/suite_test.go b/pkg/host/internal/udev/suite_test.go new file mode 100644 index 000000000..34a8820cf --- /dev/null +++ b/pkg/host/internal/udev/suite_test.go @@ -0,0 +1,30 @@ +package udev + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "go.uber.org/zap/zapcore" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" +) + +func TestUdev(t *testing.T) { + log.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.Level(zapcore.Level(-2)), + zap.UseDevMode(true))) + RegisterFailHandler(Fail) + RunSpecs(t, "Package Udev Suite") +} + +var _ = BeforeSuite(func() { + vars.SupportedVfIds = []string{"0x1017", "0x1018"} + DeferCleanup(func() { + vars.SupportedVfIds = []string{} + }) +}) diff --git a/pkg/host/internal/udev/udev.go b/pkg/host/internal/udev/udev.go index dad88913e..2b4c4c6b3 100644 --- a/pkg/host/internal/udev/udev.go +++ b/pkg/host/internal/udev/udev.go @@ -146,34 +146,64 @@ func (u *udev) WriteSwitchdevConfFile(newState *sriovnetworkv1.SriovNetworkNodeS return true, 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) - pathFile := filepath.Join(vars.FilesystemRoot, consts.UdevRulesFolder) 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) + return u.removeUdevRule(pfPciAddress, "10-nm-disable") +} + +// 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()", + "device", pfPciAddress, "name", pfName, "switch", pfSwitchID, "port", pfSwitchPort) + udevRuleContent := fmt.Sprintf(consts.SwitchdevUdevRule, pfSwitchID, strings.TrimPrefix(pfSwitchPort, "p"), pfName) + return u.addUdevRule(pfPciAddress, "20-switchdev", udevRuleContent) +} + +// RemoveVfRepresentorUdevRule removes udev rule that renames VF representors on the concrete PF +func (u *udev) RemoveVfRepresentorUdevRule(pfPciAddress string) error { + log.Log.V(2).Info("RemoveVfRepresentorUdevRule()", "device", pfPciAddress) + return u.removeUdevRule(pfPciAddress, "20-switchdev") +} - err := os.MkdirAll(pathFile, os.ModePerm) +func (u *udev) addUdevRule(pfPciAddress, ruleName, ruleContent string) error { + log.Log.V(2).Info("addUdevRule()", "device", pfPciAddress, "rule", ruleName) + rulePath := u.getRuleFolderPath() + err := os.MkdirAll(rulePath, os.ModePerm) if err != nil && !os.IsExist(err) { - log.Log.Error(err, "AddUdevRule(): failed to create dir", "path", pathFile) + log.Log.Error(err, "ensureUdevRulePathExist(): failed to create dir", "path", rulePath) return err } - - filePath := path.Join(pathFile, fmt.Sprintf("10-nm-disable-%s.rules", pfPciAddress)) - // if the file does not exist or if oldContent != newContent - // write to file and create it if it doesn't exist - err = os.WriteFile(filePath, []byte(udevRuleContent), 0666) - if err != nil { - log.Log.Error(err, "AddUdevRule(): fail to write file", "path", filePath) + filePath := u.getRulePathForPF(ruleName, pfPciAddress) + if err := os.WriteFile(filePath, []byte(ruleContent), 0666); err != nil { + log.Log.Error(err, "addUdevRule(): fail to write file", "path", filePath) return err } return nil } -func (u *udev) RemoveUdevRule(pfPciAddress string) error { - pathFile := filepath.Join(vars.FilesystemRoot, consts.UdevRulesFolder) - filePath := path.Join(pathFile, fmt.Sprintf("10-nm-disable-%s.rules", pfPciAddress)) - err := os.Remove(filePath) +func (u *udev) removeUdevRule(pfPciAddress, ruleName string) error { + log.Log.V(2).Info("removeUdevRule()", "device", pfPciAddress, "rule", ruleName) + rulePath := u.getRulePathForPF(ruleName, pfPciAddress) + err := os.Remove(rulePath) if err != nil && !os.IsNotExist(err) { + log.Log.Error(err, "removeUdevRule(): fail to remove rule file", "path", rulePath) return err } return nil } + +func (u *udev) getRuleFolderPath() string { + return filepath.Join(vars.FilesystemRoot, consts.UdevRulesFolder) +} + +func (u *udev) getRulePathForPF(ruleName, pfPciAddress string) string { + return path.Join(u.getRuleFolderPath(), fmt.Sprintf("%s-%s.rules", ruleName, pfPciAddress)) +} diff --git a/pkg/host/internal/udev/udev_test.go b/pkg/host/internal/udev/udev_test.go new file mode 100644 index 000000000..d5f222385 --- /dev/null +++ b/pkg/host/internal/udev/udev_test.go @@ -0,0 +1,117 @@ +package udev + +import ( + "os" + "path/filepath" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + "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 ( + 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"` + testExpectedSwitchdevUdevRule = `SUBSYSTEM=="net", ACTION=="add|move", ` + + `ATTRS{phys_switch_id}=="7cfe90ff2cc0", ` + + `ATTR{phys_port_name}=="pf0vf*", IMPORT{program}="/etc/udev/switchdev-vf-link-name.sh $attr{phys_port_name}", ` + + `NAME="enp216s0f0np0_$env{NUMBER}"` +) + +var _ = Describe("UDEV", func() { + var ( + s types.UdevInterface + ) + BeforeEach(func() { + s = New(nil) + }) + Context("AddUdevRule", func() { + It("Created", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{}) + Expect(s.AddUdevRule("0000:d8:00.0")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules", + testExpectedNMUdevRule) + }) + It("Overwrite", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules": []byte("something"), + }, + }) + Expect(s.AddUdevRule("0000:d8:00.0")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules", + testExpectedNMUdevRule) + }) + }) + Context("RemoveUdevRule", func() { + It("Exist", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "/etc/udev/rules.d/10-nm-disable-0000:d8:00.0.rules": []byte(testExpectedNMUdevRule), + }, + }) + Expect(s.RemoveUdevRule("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()) + }) + It("Not found", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + }) + Expect(s.RemoveUdevRule("0000:d8:00.0")).To(BeNil()) + }) + }) + Context("AddVfRepresentorUdevRule", func() { + It("Created", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{}) + Expect(s.AddVfRepresentorUdevRule("0000:d8:00.0", + "enp216s0f0np0", "7cfe90ff2cc0", "p0")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/20-switchdev-0000:d8:00.0.rules", + testExpectedSwitchdevUdevRule) + }) + It("Overwrite", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "/etc/udev/rules.d/20-switchdev-0000:d8:00.0.rules": []byte("something"), + }, + }) + Expect(s.AddVfRepresentorUdevRule("0000:d8:00.0", + "enp216s0f0np0", "7cfe90ff2cc0", "p0")).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals( + "/etc/udev/rules.d/20-switchdev-0000:d8:00.0.rules", + testExpectedSwitchdevUdevRule) + }) + }) + Context("RemoveVfRepresentorUdevRule", func() { + It("Exist", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/etc/udev/rules.d"}, + Files: map[string][]byte{ + "/etc/udev/rules.d/20-switchdev-0000:d8:00.0.rules": []byte(testExpectedSwitchdevUdevRule), + }, + }) + Expect(s.RemoveVfRepresentorUdevRule("0000:d8:00.0")).To(BeNil()) + _, err := os.Stat(filepath.Join(vars.FilesystemRoot, + "/etc/udev/rules.d/20-switchdev-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.RemoveVfRepresentorUdevRule("0000:d8:00.0")).To(BeNil()) + }) + }) +}) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 459ac594e..ddba8626e 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -53,6 +53,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) AddUdevRule(pfPciAddress interfa return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).AddUdevRule), pfPciAddress) } +// AddVfRepresentorUdevRule mocks base method. +func (m *MockHostManagerInterface) AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddVfRepresentorUdevRule", pfPciAddress, pfName, pfSwitchID, pfSwitchPort) + ret0, _ := ret[0].(error) + return ret0 +} + +// AddVfRepresentorUdevRule indicates an expected call of AddVfRepresentorUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) AddVfRepresentorUdevRule(pfPciAddress, pfName, pfSwitchID, pfSwitchPort interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddVfRepresentorUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).AddVfRepresentorUdevRule), pfPciAddress, pfName, pfSwitchID, pfSwitchPort) +} + // BindDefaultDriver mocks base method. func (m *MockHostManagerInterface) BindDefaultDriver(pciAddr string) error { m.ctrl.T.Helper() @@ -729,6 +743,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) RemoveUdevRule(pfPciAddress inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveUdevRule), pfPciAddress) } +// RemoveVfRepresentorUdevRule mocks base method. +func (m *MockHostManagerInterface) RemoveVfRepresentorUdevRule(pfPciAddress string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveVfRepresentorUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveVfRepresentorUdevRule indicates an expected call of RemoveVfRepresentorUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) RemoveVfRepresentorUdevRule(pfPciAddress interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveVfRepresentorUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveVfRepresentorUdevRule), pfPciAddress) +} + // ResetSriovDevice mocks base method. func (m *MockHostManagerInterface) ResetSriovDevice(ifaceStatus v1.InterfaceExt) error { m.ctrl.T.Helper() diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 1edea916e..6a12f29cd 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -157,10 +157,14 @@ type UdevInterface interface { // PrepareNMUdevRule creates the needed udev rules to disable NetworkManager from // our managed SR-IOV virtual functions PrepareNMUdevRule(supportedVfIds []string) error - // AddUdevRule adds a specific udev rule to the system + // AddUdevRule adds a udev rule that disables network-manager for VFs on the concrete PF AddUdevRule(pfPciAddress string) error - // RemoveUdevRule removes a udev rule from the system + // RemoveUdevRule removes a udev rule that disables network-manager for VFs on the concrete PF RemoveUdevRule(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 } type VdpaInterface interface {