Improve the virtual plugin support #262

SchSeba · 2022-03-02T11:45:30Z

This commit add the support for virtio interfaces like vhostuser for openstack virtual workers.

implementation details:

on first run (after a reboot) we get all the information we need when the devices are visible to the kernel
we match the mac address to the openstack network ID
if the sriov-network-config-daemon gets reboot it will use the initial file on the node so even if the nics are in vfio the node state will be right
introduce a new system service to remove the initial state file after a reboot to support nic changes both for virtual and BM environments

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

github-actions · 2022-03-02T11:45:42Z

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 · 2022-03-02T11:49:53Z

example:

apiVersion: v1
items:
- apiVersion: sriovnetwork.openshift.io/v1
  kind: SriovNetworkNodeState
  metadata:
    creationTimestamp: "2022-03-01T15:56:55Z"
    generation: 3
    name: dsal-nfv-mmcqx-worker-0-m8xcm
    namespace: openshift-sriov-network-operator
    ownerReferences:
    - apiVersion: sriovnetwork.openshift.io/v1
      blockOwnerDeletion: true
      controller: true
      kind: SriovNetworkNodePolicy
      name: default
      uid: 4eed7c1f-ae4e-49e6-b165-7955e932ccef
    resourceVersion: "3670266"
    uid: c8c7427c-9e6b-43ec-9da6-d831845e7f52
  spec:
    dpConfigVersion: "3220700"
    interfaces:
    - name: ens4
      numVfs: 1
      pciAddress: "0000:00:04.0"
      vfGroups:
      - deviceType: vfio-pci
        policyName: dpdk1
        resourceName: dpdk1
        vfRange: 0-0
    - numVfs: 1
      pciAddress: "0000:00:06.0"
      vfGroups:
      - deviceType: vfio-pci
        policyName: dpdk2
        resourceName: dpdk2
        vfRange: 0-0
  status:
    interfaces:
    - Vfs:
      - deviceID: "1000"
        driver: virtio-pci
        mac: fa:16:3e:1c:96:9b
        pciAddress: "0000:00:03.0"
        vendor: 1af4
        vfID: 0
      deviceID: "1000"
      driver: virtio-pci
      linkSpeed: -1 Mb/s
      linkType: ETH
      mac: fa:16:3e:1c:96:9b
      name: ens3
      netFilter: openstack/NetworkID:bfe9d7ea-0b50-4e8e-b49f-e051a9a4c1c0
      numVfs: 1
      pciAddress: "0000:00:03.0"
      totalvfs: 1
      vendor: 1af4
    - Vfs:
      - deviceID: "1000"
        driver: vfio-pci
        pciAddress: "0000:00:04.0"
        vendor: 1af4
        vfID: 0
      deviceID: "1000"
      driver: vfio-pci
      netFilter: openstack/NetworkID:32a38a94-9135-497b-9231-afe62a0e3811
      numVfs: 1
      pciAddress: "0000:00:04.0"
      totalvfs: 1
      vendor: 1af4
    - Vfs:
      - deviceID: "1515"
        driver: vfio-pci
        pciAddress: "0000:00:06.0"
        vendor: "8086"
        vfID: 0
      deviceID: "1515"
      driver: vfio-pci
      netFilter: openstack/NetworkID:b4322d87-6032-4247-be7c-9fe00b1b4c97
      numVfs: 1
      pciAddress: "0000:00:06.0"
      totalvfs: 1
      vendor: "8086"
    syncStatus: Succeeded
kind: List
metadata:
  resourceVersion: ""
  selfLink: ""

apiVersion: v1
items:
- apiVersion: sriovnetwork.openshift.io/v1
  kind: SriovNetworkNodePolicy
  metadata:
    creationTimestamp: "2022-03-01T15:56:52Z"
    generation: 1
    name: default
    namespace: openshift-sriov-network-operator
    resourceVersion: "3218392"
    uid: 4eed7c1f-ae4e-49e6-b165-7955e932ccef
  spec:
    nicSelector: {}
    nodeSelector: {}
    numVfs: 0
    resourceName: ""
