Add kernel args for rdma mode to complement the modprobe file

Signed-off-by: Sebastian Sch <[email protected]>

Makefile

@@ -226,7 +226,7 @@ test-e2e-k8s: export NAMESPACE=sriov-network-operator
 test-e2e-k8s: test-e2e
 
 test-bindata-scripts: fakechroot
-	fakechroot ./test/scripts/enable-kargs_test.sh
+	fakechroot ./test/scripts/kargs_test.sh
 
 test-%: generate manifests envtest
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./$*/... -coverprofile cover-$*.out -coverpkg ./... -v

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

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)

go.mod

@@ -38,6 +38,7 @@ require (
 	github.com/vishvananda/netlink v1.2.1-beta.2.0.20240221172127-ec7bcb248e94
 	github.com/vishvananda/netns v0.0.4
 	go.uber.org/zap v1.25.0
+	golang.org/x/net v0.23.0
 	golang.org/x/time v0.3.0
 	gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
 	gopkg.in/yaml.v3 v3.0.1
@@ -144,7 +145,6 @@ require (
 	golang.org/x/crypto v0.21.0 // indirect
 	golang.org/x/exp v0.0.0-20231006140011-7918f672742d // indirect
 	golang.org/x/mod v0.13.0 // indirect
-	golang.org/x/net v0.23.0 // indirect
 	golang.org/x/oauth2 v0.13.0 // indirect
 	golang.org/x/sync v0.4.0 // indirect
 	golang.org/x/sys v0.20.0 // indirect

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

pkg/daemon/daemon.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
-	"os/exec"
 	"reflect"
 	"sync"
 	"time"
@@ -429,16 +428,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
@@ -761,11 +750,11 @@ func (dn *Daemon) rebootNode() {
 	// However note we use `;` instead of `&&` so we keep rebooting even
 	// if kubelet failed to shutdown - that way the machine will still eventually reboot
 	// as systemd will time out the stop invocation.
-	cmd := exec.Command("systemd-run", "--unit", "sriov-network-config-daemon-reboot",
+	stdOut, StdErr, err := dn.HostHelpers.RunCommand("systemd-run", "--unit", "sriov-network-config-daemon-reboot",
 		"--description", "sriov-network-config-daemon reboot node", "/bin/sh", "-c", "systemctl stop kubelet.service; reboot")
 
-	if err := cmd.Run(); err != nil {
-		log.Log.Error(err, "failed to reboot node")
+	if err != nil {
+		log.Log.Error(err, "failed to reboot node", "stdOut", stdOut, "StdErr", StdErr)
 	}
 }
 

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

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()
 }

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

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())
@@ -297,21 +297,21 @@ var _ = Describe("Network", func() {
 			helpers.GinkgoConfigureFakeFS(&fakefilesystem.FS{
 				Dirs: []string{"/host/etc/modprobe.d"},
 				Files: map[string][]byte{
-					"/host/etc/modprobe.d/ib_core.conf": {},
+					"/host/etc/modprobe.d/sriov_network_operator_modules_config.conf": {},
 				},
 			})
 			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{
 				Dirs: []string{"/host/etc/modprobe.d"},
 				Files: map[string][]byte{
-					"/host/etc/modprobe.d/ib_core.conf": {},
+					"/host/etc/modprobe.d/sriov_network_operator_modules_config.conf": {},
 				},
 			})
 			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")
 		})
 	})
 })