vif-plug-representor: Add support for PF plugging

[dshcherb]

• Implement support for PF representor plugging
• If vf num is not present, we are plugging a PF representor
• Unit tests for various cases

The main use-case is bare metal instances where PFs need to be plugged into OVS bridges programmatically via OVN.

[fnordahl]

Hello @dshcherb, thanks a lot for working on this! The contribution is much welcome and I'll ensure we set aside some time to review it.

Your proposal continues the original intent of allowing plugging of PFs, which we never had time for in the initial implementation, so it is in line with the intent of the project and a welcome change.

I really appreciate the added test coverage.

Note that this is a first round of review and have not read through all of it yet, but I think there are some things to start on as is.

The change appears to do multiple distinct things:

• add support for plugging of the PF for the DPU use case.
• add support for plugging of PHYSICAL for non-DPU use case.

It would be easier to review if the latter use case was broken out into a separate commit with some context in the commit message for the use case.

[dshcherb]

Hi @fnordahl, thanks a lot for the quick review!

It would be easier to review if the latter use case was broken out into a separate commit with some context in the commit message for the use case.

I'll address the comments and break it up.

I wanted to have both PF and PF representor code in the same change originally because of the limitations of netdevsim w.r.t. to creating PF representors. But I ended up functionally testing on a physical DPU.

One idea that I haven't yet included in the change is tracking ownership of plugged ports: in order to avoid conflicts between netplan (or other network config management tools), different instances of ovn-controller and manual operator changes, it would be good to store some indication of ownership in ovsdb.

[fnordahl]

I wanted to have both PF and PF representor code in the same change originally because of the limitations of netdevsim w.r.t. to creating PF representors. But I ended up functionally testing on a physical DPU.

We at some point attempted to create a netdevsim based CI too, but quickly ran into similar issues. I did make an attempt at posting an update to it upstream [1] which was shot down quickly, and I have later found documentation suggesting that they only prioritize their own selftests and have no intention of adhering to kernel uAPI [2].

You're welcome to another attempt to convince them otherwise, but I think it is safe to assume that actual hardware is required for now.

One idea that I haven't yet included in the change is tracking ownership of plugged ports: in order to avoid conflicts between netplan (or other network config management tools), different instances of ovn-controller and manual operator changes, it would be good to store some indication of ownership in ovsdb.

The vif-plug infrastructure in the ovn-controller maintains an external-id for the ports it touches [3]. While not a formal ownership contract, it may cover part of this requirement?

1: https://lore.kernel.org/netdev/20211124081106.1768660-1-frode.nordahl@canonical.com/
2: https://docs.kernel.org/process/maintainer-netdev.html#netdevsim
3: https://github.com/ovn-org/ovn/blob/2f2b263df8332461d5361926febaebdbac72625c/controller/vif-plug.c#L34

[dshcherb]

I think it is safe to assume that actual hardware is required for now.

One possible direction is PCIe device emulation.

It would require some development and it's not easy to emulate an mlx5-compatible device but having a compatible PCIe test device model would be interesting to have. A device doesn't have to be mlx5-compatible but then the question is which driver to bind.

As an alternative, I also thought of including functional tests for physical hardware that would not gate ovn-vif commits but would allow manual testing on DPUs prior to change submission.

The vif-plug infrastructure in the ovn-controller maintains an external-id for the ports it touches [3]. While not a formal ownership contract, it may cover part of this requirement?

I think we need something like this stored as well:

ovn-plug-owner=<system-id> 

Quoting ovn-controller's man:

Note that the chassis name can also be provided via the system-id-override file in the local OVN "etc" directory or via the -n command-line option. The following precedence is used: first, the command-line option is read; if not present, the system-id-override file is read; if not present, then the name configured in the database is used.

That would allow ovn-vif to identify ownership if present in ovsdb.

The case that wouldn't be covered, however, would be the one where the port is used outside of OVS but there's no indication for that in ovsdb.