Support Switchdev for kernel 5.8 and above #75

Mmduh-483 · 2021-02-24T14:14:09Z

Till kernel 5.7 VF representors are exposed as virtual devices, they are not linked to a PCI address.

Starting from kernel 5.8 the VF representors are linked to their PF PCI address, so /sys/bus/pci/devices//net contains the PF net device and it VF representors net devices.

The new kernel behaviour affects 'tryGetInterfaceName' if PF in switchdev and configure-switchdev script

This commit brings needed changes to cope with new kernel behviour.

zshi-redhat · 2021-02-25T03:36:30Z

@Mmduh-483 I don't quite understand the kernel change, do you have reference to the kernel patch?

moshe010 · 2021-02-25T06:00:38Z

@zshi-redhat see this kernel change https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=123f0f53dd64b67e34142485fe866a8a581f12f1. I think it will effect RH 8.4 and above
@Mmduh-483 - please update the commits msg so it will be clear what you are fixing

Mmduh-483 · 2021-02-25T07:37:10Z

@zshi-redhat I updated the commit message and PR description to be more clear

zshi-redhat

Just a quick review, I need to check if the kernel change is in RHEL-8.4, otherwise we may need to support old kernel behaviour.

zshi-redhat · 2021-02-25T08:08:42Z

bindata/manifests/machine-config/files/configure-switchdev.sh.yaml

    names=()

    while read pci_addr num_vfs
    do
      echo "Set $num_vfs VFs on device $pci_addr"

      names+=($(ls /sys/bus/pci/devices/${pci_addr}/net/))
+      # Create udev rule to save PF name
+      add_udev_rule_for_sriov_pf $pci_addr


Should the argument of add_udev_rule_for_sriov_pf being pf name?

You r right I missed it before pushing it, it should be like

cat /etc/udev/rules.d/10-persistent-net.rules SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", KERNELS=="0000:03:00.1", NAME="enp3s0f1np1"

zshi-redhat · 2021-02-25T08:10:23Z

bindata/manifests/machine-config/files/configure-switchdev.sh.yaml

