OVS HWOL: support multiple MCPs #125

zshi-redhat · 2021-05-07T04:02:57Z

Introduced a new CRD SriovNetworkPoolConfig for pool (not device)
related configurations such as OVS HWOL. This may be extended
to support future features that require pool specific configuration.
For example, SmartNIC offload, Accelerated bridge offload etc.

Removed MCO plugins and moved the writing of switchdev device config
file into generic plugin (shared by OpenShift and Kubernetes deployment).
Removed the writing of switchdev device config file from K8s plugin.

For configuring OVS HWOL on OpenShift, the steps are changed to:

Create SriovNetworkPoolConfig to specify the name of MachineConfigPool
Configure SriovNetworkNodePolicy with eSwitchMode: switchdev (This writes switchdev.conf file and trigger drain/reboot where needed)

For configuring OVS HWOL on Kubernetes, the steps are not changed.

zshi-redhat · 2021-05-07T04:03:14Z

This PR replaces #106

pliurh · 2021-05-08T02:59:52Z

It's better to put the operator-sdk upgrade changes in separate PR.

zshi-redhat · 2021-05-08T06:04:26Z

It's better to put the operator-sdk upgrade changes in separate PR.

New PR for operator-sdk project update: #129

zshi-redhat · 2021-06-25T04:28:46Z

Resolved conflict
Updated SriovNetworkNodeConfig API to add Name match on MachineConfigPool

pliurh · 2021-06-30T03:47:27Z

controllers/sriovnetworknodeconfig_controller.go

+func (r *SriovNetworkNodeConfigReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
+	_ = context.Background()
+	// // Fetch SriovNetworkNodeConfigList
+	configList := &sriovnetworkv1.SriovNetworkNodeConfigList{}


If we have a one-to-one mapping, we don't need to fetch all the SriovNetworkNodeConfigList CRs here.

pliurh · 2021-06-30T03:50:01Z

api/v1/sriovnetworknodeconfig_types.go

+	// OVS HWOL configurations are generated automatically by Operator
+	// Labels in NodeSelector are ANDed when selecting Kubernetes Nodes
+	// On OpenShift:
+	// NodeSelector matches on Labels defined in MachineConfigPoolSpec.NodeSelector


Shall we just omit this flag for openshift? It would be more clear if we have a one-to-one mapping between the SriovNetworkNodeConfig CR and MCP CR.

I removed the NodeSelector field, and focus this PR on openshift support.
NodeSelector can be added later for k8s implementation.

pliurh · 2021-06-30T04:05:46Z

pkg/plugins/generic/generic_plugin.go

+		}
+	}
+
+	update, err := utils.WriteSwitchdevConfFile(state)


I think we shall only invoke WriteSwitchdevConfFile when the swichdev is enabled on one PF. So maybe add IsSwitchdevModeSpec check here.

pliurh · 2021-06-30T04:18:28Z

controllers/sriovnetworknodeconfig_controller.go

