diff --git a/api/v1/helper.go b/api/v1/helper.go index 9f2139437..462afa356 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -214,7 +214,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 } @@ -255,7 +261,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 @@ -296,11 +307,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 } } 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/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/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 dd1381cdb..efbefe198 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -93,9 +93,13 @@ 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 + PFNameUdevRule = `SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", KERNELS=="%s", NAME="%s"` // nolint:goconst NMUdevRule = `SUBSYSTEM=="net", ` + `ACTION=="add|change|move", ` + diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index fce5cf57e..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, @@ -156,8 +147,8 @@ 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.tryCreateSwitchdevUdevRule(); err != nil { - log.Log.Error(err, "failed to create udev files for switchdev") + if err := dn.HostHelpers.PrepareVFRepUdevRule(); err != nil { + log.Log.Error(err, "failed to prepare udev files to rename VF representors for requested VFs") } var timeout int64 = 5 @@ -221,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") - } } } } @@ -704,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/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..4d8db7bf6 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" @@ -40,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. @@ -169,17 +182,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. @@ -713,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() @@ -757,6 +784,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() @@ -772,21 +813,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() @@ -860,38 +886,32 @@ 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) { +// RemoveDisableNMUdevRule mocks base method. +func (m *MockHostHelpersInterface) RemoveDisableNMUdevRule(pfPciAddress string) 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 + ret := m.ctrl.Call(m, "RemoveDisableNMUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 } -// RemoveFromService indicates an expected call of RemoveFromService. -func (mr *MockHostHelpersInterfaceMockRecorder) RemoveFromService(service interface{}, options ...interface{}) *gomock.Call { +// RemoveDisableNMUdevRule indicates an expected call of RemoveDisableNMUdevRule. +func (mr *MockHostHelpersInterfaceMockRecorder) RemoveDisableNMUdevRule(pfPciAddress 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...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDisableNMUdevRule", reflect.TypeOf((*MockHostHelpersInterface)(nil).RemoveDisableNMUdevRule), pfPciAddress) } -// RemoveUdevRule mocks base method. -func (m *MockHostHelpersInterface) RemoveUdevRule(pfPciAddress string) error { +// 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. @@ -1206,18 +1226,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/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/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 45ce30fa1..65d18e09d 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 != "" { @@ -314,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 @@ -352,8 +360,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 } @@ -368,6 +376,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 } @@ -570,13 +582,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") @@ -591,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) @@ -604,7 +624,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 { @@ -612,11 +633,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") @@ -632,9 +648,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 @@ -891,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 } @@ -918,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 72877f23a..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) @@ -165,7 +168,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() { @@ -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) @@ -214,7 +220,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") }) @@ -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) @@ -278,7 +289,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 +312,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 +338,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() { @@ -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) @@ -358,8 +370,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 +389,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{ @@ -393,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) @@ -423,7 +436,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 2b4c4c6b3..3f828bb70 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") @@ -56,109 +49,52 @@ func (u *udev) PrepareNMUdevRule(supportedVfIds []string) 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) +// 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 { - 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 - } + log.Log.Error(err, "PrepareVFRepUdevRule(): failed to read source for representor name UDEV script") + return err } - - if bytes.Equal(newContent, oldContent) { - log.Log.V(2).Info("WriteSwitchdevConfFile(): no update") - return false, nil + if err := os.WriteFile(targetPath, data, 0755); err != nil { + log.Log.Error(err, "PrepareVFRepUdevRule(): failed to write representor name UDEV script") + return err } - 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 + 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 true, nil + 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()", @@ -173,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 d5f222385..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) + }) + + AfterEach(func() { + testCtrl.Finish() }) - Context("AddUdevRule", func() { + + 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() { @@ -114,4 +172,42 @@ 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()) + }) + }) + 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 4bc8ee630..4ea2f0e8b 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" @@ -39,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. @@ -139,17 +152,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. @@ -621,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() @@ -635,6 +662,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() @@ -650,21 +691,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() @@ -738,38 +764,32 @@ 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) { +// RemoveDisableNMUdevRule mocks base method. +func (m *MockHostManagerInterface) RemoveDisableNMUdevRule(pfPciAddress string) 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 + ret := m.ctrl.Call(m, "RemoveDisableNMUdevRule", pfPciAddress) + ret0, _ := ret[0].(error) + return ret0 } -// RemoveFromService indicates an expected call of RemoveFromService. -func (mr *MockHostManagerInterfaceMockRecorder) RemoveFromService(service interface{}, options ...interface{}) *gomock.Call { +// RemoveDisableNMUdevRule indicates an expected call of RemoveDisableNMUdevRule. +func (mr *MockHostManagerInterfaceMockRecorder) RemoveDisableNMUdevRule(pfPciAddress 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...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveDisableNMUdevRule", reflect.TypeOf((*MockHostManagerInterface)(nil).RemoveDisableNMUdevRule), pfPciAddress) } -// RemoveUdevRule mocks base method. -func (m *MockHostManagerInterface) RemoveUdevRule(pfPciAddress string) error { +// 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. @@ -1035,18 +1055,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 248d6d71e..48d47b424 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 @@ -159,25 +154,31 @@ 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 - // 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 + // PrepareVFRepUdevRule creates a script which helps to configure representor name for the VF + PrepareVFRepUdevRule() 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 { 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/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()) 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") }