- apiVersion: sriovnetwork.openshift.io/v1
  kind: SriovNetworkNodePolicy
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"sriovnetwork.openshift.io/v1","kind":"SriovNetworkNodePolicy","metadata":{"annotations":{},"generation":1,"name":"dpdk1","namespace":"openshift-sriov-network-operator"},"spec":{"deviceType":"vfio-pci","nicSelector":{"netFilter":"openstack/NetworkID:32a38a94-9135-497b-9231-afe62a0e3811"},"nodeSelector":{"feature.node.kubernetes.io/network-sriov.capable":"true"},"numVfs":1,"priority":99,"resourceName":"dpdk1"}}
    creationTimestamp: "2022-03-01T16:00:32Z"
    generation: 1
    name: dpdk1
    namespace: openshift-sriov-network-operator
    resourceVersion: "3220672"
    uid: b71eb562-5118-4353-8530-9d03a7825466
  spec:
    deviceType: vfio-pci
    nicSelector:
      netFilter: openstack/NetworkID:32a38a94-9135-497b-9231-afe62a0e3811
    nodeSelector:
      feature.node.kubernetes.io/network-sriov.capable: "true"
    numVfs: 1
    priority: 99
    resourceName: dpdk1
- apiVersion: sriovnetwork.openshift.io/v1
  kind: SriovNetworkNodePolicy
  metadata:
    annotations:
      kubectl.kubernetes.io/last-applied-configuration: |
        {"apiVersion":"sriovnetwork.openshift.io/v1","kind":"SriovNetworkNodePolicy","metadata":{"annotations":{},"name":"dpdk2","namespace":"openshift-sriov-network-operator"},"spec":{"deviceType":"vfio-pci","nicSelector":{"netFilter":"openstack/NetworkID:b4322d87-6032-4247-be7c-9fe00b1b4c97"},"nodeSelector":{"feature.node.kubernetes.io/network-sriov.capable":"true"},"numVfs":1,"priority":99,"resourceName":"dpdk2"}}
    creationTimestamp: "2022-03-01T16:00:33Z"
    generation: 1
    name: dpdk2
    namespace: openshift-sriov-network-operator
    resourceVersion: "3220681"
    uid: e4e48633-ae01-4247-bbfd-2b06c9a54bf4
  spec:
    deviceType: vfio-pci
    nicSelector:
      netFilter: openstack/NetworkID:b4322d87-6032-4247-be7c-9fe00b1b4c97
    nodeSelector:
      feature.node.kubernetes.io/network-sriov.capable: "true"
    numVfs: 1
    priority: 99
    resourceName: dpdk2
kind: List
metadata:
  resourceVersion: ""
  selfLink: ""

SchSeba · 2022-03-03T16:54:22Z

/cc @atyronesmith

github-actions · 2022-03-07T16:26:52Z

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.

bn222 · 2022-03-14T15:56:28Z

pkg/utils/utils_virtual.go

+
+	return names[0]
+}
+


Can you make this return an error instead of calling glog and handle that error in CreateOpenstackDevicesInfo?

@bn222 sorry I was not able to understand you will like this function to return (string,error) and just change