+			err = r.Get(context.TODO(), types.NamespacedName{Name: mcName}, foundMC)
+			if err != nil {
+				if !errors.IsNotFound(err) {
+					err = r.Delete(context.TODO(), foundMC)


This shall be invoked when the MC CR is found without error. It shall be moved to line 177.

pliurh · 2021-06-30T04:21:13Z

api/v1/sriovnetworknodeconfig_types.go

+	// NodeSelector matches on Labels defined in MachineConfigPoolSpec.NodeSelector
+	// OVS HWOL MachineConfigs are generated and applied to Nodes in MachineConfigPool
+	// Labels in NodeSelector are ANDed when matching on MachineConfigPoolSpec.NodeSelector
+	NodeSelector map[string]string `json:"nodeSelector,omitempty"`


Shall we also add the logic to check the NodeSelector in the k8s_plugin for the Kubernetes scenario?

zshi-redhat · 2021-07-01T06:59:10Z

/hold

zshi-redhat · 2021-07-02T06:39:10Z

/unhold

pliurh · 2021-07-05T03:12:00Z

controllers/sriovnetworkpoolconfig_controller.go

+	mcpMap := make(map[string]bool)
+
+	mcpList := &mcfgv1.MachineConfigPoolList{}
+	err := r.List(context.TODO(), mcpList, &client.ListOptions{})


Why we fetch the whole list of MCPs, instead of getting the MCP by Spec.OvsHardwareOffloadConfig.Name? We have a one-to-one mapping between SriovNetworkPoolConfig, MCP CR here, don't we?

Done, changed to get MCP according to Spec.OvsHardwareOffloadConfig.Name.

pliurh · 2021-07-06T02:54:28Z

controllers/sriovnetworkpoolconfig_controller.go

+	foundMC := &mcfgv1.MachineConfig{}
+	mcp := &mcfgv1.MachineConfigPool{}
+
+	if mcpName == "" {


Shall we move the MCP name validation to the operator webhook?

Will add the webhook check in a separate PR.

pliurh · 2021-07-06T02:57:27Z

controllers/sriovnetworkpoolconfig_controller.go

+		return nil
+	}
+
+	err := r.Get(context.TODO(), types.NamespacedName{Name: mcpName}, mcp)


The variable mcp is not used after getting? Is that only for checking the existence of the MCP?

Yes, only for checking the existence of MCP.

One node can only be added in one custom MCP pool by design of MCO. Since current ovs-hw-offload MCs are applied to hardcoded MCP ovs-hw-offload-worker, other existing custome MCPs cannot be enabled with ovs hw offload configuration. This commits allows ovs-hw-offload MCs be applied to multiple MCPs as specified in default SriovNetworkNodeConfig.

zshi-redhat · 2021-07-09T05:01:51Z

Rebased on master

pliurh · 2021-07-09T05:45:47Z

/lgtm

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from f5c463c to 224afd3 Compare May 7, 2021 04:03

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from 224afd3 to dc87455 Compare May 20, 2021 05:36

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from dc87455 to 2ce71df Compare June 25, 2021 04:27

pliurh reviewed Jun 30, 2021

View reviewed changes

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from 2ce71df to b4f2945 Compare July 1, 2021 03:07

github-actions bot added the hold label Jul 1, 2021

zshi-redhat force-pushed the ovs-hw-offload-new-api branch 2 times, most recently from bb82c48 to 4bd9cc9 Compare July 2, 2021 06:38

zshi-redhat removed the hold label Jul 2, 2021

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from 4bd9cc9 to 24116da Compare July 2, 2021 11:12

ykulazhenkov mentioned this pull request Jul 2, 2021

WIP: add API to configure RDMA subsystem network namespace mode #156

Closed

pliurh reviewed Jul 5, 2021

View reviewed changes

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from 24116da to d4786b7 Compare July 6, 2021 02:01

pliurh reviewed Jul 6, 2021

View reviewed changes

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from d4786b7 to add7d5b Compare July 6, 2021 08:37

zshi-redhat added 2 commits July 9, 2021 12:41

Initialize sriovnetworkpoolconfig API

34299d6

zshi-redhat force-pushed the ovs-hw-offload-new-api branch from add7d5b to 463afa9 Compare July 9, 2021 05:01

github-actions bot added the lgtm label Jul 9, 2021

pliurh merged commit 372cf5b into k8snetworkplumbingwg:master Jul 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OVS HWOL: support multiple MCPs #125

OVS HWOL: support multiple MCPs #125

zshi-redhat commented May 7, 2021 •

edited

Loading

zshi-redhat commented May 7, 2021

pliurh commented May 8, 2021

zshi-redhat commented May 8, 2021

zshi-redhat commented Jun 25, 2021

pliurh Jun 30, 2021

zshi-redhat Jul 1, 2021

pliurh Jun 30, 2021

zshi-redhat Jul 1, 2021

pliurh Jun 30, 2021

zshi-redhat Jul 1, 2021

pliurh Jun 30, 2021

zshi-redhat Jul 1, 2021

pliurh Jun 30, 2021

zshi-redhat commented Jul 1, 2021

zshi-redhat commented Jul 2, 2021

pliurh Jul 5, 2021

zshi-redhat Jul 6, 2021

pliurh Jul 6, 2021

zshi-redhat Jul 6, 2021

pliurh Jul 6, 2021

zshi-redhat Jul 6, 2021

zshi-redhat commented Jul 9, 2021

pliurh commented Jul 9, 2021

OVS HWOL: support multiple MCPs #125

OVS HWOL: support multiple MCPs #125

Conversation

zshi-redhat commented May 7, 2021 • edited Loading

zshi-redhat commented May 7, 2021

pliurh commented May 8, 2021

zshi-redhat commented May 8, 2021

zshi-redhat commented Jun 25, 2021

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

zshi-redhat commented Jul 1, 2021

zshi-redhat commented Jul 2, 2021

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

zshi-redhat commented Jul 9, 2021

pliurh commented Jul 9, 2021

zshi-redhat commented May 7, 2021 •

edited

Loading