Implement RDMA subsystem mode change #666

e0ne · 2024-03-24T16:39:48Z

Now it's possible to configure RDMA subsystem mode using SR-IOV Network Operator in systemd mode.

We can't configure RDMA subsystem in a daemon mode because it should be done on host before any network namespace is created.

github-actions · 2024-03-24T16:39:58Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-24T16:51:12Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-24T17:30:04Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-24T17:45:50Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-24T18:09:50Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-24T20:26:39Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

coveralls · 2024-03-24T20:32:24Z

Pull Request Test Coverage Report for Build 10772338186

Details

53 of 188 (28.19%) changed or added relevant lines in 11 files are covered.
5 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-0.4%) to 44.668%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/host/internal/lib/netlink/netlink.go	0	3	0.0%
pkg/daemon/writer.go	0	6	0.0%
controllers/sriovnetworknodepolicy_controller.go	0	7	0.0%
pkg/daemon/daemon.go	1	8	12.5%
api/v1/zz_generated.deepcopy.go	2	12	16.67%
pkg/host/internal/lib/netlink/mock/mock_netlink.go	0	11	0.0%
pkg/helper/mock/mock_helper.go	0	21	0.0%
pkg/host/mock/mock_host.go	0	21	0.0%
pkg/host/internal/network/network.go	0	24	0.0%
controllers/helper.go	49	74	66.22%

Files with Coverage Reduction	New Missed Lines	%
controllers/generic_network_controller.go	5	74.53%

Totals
Change from base Build 10734670028:	-0.4%
Covered Lines:	6606
Relevant Lines:	14789

💛 - Coveralls

ykulazhenkov · 2024-03-25T07:58:58Z

pkg/systemd/systemd.go

 	newFile := false
 	// remove the device plugin revision as we don't need it here
 	newState.Spec.DpConfigVersion = ""

+	// shared mode is a default on OS
+	rdmaMode := consts.RdmaSubsystemModeShared


I think we should try to query/change mode only in case if rdmaMode parameter is explicitly set in the poolConfig, to provide a safer behavior for ENVs which doesn't use RDMA.

ykulazhenkov · 2024-03-25T08:00:19Z

cmd/sriov-network-config-daemon/service.go