+      fi
+    }
+
+    add_udev_rule_for_sriov_pf(){


I assume this will keep the PF name as it is?
On openshift, it expects the PF name to be consistent w/ and w/o switchdev mode.

Yes, the udev rule is to save the name of the PF

moshe010 · 2021-02-25T09:11:45Z

Just a quick review, I need to check if the kernel change is in RHEL-8.4, otherwise we may need to support old kernel behaviour.

anyway we need to support old kernel. @Mmduh-483 you change support both new kernel and old ones right?

Mmduh-483 · 2021-02-25T09:36:03Z

Just a quick review, I need to check if the kernel change is in RHEL-8.4, otherwise we may need to support old kernel behaviour.

anyway we need to support old kernel. @Mmduh-483 you change support both new kernel and old ones right?

It is supporting both

zshi-redhat · 2021-02-25T10:25:31Z

Starting from kernel 5.8 the VF representors are linked to their PF PCI address, so /sys/bus/pci/devices//net contains the PF net device and it VF representors net devices.

What does the output of /sys/bus/pci/devices/<pf-pci>/net look like?

Mmduh-483 · 2021-02-25T11:29:53Z

Starting from kernel 5.8 the VF representors are linked to their PF PCI address, so /sys/bus/pci/devices//net contains the PF net device and it VF representors net devices.

What does the output of /sys/bus/pci/devices/<pf-pci>/net look like?

~# ls /sys/bus/pci/devices/0000:03:00.0/net/
enp3s0f0np0 enp3s0f0np0_0 enp3s0f0np0_1 enp3s0f0np0_2

zshi-redhat · 2021-02-25T12:27:40Z

Starting from kernel 5.8 the VF representors are linked to their PF PCI address, so /sys/bus/pci/devices//net contains the PF net device and it VF representors net devices.

What does the output of /sys/bus/pci/devices/<pf-pci>/net look like?

~# ls /sys/bus/pci/devices/0000:03:00.0/net/
enp3s0f0np0 enp3s0f0np0_0 enp3s0f0np0_1 enp3s0f0np0_2

      name=$(ls /sys/bus/pci/devices/${pci_addr}/net/)
      names+=($name)
      # Create udev rule to save PF name
      add_udev_rule_for_sriov_pf $name

When add_udev_rule_for_sriov_pf $name is called, the first argument $1 is pf name, and the rest arguments (vf rep names) are not used?

zshi-redhat · 2021-02-25T12:28:59Z

bindata/manifests/machine-config/files/configure-switchdev.sh.yaml

-      # reset the pf name
-      new_name=$(ls /sys/bus/pci/devices/${pci_addr}/net/)
-      ip link set ${new_name} down
-      ip link set ${new_name} name ${names[i]}
      ip link set ${names[i]} up


${names[i]} could be a list of pfName + vfRepNames, is it expected?

no, the names where collected in the first loop before the representors created

Mmduh-483 · 2021-02-25T12:47:48Z

When add_udev_rule_for_sriov_pf $name is called, the first argument $1 is pf name, and the rest arguments (vf rep names) are not used?

The representors are not created yet at this stage

zshi-redhat · 2021-03-01T01:45:55Z

pkg/utils/utils.go

@@ -38,6 +39,8 @@ const (
 var InitialState sriovnetworkv1.SriovNetworkNodeState
 var ClusterType string

+var pfPhysPortNameRe = regexp.MustCompile(`p\d+`)


This regexp matches all the following names: enp3s0f0np0 enp3s0f0np0_0 enp3s0f0np0_1 enp3s0f0np0_2
Maybe put a $ at the end of expression to match the PF name only? regexp.MustCompile(p\d+$)

This regex is for /sys/class/net/<netdev>/phys_port_name not the interface name
For PFs it is p/d+ e.g cat /sys/class/net/enp3s0f0np0/phys_port_name p0
For VF representors it is pf/d+vf/d+ e.g cat /sys/class/net/enp3s0f0np0_2/phys_port_name pf0vf2

zshi-redhat · 2021-03-02T09:49:00Z

overall change looks good to me, I'll run some test on openshift and comment if any issues.

adrianchiris · 2021-03-14T15:54:56Z

bindata/manifests/switchdev-config/files/configure-switchdev.sh.yaml

    if [ ! -f $input ]; then
      echo "File /etc/switchdev.conf not exist."
      exit
    fi

+    append_to_file(){
+      if ! test -f "$2"


can this be made more readable ? please assign meaningful local vars to $1 $2

Till kernel 5.7 VF representors are exposed as virtual devices, they are not linked to a PCI address. Starting from kernel 5.8 the VF representors are linked to their PF PCI address, so /sys/bus/pci/devices/<pf-pci-address>/net contains the PF net device and it VF representors net devices. The new kernel behaviour affects 'tryGetInterfaceName' if PF in switchdev and configure-switchdev script This commit brings needed changes to cope with new kernel behviour. Signed-off-by: Mamduh Alassi <mamduhala@nvidia.com>

Mmduh-483 · 2021-03-24T09:14:26Z

@zshi-redhat how did the test go? Are there any comments?

zshi-redhat · 2021-03-25T05:37:30Z

@zshi-redhat how did the test go? Are there any comments?

Sorry for the late update, I didn't run into any issue after applying this PR.

Mmduh-483 force-pushed the switchdev-kernel-5.8 branch from 2b3afa3 to ed7bfb9 Compare February 25, 2021 07:35

zshi-redhat reviewed Feb 25, 2021

View reviewed changes

adrianchiris mentioned this pull request Feb 25, 2021

Looking for github release criteria #69

Closed

Mmduh-483 force-pushed the switchdev-kernel-5.8 branch from ed7bfb9 to 5d40990 Compare February 25, 2021 09:35

zshi-redhat reviewed Feb 25, 2021

View reviewed changes

zshi-redhat reviewed Mar 1, 2021

View reviewed changes

Mmduh-483 force-pushed the switchdev-kernel-5.8 branch from 5d40990 to 9e9b93f Compare March 2, 2021 09:06

pliurh mentioned this pull request Mar 4, 2021

release 1.0.0 tracker #79

Closed

6 tasks

adrianchiris reviewed Mar 14, 2021

View reviewed changes

Mmduh-483 force-pushed the switchdev-kernel-5.8 branch from 9e9b93f to e48e2da Compare March 15, 2021 13:29

zshi-redhat merged commit 7ad0f8e into k8snetworkplumbingwg:master Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Switchdev for kernel 5.8 and above #75

Support Switchdev for kernel 5.8 and above #75

Mmduh-483 commented Feb 24, 2021 •

edited

Loading

zshi-redhat commented Feb 25, 2021

moshe010 commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021

zshi-redhat left a comment

zshi-redhat Feb 25, 2021

Mmduh-483 Feb 25, 2021

zshi-redhat Feb 25, 2021

Mmduh-483 Feb 25, 2021

moshe010 commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021 •

edited

Loading

zshi-redhat commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021

zshi-redhat commented Feb 25, 2021

zshi-redhat Feb 25, 2021

Mmduh-483 Feb 25, 2021

Mmduh-483 commented Feb 25, 2021 •

edited

Loading

zshi-redhat Mar 1, 2021 •

edited

Loading

Mmduh-483 Mar 1, 2021 •

edited

Loading

zshi-redhat Mar 2, 2021

zshi-redhat commented Mar 2, 2021

adrianchiris Mar 14, 2021

Mmduh-483 Mar 15, 2021

Mmduh-483 commented Mar 24, 2021

zshi-redhat commented Mar 25, 2021

Support Switchdev for kernel 5.8 and above #75

Support Switchdev for kernel 5.8 and above #75

Conversation

Mmduh-483 commented Feb 24, 2021 • edited Loading

zshi-redhat commented Feb 25, 2021

moshe010 commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021

zshi-redhat 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

moshe010 commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021 • edited Loading

zshi-redhat commented Feb 25, 2021

Mmduh-483 commented Feb 25, 2021

zshi-redhat commented Feb 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mmduh-483 commented Feb 25, 2021 • edited Loading

zshi-redhat Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

Mmduh-483 Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zshi-redhat commented Mar 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mmduh-483 commented Mar 24, 2021

zshi-redhat commented Mar 25, 2021

Mmduh-483 commented Feb 24, 2021 •

edited

Loading

Mmduh-483 commented Feb 25, 2021 •

edited

Loading

Mmduh-483 commented Feb 25, 2021 •

edited

Loading

zshi-redhat Mar 1, 2021 •

edited

Loading

Mmduh-483 Mar 1, 2021 •

edited

Loading