From 61d780ad0261ca869fe7660a72e3ae24a7999000 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Mon, 5 Feb 2024 18:41:00 +0200 Subject: [PATCH 1/7] Update sriov.GetVfInfo to report VDPA type Signed-off-by: Yury Kulazhenkov --- pkg/host/internal/sriov/sriov.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 45ce30fa1..9a654c439 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -122,6 +122,7 @@ func (s *sriov) GetVfInfo(pciAddr string, devices []*ghw.PCIDevice) sriovnetwork PciAddress: pciAddr, Driver: driver, VfID: id, + VdpaType: s.vdpaHelper.DiscoverVDPAType(pciAddr), } if name := s.networkHelper.TryGetInterfaceName(pciAddr); name != "" { From 253edc817c0b8f885a39c3cda47e86717f821c0f Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Fri, 23 Feb 2024 11:42:18 +0200 Subject: [PATCH 2/7] Add switchdev-related checks to api.v1.NeedToUpdateSriov function Additional checks to perform: - Eswitch mode - VdpaType for VFs Signed-off-by: Yury Kulazhenkov --- api/v1/helper.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index cb0a5a7c2..9183030c5 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -259,7 +259,12 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { return true } } - + currentEswitchMode := GetEswitchModeFromStatus(ifaceStatus) + desiredEswitchMode := GetEswitchModeFromSpec(ifaceSpec) + if currentEswitchMode != desiredEswitchMode { + log.V(2).Info("NeedToUpdateSriov(): EswitchMode needs update", "desired", desiredEswitchMode, "current", currentEswitchMode) + return true + } if ifaceSpec.NumVfs != ifaceStatus.NumVfs { log.V(2).Info("NeedToUpdateSriov(): NumVfs needs update", "desired", ifaceSpec.NumVfs, "current", ifaceStatus.NumVfs) return true @@ -300,11 +305,18 @@ func NeedToUpdateSriov(ifaceSpec *Interface, ifaceStatus *InterfaceExt) bool { return true } } + if groupSpec.VdpaType != vfStatus.VdpaType { + log.V(2).Info("NeedToUpdateSriov(): VF VdpaType mismatch", + "desired", groupSpec.VdpaType, "current", vfStatus.VdpaType) + return true + } break } } - if !ingroup && StringInArray(vfStatus.Driver, vars.DpdkDrivers) { - // VF which has DPDK driver loaded but not in any group, needs to be reset to default driver. + if !ingroup && (StringInArray(vfStatus.Driver, vars.DpdkDrivers) || vfStatus.VdpaType != "") { + // need to reset VF if it is not a part of a group and: + // a. has DPDK driver loaded + // b. has VDPA device return true } } From 309a38d167b1afa7418253765ac192899032f065 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Fri, 23 Feb 2024 11:48:26 +0200 Subject: [PATCH 3/7] Create VF representor udev script in the daemon.go PrepareVFRepUdevRule() function creates helper scrip for udev rules that rename VF representors in switchdev mode. Signed-off-by: Yury Kulazhenkov --- bindata/scripts/switchdev-vf-link-name.sh | 4 ++++ pkg/consts/constants.go | 2 ++ pkg/daemon/daemon.go | 3 +++ pkg/daemon/daemon_test.go | 1 + pkg/helper/mock/mock_helper.go | 14 ++++++++++++++ pkg/host/internal/udev/udev.go | 20 ++++++++++++++++++++ pkg/host/internal/udev/udev_test.go | 22 ++++++++++++++++++++++ pkg/host/mock/mock_host.go | 14 ++++++++++++++ pkg/host/types/interfaces.go | 2 ++ 9 files changed, 82 insertions(+) create mode 100644 bindata/scripts/switchdev-vf-link-name.sh diff --git a/bindata/scripts/switchdev-vf-link-name.sh b/bindata/scripts/switchdev-vf-link-name.sh new file mode 100644 index 000000000..b23ca327c --- /dev/null +++ b/bindata/scripts/switchdev-vf-link-name.sh @@ -0,0 +1,4 @@ +#!/bin/bash +set -x +PORT="$1" +echo "NUMBER=${PORT##pf*vf}" diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index 86af30bef..78e463684 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -93,9 +93,11 @@ const ( BusVdpa = "vdpa" UdevFolder = "/etc/udev" + HostUdevFolder = Host + UdevFolder UdevRulesFolder = UdevFolder + "/rules.d" HostUdevRulesFolder = Host + UdevRulesFolder UdevDisableNM = "/bindata/scripts/udev-find-sriov-pf.sh" + UdevRepName = "/bindata/scripts/switchdev-vf-link-name.sh" // nolint:goconst NMUdevRule = `SUBSYSTEM=="net", ` + `ACTION=="add|change|move", ` + diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index fce5cf57e..e977f4215 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -156,6 +156,9 @@ func (dn *Daemon) Run(stopCh <-chan struct{}, exitCh <-chan error) error { if err := dn.prepareNMUdevRule(); err != nil { log.Log.Error(err, "failed to prepare udev files to disable network manager on requested VFs") } + if err := dn.HostHelpers.PrepareVFRepUdevRule(); err != nil { + log.Log.Error(err, "failed to prepare udev files to rename VF representors for requested VFs") + } if err := dn.tryCreateSwitchdevUdevRule(); err != nil { log.Log.Error(err, "failed to create udev files for switchdev") } diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index 253c29112..54b75e3e8 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -149,6 +149,7 @@ var _ = Describe("Config Daemon", func() { vendorHelper.EXPECT().TryEnableVhostNet().AnyTimes() vendorHelper.EXPECT().TryEnableTun().AnyTimes() vendorHelper.EXPECT().PrepareNMUdevRule([]string{"0x1014", "0x154c"}).Return(nil).AnyTimes() + vendorHelper.EXPECT().PrepareVFRepUdevRule().Return(nil).AnyTimes() sut = New( kClient, diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 4a02803c9..643c93c82 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -757,6 +757,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) PrepareNMUdevRule(supportedVfIds return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrepareNMUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).PrepareNMUdevRule), supportedVfIds) } +// PrepareVFRepUdevRule mocks base method. +func (m *MockHostHelpersInterface) PrepareVFRepUdevRule() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PrepareVFRepUdevRule") + ret0, _ := ret[0].(error) + return ret0 +} + +// PrepareVFRepUdevRule indicates an expected call of PrepareVFRepUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) PrepareVFRepUdevRule() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrepareVFRepUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).PrepareVFRepUdevRule)) +} + // RdmaIsLoaded mocks base method. func (m *MockHostHelpersInterface) RdmaIsLoaded() (bool, error) { m.ctrl.T.Helper() diff --git a/pkg/host/internal/udev/udev.go b/pkg/host/internal/udev/udev.go index 2b4c4c6b3..a8506e4ae 100644 --- a/pkg/host/internal/udev/udev.go +++ b/pkg/host/internal/udev/udev.go @@ -56,6 +56,26 @@ func (u *udev) PrepareNMUdevRule(supportedVfIds []string) error { return nil } +// PrepareVFRepUdevRule creates a script which helps to configure representor name for the VF +func (u *udev) PrepareVFRepUdevRule() error { + log.Log.V(2).Info("PrepareVFRepUdevRule()") + targetPath := filepath.Join(vars.FilesystemRoot, consts.HostUdevFolder, filepath.Base(consts.UdevRepName)) + data, err := os.ReadFile(filepath.Join(vars.FilesystemRoot, consts.UdevRepName)) + if err != nil { + log.Log.Error(err, "PrepareVFRepUdevRule(): failed to read source for representor name UDEV script") + return err + } + if err := os.WriteFile(targetPath, data, 0755); err != nil { + log.Log.Error(err, "PrepareVFRepUdevRule(): failed to write representor name UDEV script") + return err + } + if err := os.Chmod(targetPath, 0755); err != nil { + log.Log.Error(err, "PrepareVFRepUdevRule(): failed to set permissions on representor name UDEV script") + return err + } + return nil +} + func (u *udev) WriteSwitchdevConfFile(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToSkip map[string]bool) (bool, error) { cfg := config{} for _, iface := range newState.Spec.Interfaces { diff --git a/pkg/host/internal/udev/udev_test.go b/pkg/host/internal/udev/udev_test.go index d5f222385..e002aee4e 100644 --- a/pkg/host/internal/udev/udev_test.go +++ b/pkg/host/internal/udev/udev_test.go @@ -114,4 +114,26 @@ var _ = Describe("UDEV", func() { Expect(s.RemoveVfRepresentorUdevRule("0000:d8:00.0")).To(BeNil()) }) }) + Context("PrepareVFRepUdevRule", func() { + It("Already Exist", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/host/etc/udev", "/bindata/scripts"}, + Files: map[string][]byte{ + "/host/etc/udev/switchdev-vf-link-name.sh": []byte("before"), + "/bindata/scripts/switchdev-vf-link-name.sh": []byte("script"), + }, + }) + Expect(s.PrepareVFRepUdevRule()).To(BeNil()) + helpers.GinkgoAssertFileContentsEquals("/host/etc/udev/switchdev-vf-link-name.sh", "script") + }) + It("Fail - no folder", func() { + helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ + Dirs: []string{"/bindata/scripts"}, + Files: map[string][]byte{ + "/bindata/scripts/switchdev-vf-link-name.sh": []byte("script"), + }, + }) + Expect(s.PrepareVFRepUdevRule()).NotTo(BeNil()) + }) + }) }) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 4bc8ee630..e3df6a71d 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -635,6 +635,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) PrepareNMUdevRule(supportedVfIds return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrepareNMUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).PrepareNMUdevRule), supportedVfIds) } +// PrepareVFRepUdevRule mocks base method. +func (m *MockHostManagerInterface) PrepareVFRepUdevRule() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PrepareVFRepUdevRule") + ret0, _ := ret[0].(error) + return ret0 +} + +// PrepareVFRepUdevRule indicates an expected call of PrepareVFRepUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) PrepareVFRepUdevRule() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrepareVFRepUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).PrepareVFRepUdevRule)) +} + // RdmaIsLoaded mocks base method. func (m *MockHostManagerInterface) RdmaIsLoaded() (bool, error) { m.ctrl.T.Helper() diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 248d6d71e..92004fb84 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -170,6 +170,8 @@ type UdevInterface interface { // PrepareNMUdevRule creates the needed udev rules to disable NetworkManager from // our managed SR-IOV virtual functions 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 From 18a93cf5e73a1f9a28248f6846564e1635fa485c Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Mon, 5 Feb 2024 16:59:47 +0200 Subject: [PATCH 4/7] Remove deployment of old switchdev implementation This commit removes services and scripts related to old switchdev implementations Signed-off-by: Yury Kulazhenkov --- .../switchdev-configuration-after-nm.sh.yaml | 76 --- .../switchdev-configuration-before-nm.sh.yaml | 78 --- .../files/switchdev-vf-link-name.sh.yaml | 9 - .../switchdev-config/machineconfigpool.yaml | 12 - .../NetworkManager.service.yaml | 9 - .../switchdev-configuration-after-nm.yaml | 19 - .../switchdev-configuration-before-nm.yaml | 19 - bindata/scripts/clean-k8s-services.sh | 34 +- pkg/helper/mock/mock_helper.go | 36 -- pkg/host/internal/service/service.go | 48 +- pkg/host/mock/mock_host.go | 36 -- pkg/host/types/interfaces.go | 7 +- pkg/host/types/types.go | 13 - pkg/plugins/k8s/k8s_plugin.go | 448 +++++------------- pkg/plugins/k8s/k8s_plugin_test.go | 69 ++- pkg/render/render.go | 36 +- 16 files changed, 182 insertions(+), 767 deletions(-) delete mode 100644 bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml delete mode 100644 bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml delete mode 100644 bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml delete mode 100644 bindata/manifests/switchdev-config/machineconfigpool.yaml delete mode 100644 bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml delete mode 100644 bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml delete mode 100644 bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml diff --git a/bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml b/bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml deleted file mode 100644 index 00a4019b4..000000000 --- a/bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml +++ /dev/null @@ -1,76 +0,0 @@ -mode: 0755 -overwrite: true -path: "/usr/local/bin/switchdev-configuration-after-nm.sh" -contents: - inline: | - #!/bin/bash - set -eux - input="/etc/sriov-operator/sriov_config.json" - - minId=-1 - maxId=-1 - - extract_min_max_ids() { - range=$(jq -c '.vfRange' -r <<< $group) - ids=(${range//-/ }) - length=${#ids[@]} - minId=-1 - maxId=-1 - if [[ length -eq 2 ]]; then - minId=${ids[0]} - maxId=${ids[1]} - elif [[ length -eq 1 ]]; then - minId=${ids[0]} - maxId=$minId - fi - } - - if [ ! -f $input ]; then - echo "File /etc/sriov-operator/sriov_config.json not exist." - exit - fi - - # Required for NetworkManager configuration(e.g. bond) to settle down - sleep 3 - - jq -c '.interfaces[]' $input | while read iface; - do - eswitch_mode=$(echo $iface | jq '.eSwitchMode' -r) - if [[ "$eswitch_mode" == "switchdev" ]]; then - pci_addr=$(echo $iface | jq '.pciAddress' -r) - name=$(echo $iface | jq '.name' -r) - - echo "load VF driver for $pci_addr" - VfDirs=$(ls /sys/bus/pci/devices/${pci_addr} | grep virtfn) - - # load VF driver and configure vdpa if needed - for VfDir in $VfDirs - do - VfPciAddr=$(basename "$( readlink -f /sys/bus/pci/devices/${pci_addr}/$VfDir )") - echo $VfPciAddr > /sys/bus/pci/drivers_probe - - # extract VF id from a path like '/sys/bus/pci/devices/0000:65:00.0/virtfn1' - # the VF id is the 7th character after 'virtfn' - vfid=${VfDir:6} - - # check if vfid in VF group range - jq -c '.vfGroups[]' <<< "$iface" | while read group; - do - extract_min_max_ids - vdpaType=$(jq -c '.vdpaType' -r <<< $group) - if [ $vfid -le $maxId ] && [ $vfid -ge $minId ] && ([ $vdpaType == "virtio" ] || [ $vdpaType == "vhost" ]); then - vdpa_name=vdpa:${VfPciAddr} - vdpa_cmd="vdpa dev add name "${vdpa_name}" mgmtdev pci/"${VfPciAddr}" max_vqp 32" - eval $vdpa_cmd - # set the driver_override. When the specified driver will be loaded in the kernel - # it will be automatically bound to the vdpa device - driver_override=virtio_vdpa - if [ $vdpaType == "vhost" ]; then - driver_override=vhost_vdpa - fi - echo $driver_override > /sys/bus/vdpa/devices/$vdpa_name/driver_override - fi - done - done - fi - done diff --git a/bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml b/bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml deleted file mode 100644 index 6455716f8..000000000 --- a/bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml +++ /dev/null @@ -1,78 +0,0 @@ -mode: 0755 -overwrite: true -path: "/usr/local/bin/switchdev-configuration-before-nm.sh" -contents: - inline: | - #!/bin/bash - set -eux - input="/etc/sriov-operator/sriov_config.json" - UDEV_RULE_FILE='/etc/udev/rules.d/10-persistent-net.rules' - - if [ ! -f $input ]; then - echo "File /etc/sriov-operator/sriov_config.json not exist." - exit - fi - - which jq - - append_to_file(){ - content="$1" - file_name="$2" - if ! test -f "$file_name" - then - echo "$content" > "$file_name" - else - if ! grep -Fxq "$content" "$file_name" - then - echo "$content" >> "$file_name" - fi - fi - } - - add_udev_rule_for_sriov_pf(){ - pf_pci=$(grep PCI_SLOT_NAME /sys/class/net/$1/device/uevent | cut -d'=' -f2) - udev_data_line="SUBSYSTEM==\"net\", ACTION==\"add\", DRIVERS==\"?*\", KERNELS==\"$pf_pci\", NAME=\"$1\"" - append_to_file "$udev_data_line" "$UDEV_RULE_FILE" - } - - jq -c '.interfaces[]' ${input} | while read iface;do - num_vfs=$(echo $iface | jq '.numVfs' -r) - pci_addr=$(echo $iface | jq '.pciAddress' -r) - echo "Set $num_vfs VFs on device $pci_addr" - - # create VFs - echo $num_vfs > /sys/bus/pci/devices/${pci_addr}/sriov_numvfs - done - # wait for vfs to be ready - sleep 5 - - jq -c '.interfaces[]' $input | while read iface; - do - eswitch_mode=$(echo $iface | jq '.eSwitchMode' -r) - if [[ "$eswitch_mode" == "switchdev" ]]; then - pci_addr=$(echo $iface | jq '.pciAddress' -r) - name=$(echo $iface | jq '.name' -r) - - # Create udev rule to save PF name - add_udev_rule_for_sriov_pf $name - - echo "Unload VF driver for $pci_addr" - VfDirs=$(ls /sys/bus/pci/devices/${pci_addr} | grep virtfn) - for VfDir in $VfDirs - do - VfPciAddr=$(basename "$( readlink -f /sys/bus/pci/devices/${pci_addr}/$VfDir )") - echo $VfPciAddr > /sys/bus/pci/drivers/mlx5_core/unbind || true - done - # set flow steering mode before entering switchdev mode - echo "Set flow steering mode to smfs" - devlink dev param set pci/${pci_addr} name flow_steering_mode value smfs cmode runtime - - # set PF to switchdev mode - echo "Set eswitch mode to switchdev" - devlink dev eswitch set pci/${pci_addr} mode switchdev - ip link set ${name} up - - # turn hw-tc-offload on - /usr/sbin/ethtool -K ${name} hw-tc-offload on - fi - done diff --git a/bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml b/bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml deleted file mode 100644 index 7b0568e51..000000000 --- a/bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml +++ /dev/null @@ -1,9 +0,0 @@ -mode: 0755 -overwrite: true -path: "/etc/udev/switchdev-vf-link-name.sh" -contents: - inline: | - #!/bin/bash - set -x - PORT="$1" - echo "NUMBER=${PORT##pf*vf}" diff --git a/bindata/manifests/switchdev-config/machineconfigpool.yaml b/bindata/manifests/switchdev-config/machineconfigpool.yaml deleted file mode 100644 index 23e3a2399..000000000 --- a/bindata/manifests/switchdev-config/machineconfigpool.yaml +++ /dev/null @@ -1,12 +0,0 @@ ---- -apiVersion: machineconfiguration.openshift.io/v1 -kind: MachineConfigPool -metadata: - name: {{.HwOffloadNodeLabel}} -spec: - machineConfigSelector: - matchExpressions: - - {key: machineconfiguration.openshift.io/role, operator: In, values: [worker,{{.HwOffloadNodeLabel}}]} - nodeSelector: - matchLabels: - node-role.kubernetes.io/{{.HwOffloadNodeLabel}}: "" diff --git a/bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml b/bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml deleted file mode 100644 index 4765e88d9..000000000 --- a/bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml +++ /dev/null @@ -1,9 +0,0 @@ -name: NetworkManager.service -dropins: -- name: 10-switchdev.conf - contents: | - [Unit] - Wants=switchdev-configuration-before-nm.service - Wants=switchdev-configuration-after-nm.service - After=switchdev-configuration-before-nm.service - Before=switchdev-configuration-after-nm.service diff --git a/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml b/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml deleted file mode 100644 index 59aba324a..000000000 --- a/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml +++ /dev/null @@ -1,19 +0,0 @@ -contents: | - [Unit] - Description=Binds SRIOV VFs to switchdev driver - # Removal of this file signals firstboot completion - ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json - # This service is used to rebind SR-IOV netdev driver after switchdev and NetworkManager configurations - Wants=NetworkManager.service - After=NetworkManager.service - - [Service] - Type=oneshot - ExecStart=/usr/local/bin/switchdev-configuration-after-nm.sh - StandardOutput=journal+console - StandardError=journal+console - - [Install] - WantedBy=network-online.target -enabled: true -name: switchdev-configuration-after-nm.service diff --git a/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml b/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml deleted file mode 100644 index 7500a5e76..000000000 --- a/bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml +++ /dev/null @@ -1,19 +0,0 @@ -contents: | - [Unit] - Description=Configures SRIOV NIC to switchdev mode - # Removal of this file signals firstboot completion - ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json - # This service is used to move a SRIOV NIC to switchdev mode - Wants=network-pre.target - Before=network-pre.target - - [Service] - Type=oneshot - ExecStart=/usr/local/bin/switchdev-configuration-before-nm.sh - StandardOutput=journal+console - StandardError=journal+console - - [Install] - WantedBy=network-online.target -enabled: true -name: switchdev-configuration-before-nm.service diff --git a/bindata/scripts/clean-k8s-services.sh b/bindata/scripts/clean-k8s-services.sh index 59cbc5fd7..86726b663 100755 --- a/bindata/scripts/clean-k8s-services.sh +++ b/bindata/scripts/clean-k8s-services.sh @@ -7,36 +7,12 @@ fi chroot_path="/host" -function clean_services() { - # Remove switchdev service files - rm -f $chroot_path/etc/systemd/system/switchdev-configuration-after-nm.service - rm -f $chroot_path/etc/systemd/system/switchdev-configuration-before-nm.service - rm -f $chroot_path/usr/local/bin/switchdev-configuration-after-nm.sh - rm -f $chroot_path/usr/local/bin/switchdev-configuration-before-nm.sh - rm -f $chroot_path/etc/switchdev.conf - rm -f $chroot_path/etc/udev/switchdev-vf-link-name.sh - # The following files are no longer created by config daemon - # Remove them in case of leftovers from earlier SR-IOV network operator - rm -f $chroot_path/usr/local/bin/configure-switchdev.sh - rm -f $chroot_path/etc/systemd/system/switchdev-configuration.service - # clean NetworkManager and ovs-vswitchd services - network_manager_service=$chroot_path/usr/lib/systemd/system/NetworkManager.service - ovs_service=$chroot_path/usr/lib/systemd/system/ovs-vswitchd.service +ovs_service=$chroot_path/usr/lib/systemd/system/ovs-vswitchd.service - if [ -f $network_manager_service ]; then - sed -i.bak '/switchdev-configuration.service/d' $network_manager_service - fi - - if [ -f $ovs_service ]; then +if [ -f $ovs_service ]; then + if grep -q hw-offload $ovs_service; then sed -i.bak '/hw-offload/d' $ovs_service + chroot $chroot_path /bin/bash -c systemctl daemon-reload >/dev/null 2>&1 || true fi -} - -clean_services -# Reload host services -chroot $chroot_path /bin/bash -c systemctl daemon-reload >/dev/null 2>&1 || true - -# Restart system services -chroot $chroot_path /bin/bash -c systemctl restart NetworkManager.service >/dev/null 2>&1 || true -chroot $chroot_path /bin/bash -c systemctl restart ovs-vswitchd.service >/dev/null 2>&1 || true +fi diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 643c93c82..1ca13545e 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -7,7 +7,6 @@ package mock_helper import ( reflect "reflect" - unit "github.com/coreos/go-systemd/v22/unit" gomock "github.com/golang/mock/gomock" ghw "github.com/jaypipes/ghw" v1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" @@ -786,21 +785,6 @@ func (mr *MockHostHelpersInterfaceMockRecorder) RdmaIsLoaded() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RdmaIsLoaded", reflect.TypeOf((*MockHostHelpersInterface)(nil).RdmaIsLoaded)) } -// ReadScriptManifestFile mocks base method. -func (m *MockHostHelpersInterface) ReadScriptManifestFile(path string) (*types.ScriptManifestFile, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReadScriptManifestFile", path) - ret0, _ := ret[0].(*types.ScriptManifestFile) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ReadScriptManifestFile indicates an expected call of ReadScriptManifestFile. -func (mr *MockHostHelpersInterfaceMockRecorder) ReadScriptManifestFile(path interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadScriptManifestFile", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReadScriptManifestFile), path) -} - // ReadService mocks base method. func (m *MockHostHelpersInterface) ReadService(servicePath string) (*types.Service, error) { m.ctrl.T.Helper() @@ -874,26 +858,6 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ReloadDriver(driver interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadDriver", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReloadDriver), driver) } -// RemoveFromService mocks base method. -func (m *MockHostHelpersInterface) RemoveFromService(service *types.Service, options ...*unit.UnitOption) (*types.Service, error) { - m.ctrl.T.Helper() - varargs := []interface{}{service} - for _, a := range options { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "RemoveFromService", varargs...) - ret0, _ := ret[0].(*types.Service) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// RemoveFromService indicates an expected call of RemoveFromService. -func (mr *MockHostHelpersInterfaceMockRecorder) RemoveFromService(service interface{}, options ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{service}, options...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveFromService", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveFromService), varargs...) -} - // RemoveUdevRule mocks base method. func (m *MockHostHelpersInterface) RemoveUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() diff --git a/pkg/host/internal/service/service.go b/pkg/host/internal/service/service.go index 4ecec5b5f..f1574526f 100644 --- a/pkg/host/internal/service/service.go +++ b/pkg/host/internal/service/service.go @@ -102,7 +102,7 @@ func (s *service) EnableService(service *types.Service) error { return err } -// CompareServices compare 2 service and return true if serviceA has all the fields of serviceB +// CompareServices returns true if serviceA needs update(doesn't contain all fields from service B) func (s *service) CompareServices(serviceA, serviceB *types.Service) (bool, error) { optsA, err := unit.Deserialize(strings.NewReader(serviceA.Content)) if err != nil { @@ -127,37 +127,6 @@ OUTER: return false, nil } -// RemoveFromService removes given fields from service -func (s *service) RemoveFromService(service *types.Service, options ...*unit.UnitOption) (*types.Service, error) { - opts, err := unit.Deserialize(strings.NewReader(service.Content)) - if err != nil { - return nil, err - } - - var newServiceOptions []*unit.UnitOption -OUTER: - for _, opt := range opts { - for _, optRemove := range options { - if opt.Match(optRemove) { - continue OUTER - } - } - - newServiceOptions = append(newServiceOptions, opt) - } - - data, err := io.ReadAll(unit.Serialize(newServiceOptions)) - if err != nil { - return nil, err - } - - return &types.Service{ - Name: service.Name, - Path: service.Path, - Content: string(data), - }, nil -} - // ReadServiceInjectionManifestFile reads service injection file func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { data, err := os.ReadFile(path) @@ -196,21 +165,6 @@ func (s *service) ReadServiceManifestFile(path string) (*types.Service, error) { }, nil } -// ReadScriptManifestFile reads script file -func (s *service) ReadScriptManifestFile(path string) (*types.ScriptManifestFile, error) { - data, err := os.ReadFile(path) - if err != nil { - return nil, err - } - - var scriptFile *types.ScriptManifestFile - if err := yaml.Unmarshal(data, &scriptFile); err != nil { - return nil, err - } - - return scriptFile, nil -} - func (s *service) UpdateSystemService(serviceObj *types.Service) error { systemService, err := s.ReadService(serviceObj.Path) if err != nil { diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index e3df6a71d..2ccad377e 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -7,7 +7,6 @@ package mock_host import ( reflect "reflect" - unit "github.com/coreos/go-systemd/v22/unit" gomock "github.com/golang/mock/gomock" ghw "github.com/jaypipes/ghw" v1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" @@ -664,21 +663,6 @@ func (mr *MockHostManagerInterfaceMockRecorder) RdmaIsLoaded() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RdmaIsLoaded", reflect.TypeOf((*MockHostManagerInterface)(nil).RdmaIsLoaded)) } -// ReadScriptManifestFile mocks base method. -func (m *MockHostManagerInterface) ReadScriptManifestFile(path string) (*types.ScriptManifestFile, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReadScriptManifestFile", path) - ret0, _ := ret[0].(*types.ScriptManifestFile) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ReadScriptManifestFile indicates an expected call of ReadScriptManifestFile. -func (mr *MockHostManagerInterfaceMockRecorder) ReadScriptManifestFile(path interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadScriptManifestFile", reflect.TypeOf((*MockHostManagerInterface)(nil).ReadScriptManifestFile), path) -} - // ReadService mocks base method. func (m *MockHostManagerInterface) ReadService(servicePath string) (*types.Service, error) { m.ctrl.T.Helper() @@ -752,26 +736,6 @@ func (mr *MockHostManagerInterfaceMockRecorder) ReloadDriver(driver interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadDriver", reflect.TypeOf((*MockHostManagerInterface)(nil).ReloadDriver), driver) } -// RemoveFromService mocks base method. -func (m *MockHostManagerInterface) RemoveFromService(service *types.Service, options ...*unit.UnitOption) (*types.Service, error) { - m.ctrl.T.Helper() - varargs := []interface{}{service} - for _, a := range options { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "RemoveFromService", varargs...) - ret0, _ := ret[0].(*types.Service) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// RemoveFromService indicates an expected call of RemoveFromService. -func (mr *MockHostManagerInterfaceMockRecorder) RemoveFromService(service interface{}, options ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{service}, options...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveFromService", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveFromService), varargs...) -} - // RemoveUdevRule mocks base method. func (m *MockHostManagerInterface) RemoveUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 92004fb84..9e8e0d3d6 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -1,7 +1,6 @@ package types import ( - "github.com/coreos/go-systemd/v22/unit" "github.com/jaypipes/ghw" "github.com/vishvananda/netlink" @@ -119,13 +118,9 @@ type ServiceInterface interface { EnableService(service *Service) error // ReadServiceManifestFile reads the systemd manifest for a specific service ReadServiceManifestFile(path string) (*Service, error) - // RemoveFromService removes a systemd service from the host - RemoveFromService(service *Service, options ...*unit.UnitOption) (*Service, error) - // ReadScriptManifestFile reads the script manifest from a systemd service - ReadScriptManifestFile(path string) (*ScriptManifestFile, error) // ReadServiceInjectionManifestFile reads the injection manifest file for the systemd service ReadServiceInjectionManifestFile(path string) (*Service, error) - // CompareServices compare two servers and return true if they are equal + // CompareServices returns true if serviceA needs update(doesn't contain all fields from service B) CompareServices(serviceA, serviceB *Service) (bool, error) // UpdateSystemService updates a system service on the host UpdateSystemService(serviceObj *Service) error diff --git a/pkg/host/types/types.go b/pkg/host/types/types.go index 935a34bfd..248163670 100644 --- a/pkg/host/types/types.go +++ b/pkg/host/types/types.go @@ -1,9 +1,5 @@ package types -import ( - "github.com/coreos/go-systemd/v22/unit" -) - // Service contains info about systemd service type Service struct { Name string @@ -32,12 +28,3 @@ type ScriptManifestFile struct { Inline string } } - -var ( - // Remove run condition form the service - ConditionOpt = &unit.UnitOption{ - Section: "Unit", - Name: "ConditionPathExists", - Value: "!/etc/ignition-machine-config-encapsulated.json", - } -) diff --git a/pkg/plugins/k8s/k8s_plugin.go b/pkg/plugins/k8s/k8s_plugin.go index c5079473e..118073ee5 100644 --- a/pkg/plugins/k8s/k8s_plugin.go +++ b/pkg/plugins/k8s/k8s_plugin.go @@ -1,9 +1,6 @@ package k8s import ( - "fmt" - "os" - "path" "strings" "sigs.k8s.io/controller-runtime/pkg/log" @@ -19,96 +16,80 @@ import ( var PluginName = "k8s" type K8sPlugin struct { - PluginName string - SpecVersion string - switchdevBeforeNMRunScript *hostTypes.ScriptManifestFile - switchdevAfterNMRunScript *hostTypes.ScriptManifestFile - switchdevUdevScript *hostTypes.ScriptManifestFile - switchdevBeforeNMService *hostTypes.Service - switchdevAfterNMService *hostTypes.Service - openVSwitchService *hostTypes.Service - networkManagerService *hostTypes.Service - sriovService *hostTypes.Service - sriovPostNetworkService *hostTypes.Service - updateTarget *k8sUpdateTarget - hostHelper helper.HostHelpersInterface + PluginName string + SpecVersion string + hostHelper helper.HostHelpersInterface + + openVSwitchService *hostTypes.Service + sriovService *hostTypes.Service + sriovPostNetworkService *hostTypes.Service + + updateTarget *k8sUpdateTarget +} +type updateTargetReq struct { + update bool + reboot bool } -type k8sUpdateTarget struct { - switchdevBeforeNMService bool - switchdevAfterNMService bool - switchdevBeforeNMRunScript bool - switchdevAfterNMRunScript bool - switchdevUdevScript bool - sriovScript bool - sriovPostNetworkScript bool - systemServices []*hostTypes.Service +// set need update flag for updateTargetReq +func (u *updateTargetReq) SetNeedUpdate() { + u.update = true } -func (u *k8sUpdateTarget) needUpdate() bool { - return u.switchdevBeforeNMService || - u.switchdevAfterNMService || - u.switchdevBeforeNMRunScript || - u.switchdevAfterNMRunScript || - u.switchdevUdevScript || - u.sriovScript || - u.sriovPostNetworkScript || - len(u.systemServices) > 0 +// set need update and reboot flags for updateTargetReq +func (u *updateTargetReq) SetNeedReboot() { + u.update = true + u.reboot = true } -func (u *k8sUpdateTarget) needReboot() bool { - return u.switchdevBeforeNMService || - u.switchdevAfterNMService || - u.switchdevBeforeNMRunScript || - u.switchdevAfterNMRunScript || - u.switchdevUdevScript || - u.sriovScript || - u.sriovPostNetworkScript +// returns state of the update flag +func (u *updateTargetReq) NeedUpdate() bool { + return u.update } -func (u *k8sUpdateTarget) reset() { - u.switchdevBeforeNMService = false - u.switchdevAfterNMService = false - u.switchdevBeforeNMRunScript = false - u.switchdevAfterNMRunScript = false - u.switchdevUdevScript = false - u.sriovScript = false - u.sriovPostNetworkScript = false - u.systemServices = []*hostTypes.Service{} +// returns state of the reboot flag +func (u *updateTargetReq) NeedReboot() bool { + return u.reboot +} + +type k8sUpdateTarget struct { + sriovScript updateTargetReq + sriovPostNetworkScript updateTargetReq + openVSwitch updateTargetReq } func (u *k8sUpdateTarget) String() string { var updateList []string - if u.switchdevBeforeNMService || u.switchdevAfterNMService { - updateList = append(updateList, "SwitchdevService") + if u.sriovScript.NeedReboot() { + updateList = append(updateList, "sriov-config.service") } - if u.switchdevBeforeNMRunScript || u.switchdevAfterNMRunScript { - updateList = append(updateList, "SwitchdevRunScript") + if u.sriovPostNetworkScript.NeedReboot() { + updateList = append(updateList, "sriov-config-post-network.service") } - if u.switchdevUdevScript { - updateList = append(updateList, "SwitchdevUdevScript") + if u.openVSwitch.NeedReboot() { + updateList = append(updateList, "ovs-vswitchd.service") } - for _, s := range u.systemServices { - updateList = append(updateList, s.Name) - } - return strings.Join(updateList, ",") } +func (u *k8sUpdateTarget) needReboot() bool { + return u.sriovScript.NeedReboot() || u.sriovPostNetworkScript.NeedReboot() || u.openVSwitch.NeedReboot() +} + +func (u *k8sUpdateTarget) reset() { + u.sriovScript = updateTargetReq{} + u.sriovPostNetworkScript = updateTargetReq{} + u.openVSwitch = updateTargetReq{} +} + const ( - bindataManifestPath = "bindata/manifests/" - switchdevManifestPath = bindataManifestPath + "switchdev-config/" - switchdevUnits = switchdevManifestPath + "switchdev-units/" - sriovUnits = bindataManifestPath + "sriov-config-service/kubernetes/" - sriovUnitFile = sriovUnits + "sriov-config-service.yaml" - sriovPostNetworkUnitFile = sriovUnits + "sriov-config-post-network-service.yaml" - switchdevBeforeNMUnitFile = switchdevUnits + "switchdev-configuration-before-nm.yaml" - switchdevAfterNMUnitFile = switchdevUnits + "switchdev-configuration-after-nm.yaml" - networkManagerUnitFile = switchdevUnits + "NetworkManager.service.yaml" - ovsUnitFile = switchdevManifestPath + "ovs-units/ovs-vswitchd.service.yaml" - configuresSwitchdevBeforeNMScript = switchdevManifestPath + "files/switchdev-configuration-before-nm.sh.yaml" - configuresSwitchdevAfterNMScript = switchdevManifestPath + "files/switchdev-configuration-after-nm.sh.yaml" - switchdevRenamingUdevScript = switchdevManifestPath + "files/switchdev-vf-link-name.sh.yaml" + bindataManifestPath = "bindata/manifests/" + switchdevManifestPath = bindataManifestPath + "switchdev-config/" + switchdevUnits = switchdevManifestPath + "switchdev-units/" + sriovUnits = bindataManifestPath + "sriov-config-service/kubernetes/" + sriovUnitFile = sriovUnits + "sriov-config-service.yaml" + sriovPostNetworkUnitFile = sriovUnits + "sriov-config-post-network-service.yaml" + ovsUnitFile = switchdevManifestPath + "ovs-units/ovs-vswitchd.service.yaml" ) // Initialize our plugin and set up initial values @@ -148,7 +129,7 @@ func (p *K8sPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) if sriovnetworkv1.IsSwitchdevModeSpec(new.Spec) { // Check services - err = p.switchDevServicesStateUpdate() + err = p.ovsServiceStateUpdate() if err != nil { log.Log.Error(err, "k8s plugin OnNodeStateChange(): failed") return @@ -164,14 +145,10 @@ func (p *K8sPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) } } - if p.updateTarget.needUpdate() { + if p.updateTarget.needReboot() { needDrain = true - if p.updateTarget.needReboot() { - needReboot = true - log.Log.Info("k8s plugin OnNodeStateChange(): needReboot to update", "target", p.updateTarget) - } else { - log.Log.Info("k8s plugin OnNodeStateChange(): needDrain to update", "target", p.updateTarget) - } + needReboot = true + log.Log.Info("k8s plugin OnNodeStateChange(): needReboot to update", "target", p.updateTarget) } return @@ -180,77 +157,12 @@ func (p *K8sPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) // Apply config change func (p *K8sPlugin) Apply() error { log.Log.Info("k8s plugin Apply()") - if err := p.updateSwitchdevService(); err != nil { - return err - } - if vars.UsingSystemdMode { if err := p.updateSriovServices(); err != nil { return err } } - - for _, systemService := range p.updateTarget.systemServices { - if err := p.hostHelper.UpdateSystemService(systemService); err != nil { - return err - } - } - - return nil -} - -func (p *K8sPlugin) readSwitchdevManifest() error { - // Read switchdev service - switchdevBeforeNMService, err := p.hostHelper.ReadServiceManifestFile(switchdevBeforeNMUnitFile) - if err != nil { - return err - } - switchdevAfterNMService, err := p.hostHelper.ReadServiceManifestFile(switchdevAfterNMUnitFile) - if err != nil { - return err - } - - switchdevBeforeNMService, err = p.hostHelper.RemoveFromService(switchdevBeforeNMService, hostTypes.ConditionOpt) - if err != nil { - return err - } - switchdevAfterNMService, err = p.hostHelper.RemoveFromService(switchdevAfterNMService, hostTypes.ConditionOpt) - if err != nil { - return err - } - p.switchdevBeforeNMService = switchdevBeforeNMService - p.switchdevAfterNMService = switchdevAfterNMService - - // Read switchdev run script - switchdevBeforeNMRunScript, err := p.hostHelper.ReadScriptManifestFile(configuresSwitchdevBeforeNMScript) - if err != nil { - return err - } - switchdevAfterNMRunScript, err := p.hostHelper.ReadScriptManifestFile(configuresSwitchdevAfterNMScript) - if err != nil { - return err - } - p.switchdevBeforeNMRunScript = switchdevBeforeNMRunScript - p.switchdevAfterNMRunScript = switchdevAfterNMRunScript - - // Read switchdev udev script - switchdevUdevScript, err := p.hostHelper.ReadScriptManifestFile(switchdevRenamingUdevScript) - if err != nil { - return err - } - p.switchdevUdevScript = switchdevUdevScript - - return nil -} - -func (p *K8sPlugin) readNetworkManagerManifest() error { - networkManagerService, err := p.hostHelper.ReadServiceInjectionManifestFile(networkManagerUnitFile) - if err != nil { - return err - } - - p.networkManagerService = networkManagerService - return nil + return p.updateOVSService() } func (p *K8sPlugin) readOpenVSwitchdManifest() error { @@ -258,7 +170,6 @@ func (p *K8sPlugin) readOpenVSwitchdManifest() error { if err != nil { return err } - p.openVSwitchService = openVSwitchService return nil } @@ -268,7 +179,6 @@ func (p *K8sPlugin) readSriovServiceManifest() error { if err != nil { return err } - p.sriovService = sriovService return nil } @@ -278,76 +188,27 @@ func (p *K8sPlugin) readSriovPostNetworkServiceManifest() error { if err != nil { return err } - p.sriovPostNetworkService = sriovService return nil } func (p *K8sPlugin) readManifestFiles() error { - if err := p.readSwitchdevManifest(); err != nil { - return err - } - - if err := p.readNetworkManagerManifest(); err != nil { - return err - } - if err := p.readOpenVSwitchdManifest(); err != nil { return err } - if err := p.readSriovServiceManifest(); err != nil { return err } - if err := p.readSriovPostNetworkServiceManifest(); err != nil { return err } - - return nil -} - -func (p *K8sPlugin) switchdevServiceStateUpdate() error { - // Check switchdev service - needUpdate, err := p.isSwitchdevServiceNeedUpdate(p.switchdevBeforeNMService) - if err != nil { - return err - } - p.updateTarget.switchdevBeforeNMService = needUpdate - needUpdate, err = p.isSwitchdevServiceNeedUpdate(p.switchdevAfterNMService) - if err != nil { - return err - } - p.updateTarget.switchdevAfterNMService = needUpdate - - // Check switchdev run script - needUpdate, err = p.isSwitchdevScriptNeedUpdate(p.switchdevBeforeNMRunScript) - if err != nil { - return err - } - p.updateTarget.switchdevBeforeNMRunScript = needUpdate - needUpdate, err = p.isSwitchdevScriptNeedUpdate(p.switchdevAfterNMRunScript) - if err != nil { - return err - } - p.updateTarget.switchdevAfterNMRunScript = needUpdate - - // Check switchdev udev script - needUpdate, err = p.isSwitchdevScriptNeedUpdate(p.switchdevUdevScript) - if err != nil { - return err - } - p.updateTarget.switchdevUdevScript = needUpdate - return nil } func (p *K8sPlugin) sriovServicesStateUpdate() error { - log.Log.Info("sriovServicesStateUpdate()") - for _, s := range []struct { srv *hostTypes.Service - update *bool + update *updateTargetReq }{ {srv: p.sriovService, update: &p.updateTarget.sriovScript}, {srv: p.sriovPostNetworkService, update: &p.updateTarget.sriovPostNetworkScript}, @@ -358,164 +219,101 @@ func (p *K8sPlugin) sriovServicesStateUpdate() error { } // create and enable the service if it doesn't exist or is not enabled if !isServiceEnabled { - *s.update = true + s.update.SetNeedReboot() } else { - *s.update = p.isSystemServiceNeedUpdate(s.srv) - } - if *s.update { - p.updateTarget.systemServices = append(p.updateTarget.systemServices, s.srv) + if p.isSystemDServiceNeedUpdate(s.srv) { + s.update.SetNeedReboot() + } } } - return nil } -func (p *K8sPlugin) getSwitchDevSystemServices() []*hostTypes.Service { - return []*hostTypes.Service{p.networkManagerService, p.openVSwitchService} -} - -func (p *K8sPlugin) isSwitchdevScriptNeedUpdate(scriptObj *hostTypes.ScriptManifestFile) (needUpdate bool, err error) { - data, err := os.ReadFile(path.Join(consts.Host, scriptObj.Path)) - if err != nil { - if !os.IsNotExist(err) { - return false, err +func (p *K8sPlugin) updateSriovServices() error { + for _, s := range []struct { + srv *hostTypes.Service + update *updateTargetReq + }{ + {srv: p.sriovService, update: &p.updateTarget.sriovScript}, + {srv: p.sriovPostNetworkService, update: &p.updateTarget.sriovPostNetworkScript}, + } { + if s.update.NeedUpdate() { + err := p.hostHelper.EnableService(s.srv) + if err != nil { + return err + } } - return true, nil - } else if string(data) != scriptObj.Contents.Inline { - return true, nil } - return false, nil + return nil } -func (p *K8sPlugin) isSwitchdevServiceNeedUpdate(serviceObj *hostTypes.Service) (needUpdate bool, err error) { - swdService, err := p.hostHelper.ReadService(serviceObj.Path) +func (p *K8sPlugin) ovsServiceStateUpdate() error { + exist, err := p.hostHelper.IsServiceExist(p.openVSwitchService.Path) if err != nil { - if !os.IsNotExist(err) { - return false, err - } - // service not exists - return true, nil + return err + } + if !exist { + log.Log.Info("k8s plugin systemServicesStateUpdate(): WARNING! openvswitch system service not found, skip update", + "service", p.openVSwitchService.Name) + return nil + } + if !p.isSystemDServiceNeedUpdate(p.openVSwitchService) { + // service is up to date + return nil + } + if p.isOVSHwOffloadingEnabled() { + p.updateTarget.openVSwitch.SetNeedUpdate() } else { - needChange, err := p.hostHelper.CompareServices(swdService, serviceObj) - if err != nil { - return false, err - } - return needChange, nil + p.updateTarget.openVSwitch.SetNeedReboot() + } + return nil +} + +func (p *K8sPlugin) updateOVSService() error { + if p.updateTarget.openVSwitch.NeedUpdate() { + return p.hostHelper.UpdateSystemService(p.openVSwitchService) } + return nil } -func (p *K8sPlugin) isSystemServiceNeedUpdate(serviceObj *hostTypes.Service) bool { - log.Log.Info("isSystemServiceNeedUpdate()") +func (p *K8sPlugin) isSystemDServiceNeedUpdate(serviceObj *hostTypes.Service) bool { systemService, err := p.hostHelper.ReadService(serviceObj.Path) if err != nil { - log.Log.Error(err, "k8s plugin isSystemServiceNeedUpdate(): failed to read sriov-config service file, ignoring", + log.Log.Error(err, "k8s plugin isSystemDServiceNeedUpdate(): failed to read service file, ignoring", "path", serviceObj.Path) return false } if systemService != nil { needChange, err := p.hostHelper.CompareServices(systemService, serviceObj) if err != nil { - log.Log.Error(err, "k8s plugin isSystemServiceNeedUpdate(): failed to compare sriov-config service, ignoring") + log.Log.Error(err, "k8s plugin isSystemDServiceNeedUpdate(): failed to compare service, ignoring") return false } return needChange } - return false } -func (p *K8sPlugin) systemServicesStateUpdate() error { - var services []*hostTypes.Service - for _, systemService := range p.getSwitchDevSystemServices() { - exist, err := p.hostHelper.IsServiceExist(systemService.Path) - if err != nil { - return err - } - if !exist { - return fmt.Errorf("k8s plugin systemServicesStateUpdate(): %q not found", systemService.Name) - } - if p.isSystemServiceNeedUpdate(systemService) { - services = append(services, systemService) - } - } - - p.updateTarget.systemServices = services - return nil -} - -func (p *K8sPlugin) switchDevServicesStateUpdate() error { - // Check switchdev - err := p.switchdevServiceStateUpdate() +// try to check if OVS HW offloading is already enabled +// required to avoid unneeded reboots in case if HW offloading is already enabled by different entity +// TODO move to the right package and avoid ovs-vsctl binary call +// the function should be revisited when support for software bridge configuration +// is implemented +func (p *K8sPlugin) isOVSHwOffloadingEnabled() bool { + log.Log.V(2).Info("isOVSHwOffloadingEnabled()") + exit, err := p.hostHelper.Chroot(consts.Chroot) if err != nil { - return err + return false } - - // Check system services - err = p.systemServicesStateUpdate() + defer exit() + out, _, err := p.hostHelper.RunCommand("ovs-vsctl", "get", "Open_vSwitch", ".", "other_config:hw-offload") if err != nil { - return err - } - - return nil -} - -func (p *K8sPlugin) updateSriovServices() error { - for _, s := range []struct { - srv *hostTypes.Service - update bool - }{ - {srv: p.sriovService, update: p.updateTarget.sriovScript}, - {srv: p.sriovPostNetworkService, update: p.updateTarget.sriovPostNetworkScript}, - } { - if s.update { - err := p.hostHelper.EnableService(s.srv) - if err != nil { - return err - } - } - } - - return nil -} - -func (p *K8sPlugin) updateSwitchdevService() error { - if p.updateTarget.switchdevBeforeNMService { - err := p.hostHelper.EnableService(p.switchdevBeforeNMService) - if err != nil { - return err - } - } - - if p.updateTarget.switchdevAfterNMService { - err := p.hostHelper.EnableService(p.switchdevAfterNMService) - if err != nil { - return err - } - } - - if p.updateTarget.switchdevBeforeNMRunScript { - err := os.WriteFile(path.Join(consts.Host, p.switchdevBeforeNMRunScript.Path), - []byte(p.switchdevBeforeNMRunScript.Contents.Inline), 0755) - if err != nil { - return err - } - } - - if p.updateTarget.switchdevAfterNMRunScript { - err := os.WriteFile(path.Join(consts.Host, p.switchdevAfterNMRunScript.Path), - []byte(p.switchdevAfterNMRunScript.Contents.Inline), 0755) - if err != nil { - return err - } + log.Log.V(2).Info("isOVSHwOffloadingEnabled() check failed, assume offloading is disabled", "error", err.Error()) + return false } - - if p.updateTarget.switchdevUdevScript { - err := os.WriteFile(path.Join(consts.Host, p.switchdevUdevScript.Path), - []byte(p.switchdevUdevScript.Contents.Inline), 0755) - if err != nil { - return err - } + if strings.Trim(out, "\n") == `"true"` { + log.Log.V(2).Info("isOVSHwOffloadingEnabled() offloading is already enabled") + return true } - - return nil + return false } diff --git a/pkg/plugins/k8s/k8s_plugin_test.go b/pkg/plugins/k8s/k8s_plugin_test.go index c3a64725b..28a2e85c1 100644 --- a/pkg/plugins/k8s/k8s_plugin_test.go +++ b/pkg/plugins/k8s/k8s_plugin_test.go @@ -1,6 +1,7 @@ package k8s import ( + "fmt" "os" "testing" @@ -35,7 +36,7 @@ func registerCall(m *gomock.Call, realF interface{}) *gomock.Call { os.Chdir("../../..") }).DoAndReturn(realF).Do(func(_ ...interface{}) { os.Chdir(cur) - }).AnyTimes() + }) } func setIsSystemdMode(val bool) { @@ -82,28 +83,12 @@ var _ = Describe("K8s plugin", func() { // proxy some functions to real host manager to simplify testing and to additionally validate manifests for _, f := range []string{ - "bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml", - "bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml", - "bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml", - } { - registerCall(hostHelper.EXPECT().ReadScriptManifestFile(f), realHostMgr.ReadScriptManifestFile) - } - for _, f := range []string{ - "bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml", - "bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml", "bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml", "bindata/manifests/sriov-config-service/kubernetes/sriov-config-post-network-service.yaml", } { registerCall(hostHelper.EXPECT().ReadServiceManifestFile(f), realHostMgr.ReadServiceManifestFile) } for _, s := range []string{ - "switchdev-configuration-before-nm.service", - "switchdev-configuration-after-nm.service", - } { - registerCall(hostHelper.EXPECT().RemoveFromService(newServiceNameMatcher(s), gomock.Any()), realHostMgr.RemoveFromService) - } - for _, s := range []string{ - "bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml", "bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml", } { registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s), realHostMgr.ReadServiceInjectionManifestFile) @@ -132,8 +117,6 @@ var _ = Describe("K8s plugin", func() { hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config-post-network.service").Return(false, nil) hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config.service")).Return(nil) hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config-post-network.service")).Return(nil) - hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config.service")).Return(nil) - hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config-post-network.service")).Return(nil) needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{}) Expect(err).ToNot(HaveOccurred()) @@ -176,7 +159,6 @@ var _ = Describe("K8s plugin", func() { newServiceNameMatcher("sriov-config.service"), ).Return(true, nil) hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config.service")).Return(nil) - hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config.service")).Return(nil) hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config-post-network.service").Return(true, nil) hostHelper.EXPECT().ReadService("/etc/systemd/system/sriov-config-post-network.service").Return( @@ -191,4 +173,51 @@ var _ = Describe("K8s plugin", func() { Expect(needDrain).To(BeTrue()) Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) }) + It("ovs service not found", func() { + setIsSystemdMode(false) + hostHelper.EXPECT().IsServiceExist("/usr/lib/systemd/system/ovs-vswitchd.service").Return(false, nil) + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{Interfaces: []sriovnetworkv1.Interface{{EswitchMode: "switchdev"}}}}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) + It("ovs service updated", func() { + setIsSystemdMode(false) + hostHelper.EXPECT().IsServiceExist("/usr/lib/systemd/system/ovs-vswitchd.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/usr/lib/systemd/system/ovs-vswitchd.service").Return( + &hostTypes.Service{Name: "ovs-vswitchd.service"}, nil) + hostHelper.EXPECT().CompareServices( + &hostTypes.Service{Name: "ovs-vswitchd.service"}, + newServiceNameMatcher("ovs-vswitchd.service"), + ).Return(true, nil) + hostHelper.EXPECT().Chroot("/host").Return(nil, fmt.Errorf("test")) + hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{Interfaces: []sriovnetworkv1.Interface{{EswitchMode: "switchdev"}}}}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeTrue()) + Expect(needDrain).To(BeTrue()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) + It("ovs service updated - hw offloading already enabled", func() { + setIsSystemdMode(false) + hostHelper.EXPECT().IsServiceExist("/usr/lib/systemd/system/ovs-vswitchd.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/usr/lib/systemd/system/ovs-vswitchd.service").Return( + &hostTypes.Service{Name: "ovs-vswitchd.service"}, nil) + hostHelper.EXPECT().CompareServices( + &hostTypes.Service{Name: "ovs-vswitchd.service"}, + newServiceNameMatcher("ovs-vswitchd.service"), + ).Return(true, nil) + hostHelper.EXPECT().Chroot("/host").Return(func() error { return nil }, nil) + hostHelper.EXPECT().RunCommand("ovs-vsctl", "get", "Open_vSwitch", ".", "other_config:hw-offload").Return("\"true\"\n", "", nil) + hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{Interfaces: []sriovnetworkv1.Interface{{EswitchMode: "switchdev"}}}}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) }) diff --git a/pkg/render/render.go b/pkg/render/render.go index 8763cc8f0..aa26b018b 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -36,9 +36,7 @@ type DeviceInfo struct { } const ( - filesDir = "files" - ovsUnitsDir = "ovs-units" - switchdevUnitsDir = "switchdev-units" + ovsUnitsDir = "ovs-units" ) func MakeRenderData() RenderData { @@ -159,26 +157,10 @@ func GenerateMachineConfig(path, name, mcRole string, ovsOffload bool, d *Render if !exists { return nil, errors.Errorf("%s is not a directory", path) } - files := map[string]string{} units := map[string]string{} - // if err := filterTemplates(files, path, d); err != nil { - // return nil, err - // } - - p := filepath.Join(path, filesDir) - exists, err = existsDir(p) - if err != nil { - return nil, err - } - if exists { - if err := filterTemplates(files, p, d); err != nil { - return nil, err - } - } - if ovsOffload { - p = filepath.Join(path, ovsUnitsDir) + p := filepath.Join(path, ovsUnitsDir) exists, err = existsDir(p) if err != nil { return nil, err @@ -189,18 +171,6 @@ func GenerateMachineConfig(path, name, mcRole string, ovsOffload bool, d *Render } } } - - p = filepath.Join(path, switchdevUnitsDir) - exists, err = existsDir(p) - if err != nil { - return nil, err - } - if exists { - if err := filterTemplates(units, p, d); err != nil { - return nil, err - } - } - // keySortVals returns a list of values, sorted by key // we need the lists of files and units to have a stable ordering for the checksum keySortVals := func(m map[string]string) []string { @@ -218,7 +188,7 @@ func GenerateMachineConfig(path, name, mcRole string, ovsOffload bool, d *Render return vs } - ignCfg, err := common.TranspileCoreOSConfigToIgn(keySortVals(files), keySortVals(units)) + ignCfg, err := common.TranspileCoreOSConfigToIgn(nil, keySortVals(units)) if err != nil { return nil, errors.Wrap(err, "error transpiling CoreOS config to Ignition config") } From f296db72ba4b3a90b1ba0801e3a0928633ca9669 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Fri, 23 Feb 2024 17:37:58 +0200 Subject: [PATCH 5/7] Remove old switchdev implementation Signed-off-by: Yury Kulazhenkov --- bindata/scripts/load-udev.sh | 25 ------ pkg/daemon/daemon.go | 85 ------------------- pkg/helper/mock/mock_helper.go | 23 +---- pkg/host/internal/sriov/sriov.go | 16 ++-- pkg/host/internal/sriov/sriov_test.go | 18 ++-- pkg/host/internal/udev/udev.go | 97 ---------------------- pkg/host/mock/mock_host.go | 23 +---- pkg/host/types/interfaces.go | 4 +- pkg/plugins/generic/generic_plugin.go | 73 +--------------- pkg/plugins/generic/generic_plugin_test.go | 4 - 10 files changed, 23 insertions(+), 345 deletions(-) delete mode 100755 bindata/scripts/load-udev.sh diff --git a/bindata/scripts/load-udev.sh b/bindata/scripts/load-udev.sh deleted file mode 100755 index d65830ed0..000000000 --- a/bindata/scripts/load-udev.sh +++ /dev/null @@ -1,25 +0,0 @@ -#!/bin/bash - -REDHAT_RELEASE_FILE="/host/etc/redhat-release" -udevadm_bin="" - -if [ -f "$REDHAT_RELEASE_FILE" ]; then - udevadm_bin="/usr/sbin/udevadm" -elif grep -i --quiet 'ubuntu' /host/etc/os-release; then - if grep -i --quiet '20' /host/etc/os-release; then - udevadm_bin="/usr/bin/udevadm" - elif grep -i --quiet '16\|18\|14' /host/etc/os-release; then - udevadm_bin="/sbin/udevadm" - fi -fi - -if [ -z "$udevadm_bin" ]; then - echo "udevadm not found" - exit 1 -fi - -echo "Reload udev rules: $udevadm_bin control --reload-rules" -chroot /host/ $udevadm_bin control --reload-rules - -echo "Trigger udev event: $udevadm_bin trigger --action add --attr-match subsystem=net" -chroot /host/ $udevadm_bin trigger --action add --attr-match subsystem=net diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index e977f4215..a389c1758 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -1,15 +1,10 @@ package daemon import ( - "bytes" "context" "fmt" "math/rand" - "os" "os/exec" - "path" - "strconv" - "strings" "sync" "time" @@ -89,10 +84,6 @@ type Daemon struct { eventRecorder *EventRecorder } -const ( - udevScriptsPath = "/bindata/scripts/load-udev.sh" -) - func New( client client.Client, sriovClient snclientset.Interface, @@ -159,9 +150,6 @@ func (dn *Daemon) Run(stopCh <-chan struct{}, exitCh <-chan error) error { if err := dn.HostHelpers.PrepareVFRepUdevRule(); err != nil { log.Log.Error(err, "failed to prepare udev files to rename VF representors for requested VFs") } - if err := dn.tryCreateSwitchdevUdevRule(); err != nil { - log.Log.Error(err, "failed to create udev files for switchdev") - } var timeout int64 = 5 var metadataKey = "metadata.name" @@ -224,11 +212,6 @@ func (dn *Daemon) Run(stopCh <-chan struct{}, exitCh <-chan error) error { } } return err - case <-time.After(30 * time.Second): - log.Log.V(2).Info("Run(): period refresh") - if err := dn.tryCreateSwitchdevUdevRule(); err != nil { - log.Log.V(2).Error(err, "Could not create udev rule") - } } } } @@ -707,74 +690,6 @@ func (dn *Daemon) rebootNode() { } } -func (dn *Daemon) tryCreateSwitchdevUdevRule() error { - log.Log.V(2).Info("tryCreateSwitchdevUdevRule()") - nodeState, nodeStateErr := dn.sriovClient.SriovnetworkV1().SriovNetworkNodeStates(vars.Namespace).Get( - context.Background(), - vars.NodeName, - metav1.GetOptions{}, - ) - if nodeStateErr != nil { - log.Log.Error(nodeStateErr, "could not fetch node state, skip updating switchdev udev rules", "name", vars.NodeName) - return nil - } - - var newContent string - filePath := path.Join(vars.FilesystemRoot, "/host/etc/udev/rules.d/20-switchdev.rules") - - for _, ifaceStatus := range nodeState.Status.Interfaces { - if ifaceStatus.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { - switchID, err := dn.HostHelpers.GetPhysSwitchID(ifaceStatus.Name) - if err != nil { - return err - } - portName, err := dn.HostHelpers.GetPhysPortName(ifaceStatus.Name) - if err != nil { - return err - } - newContent = newContent + fmt.Sprintf("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}\"\n", switchID, strings.TrimPrefix(portName, "p"), ifaceStatus.Name) - } - } - - oldContent, err := os.ReadFile(filePath) - // if oldContent = newContent, don't do anything - if err == nil && newContent == string(oldContent) { - return nil - } - - log.Log.V(2).Info("Old udev content and new content differ. Writing new content to file.", - "old-content", strings.TrimSuffix(string(oldContent), "\n"), - "new-content", strings.TrimSuffix(newContent, "\n"), - "path", filePath) - - // 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(newContent), 0664) - if err != nil { - log.Log.Error(err, "tryCreateSwitchdevUdevRule(): fail to write file") - return err - } - - var stdout, stderr bytes.Buffer - cmd := exec.Command("/bin/bash", path.Join(vars.FilesystemRoot, udevScriptsPath)) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - return err - } - log.Log.V(2).Info("tryCreateSwitchdevUdevRule(): stdout", "output", cmd.Stdout) - - i, err := strconv.Atoi(strings.TrimSpace(stdout.String())) - if err == nil { - if i == 0 { - log.Log.V(2).Info("tryCreateSwitchdevUdevRule(): switchdev udev rules loaded") - } else { - log.Log.V(2).Info("tryCreateSwitchdevUdevRule(): switchdev udev rules not loaded") - } - } - return nil -} - func (dn *Daemon) prepareNMUdevRule() error { // we need to remove the Red Hat Virtio network device from the udev rule configuration // if we don't remove it when running the config-daemon on a virtual node it will disconnect the node after a reboot diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 1ca13545e..853c5e954 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -168,17 +168,17 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ConfigSriovDeviceVirtual(iface i } // ConfigSriovInterfaces mocks base method. -func (m *MockHostHelpersInterface) ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []v1.Interface, ifaceStatuses []v1.InterfaceExt, pfsToConfig map[string]bool, skipVFConfiguration bool) error { +func (m *MockHostHelpersInterface) ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []v1.Interface, ifaceStatuses []v1.InterfaceExt, skipVFConfiguration bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ConfigSriovInterfaces", storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration) + ret := m.ctrl.Call(m, "ConfigSriovInterfaces", storeManager, interfaces, ifaceStatuses, skipVFConfiguration) ret0, _ := ret[0].(error) return ret0 } // ConfigSriovInterfaces indicates an expected call of ConfigSriovInterfaces. -func (mr *MockHostHelpersInterfaceMockRecorder) ConfigSriovInterfaces(storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration interface{}) *gomock.Call { +func (mr *MockHostHelpersInterfaceMockRecorder) ConfigSriovInterfaces(storeManager, interfaces, ifaceStatuses, skipVFConfiguration interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostHelpersInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostHelpersInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, skipVFConfiguration) } // CreateVDPADevice mocks base method. @@ -1184,18 +1184,3 @@ func (mr *MockHostHelpersInterfaceMockRecorder) WriteCheckpointFile(arg0 interfa mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteCheckpointFile", reflect.TypeOf((*MockHostHelpersInterface)(nil).WriteCheckpointFile), arg0) } - -// WriteSwitchdevConfFile mocks base method. -func (m *MockHostHelpersInterface) WriteSwitchdevConfFile(newState *v1.SriovNetworkNodeState, pfsToSkip map[string]bool) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "WriteSwitchdevConfFile", newState, pfsToSkip) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// WriteSwitchdevConfFile indicates an expected call of WriteSwitchdevConfFile. -func (mr *MockHostHelpersInterfaceMockRecorder) WriteSwitchdevConfFile(newState, pfsToSkip interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteSwitchdevConfFile", reflect.TypeOf((*MockHostHelpersInterface)(nil).WriteSwitchdevConfFile), newState, pfsToSkip) -} diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 9a654c439..76c11ac09 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -571,13 +571,13 @@ func (s *sriov) configSriovDevice(iface *sriovnetworkv1.Interface, skipVFConfigu } func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface, - interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool, skipVFConfiguration bool) error { + interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, skipVFConfiguration bool) error { if s.kernelHelper.IsKernelLockdownMode() && mlx.HasMellanoxInterfacesInSpec(ifaceStatuses, interfaces) { log.Log.Error(nil, "cannot use mellanox devices when in kernel lockdown mode") return fmt.Errorf("cannot use mellanox devices when in kernel lockdown mode") } - toBeConfigured, toBeResetted, err := s.getConfigureAndReset(storeManager, interfaces, ifaceStatuses, pfsToConfig) + toBeConfigured, toBeResetted, err := s.getConfigureAndReset(storeManager, interfaces, ifaceStatuses) if err != nil { log.Log.Error(err, "cannot get a list of interfaces to configure") return fmt.Errorf("cannot get a list of interfaces to configure") @@ -605,7 +605,8 @@ func (s *sriov) ConfigSriovInterfaces(storeManager store.ManagerInterface, return nil } -func (s *sriov) getConfigureAndReset(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.Interface, ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool) ([]interfaceToConfigure, []sriovnetworkv1.InterfaceExt, error) { +func (s *sriov) getConfigureAndReset(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.Interface, + ifaceStatuses []sriovnetworkv1.InterfaceExt) ([]interfaceToConfigure, []sriovnetworkv1.InterfaceExt, error) { toBeConfigured := []interfaceToConfigure{} toBeResetted := []sriovnetworkv1.InterfaceExt{} for _, ifaceStatus := range ifaceStatuses { @@ -613,11 +614,6 @@ func (s *sriov) getConfigureAndReset(storeManager store.ManagerInterface, interf for _, iface := range interfaces { if iface.PciAddress == ifaceStatus.PciAddress { configured = true - - if skip := pfsToConfig[iface.PciAddress]; skip { - break - } - skip, err := skipSriovConfig(&iface, &ifaceStatus, storeManager) if err != nil { log.Log.Error(err, "getConfigureAndReset(): failed to check interface") @@ -633,9 +629,7 @@ func (s *sriov) getConfigureAndReset(storeManager store.ManagerInterface, interf } if !configured && ifaceStatus.NumVfs > 0 { - if skip := pfsToConfig[ifaceStatus.PciAddress]; !skip { - toBeResetted = append(toBeResetted, ifaceStatus) - } + toBeResetted = append(toBeResetted, ifaceStatus) } } return toBeConfigured, toBeResetted, nil diff --git a/pkg/host/internal/sriov/sriov_test.go b/pkg/host/internal/sriov/sriov_test.go index 72877f23a..020a930cb 100644 --- a/pkg/host/internal/sriov/sriov_test.go +++ b/pkg/host/internal/sriov/sriov_test.go @@ -165,7 +165,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}, {PciAddress: "0000:d8:00.1"}}, - map[string]bool{"0000:d8:00.1": true}, false)).NotTo(HaveOccurred()) + false)).NotTo(HaveOccurred()) helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "2") }) It("should configure IB", func() { @@ -214,7 +214,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}}, - map[string]bool{}, false)).NotTo(HaveOccurred()) + false)).NotTo(HaveOccurred()) helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "1") }) @@ -278,7 +278,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}}, - map[string]bool{}, false)).NotTo(HaveOccurred()) + false)).NotTo(HaveOccurred()) helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "1") }) @@ -301,7 +301,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}}, - map[string]bool{}, false)).To(HaveOccurred()) + false)).To(HaveOccurred()) }) It("externally managed - wrong MTU", func() { @@ -327,7 +327,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}}, - map[string]bool{}, false)).To(HaveOccurred()) + false)).To(HaveOccurred()) }) It("reset device", func() { @@ -358,8 +358,7 @@ var _ = Describe("SRIOV", func() { LinkType: "ETH", NumVfs: 2, TotalVfs: 2, - }}, - map[string]bool{}, false)).NotTo(HaveOccurred()) + }}, false)).NotTo(HaveOccurred()) helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "0") }) It("reset device - skip external", func() { @@ -378,8 +377,7 @@ var _ = Describe("SRIOV", func() { PciAddress: "0000:d8:00.0", NumVfs: 2, TotalVfs: 2, - }}, - map[string]bool{}, false)).NotTo(HaveOccurred()) + }}, false)).NotTo(HaveOccurred()) }) It("should configure - skipVFConfiguration is true", func() { helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ @@ -423,7 +421,7 @@ var _ = Describe("SRIOV", func() { }}, }}, []sriovnetworkv1.InterfaceExt{{PciAddress: "0000:d8:00.0"}}, - map[string]bool{}, true)).NotTo(HaveOccurred()) + true)).NotTo(HaveOccurred()) helpers.GinkgoAssertFileContentsEquals("/sys/bus/pci/devices/0000:d8:00.0/sriov_numvfs", "2") }) }) diff --git a/pkg/host/internal/udev/udev.go b/pkg/host/internal/udev/udev.go index a8506e4ae..13602ae8e 100644 --- a/pkg/host/internal/udev/udev.go +++ b/pkg/host/internal/udev/udev.go @@ -1,8 +1,6 @@ package udev import ( - "bytes" - "encoding/json" "fmt" "os" "path" @@ -11,7 +9,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" - sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" @@ -26,10 +23,6 @@ func New(utilsHelper utils.CmdInterface) types.UdevInterface { return &udev{utilsHelper: utilsHelper} } -type config struct { - Interfaces []sriovnetworkv1.Interface `json:"interfaces"` -} - func (u *udev) PrepareNMUdevRule(supportedVfIds []string) error { log.Log.V(2).Info("PrepareNMUdevRule()") filePath := filepath.Join(vars.FilesystemRoot, consts.HostUdevRulesFolder, "10-nm-unmanaged.rules") @@ -76,96 +69,6 @@ func (u *udev) PrepareVFRepUdevRule() error { return nil } -func (u *udev) WriteSwitchdevConfFile(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToSkip map[string]bool) (bool, error) { - cfg := config{} - for _, iface := range newState.Spec.Interfaces { - for _, ifaceStatus := range newState.Status.Interfaces { - if iface.PciAddress != ifaceStatus.PciAddress { - continue - } - - if skip := pfsToSkip[iface.PciAddress]; !skip { - continue - } - - if iface.NumVfs > 0 { - var vfGroups []sriovnetworkv1.VfGroup = nil - ifc, err := sriovnetworkv1.FindInterface(newState.Spec.Interfaces, iface.Name) - if err != nil { - log.Log.Error(err, "WriteSwitchdevConfFile(): fail find interface") - } else { - vfGroups = ifc.VfGroups - } - i := sriovnetworkv1.Interface{ - // Not passing all the contents, since only NumVfs and EswitchMode can be configured by configure-switchdev.sh currently. - Name: iface.Name, - PciAddress: iface.PciAddress, - NumVfs: iface.NumVfs, - Mtu: iface.Mtu, - VfGroups: vfGroups, - } - - if iface.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { - i.EswitchMode = iface.EswitchMode - } - cfg.Interfaces = append(cfg.Interfaces, i) - } - } - } - _, err := os.Stat(consts.SriovHostSwitchDevConfPath) - if err != nil { - if os.IsNotExist(err) { - if len(cfg.Interfaces) == 0 { - return false, nil - } - - // TODO: refactor this function to allow using vars.FilesystemRoot for unit-tests - // Create the sriov-operator folder on the host if it doesn't exist - if _, err := os.Stat(consts.Host + consts.SriovConfBasePath); os.IsNotExist(err) { - err = os.Mkdir(consts.Host+consts.SriovConfBasePath, os.ModeDir) - if err != nil { - log.Log.Error(err, "WriteConfFile(): failed to create sriov-operator folder") - return false, err - } - } - - log.Log.V(2).Info("WriteSwitchdevConfFile(): file not existed, create it") - _, err = os.Create(consts.SriovHostSwitchDevConfPath) - if err != nil { - log.Log.Error(err, "WriteSwitchdevConfFile(): failed to create file") - return false, err - } - } else { - return false, err - } - } - oldContent, err := os.ReadFile(consts.SriovHostSwitchDevConfPath) - if err != nil { - log.Log.Error(err, "WriteSwitchdevConfFile(): failed to read file") - return false, err - } - var newContent []byte - if len(cfg.Interfaces) != 0 { - newContent, err = json.Marshal(cfg) - if err != nil { - log.Log.Error(err, "WriteSwitchdevConfFile(): fail to marshal config") - return false, err - } - } - - if bytes.Equal(newContent, oldContent) { - log.Log.V(2).Info("WriteSwitchdevConfFile(): no update") - return false, nil - } - log.Log.V(2).Info("WriteSwitchdevConfFile(): write to switchdev.conf", "content", newContent) - err = os.WriteFile(consts.SriovHostSwitchDevConfPath, newContent, 0644) - if err != nil { - log.Log.Error(err, "WriteSwitchdevConfFile(): failed to write file") - return false, err - } - 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) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 2ccad377e..188c2fb27 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -138,17 +138,17 @@ func (mr *MockHostManagerInterfaceMockRecorder) ConfigSriovDeviceVirtual(iface i } // ConfigSriovInterfaces mocks base method. -func (m *MockHostManagerInterface) ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []v1.Interface, ifaceStatuses []v1.InterfaceExt, pfsToConfig map[string]bool, skipVFConfiguration bool) error { +func (m *MockHostManagerInterface) ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []v1.Interface, ifaceStatuses []v1.InterfaceExt, skipVFConfiguration bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ConfigSriovInterfaces", storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration) + ret := m.ctrl.Call(m, "ConfigSriovInterfaces", storeManager, interfaces, ifaceStatuses, skipVFConfiguration) ret0, _ := ret[0].(error) return ret0 } // ConfigSriovInterfaces indicates an expected call of ConfigSriovInterfaces. -func (mr *MockHostManagerInterfaceMockRecorder) ConfigSriovInterfaces(storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration interface{}) *gomock.Call { +func (mr *MockHostManagerInterfaceMockRecorder) ConfigSriovInterfaces(storeManager, interfaces, ifaceStatuses, skipVFConfiguration interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostManagerInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, pfsToConfig, skipVFConfiguration) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigSriovInterfaces", reflect.TypeOf((*MockHostManagerInterface)(nil).ConfigSriovInterfaces), storeManager, interfaces, ifaceStatuses, skipVFConfiguration) } // CreateVDPADevice mocks base method. @@ -1013,18 +1013,3 @@ func (mr *MockHostManagerInterfaceMockRecorder) VFIsReady(pciAddr interface{}) * mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VFIsReady", reflect.TypeOf((*MockHostManagerInterface)(nil).VFIsReady), pciAddr) } - -// WriteSwitchdevConfFile mocks base method. -func (m *MockHostManagerInterface) WriteSwitchdevConfFile(newState *v1.SriovNetworkNodeState, pfsToSkip map[string]bool) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "WriteSwitchdevConfFile", newState, pfsToSkip) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// WriteSwitchdevConfFile indicates an expected call of WriteSwitchdevConfFile. -func (mr *MockHostManagerInterfaceMockRecorder) WriteSwitchdevConfFile(newState, pfsToSkip interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WriteSwitchdevConfFile", reflect.TypeOf((*MockHostManagerInterface)(nil).WriteSwitchdevConfFile), newState, pfsToSkip) -} diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 9e8e0d3d6..05fc6a6fb 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -154,14 +154,12 @@ type SriovInterface interface { // ConfigSriovInterfaces configure multiple SR-IOV devices with the desired configuration // if skipVFConfiguration flag is set, the function will configure PF and create VFs on it, but will skip VFs configuration ConfigSriovInterfaces(storeManager store.ManagerInterface, interfaces []sriovnetworkv1.Interface, - ifaceStatuses []sriovnetworkv1.InterfaceExt, pfsToConfig map[string]bool, skipVFConfiguration bool) error + ifaceStatuses []sriovnetworkv1.InterfaceExt, skipVFConfiguration bool) error // ConfigSriovInterfaces configure virtual functions for virtual environments with the desired configuration ConfigSriovDeviceVirtual(iface *sriovnetworkv1.Interface) error } type UdevInterface interface { - // WriteSwitchdevConfFile writes the needed switchdev configuration files for HW offload support - WriteSwitchdevConfFile(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToSkip map[string]bool) (bool, error) // PrepareNMUdevRule creates the needed udev rules to disable NetworkManager from // our managed SR-IOV virtual functions PrepareNMUdevRule(supportedVfIds []string) error diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index 8a510417c..a7a71ca15 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -3,7 +3,6 @@ package generic import ( "bytes" "errors" - "fmt" "os/exec" "reflect" "strconv" @@ -18,7 +17,6 @@ import ( plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" - mlx "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vendors/mellanox" ) var PluginName = "generic" @@ -57,7 +55,6 @@ type GenericPlugin struct { LastState *sriovnetworkv1.SriovNetworkNodeState DriverStateMap DriverStateMapType DesiredKernelArgs map[string]bool - pfsToSkip map[string]bool helpers helper.HostHelpersInterface skipVFConfiguration bool } @@ -112,7 +109,6 @@ func NewGenericPlugin(helpers helper.HostHelpersInterface, options ...Option) (p SpecVersion: "1.0", DriverStateMap: driverStateMap, DesiredKernelArgs: make(map[string]bool), - pfsToSkip: make(map[string]bool), helpers: helpers, skipVFConfiguration: cfg.skipVFConfiguration, }, nil @@ -185,7 +181,7 @@ func (p *GenericPlugin) Apply() error { } if err := p.helpers.ConfigSriovInterfaces(p.helpers, p.DesireState.Spec.Interfaces, - p.DesireState.Status.Interfaces, p.pfsToSkip, p.skipVFConfiguration); err != nil { + p.DesireState.Status.Interfaces, p.skipVFConfiguration); err != nil { // Catch the "cannot allocate memory" error and try to use PCI realloc if errors.Is(err, syscall.ENOMEM) { p.addToDesiredKernelArgs(consts.KernelArgPciRealloc) @@ -368,76 +364,9 @@ func (p *GenericPlugin) needRebootNode(state *sriovnetworkv1.SriovNetworkNodeSta log.Log.V(2).Info("generic plugin needRebootNode(): need reboot for updating kernel arguments") needReboot = true } - - // Create a map with all the PFs we will need to configure - // we need to create it here before we access the host file system using the chroot function - // because the skipConfigVf needs the mstconfig package that exist only inside the sriov-config-daemon file system - pfsToSkip, err := getPfsToSkip(p.DesireState, p.helpers) - if err != nil { - return false, err - } - p.pfsToSkip = pfsToSkip - - updateNode, err = p.helpers.WriteSwitchdevConfFile(state, p.pfsToSkip) - if err != nil { - log.Log.Error(err, "generic plugin needRebootNode(): fail to write switchdev device config file") - return false, err - } - if updateNode { - log.Log.V(2).Info("generic plugin needRebootNode(): need reboot for updating switchdev device configuration") - needReboot = true - } - return needReboot, nil } -// getPfsToSkip return a map of devices pci addresses to should be configured via systemd instead if the legacy mode -// we skip devices in switchdev mode and Bluefield card in ConnectX mode -func getPfsToSkip(ns *sriovnetworkv1.SriovNetworkNodeState, mlxHelper mlx.MellanoxInterface) (map[string]bool, error) { - pfsToSkip := map[string]bool{} - for _, ifaceStatus := range ns.Status.Interfaces { - for _, iface := range ns.Spec.Interfaces { - if iface.PciAddress == ifaceStatus.PciAddress { - skip, err := skipConfigVf(iface, ifaceStatus, mlxHelper) - if err != nil { - log.Log.Error(err, "GetPfsToSkip(): fail to check for skip VFs", "device", iface.PciAddress) - return pfsToSkip, err - } - pfsToSkip[iface.PciAddress] = skip - break - } - } - } - - return pfsToSkip, nil -} - -// skipConfigVf Use systemd service to configure switchdev mode or BF-2 NICs in OpenShift -func skipConfigVf(ifSpec sriovnetworkv1.Interface, ifStatus sriovnetworkv1.InterfaceExt, mlxHelper mlx.MellanoxInterface) (bool, error) { - if ifSpec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { - log.Log.V(2).Info("skipConfigVf(): skip config VF for switchdev device") - return true, nil - } - - // NVIDIA BlueField 2 and BlueField3 in OpenShift - if vars.ClusterType == consts.ClusterTypeOpenshift && ifStatus.Vendor == mlx.VendorMellanox && (ifStatus.DeviceID == mlx.DeviceBF2 || ifStatus.DeviceID == mlx.DeviceBF3) { - // TODO: remove this when switch to the systemd configuration support. - mode, err := mlxHelper.GetMellanoxBlueFieldMode(ifStatus.PciAddress) - if err != nil { - return false, fmt.Errorf("failed to read Mellanox Bluefield card mode for %s,%v", ifStatus.PciAddress, err) - } - - if mode == mlx.BluefieldConnectXMode { - return false, nil - } - - log.Log.V(2).Info("skipConfigVf(): skip config VF for Bluefiled card on DPU mode") - return true, nil - } - - return false, nil -} - // ////////////// for testing purposes only /////////////////////// func (p *GenericPlugin) getDriverStateMap() DriverStateMapType { return p.DriverStateMap diff --git a/pkg/plugins/generic/generic_plugin_test.go b/pkg/plugins/generic/generic_plugin_test.go index 881fc87d4..9f9c1be83 100644 --- a/pkg/plugins/generic/generic_plugin_test.go +++ b/pkg/plugins/generic/generic_plugin_test.go @@ -77,8 +77,6 @@ var _ = Describe("Generic plugin", func() { }}, }, } - - hostHelper.EXPECT().WriteSwitchdevConfFile(networkNodeState, map[string]bool{"0000:00:00.0": false}).Return(false, nil) needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) Expect(err).ToNot(HaveOccurred()) Expect(needReboot).To(BeFalse()) @@ -133,8 +131,6 @@ var _ = Describe("Generic plugin", func() { }}, }, } - - hostHelper.EXPECT().WriteSwitchdevConfFile(networkNodeState, map[string]bool{"0000:00:00.0": false}).Return(false, nil) needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) Expect(err).ToNot(HaveOccurred()) Expect(needReboot).To(BeFalse()) From 68ef448cb5aeafdf942dcdaeab582ccb862c2ca5 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Wed, 20 Mar 2024 17:05:35 +0200 Subject: [PATCH 6/7] Fix sriov.configureHWOptionsForSwitchdev function Fix the function to correctly handle devices which doesn't support software_steering Signed-off-by: Yury Kulazhenkov --- pkg/host/internal/sriov/sriov.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 76c11ac09..a09231e14 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -353,8 +353,8 @@ func (s *sriov) configureHWOptionsForSwitchdev(iface *sriovnetworkv1.Interface) desiredFlowSteeringMode := "smfs" currentFlowSteeringMode, err := s.networkHelper.GetDevlinkDeviceParam(iface.PciAddress, "flow_steering_mode") if err != nil { - if errors.Is(err, syscall.EINVAL) { - log.Log.V(2).Info("configureHWOptionsForSwitchdev(): software flow steering is not supported by the device, skip configuration", + if errors.Is(err, syscall.EINVAL) || errors.Is(err, syscall.ENODEV) { + log.Log.V(2).Info("configureHWOptionsForSwitchdev(): device has no flow_steering_mode parameter, skip", "device", iface.PciAddress) return nil } @@ -369,6 +369,10 @@ func (s *sriov) configureHWOptionsForSwitchdev(iface *sriovnetworkv1.Interface) s.setEswitchModeAndNumVFs(iface.PciAddress, sriovnetworkv1.ESwithModeLegacy, 0) } if err := s.networkHelper.SetDevlinkDeviceParam(iface.PciAddress, "flow_steering_mode", desiredFlowSteeringMode); err != nil { + if errors.Is(err, syscall.ENOTSUP) { + log.Log.V(2).Info("configureHWOptionsForSwitchdev(): device doesn't support changing of flow_steering_mode, skip", "device", iface.PciAddress) + return nil + } log.Log.Error(err, "configureHWOptionsForSwitchdev(): fail to configure flow steering mode for the device", "device", iface.PciAddress) return err } From 0bdf464a07cac23c79ef3a2e2986bc97a8ea1e29 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Wed, 20 Mar 2024 20:49:10 +0200 Subject: [PATCH 7/7] 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 {