if name := tryToGetVirtualInterfaceName(device.Address); name != "" {

to

if name,error := tryToGetVirtualInterfaceName(device.Address); err != nil {

Yes, that's correct.

pliurh · 2022-03-16T04:28:27Z

pkg/daemon/writer.go

+	return &utils.InitialState, nil
+}
+
+func (w *NodeStateStatusWriter) createCleanUpServiceIfNeeded() error {


Do you want this to work on RHCOS nodes? I think we need to use MCO to inject the systemd service for RHCOS nodes.

Hi @pliurh I test this one also on RHCOS and it's working the service is created.

another option will be for the sriov operator to create this system service always for masters and workers pools is that a valid solution?

SchSeba · 2022-03-21T14:50:59Z

rebased on master

github-actions · 2022-03-21T14:51:00Z

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.

EmilienM · 2022-03-25T12:53:14Z

LGTM

github-actions · 2022-03-31T10:42:25Z

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 · 2022-03-31T10:46: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 · 2022-03-31T11:59:38Z

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 · 2022-03-31T12:00:23Z

@pliurh can you please have a look?

I change the location of the initial file to be in /tmp on the host file system as requested. this way the file we get removed after every reboot without the need of a system service

MaysaMacedo · 2022-03-31T11:36:04Z

deploy/configmap.yaml

@@ -21,4 +21,4 @@ data:
  Broadcom_bnxt_BCM57414_2x25G: "14e4 16d7 16dc"
  Broadcom_bnxt_BCM75508_2x100G: "14e4 1750 1806"
  Qlogic_qede_QL45000_50G: "1077 1654 1664"
-
+  Red_Hat_Virtio_network_device: "1af4 1000 1000"


Can you also add to:
https://github.com/openshift/sriov-network-operator/blob/master/manifests/stable/supported-nic-ids_v1_configmap.yaml (once we backport it)?

can you clarify on the 1000 I see it configured as 0000
https://github.com/k8snetworkplumbingwg/sriov-network-operator/pull/271/files#r839536286

I will add it when I rebase the openshift repo base on this PR.

The deviceID is 1000 I checked it again

github-actions · 2022-03-31T14:43:05Z

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.

MaysaMacedo · 2022-03-31T15:47:34Z

pkg/webhook/validate.go

 	for key := range utils.PlatformMap {
-		if strings.Contains(strings.ToLower(node.Spec.ProviderID), strings.ToLower(key)) && sriovnetworkv1.IsVfSupportedModel(iface.Vendor, iface.DeviceID) {
+		if strings.Contains(strings.ToLower(node.Spec.ProviderID), strings.ToLower(key)) &&
+			selector.NetFilter != "" && selector.NetFilter == iface.NetFilter &&


EmilienM · 2022-04-04T14:08:11Z

LGTM

pliurh · 2022-04-07T07:43:26Z

bindata/manifests/config-units/initial-config-reset.yaml

@@ -0,0 +1,17 @@
+contents: |


We don't need this systemd service anymore, do we?

right! done

pliurh · 2022-04-07T07:45:36Z

pkg/daemon/writer.go

 )

 const (
-	CheckpointFileName = "sno-initial-node-state.json"
+	CheckpointFileName                = "sno-initial-node-state.json"
+	InitialConfigurationResetUnitFile = "bindata/manifests/config-units/initial-config-reset.yaml"


zeeke · 2022-04-07T14:52:29Z

pkg/daemon/writer.go

 	glog.V(0).Infof("Run(): start writer")
 	msg := Message{}

-	var err error
+	if runonce {


What about refactoring this if-body to a different function and getting rid of the runonce parameters?

atyronesmith · 2022-04-11T16:26:44Z

lgtm

This commit add the support for virtio interfaces like vhostuser for openstack virtual workers. implementation details: * on first run (after a reboot) we get all the information we need when the devices are visible to the kernel * we match the mac address to the openstack network ID * if the sriov-network-config-daemon gets reboot it will use the initial file on the node so even if the nics are in vfio the node state will be right move the operator initial state file to /tmp on the host so it will get deleted on every reboot to support nic changes both for virtual and BM environments Signed-off-by: Sebastian Sch <[email protected]>

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

github-actions · 2022-04-13T10:07: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.

Eoghan1232

lgtm

pkg/daemon/writer.go

adrianchiris · 2022-04-13T17:00:58Z

pkg/daemon/writer.go

+				return err
+			}
+		} else {
+			devicesInfo := make(utils.OSPDevicesInfo)


nit: can we move this logic to: utils.CreateOpenstackDevicesInfoFromNodeStatus(ns) or similar ?

pkg/daemon/writer.go

pkg/utils/utils.go

pliurh · 2022-04-14T04:41:01Z

/lgtm

this script is gathering the following: * openshift-sriov-network-operator namespace. * the following resources: - sriovnetworknodepolicies. - sriovnetworknodestates. - sriovnetworkpoolconfigs. - sriovnetworks. - sriovoperatorconfigs. - sriovibnetworks. * the following logs from the sriov-config-daemon: - /etc/sno-initial-node-state.json. (*) - /proc/cmdline. - dmseg. - ip link. - var/multus.log (if exists). - netns and output of `ip a` fron every ns. - NIC firmware and driver from ethtool. (*) Need to update the path to /tmp/sno-initial-node-state.json after: k8snetworkplumbingwg/sriov-network-operator#262

we can't run the udev rule on virtual environments because all the nics in that type of deployment will be presented as VFs. this means that we are going to disable nics on the machine itself Signed-off-by: Sebastian Sch <[email protected]>

github-actions · 2022-04-18T14:19:44Z

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 · 2022-04-18T14:32:09Z

@adrianchiris done can you make another round please :)

e0ne · 2022-04-19T13:15:06Z

/test-all

e0ne

LGTM

adrianchiris

was accidently logged in as npwg-robot :) LGTM

I merged this.

SchSeba requested review from pliurh and adrianchiris March 2, 2022 11:45

SchSeba force-pushed the support_vhostuser_shiftstack branch from 315b4be to 375a490 Compare March 7, 2022 16:26

bn222 reviewed Mar 14, 2022

View reviewed changes

pliurh reviewed Mar 16, 2022

View reviewed changes

SchSeba force-pushed the support_vhostuser_shiftstack branch from 375a490 to df4f8f4 Compare March 21, 2022 14:50

SchSeba force-pushed the support_vhostuser_shiftstack branch from df4f8f4 to 38612c9 Compare March 31, 2022 10:42

SchSeba force-pushed the support_vhostuser_shiftstack branch from 38612c9 to b76e3b8 Compare March 31, 2022 10:45

SchSeba force-pushed the support_vhostuser_shiftstack branch from b76e3b8 to d436eff Compare March 31, 2022 11:59

MaysaMacedo reviewed Mar 31, 2022

View reviewed changes

Eoghan1232 self-requested a review April 5, 2022 13:06

SchSeba mentioned this pull request Apr 6, 2022

OpenStack: Support generic virtio device for DPDK #271

Closed

pliurh reviewed Apr 7, 2022

View reviewed changes

zeeke reviewed Apr 7, 2022

View reviewed changes

Fix the validation webhook for virtual environments

04a3f69

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

SchSeba force-pushed the support_vhostuser_shiftstack branch from 33ec717 to 5243dc1 Compare April 13, 2022 10:06

Eoghan1232 approved these changes Apr 13, 2022

View reviewed changes

adrianchiris reviewed Apr 13, 2022

View reviewed changes

github-actions bot added the lgtm label Apr 14, 2022

liornoy mentioned this pull request Apr 17, 2022

Add SR-IOV gather script openshift/must-gather#297

Merged

SchSeba force-pushed the support_vhostuser_shiftstack branch from 5243dc1 to 346de67 Compare April 18, 2022 14:19

e0ne approved these changes Apr 19, 2022

View reviewed changes

npwg-robot approved these changes Apr 19, 2022

View reviewed changes

npwg-robot merged commit 70b25f6 into k8snetworkplumbingwg:master Apr 19, 2022

adrianchiris reviewed Apr 19, 2022

View reviewed changes

Improve the virtual plugin support #262

Improve the virtual plugin support #262

Conversation

SchSeba commented Mar 2, 2022

github-actions bot commented Mar 2, 2022

SchSeba commented Mar 2, 2022

SchSeba commented Mar 3, 2022

github-actions bot commented Mar 7, 2022

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 Mar 21, 2022

github-actions bot commented Mar 21, 2022

EmilienM commented Mar 25, 2022

github-actions bot commented Mar 31, 2022

github-actions bot commented Mar 31, 2022

github-actions bot commented Mar 31, 2022

SchSeba commented Mar 31, 2022

MaysaMacedo Mar 31, 2022 • edited Loading

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 31, 2022

Choose a reason for hiding this comment

EmilienM commented Apr 4, 2022

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

atyronesmith commented Apr 11, 2022

github-actions bot commented Apr 13, 2022

Eoghan1232 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pliurh commented Apr 14, 2022

github-actions bot commented Apr 18, 2022

SchSeba commented Apr 18, 2022

e0ne commented Apr 19, 2022

e0ne left a comment

Choose a reason for hiding this comment

adrianchiris left a comment

Choose a reason for hiding this comment

MaysaMacedo Mar 31, 2022 •

edited

Loading