@@ -152,6 +152,17 @@ func phasePre(setupLog logr.Logger, conf *systemd.SriovConfig, hostHelpers helpe
 	hostHelpers.TryEnableTun()
 	hostHelpers.TryEnableVhostNet()

+	rdmaSubsystem, err := hostHelpers.GetRDMASubsystem()


I think we should execute this logic only if mode configuration is explicitly requested by a user.

ykulazhenkov · 2024-03-25T08:01:10Z

api/v1/sriovnetworkpoolconfig_types.go

+
+	// +kubebuilder:validation:Enum=shared;exclusive
+	// RDMA subsystem. Allowed value "shared", "exclusive".
+	RdmaMode string `json:"rdmaMode,omitempty"`


This option is only valid for systemd mode?
Do we want to document this somehow?

done as log message in a SriovNetworkPoolConfig controller

@e0ne cant we set this using module parameter ?

https://github.com/torvalds/linux/blob/55027e689933ba2e64f3d245fb1ff185b3e7fc81/drivers/infiniband/core/device.c#L128C1-L128C19

github-actions · 2024-03-25T20:11:11Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-03-25T20:39:21Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

ykulazhenkov

I added few additional comments

ykulazhenkov · 2024-03-26T08:12:34Z

cmd/sriov-network-config-daemon/service.go

+	if conf.RdmaMode != "" {
+		rdmaSubsystem, err := hostHelpers.GetRDMASubsystem()
+		if err != nil {
+			setupLog.Error(err, "failed to get RDMA subsystem mode")


If conf.RdmaMode is not empty string, then the user explicitly requested RDMA mode configuration. I think we can return error in this case. WDYT?

ykulazhenkov · 2024-03-26T08:13:11Z

cmd/sriov-network-config-daemon/service.go

+		if rdmaSubsystem != conf.RdmaMode {
+			err = hostHelpers.SetRDMASubsystem(conf.RdmaMode)
+			if err != nil {
+				setupLog.Error(err, "failed to set RDMA subsystem mode")


Do we want to return error here?

we want, thanks!

ykulazhenkov · 2024-03-26T08:14:48Z

pkg/host/internal/kernel/kernel.go

@@ -522,6 +522,34 @@ func (k *kernel) InstallRDMA(packageManager string) error {
 	return nil
 }

+func (k *kernel) GetRDMASubsystem() (string, error) {
+	log.Log.Info("GetRDMASubsystem(): retrieving RDMA subsystem mode")
+	chrootDefinition := utils.GetChrootExtension()


We have helper to enter chroot (part of utilsHelper). Do we want to use it here?

We'd got the same implementation in all `kernel' methods. Let's do it in a scope of a separate PR

github-actions · 2024-03-26T16:04:12Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-04-03T14:53:48Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-04-11T12:20:29Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-04-11T14:43:32Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

e0ne · 2024-04-15T07:02:42Z

@SchSeba could you please review this PR?

github-actions · 2024-05-13T09:29:29Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-05-13T13:42:08Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-05-13T13:50:43Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

SchSeba

nice work getting close to be ready :)

SchSeba · 2024-05-27T14:24:15Z

api/v1/sriovnetworknodestate_types.go

@@ -23,9 +23,14 @@ import (
 // EDIT THIS FILE!  THIS IS SCAFFOLDING FOR YOU TO OWN!
 // NOTE: json tags are required.  Any new fields you add must have json tags for the fields to be serialized.

+type System struct {


nit: please move this one below the SriovNetworkNodeStateSpec struct

SchSeba · 2024-05-27T14:28:40Z

cmd/sriov-network-config-daemon/service.go

@@ -152,6 +153,21 @@ func phasePre(setupLog logr.Logger, conf *systemd.SriovConfig, hostHelpers helpe
 	hostHelpers.TryEnableTun()
 	hostHelpers.TryEnableVhostNet()

+	if conf.Spec.System.RdmaMode != "" {
+		rdmaSubsystem, err := netlink.RdmaSystemGetNetnsMode()


please use the hostHelper interface don't call the netlink lib directly so it will be easier to add unit-tests

SchSeba · 2024-05-27T14:28:45Z

cmd/sriov-network-config-daemon/service.go

+			return fmt.Errorf("failed to get RDMA subsystem mode: %v", err)
+		}
+		if rdmaSubsystem != conf.Spec.System.RdmaMode {
+			err = netlink.RdmaSystemSetNetnsMode(conf.Spec.System.RdmaMode)


please use the hostHelper interface don't call the netlink lib directly so it will be easier to add unit-tests

SchSeba · 2024-05-27T14:33:45Z

deployment/sriov-network-operator/templates/clusterrole.yaml

@@ -58,3 +58,6 @@ rules:
  - apiGroups: [ "config.openshift.io" ]
    resources: [ "infrastructures" ]
    verbs: [ "get", "list", "watch" ]
+  - apiGroups: [ "sriovnetwork.openshift.io" ]


I don't think we need this one as the daemon will continue to only see the nodeState

SchSeba · 2024-05-27T14:41:25Z

pkg/daemon/writer.go

@@ -127,7 +128,13 @@ func (w *NodeStateStatusWriter) pollNicStatus() error {
 	if err != nil {
 		return err
 	}
+	rdmaMode, err = w.hostHelper.GetRDMASubsystem()


please lets have a nice function in hostHelper like DiscoverSriovDevices to DiscoverSystemConfig() or something like that it will return System struct

renamed to DiscoverRDMASubsystem. It will always return a string so there is no need to introduce a new type

github-actions · 2024-05-29T10:26:27Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-06-12T06:13:27Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

adrianchiris · 2024-06-25T09:14:56Z

@e0ne can you rebase the pr ?

github-actions · 2024-06-25T09:32:18Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

github-actions · 2024-06-25T09:33:49Z

Thanks for your PR,
To run vendors CIs use one of:

/test-all: To run all tests for all vendors.
/test-e2e-all: To run all E2E tests for all vendors.
/test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

/skip-all: To skip all tests for all vendors.
/skip-e2e-all: To skip all E2E tests for all vendors.
/skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
Best regards.

d

adrianchiris · 2024-06-25T11:08:21Z

pkg/utils/cluster.go

@@ -161,3 +171,94 @@ func AnnotateNode(ctx context.Context, nodeName string, key, value string, c cli

 	return AnnotateObject(ctx, node, key, value, c)
 }
+
+func FindNodePoolConfig(ctx context.Context, node *corev1.Node, c client.Client) (*sriovnetworkv1.SriovNetworkPoolConfig, []corev1.Node, error) {


nit: add docstring

also im thinking we should have two functions:

find node pool for node

find nodes for node pool (with special handling for case where default node pool was provided)

WDYT ?

Also please add UT for whatever we end up with

adrianchiris · 2024-06-25T11:09:50Z

pkg/utils/cluster.go

@@ -26,6 +28,14 @@ const (
 	controlPlaneNodeLabelKey = "node-role.kubernetes.io/control-plane"
 )

+var (
+	oneNode     = intstr.FromInt32(1)
+	defaultNpcl = &sriovnetworkv1.SriovNetworkPoolConfig{Spec: sriovnetworkv1.SriovNetworkPoolConfigSpec{


nit: can we use full name here ? e.g defaultPoolConfig ?

also the 'l' at the end is not related

adrianchiris · 2024-06-25T11:24:19Z

pkg/utils/cluster.go

+			return nil, nil, err
+		}
+
+		// list all the nodes that are also part of this pool and return them


for those nodes why arent we validating they match exactly one ncp ? like in L223

adrianchiris · 2024-06-25T11:38:15Z

pkg/daemon/daemon.go

@@ -420,6 +421,13 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 	// When using systemd configuration we write the file
 	if vars.UsingSystemdMode {
 		log.Log.V(0).Info("nodeStateSyncHandler(): writing systemd config file to host")
+		// get node object
+		node := &corev1.Node{}


i dont see node is being used in this scope.

SchSeba · 2024-07-15T11:57:38Z

bindata/manifests/daemon/daemonset.yaml

@@ -92,6 +92,23 @@ spec:
              mountPath: /host/etc/os-release
              readOnly: true
        {{- end }}
+        {{- if .RDMACNIImage }}


Hi please rebase this PR now that we merged the rdma-cni deployment

SchSeba · 2024-07-15T11:58:54Z

cmd/sriov-network-config-daemon/service.go

@@ -152,6 +152,21 @@ func phasePre(setupLog logr.Logger, conf *systemd.SriovConfig, hostHelpers helpe
 	hostHelpers.TryEnableTun()
 	hostHelpers.TryEnableVhostNet()

+	if conf.Spec.System.RdmaMode != "" {


I think we can remove this one as we do the configure via the modeprobe file

SchSeba · 2024-07-15T12:00:24Z

api/v1/sriovnetworknodestate_types.go

@@ -114,10 +115,15 @@ type OVSUplinkConfigExt struct {
 	Interface OVSInterfaceConfig `json:"interface,omitempty"`
 }

+type System struct {
+	RdmaMode string `json:"rdmaMode,omitempty"`


I think you can add here also

// +kubebuilder:validation:Enum=shared;exclusive // RDMA subsystem. Allowed value "shared", "exclusive".

SchSeba · 2024-07-15T12:01:35Z

controllers/sriovnetworknodepolicy_controller.go

@@ -269,6 +270,13 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con
 		ns.Name = node.Name
 		ns.Namespace = vars.Namespace
 		j, _ := json.Marshal(ns)
+		netPoolConfig, _, err := utils.FindNodePoolConfig(context.Background(), &node, r.Client)


please use the context from the function don't create a new one

SchSeba · 2024-07-15T12:02:57Z

controllers/sriovnetworkpoolconfig_controller.go

@@ -73,6 +73,19 @@ func (r *SriovNetworkPoolConfigReconciler) Reconcile(ctx context.Context, req ct
 		return reconcile.Result{}, err
 	}

+	// RdmaMode could be set in systemd mode only
+	if instance.Spec.RdmaMode != "" {


please remove this one as we support this on both modes

SchSeba · 2024-07-15T12:04:43Z

pkg/host/internal/kernel/kernel.go

@@ -522,6 +523,29 @@ func (k *kernel) InstallRDMA(packageManager string) error {
 	return nil
 }

+func (k *kernel) DiscoverRDMASubsystem() (string, error) {


I think we can move this function to the network or sriov package

SchSeba · 2024-07-15T12:05:06Z

pkg/host/internal/kernel/kernel.go

@@ -522,6 +523,29 @@ func (k *kernel) InstallRDMA(packageManager string) error {
 	return nil
 }

+func (k *kernel) DiscoverRDMASubsystem() (string, error) {
+	log.Log.Info("DiscoverRDMASubsystem(): retrieving RDMA subsystem mode")
+	subsystem, err := netlink.RdmaSystemGetNetnsMode()


please use the netlink interface in the project so we can have a mock for it on unit tests

SchSeba · 2024-07-15T12:05:27Z

pkg/host/internal/kernel/kernel.go

+	return subsystem, nil
+}
+
+func (k *kernel) SetRDMASubsystem(mode string) error {


I think this function is no needed now that we use the modprobe file

SchSeba · 2024-07-15T12:06:13Z

pkg/utils/cluster.go

@@ -161,3 +171,94 @@ func AnnotateNode(ctx context.Context, nodeName string, key, value string, c cli

 	return AnnotateObject(ctx, node, key, value, c)
 }
+
+func FindNodePoolConfig(ctx context.Context, node *corev1.Node, c client.Client) (*sriovnetworkv1.SriovNetworkPoolConfig, []corev1.Node, error) {


SchSeba · 2024-08-27T19:48:00Z

Hi @e0ne can you please rebase the PR?

e0ne · 2024-08-27T19:51:10Z

Hi @e0ne can you please rebase the PR?

done

SchSeba

nice work!

I left some small comments

SchSeba · 2024-09-01T15:03:52Z

controllers/drain_controller.go

-		}
-		return defaultNpcl, defaultNodeLists, nil
-	}
+	return utils.FindNodePoolConfig(ctx, node, dr.Client)


should we put this in the helper of the controllers?
I don't want to utils to start growing again

it makes sense. done

SchSeba · 2024-09-01T16:22:14Z

controllers/sriovnetworknodepolicy_controller.go

@@ -272,6 +273,13 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con
 		ns.Name = node.Name
 		ns.Namespace = vars.Namespace
 		j, _ := json.Marshal(ns)
+		netPoolConfig, _, err := utils.FindNodePoolConfig(ctx, &node, r.Client)


just a general todo here we should have in memory map I think for this

could you please elaborate on this?

SchSeba · 2024-09-01T16:23:46Z

pkg/host/internal/network/network.go

@@ -23,6 +24,8 @@ import (
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
 )

+var ManifestsPath = "./bindata/manifests"


lets put this in consts

It's not needed anymore, so I deleted it

SchSeba · 2024-09-01T16:25:23Z

pkg/host/internal/network/network.go

+	if mode == "exclusive" {
+		modeValue = 0
+	}
+	config := fmt.Sprintf("options ib_core netns_mode=%d\n", modeValue)


I think if you use this we can remove the other file in the manifests no?

it's deleted now

SchSeba · 2024-09-01T16:25:48Z

pkg/host/internal/network/network.go

+		modeValue = 0
+	}
+	config := fmt.Sprintf("options ib_core netns_mode=%d\n", modeValue)
+	err := os.WriteFile("/etc/modprobe.d/ib_core.conf", []byte(config), 0644)


please use the getExtention here so we know if we are not inside a chroot

SchSeba · 2024-09-01T16:26:13Z

pkg/host/internal/network/network.go

+		return fmt.Errorf("failed to write ib_core config: %v", err)
+	}
+
+	err = os.WriteFile(path.Join(consts.Chroot, "/etc/modprobe.d/ib_core.conf"), []byte(config), 0644)


this looks like a duplicate?

rebase issue, it's deleted now

SchSeba · 2024-09-01T16:27:11Z

pkg/render/render.go

@@ -77,6 +77,14 @@ func RenderDir(manifestDir string, d *RenderData) ([]*unstructured.Unstructured,
 	return out, nil
 }

+func RenderToString(path string, d *RenderData) (string, error) {


I don't think we need this function where we use it?

SchSeba · 2024-09-01T16:28:01Z

pkg/utils/cluster.go

@@ -161,3 +171,94 @@ func AnnotateNode(ctx context.Context, nodeName string, key, value string, c cli

 	return AnnotateObject(ctx, node, key, value, c)
 }
+
+func FindNodePoolConfig(ctx context.Context, node *corev1.Node, c client.Client) (*sriovnetworkv1.SriovNetworkPoolConfig, []corev1.Node, error) {


please move this function to the helpers in controllers better then adding more stuff to utils

e0ne force-pushed the rdma-subsytem-mode branch from c986251 to d3641ab Compare March 24, 2024 16:51

e0ne force-pushed the rdma-subsytem-mode branch from d3641ab to a0af988 Compare March 24, 2024 17:29

e0ne force-pushed the rdma-subsytem-mode branch from a0af988 to a3ce3a3 Compare March 24, 2024 17:45

e0ne force-pushed the rdma-subsytem-mode branch from a3ce3a3 to 3952e04 Compare March 24, 2024 18:09

e0ne force-pushed the rdma-subsytem-mode branch from 3952e04 to bcb0804 Compare March 24, 2024 20:26

ykulazhenkov reviewed Mar 25, 2024

View reviewed changes

e0ne force-pushed the rdma-subsytem-mode branch from bcb0804 to 79013d7 Compare March 25, 2024 20:11

e0ne force-pushed the rdma-subsytem-mode branch from 79013d7 to f60fdf7 Compare March 25, 2024 20:39

ykulazhenkov reviewed Mar 26, 2024

View reviewed changes

e0ne force-pushed the rdma-subsytem-mode branch from f60fdf7 to a5f0d3b Compare March 26, 2024 16:04

e0ne force-pushed the rdma-subsytem-mode branch from a5f0d3b to feb4bd0 Compare April 3, 2024 14:53

e0ne force-pushed the rdma-subsytem-mode branch from feb4bd0 to ab92392 Compare April 11, 2024 12:20

e0ne force-pushed the rdma-subsytem-mode branch from ab92392 to f37c6d1 Compare April 11, 2024 14:43

e0ne force-pushed the rdma-subsytem-mode branch from f37c6d1 to 3d19033 Compare April 16, 2024 15:05

e0ne force-pushed the rdma-subsytem-mode branch from 5bd7f59 to 7134d2f Compare May 13, 2024 13:41

e0ne force-pushed the rdma-subsytem-mode branch from 7134d2f to f381fc5 Compare May 13, 2024 13:50

SchSeba reviewed May 27, 2024

View reviewed changes

e0ne force-pushed the rdma-subsytem-mode branch from f381fc5 to 1bc807a Compare May 29, 2024 10:26

e0ne force-pushed the rdma-subsytem-mode branch from 1bc807a to bae82f1 Compare June 12, 2024 06:13

e0ne requested review from SchSeba and ykulazhenkov June 12, 2024 06:13

e0ne force-pushed the rdma-subsytem-mode branch from bae82f1 to b032b21 Compare June 25, 2024 09:32

e0ne force-pushed the rdma-subsytem-mode branch from b032b21 to e0ae318 Compare June 25, 2024 09:33

adrianchiris previously approved these changes Jun 25, 2024

View reviewed changes

adrianchiris reviewed Jun 25, 2024

View reviewed changes

SchSeba reviewed Jul 15, 2024

View reviewed changes

e0ne force-pushed the rdma-subsytem-mode branch from e0ae318 to 8ce9b72 Compare August 27, 2024 18:47

e0ne force-pushed the rdma-subsytem-mode branch 2 times, most recently from e294415 to 288e028 Compare August 28, 2024 08:34

SchSeba reviewed Sep 1, 2024

View reviewed changes

RDMA subsystem is implemented via ib_core module config.

596e73d

e0ne force-pushed the rdma-subsytem-mode branch from 288e028 to 596e73d Compare September 9, 2024 11:33

e0ne requested a review from SchSeba September 9, 2024 11:34

Implement RDMA subsystem mode change #666

Are you sure you want to change the base?

Implement RDMA subsystem mode change #666

Conversation

e0ne commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

github-actions bot commented Mar 24, 2024

coveralls commented Mar 24, 2024 • edited Loading

Pull Request Test Coverage Report for Build 10772338186

Details

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 25, 2024

github-actions bot commented Mar 25, 2024

ykulazhenkov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 26, 2024

github-actions bot commented Apr 3, 2024

github-actions bot commented Apr 11, 2024

github-actions bot commented Apr 11, 2024

e0ne commented Apr 15, 2024

github-actions bot commented May 13, 2024

github-actions bot commented May 13, 2024

github-actions bot commented May 13, 2024

SchSeba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 29, 2024

github-actions bot commented Jun 12, 2024

adrianchiris commented Jun 25, 2024

github-actions bot commented Jun 25, 2024

github-actions bot commented Jun 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SchSeba commented Aug 27, 2024

e0ne commented Aug 27, 2024

SchSeba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 24, 2024 •

edited

Loading