VF/Switchdev abstraction

[jtluka]

Description

This MR extends the Device class with methods and properties required to configure and use vfs of an ethernet device in switchdev mode.

User would typically configure the device as follows (see also the change to BaseSRIOVNetnsTcRecipe):

class SwitchdevRecipe(BaseRecipe):
    host1 = HostReq()
    host1.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver"))

    host2 = HostReq()
    host2.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver"))

    def run(self):
        host1, host2 = self.matched.host1, self.matched.host2

        # create virtual functions
        for host in [host1, host2]:
            host.eth0.eswitch_mode = "switchdev"
            host.eth0.create_vfs(1)
            vf_device = host.eth0.vf_devices[0]
            vf_rep_device = host.eth0.vf_rep_devices[0]

            host.map_device("vf_representor_eth0", {"ifname": vf_rep_device.name})

            host.run(f"ethtool -K {host.vf_representor_eth0.name} hw-tc-offload on")
            host.run(f"ethtool -K {host.eth0.name} hw-tc-offload on")

Known issues:

• for i40e NICs, the code does not work as there are no representors by the NIC design
• for bnxt_en NICs, the code does not work on RHEL distros due to missing patch [PATCH net-next v2] bnxt_en: Allow to set switchdev mode without existing VFs

Tests

J:8572626 J:8572625 J:8572624 J:8572623

Reviews

@olichtne @Axonis

Closes: #345 #344

[jtluka]

Tests done through beaker job ran without issues.

I'd hold the MR until I get a feedback from Ivan whether the use case when VFs are created before setting the switchdev mode is valid and expected. If it is valid this will need to be reworked.

[jtluka]

Just a note that here's the RFC for unfication of PF/VF/VF_REP names: https://patchwork.ozlabs.org/project/netdev/patch/[email protected]/

[jtluka]

I'd hold the MR until I get a feedback from Ivan whether the use case when VFs are created before setting the switchdev mode is valid and expected. If it is valid this will need to be reworked.

Apparently it is possible to create VFs without setting the switchdev mode - this is typical scenario where tuntap is substituted with virtual function devices.

The MR should be updated and create_vfs should be updated to support both non-switchdev and switchdev scenarios.

[jtluka]

Note, for mlx5 it is not possible to create vfs and then set the switchdev mode:

[root@wsfd-advnetlab57 ~]# echo 1 > /sys/class/net/ens5f0/device/sriov_numvfs 
[root@wsfd-advnetlab57 ~]# cat /sys/class/net/ens5f0/device/sriov_numvfs 
1
[root@wsfd-advnetlab57 ~]# devlink dev eswitch set pci/0000:81:00.0 mode switchdev
Error: mlx5_core: Inline mode is different between vports.
kernel answers: Invalid argument
[root@wsfd-advnetlab57 ~]# dmesg | tail
[  165.786118] mlx5_core 0000:81:00.2 ens5f0v0: renamed from eth0
[  165.810195] IPv6: ADDRCONF(NETDEV_UP): ens5f0v0: link is not ready
[  165.955773] mlx5_core 0000:81:00.2 ens5f0v0: Link up
[  165.962261] IPv6: ADDRCONF(NETDEV_UP): ens5f0v0: link is not ready
[  165.968699] IPv6: ADDRCONF(NETDEV_CHANGE): ens5f0v0: link becomes ready
[  264.102362] mlx5_core 0000:81:00.0: E-Switch: Disable: mode(LEGACY), nvfs(1), active vports(2)
[  264.121696] mlx5_core 0000:81:00.2 ens5f0v0: Link down
[  265.831026] mlx5_core 0000:81:00.0: mlx5_cmd_out_err:806:(pid 2226): CREATE_FLOW_TABLE(0x930) op_mod(0x0) failed, status bad resource state(0x9), syndrome (0x98afbb), err(-22)
[  265.846675] mlx5_core 0000:81:00.0: mlx5_cmd_dr_create_flow_table:86:(pid 2226): Failed creating dr flow_table
[  265.856671] mlx5_core 0000:81:00.0: E-Switch: Failed to create slow path FDB Table err -22

[Axonis]

With all the differences between drivers/NICs, I think it makes sense to split all VF/SRIOV related actions to separate commands. What do you think?

[jtluka]

With all the differences between drivers/NICs, I think it makes sense to split all VF/SRIOV related actions to separate commands. What do you think?

The only difference is the bnxt_en driver. The fact that you can't set switchdevice mode unless you have virtual functions created is considered a bug that has been already fixed in upstream kernel. I asked Ivan to update RHEL kernels.

I could expose more Device methods, but you would have to be more specific. Eventually these methods can be exposed with followup patches.

The example in the MR description was a bit out of date, maybe that would fit what you're asking for. I updated it, so please check.

[jtluka]

With all the differences between drivers/NICs, I think it makes sense to split all VF/SRIOV related actions to separate commands. What do you think?

The only difference is the bnxt_en driver. The fact that you can't set switchdevice mode unless you have virtual functions created is considered a bug that has been already fixed in upstream kernel. I asked Ivan to update RHEL kernels.

I've checked with Ivan and there's already a MR for RHEL9 kernel to update the bnxt_en driver, so there's no other difference across the drivers.

[Axonis]

With all the differences between drivers/NICs, I think it makes sense to split all VF/SRIOV related actions to separate commands. What do you think?

The only difference is the bnxt_en driver. The fact that you can't set switchdevice mode unless you have virtual functions created is considered a bug that has been already fixed in upstream kernel. I asked Ivan to update RHEL kernels.

I could expose more Device methods, but you would have to be more specific. Eventually these methods can be exposed with followup patches.

The example in the MR description was a bit out of date, maybe that would fit what you're asking for. I updated it, so please check.

I'm fine with current implementation, makes sense.

[jtluka]

Based on a discussion with LNST team I plan to update the code.

The main issue detected was with vf_devices property in following use case:

host1.phys_dev.create_vfs()
vf_device = host1.phys_dev.vf_devices[0]
host1.newns = NetNamespace("lnst") 
host1.newns.vf_eth0 = vf_device
...
vf_device = host1.phys_dev.vf_devices[0]

The last statement will not work because phys_dev is in different namespace than the vf device and would not be able to fetch it from netlink messages during rescan of available devices.

We came to conclusion that vf_devices and vf_rep_devices properties will be removed and instead the create_vfs() method will return the vf/vf_rep devices directly.

[jtluka]

Testing is blocked by podman dependency issue containers/podman-py#350

[olichtne]

looks ok to me.

[jtluka]

I've cleaned up the commit history and added one more patch for SRIOVDevices class, where I noticed that device method calls printed None as the _id of vf_rep devices (but applies also to vfs if they're not moved to a namespace). I added map_device() calls for created vf/vf_reps.

I'll schedule a Beaker test job to verify the functionality and this can be merged.

[jtluka]

New test jobs: J:8640504 J:8640505 J:8640506 J:8640507

[jtluka]

New test jobs: J:8640504 J:8640505 J:8640506 J:8640507

These tests passed. Note that bnxt_en driver contains bug where the vfs need to be created before moving to switchdev mode.

[Axonis]

Ack, no more comments at the moment.