From 511c308c98d7c91864793562a2fa0b49434aa240 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 28 Oct 2024 15:28:41 +0200 Subject: [PATCH] Add kernel args for rdma mode to complement the modprobe file Signed-off-by: Sebastian Sch --- bindata/scripts/enable-kargs.sh | 33 ---- bindata/scripts/kargs.sh | 55 ++++++ .../sriovnetworknodepolicy_controller.go | 13 +- pkg/consts/constants.go | 8 +- pkg/daemon/daemon.go | 10 - pkg/daemon/writer.go | 1 + .../internal/lib/netlink/mock/mock_netlink.go | 30 +-- pkg/host/internal/lib/netlink/netlink.go | 8 +- pkg/host/internal/network/network.go | 24 ++- pkg/host/internal/network/network_test.go | 6 +- pkg/plugins/generic/generic_plugin.go | 173 ++++++++++-------- pkg/plugins/generic/generic_plugin_test.go | 11 +- 12 files changed, 212 insertions(+), 160 deletions(-) delete mode 100755 bindata/scripts/enable-kargs.sh create mode 100755 bindata/scripts/kargs.sh diff --git a/bindata/scripts/enable-kargs.sh b/bindata/scripts/enable-kargs.sh deleted file mode 100755 index 0dc18c784..000000000 --- a/bindata/scripts/enable-kargs.sh +++ /dev/null @@ -1,33 +0,0 @@ -#!/bin/bash -set -x - -declare -a kargs=( "$@" ) -ret=0 -args=$(chroot /host/ cat /proc/cmdline) - -if chroot /host/ test -f /run/ostree-booted ; then - for t in "${kargs[@]}";do - if [[ $args != *${t}* ]];then - if chroot /host/ rpm-ostree kargs | grep -vq ${t}; then - chroot /host/ rpm-ostree kargs --append ${t} > /dev/null 2>&1 - fi - let ret++ - fi - done -else - chroot /host/ which grubby > /dev/null 2>&1 - # if grubby is not there, let's tell it - if [ $? -ne 0 ]; then - exit 127 - fi - for t in "${kargs[@]}";do - if [[ $args != *${t}* ]];then - if chroot /host/ grubby --info=DEFAULT | grep args | grep -vq ${t}; then - chroot /host/ grubby --update-kernel=DEFAULT --args=${t} > /dev/null 2>&1 - fi - let ret++ - fi - done -fi - -echo $ret diff --git a/bindata/scripts/kargs.sh b/bindata/scripts/kargs.sh new file mode 100755 index 000000000..8d118456e --- /dev/null +++ b/bindata/scripts/kargs.sh @@ -0,0 +1,55 @@ +#!/bin/bash +set -x + +command=$1 +shift +declare -a kargs=( "$@" ) +ret=0 +args=$(chroot /host/ cat /proc/cmdline) + +if chroot /host/ test -f /run/ostree-booted ; then + for t in "${kargs[@]}";do + if [[ $command == "add" ]];then + if [[ $args != *${t}* ]];then + if chroot /host/ rpm-ostree kargs | grep -vq ${t}; then + chroot /host/ rpm-ostree kargs --append ${t} > /dev/null 2>&1 + fi + let ret++ + fi + fi + if [[ $command == "remove" ]];then + if [[ $args == *${t}* ]];then + if chroot /host/ rpm-ostree kargs | grep -q ${t}; then + chroot /host/ rpm-ostree kargs --delete ${t} > /dev/null 2>&1 + fi + let ret++ + fi + fi + done +else + chroot /host/ which grubby > /dev/null 2>&1 + # if grubby is not there, let's tell it + if [ $? -ne 0 ]; then + exit 127 + fi + for t in "${kargs[@]}";do + if [[ $command == "add" ]];then + if [[ $args != *${t}* ]];then + if chroot /host/ grubby --info=DEFAULT | grep args | grep -vq ${t}; then + chroot /host/ grubby --update-kernel=DEFAULT --args=${t} > /dev/null 2>&1 + fi + let ret++ + fi + fi + if [[ $command == "remove" ]];then + if [[ $args == *${t}* ]];then + if chroot /host/ grubby --info=DEFAULT | grep args | grep -q ${t}; then + chroot /host/ grubby --update-kernel=DEFAULT --remove-args=${t} > /dev/null 2>&1 + fi + let ret++ + fi + fi + done +fi + +echo $ret diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index 1d2811fac..62218436f 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -155,22 +155,22 @@ func (r *SriovNetworkNodePolicyReconciler) SetupWithManager(mgr ctrl.Manager) er delayedEventHandler := handler.Funcs{ CreateFunc: func(ctx context.Context, e event.CreateEvent, q workqueue.RateLimitingInterface) { log.Log.WithName("SriovNetworkNodePolicy"). - Info("Enqueuing sync for create event", "resource", e.Object.GetName()) + Info("Enqueuing sync for create event", "resource", e.Object.GetName(), "type", e.Object.GetObjectKind().GroupVersionKind().String()) qHandler(q) }, UpdateFunc: func(ctx context.Context, e event.UpdateEvent, q workqueue.RateLimitingInterface) { log.Log.WithName("SriovNetworkNodePolicy"). - Info("Enqueuing sync for update event", "resource", e.ObjectNew.GetName()) + Info("Enqueuing sync for update event", "resource", e.ObjectNew.GetName(), "type", e.ObjectNew.GetObjectKind().GroupVersionKind().String()) qHandler(q) }, DeleteFunc: func(ctx context.Context, e event.DeleteEvent, q workqueue.RateLimitingInterface) { log.Log.WithName("SriovNetworkNodePolicy"). - Info("Enqueuing sync for delete event", "resource", e.Object.GetName()) + Info("Enqueuing sync for delete event", "resource", e.Object.GetName(), "type", e.Object.GetObjectKind().GroupVersionKind().String()) qHandler(q) }, GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) { log.Log.WithName("SriovNetworkNodePolicy"). - Info("Enqueuing sync for generic event", "resource", e.Object.GetName()) + Info("Enqueuing sync for generic event", "resource", e.Object.GetName(), "type", e.Object.GetObjectKind().GroupVersionKind().String()) qHandler(q) }, } @@ -199,6 +199,7 @@ func (r *SriovNetworkNodePolicyReconciler) SetupWithManager(mgr ctrl.Manager) er For(&sriovnetworkv1.SriovNetworkNodePolicy{}). Watches(&corev1.Node{}, nodeEvenHandler). Watches(&sriovnetworkv1.SriovNetworkNodePolicy{}, delayedEventHandler). + Watches(&sriovnetworkv1.SriovNetworkPoolConfig{}, delayedEventHandler). WatchesRawSource(&source.Channel{Source: eventChan}, delayedEventHandler). Complete(r) } @@ -271,14 +272,14 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con ns := &sriovnetworkv1.SriovNetworkNodeState{} ns.Name = node.Name ns.Namespace = vars.Namespace - j, _ := json.Marshal(ns) netPoolConfig, _, err := findNodePoolConfig(ctx, &node, r.Client) if err != nil { - log.Log.Error(err, "nodeStateSyncHandler(): failed to get SriovNetworkPoolConfig for the current node") + logger.Error(err, "failed to get SriovNetworkPoolConfig for the current node") } if netPoolConfig != nil { ns.Spec.System.RdmaMode = netPoolConfig.Spec.RdmaMode } + j, _ := json.Marshal(ns) logger.V(2).Info("SriovNetworkNodeState CR", "content", j) if err := r.syncSriovNetworkNodeState(ctx, dc, npl, ns, &node); err != nil { logger.Error(err, "Fail to sync", "SriovNetworkNodeState", ns.Name) diff --git a/pkg/consts/constants.go b/pkg/consts/constants.go index f62cd3195..890a05544 100644 --- a/pkg/consts/constants.go +++ b/pkg/consts/constants.go @@ -124,9 +124,11 @@ const ( `IMPORT{program}="/etc/udev/switchdev-vf-link-name.sh $attr{phys_port_name}", ` + `NAME="%s_$env{NUMBER}"` - KernelArgPciRealloc = "pci=realloc" - KernelArgIntelIommu = "intel_iommu=on" - KernelArgIommuPt = "iommu=pt" + KernelArgPciRealloc = "pci=realloc" + KernelArgIntelIommu = "intel_iommu=on" + KernelArgIommuPt = "iommu=pt" + KernelArgRdmaShared = "ib_core.netns_mode=1" + KernelArgRdmaExclusive = "ib_core.netns_mode=0" // Feature gates // ParallelNicConfigFeatureGate: allow to configure nics in parallel diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 0867685dc..ff7f326dc 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -429,16 +429,6 @@ func (dn *Daemon) nodeStateSyncHandler() error { reqReboot = reqReboot || r } - if dn.currentNodeState.Status.System.RdmaMode != dn.desiredNodeState.Spec.System.RdmaMode { - err = dn.HostHelpers.SetRDMASubsystem(dn.desiredNodeState.Spec.System.RdmaMode) - if err != nil { - log.Log.Error(err, "nodeStateSyncHandler(): failed to set RDMA subsystem") - return err - } - reqReboot = true - reqDrain = true - } - // When running using systemd check if the applied configuration is the latest one // or there is a new config we need to apply // When using systemd configuration we write the file diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index 60d4e8d91..42eeb2928 100644 --- a/pkg/daemon/writer.go +++ b/pkg/daemon/writer.go @@ -189,6 +189,7 @@ func (w *NodeStateStatusWriter) setNodeStateStatus(msg Message) (*sriovnetworkv1 nodeState, err := w.updateNodeStateStatusRetry(func(nodeState *sriovnetworkv1.SriovNetworkNodeState) { nodeState.Status.Interfaces = w.status.Interfaces nodeState.Status.Bridges = w.status.Bridges + nodeState.Status.System = w.status.System if msg.lastSyncError != "" || msg.syncStatus == consts.SyncStatusSucceeded { // clear lastSyncError when sync Succeeded nodeState.Status.LastSyncError = msg.lastSyncError diff --git a/pkg/host/internal/lib/netlink/mock/mock_netlink.go b/pkg/host/internal/lib/netlink/mock/mock_netlink.go index 758346a3f..ec136bf29 100644 --- a/pkg/host/internal/lib/netlink/mock/mock_netlink.go +++ b/pkg/host/internal/lib/netlink/mock/mock_netlink.go @@ -145,21 +145,6 @@ func (mr *MockNetlinkLibMockRecorder) DevlinkSetDeviceParam(bus, device, param, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DevlinkSetDeviceParam", reflect.TypeOf((*MockNetlinkLib)(nil).DevlinkSetDeviceParam), bus, device, param, cmode, value) } -// DiscoverRDMASubsystem mocks base method. -func (m *MockNetlinkLib) DiscoverRDMASubsystem() (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DiscoverRDMASubsystem") - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DiscoverRDMASubsystem indicates an expected call of DiscoverRDMASubsystem. -func (mr *MockNetlinkLibMockRecorder) DiscoverRDMASubsystem() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DiscoverRDMASubsystem", reflect.TypeOf((*MockNetlinkLib)(nil).DiscoverRDMASubsystem)) -} - // IsLinkAdminStateUp mocks base method. func (m *MockNetlinkLib) IsLinkAdminStateUp(link netlink.Link) bool { m.ctrl.T.Helper() @@ -304,6 +289,21 @@ func (mr *MockNetlinkLibMockRecorder) RdmaLinkByName(name interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RdmaLinkByName", reflect.TypeOf((*MockNetlinkLib)(nil).RdmaLinkByName), name) } +// RdmaSystemGetNetnsMode mocks base method. +func (m *MockNetlinkLib) RdmaSystemGetNetnsMode() (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RdmaSystemGetNetnsMode") + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// RdmaSystemGetNetnsMode indicates an expected call of RdmaSystemGetNetnsMode. +func (mr *MockNetlinkLibMockRecorder) RdmaSystemGetNetnsMode() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RdmaSystemGetNetnsMode", reflect.TypeOf((*MockNetlinkLib)(nil).RdmaSystemGetNetnsMode)) +} + // VDPADelDev mocks base method. func (m *MockNetlinkLib) VDPADelDev(name string) error { m.ctrl.T.Helper() diff --git a/pkg/host/internal/lib/netlink/netlink.go b/pkg/host/internal/lib/netlink/netlink.go index 7d857921d..ad6056710 100644 --- a/pkg/host/internal/lib/netlink/netlink.go +++ b/pkg/host/internal/lib/netlink/netlink.go @@ -68,8 +68,8 @@ type NetlinkLib interface { RdmaLinkByName(name string) (*netlink.RdmaLink, error) // IsLinkAdminStateUp checks if the admin state of a link is up IsLinkAdminStateUp(link Link) bool - // DiscoverRDMASubsystem returns RDMA subsystem mode - DiscoverRDMASubsystem() (string, error) + // RdmaSystemGetNetnsMode returns RDMA subsystem mode + RdmaSystemGetNetnsMode() (string, error) } type libWrapper struct{} @@ -188,7 +188,7 @@ func (w *libWrapper) IsLinkAdminStateUp(link Link) bool { return link.Attrs().Flags&net.FlagUp == 1 } -// DiscoverRDMASubsystem returns RDMA subsystem mode -func (w *libWrapper) DiscoverRDMASubsystem() (string, error) { +// RdmaSystemGetNetnsMode returns RDMA subsystem mode +func (w *libWrapper) RdmaSystemGetNetnsMode() (string, error) { return netlink.RdmaSystemGetNetnsMode() } diff --git a/pkg/host/internal/network/network.go b/pkg/host/internal/network/network.go index 7349c8697..621ceef93 100644 --- a/pkg/host/internal/network/network.go +++ b/pkg/host/internal/network/network.go @@ -432,30 +432,40 @@ func (n *network) GetPciAddressFromInterfaceName(interfaceName string) (string, func (n *network) DiscoverRDMASubsystem() (string, error) { log.Log.Info("DiscoverRDMASubsystem(): retrieving RDMA subsystem mode") - subsystem, err := n.netlinkLib.DiscoverRDMASubsystem() + subsystem, err := n.netlinkLib.RdmaSystemGetNetnsMode() if err != nil { log.Log.Error(err, "DiscoverRDMASubsystem(): failed to get RDMA subsystem mode") return "", err } + log.Log.V(2).Info("DiscoverRDMASubsystem(): RDMA subsystem mode", "mode", subsystem) return subsystem, nil } func (n *network) SetRDMASubsystem(mode string) error { - log.Log.Info("SetRDMASubsystem(): Updating RDMA subsystem mode") + log.Log.Info("SetRDMASubsystem(): Updating RDMA subsystem mode", "mode", mode) + path := filepath.Join(vars.FilesystemRoot, consts.Host, "etc", "modprobe.d", "sriov_network_operator_modules_config.conf") + + if mode == "" { + err := os.Remove(path) + if err != nil && !errors.Is(err, os.ErrNotExist) { + log.Log.Error(err, "failed to remove ib_core config file") + return err + } + return nil + } modeValue := 1 if mode == "exclusive" { modeValue = 0 } - config := fmt.Sprintf("options ib_core netns_mode=%d\n", modeValue) - path := filepath.Join(vars.FilesystemRoot, consts.Host, "etc", "modprobe.d", "ib_core.conf") - err := os.WriteFile(path, []byte(config), 0644) + config := fmt.Sprintf("# This file is managed by sriov-network-operator do not edit.\noptions ib_core netns_mode=%d\n", modeValue) + err := os.WriteFile(path, []byte(config), 0644) if err != nil { - log.Log.Error(err, "SetRDMASubsystem(): failed to write ib_core config") - return fmt.Errorf("failed to write ib_core config: %v", err) + log.Log.Error(err, "SetRDMASubsystem(): failed to write sriov_network_operator_modules_config.conf") + return fmt.Errorf("failed to write sriov_network_operator_modules_config.conf: %v", err) } return nil diff --git a/pkg/host/internal/network/network_test.go b/pkg/host/internal/network/network_test.go index 51c56b875..1bbfa6684 100644 --- a/pkg/host/internal/network/network_test.go +++ b/pkg/host/internal/network/network_test.go @@ -285,7 +285,7 @@ var _ = Describe("Network", func() { }) Context("DiscoverRDMASubsystem", func() { It("Should get RDMA Subsystem using netlink", func() { - netlinkLibMock.EXPECT().DiscoverRDMASubsystem().Return("shared", nil) + netlinkLibMock.EXPECT().RdmaSystemGetNetnsMode().Return("shared", nil) pci, err := n.DiscoverRDMASubsystem() Expect(err).NotTo(HaveOccurred()) @@ -301,7 +301,7 @@ var _ = Describe("Network", func() { }, }) Expect(n.SetRDMASubsystem("shared")).NotTo(HaveOccurred()) - helpers.GinkgoAssertFileContentsEquals("/host/etc/modprobe.d/ib_core.conf", "options ib_core netns_mode=1\n") + helpers.GinkgoAssertFileContentsEquals("/host/etc/modprobe.d/sriov_network_operator_modules_config.conf", "# This file is managed by sriov-network-operator do not edit.\noptions ib_core netns_mode=1\n") }) It("Should set RDMA Subsystem exclusive mode", func() { helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{ @@ -311,7 +311,7 @@ var _ = Describe("Network", func() { }, }) Expect(n.SetRDMASubsystem("exclusive")).NotTo(HaveOccurred()) - helpers.GinkgoAssertFileContentsEquals("/host/etc/modprobe.d/ib_core.conf", "options ib_core netns_mode=0\n") + helpers.GinkgoAssertFileContentsEquals("/host/etc/modprobe.d/sriov_network_operator_modules_config.conf", "# This file is managed by sriov-network-operator do not edit.\noptions ib_core netns_mode=0\n") }) }) }) diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index 14b1903e5..29b558597 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -3,9 +3,8 @@ package generic import ( "bytes" "errors" + "fmt" "os/exec" - "strconv" - "strings" "syscall" "sigs.k8s.io/controller-runtime/pkg/log" @@ -47,12 +46,14 @@ type DriverState struct { type DriverStateMapType map[uint]*DriverState +type KargStateMapType map[string]bool + type GenericPlugin struct { PluginName string SpecVersion string DesireState *sriovnetworkv1.SriovNetworkNodeState DriverStateMap DriverStateMapType - DesiredKernelArgs map[string]bool + DesiredKernelArgs KargStateMapType helpers helper.HostHelpersInterface skipVFConfiguration bool skipBridgeConfiguration bool @@ -81,7 +82,7 @@ type genericPluginOptions struct { skipBridgeConfiguration bool } -const scriptsPath = "bindata/scripts/enable-kargs.sh" +const scriptsPath = "bindata/scripts/kargs.sh" // Initialize our plugin and set up initial values func NewGenericPlugin(helpers helper.HostHelpersInterface, options ...Option) (plugin.VendorPlugin, error) { @@ -111,11 +112,20 @@ func NewGenericPlugin(helpers helper.HostHelpersInterface, options ...Option) (p NeedDriverFunc: needDriverCheckVdpaType, DriverLoaded: false, } + + desiredKernelArgs := KargStateMapType{ + consts.KernelArgPciRealloc: false, + consts.KernelArgIntelIommu: false, + consts.KernelArgIommuPt: false, + consts.KernelArgRdmaShared: false, + consts.KernelArgRdmaExclusive: false, + } + return &GenericPlugin{ PluginName: PluginName, SpecVersion: "1.0", DriverStateMap: driverStateMap, - DesiredKernelArgs: make(map[string]bool), + DesiredKernelArgs: desiredKernelArgs, helpers: helpers, skipVFConfiguration: cfg.skipVFConfiguration, skipBridgeConfiguration: cfg.skipBridgeConfiguration, @@ -178,18 +188,13 @@ func (p *GenericPlugin) CheckStatusChanges(current *sriovnetworkv1.SriovNetworkN } } - missingKernelArgs, err := p.getMissingKernelArgs() + shouldUpdate, err := p.shouldUpdateKernelArgs() if err != nil { log.Log.Error(err, "generic-plugin CheckStatusChanges(): failed to verify missing kernel arguments") return false, err } - if len(missingKernelArgs) != 0 { - log.Log.V(0).Info("generic-plugin CheckStatusChanges(): kernel args missing", - "kernelArgs", missingKernelArgs) - } - - return len(missingKernelArgs) != 0, nil + return shouldUpdate, nil } func (p *GenericPlugin) syncDriverState() error { @@ -227,7 +232,7 @@ func (p *GenericPlugin) Apply() error { 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) + p.enableDesiredKernelArgs(consts.KernelArgPciRealloc) } return err } @@ -264,10 +269,10 @@ func needDriverCheckVdpaType(state *sriovnetworkv1.SriovNetworkNodeState, driver } // setKernelArg Tries to add the kernel args via ostree or grubby. -func setKernelArg(karg string) (bool, error) { +func setKernelArg(mode, karg string) error { log.Log.Info("generic plugin setKernelArg()") var stdout, stderr bytes.Buffer - cmd := exec.Command("/bin/sh", scriptsPath, karg) + cmd := exec.Command("/bin/sh", scriptsPath, mode, karg) cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -276,72 +281,75 @@ func setKernelArg(karg string) (bool, error) { if utils.IsCommandNotFound(err) { log.Log.Error(err, "generic plugin setKernelArg(): grubby or ostree command not found. Please ensure that kernel arg are set", "kargs", karg) - return false, nil + return nil } log.Log.Error(err, "generic plugin setKernelArg(): fail to enable kernel arg", "karg", karg) - return false, err - } - - i, err := strconv.Atoi(strings.TrimSpace(stdout.String())) - if err == nil { - if i > 0 { - log.Log.Info("generic plugin setKernelArg(): need to reboot node for kernel arg", "karg", karg) - return true, nil - } + return err } - return false, err + return nil } -// addToDesiredKernelArgs Should be called to queue a kernel arg to be added to the node. -func (p *GenericPlugin) addToDesiredKernelArgs(karg string) { - if _, ok := p.DesiredKernelArgs[karg]; !ok { - log.Log.Info("generic plugin addToDesiredKernelArgs(): Adding to desired kernel arg", "karg", karg) - p.DesiredKernelArgs[karg] = false - } +// enableDesiredKernelArgs Should be called to mark a kernel arg as enabled. +func (p *GenericPlugin) enableDesiredKernelArgs(karg string) { + log.Log.Info("generic plugin enableDesiredKernelArgs(): enable kernel arg", "karg", karg) + p.DesiredKernelArgs[karg] = true } -// getMissingKernelArgs gets Kernel arguments that have not been set. -func (p *GenericPlugin) getMissingKernelArgs() ([]string, error) { - missingArgs := make([]string, 0, len(p.DesiredKernelArgs)) - if len(p.DesiredKernelArgs) == 0 { - return nil, nil - } +// disableDesiredKernelArgs Should be called to mark a kernel arg as disabled. +func (p *GenericPlugin) disableDesiredKernelArgs(karg string) { + log.Log.Info("generic plugin disableDesiredKernelArgs(): disable kernel arg", "karg", karg) + p.DesiredKernelArgs[karg] = false +} +// getKernelArgs gets Kernel arguments on the running sy. +func (p *GenericPlugin) shouldUpdateKernelArgs() (bool, error) { kargs, err := p.helpers.GetCurrentKernelArgs() if err != nil { - return nil, err + return false, err } - for desiredKarg := range p.DesiredKernelArgs { - if !p.helpers.IsKernelArgsSet(kargs, desiredKarg) { - missingArgs = append(missingArgs, desiredKarg) + for karg, kargState := range p.DesiredKernelArgs { + if kargState && !p.helpers.IsKernelArgsSet(kargs, karg) { + return true, nil + } + + if !kargState && p.helpers.IsKernelArgsSet(kargs, karg) { + return true, nil } } - return missingArgs, nil + return false, nil } // syncDesiredKernelArgs should be called to set all the kernel arguments. Returns bool if node update is needed. -func (p *GenericPlugin) syncDesiredKernelArgs(kargs []string) (bool, error) { +func (p *GenericPlugin) syncDesiredKernelArgs() (bool, error) { + kargs, err := p.helpers.GetCurrentKernelArgs() + if err != nil { + return false, err + } + needReboot := false + for karg, kargState := range p.DesiredKernelArgs { + if kargState { + err = setKernelArg("add", karg) + if err != nil { + log.Log.Error(err, "generic-plugin syncDesiredKernelArgs(): fail to set kernel arg", "karg", karg) + return false, err + } - for _, karg := range kargs { - if p.DesiredKernelArgs[karg] { - log.Log.V(2).Info("generic-plugin syncDesiredKernelArgs(): previously attempted to set kernel arg", - "karg", karg) - } - // There is a case when we try to set the kernel argument here, the daemon could decide to not reboot because - // the daemon encountered a potentially one-time error. However we always want to make sure that the kernel - // argument is set once the daemon goes through node state sync again. - update, err := setKernelArg(karg) - if err != nil { - log.Log.Error(err, "generic-plugin syncDesiredKernelArgs(): fail to set kernel arg", "karg", karg) - return false, err - } - if update { - needReboot = true - log.Log.V(2).Info("generic-plugin syncDesiredKernelArgs(): need reboot for setting kernel arg", "karg", karg) + if !p.helpers.IsKernelArgsSet(kargs, karg) { + needReboot = true + } + } else { + err = setKernelArg("remove", karg) + if err != nil { + log.Log.Error(err, "generic-plugin syncDesiredKernelArgs(): fail to remove kernel arg", "karg", karg) + return false, err + } + + if p.helpers.IsKernelArgsSet(kargs, karg) { + needReboot = true + } } - p.DesiredKernelArgs[karg] = true } return needReboot, nil } @@ -420,31 +428,46 @@ func (p *GenericPlugin) shouldConfigureBridges() bool { func (p *GenericPlugin) addVfioDesiredKernelArg(state *sriovnetworkv1.SriovNetworkNodeState) { driverState := p.DriverStateMap[Vfio] if !driverState.DriverLoaded && driverState.NeedDriverFunc(state, driverState) { - p.addToDesiredKernelArgs(consts.KernelArgIntelIommu) - p.addToDesiredKernelArgs(consts.KernelArgIommuPt) + p.enableDesiredKernelArgs(consts.KernelArgIntelIommu) + p.enableDesiredKernelArgs(consts.KernelArgIommuPt) + } +} + +func (p *GenericPlugin) configRdmaKernelArg(state *sriovnetworkv1.SriovNetworkNodeState) error { + if state.Spec.System.RdmaMode == "" { + p.disableDesiredKernelArgs(consts.KernelArgRdmaExclusive) + p.disableDesiredKernelArgs(consts.KernelArgRdmaShared) + } else if state.Spec.System.RdmaMode == "shared" { + p.enableDesiredKernelArgs(consts.KernelArgRdmaShared) + p.disableDesiredKernelArgs(consts.KernelArgRdmaExclusive) + } else if state.Spec.System.RdmaMode == "exclusive" { + p.enableDesiredKernelArgs(consts.KernelArgRdmaExclusive) + p.disableDesiredKernelArgs(consts.KernelArgRdmaShared) + } else { + err := fmt.Errorf("unexpected rdma mode: %s", state.Spec.System.RdmaMode) + log.Log.Error(err, "generic-plugin configRdmaKernelArg(): failed to configure kernel arguments for rdma") + return err } + + return p.helpers.SetRDMASubsystem(state.Spec.System.RdmaMode) } func (p *GenericPlugin) needRebootNode(state *sriovnetworkv1.SriovNetworkNodeState) (bool, error) { needReboot := false p.addVfioDesiredKernelArg(state) - - missingKernelArgs, err := p.getMissingKernelArgs() + err := p.configRdmaKernelArg(state) if err != nil { - log.Log.Error(err, "generic-plugin needRebootNode(): failed to verify missing kernel arguments") return false, err } - if len(missingKernelArgs) != 0 { - needReboot, err = p.syncDesiredKernelArgs(missingKernelArgs) - if err != nil { - log.Log.Error(err, "generic-plugin needRebootNode(): failed to set the desired kernel arguments") - return false, err - } - if needReboot { - log.Log.V(2).Info("generic-plugin needRebootNode(): need reboot for updating kernel arguments") - } + needReboot, err = p.syncDesiredKernelArgs() + if err != nil { + log.Log.Error(err, "generic-plugin needRebootNode(): failed to set the desired kernel arguments") + return false, err + } + if needReboot { + log.Log.V(2).Info("generic-plugin needRebootNode(): need reboot for updating kernel arguments") } return needReboot, nil diff --git a/pkg/plugins/generic/generic_plugin_test.go b/pkg/plugins/generic/generic_plugin_test.go index 0d6701a64..ca2725acf 100644 --- a/pkg/plugins/generic/generic_plugin_test.go +++ b/pkg/plugins/generic/generic_plugin_test.go @@ -33,6 +33,13 @@ var _ = Describe("Generic plugin", func() { ctrl = gomock.NewController(t) hostHelper = mock_helper.NewMockHostHelpersInterface(ctrl) + hostHelper.EXPECT().SetRDMASubsystem("").Return(nil) + hostHelper.EXPECT().GetCurrentKernelArgs().Return("", nil) + hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIntelIommu).Return(false) + hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIommuPt).Return(false) + hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgPciRealloc).Return(false) + hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgRdmaExclusive).Return(false) + hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgRdmaShared).Return(false) genericPlugin, err = NewGenericPlugin(hostHelper) Expect(err).ToNot(HaveOccurred()) @@ -899,10 +906,6 @@ var _ = Describe("Generic plugin", func() { // Load required kernel args. genericPlugin.(*GenericPlugin).addVfioDesiredKernelArg(networkNodeState) - hostHelper.EXPECT().GetCurrentKernelArgs().Return("", nil) - hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIntelIommu).Return(false) - hostHelper.EXPECT().IsKernelArgsSet("", consts.KernelArgIommuPt).Return(false) - changed, err := genericPlugin.CheckStatusChanges(networkNodeState) Expect(err).ToNot(HaveOccurred()) Expect(changed).To(BeTrue())