[rfe] Non-admin users unable to create SR-IOV switch dev ports

[John Garbutt]

Currently only admin users can create SR-IOV direct ports that are marked as switch_dev, i.e. the ones that do OVS hardware offload.

If we use Neutron RBAC to allow access to the binding profile, users can create big problems by modifying it after port binding when nova has stored private details in their like the specific PCI device that has been bound.

Example operator deployments:
* all SR-IOV direct ports are hardware offloaded with switch_dev using ovn
* all SR-IOV direct ports are hardware offloaded with switch_dev using ovs ml2
* all SR-IOV direct ports created using legacy SR-IOV driver
* (not a use case I have, but theoretically its possible) some mix of the above

From and end user, the direct nic, hardware offloaded or not, by ovs ml2 or ovn, all appear the same. Its operator internal setup details on what happens. Why do we expose this to the end user in this way? Ideally the user doesn't care between these, and the neutron API is able to extract away the implementation details of getting a direct type of vnic.

Moreover we don't want users to have to know how their operator has configured their system, be it ovn or ovs or sriov. They just want the direct NIC that get them RDMA within their VM for the requested nova flavor of their instance.

This could be fixed by having ovn drivers and ovs ml2 drivers respect a configuration like this, for the non mixed case:
bind_direct_nics_as_switch_dev = True/False

At the PTG it was mentioned that this was configuration that change what the API does. But I don't understand how using ovn vs ovs ml2 is any different to hardware offloaded vs not-hardware offloads direct NICs.

I have a local workaround, for ovs ml2 only, I comment out this line and just use direct ports:
https://github.com/openstack/neutron/blob/b73399fa746d951a99fdf29950a1c0a801e941a2/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#L128

Note, if we add a new API for this (rather than just a new VNIC type), we would need to add that to all these things before we can use it with customers, once we have upgrade to the release that API change has landed in:
* terraform
* k8s cluster api provider openstack
* openstack CLI

[John Garbutt (johngarbutt)]

As discussed at the bobcat PTG operator session: https://etherpad.opendev.org/p/neutron-bobcat-ptg

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

This RFE has been approved with the following comments (see drivers meeting logs):
* Create a SPEC.
* The RFE will propose a new port extension, e.g. hardware_offload, that will replace the port binding definition.
* The feature will be implemented in 2 phases:
** Create the port extension, changes in the SDK/CLI code and Neutron core. But pass to Nova the port binding information as before. That will avoid the need to make any change in the Nova code yet.
** Once implemented, remove the Neutron code that changes the port binding information.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-specs/+/882272

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron-lib/+/882726

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/882832

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron-specs/+/882272
Committed: https://opendev.org/openstack/neutron-specs/commit/ec0a0fb2a2b3eb71b7d08d39b2d73ec9de55b537
Submitter: "Zuul (22348)"
Branch: master

commit ec0a0fb2a2b3eb71b7d08d39b2d73ec9de55b537
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Thu May 4 11:18:39 2023 +0000

    Port extension to create hardware offloaded ports

    The aim of the RFE is to create a new port extension to allow to create
    ports with hardware offloaded capabilities.

    Related-Bug: #2013228
    Change-Id: I0ee77fd4a0b1fe28b206db7dc13e105ffff122a6

[OpenStack Infra (hudson-openstack)]

Change abandoned by "Slawek Kaplonski <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/882832
Reason: This review is > 4 weeks without comment, and failed Zuul jobs the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[John Garbutt (johngarbutt)]

Thank you for your efforts on this. Appologies that I only just spotted there was progress.

My main question is why does the user need to request that the direct port is hardware offloaded?

As a user I don't need to create different ports when my operator has installed OVN vs OVS? This seems a very similar case where the direct mode port happens to be implemented differently?

Granted there are cases in a mixed cloud where you might need both types of direct port. I have never seen such a cloud myself, and we use a lot of hardware offloaded direct mode ports. It would be nice to configure a default for a given cloud, i.e. default to hardware offload. Then any user that cares about the distinction gets to request the specific one?

[Rodolfo Alonso (rodolfo-alonso-hernandez)]

Hello John:

We discussed this during the PTG. The HW offload capability is requested in the port definition, this is not implicit in the backend. As you commented, any HW offload deployment can host kernel and HW offloaded ports.

The other justification is that, by default in Neutron, we don't implement config driven API features. In other words: we don't implement variables that change the API, in this case the port creation, always defining this new parameter to "switchdev".

Now we have defined a parameter outside the port binding dictionary, you can lecture your users to request both "direct" and "hardware_offload_type" in the port creation definition.

Regards.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/neutron/+/905106

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/882832
Committed: https://opendev.org/openstack/neutron/commit/80f547ad1d1e95a05570d9a7db2dac7cad7ff33a
Submitter: "Zuul (22348)"
Branch: master

commit 80f547ad1d1e95a05570d9a7db2dac7cad7ff33a
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Wed May 10 02:32:46 2023 +0200

    Add a "port" child table "porthardwareoffloadtype"

    This table has a 1:1 relationship with the "port" table, providing
    the "hardware_offload_type" field (string).

    The "neutron-lib" library minimum version is 3.8.0, that contains
    [1].

    NOTE: once the OSC patch is merged [2], the documentation will be
    updated to reflect how to create a hardware offloaded port without
    manually defining the port binding profile,

    [1]https://review.opendev.org/c/openstack/neutron-lib/+/882726
    [2]https://review.opendev.org/c/openstack/python-openstackclient/+/892792

    Partial-Bug: #2013228
    Change-Id: I04f232d6c43e39f254c4559caf041dcf05acec21

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/905106
Committed: https://opendev.org/openstack/neutron/commit/de40bfbafb03fcb14299cd5da4459d3927aaaef3
Submitter: "Zuul (22348)"
Branch: master

commit de40bfbafb03fcb14299cd5da4459d3927aaaef3
Author: Rodolfo Alonso Hernandez <email address hidden>
Date: Wed Jan 17 17:04:09 2024 +0000

    Update exception conditions in ``_process_create_port``

    This is a follow-up patch to handle a missing comment. The except
    branch should never catch a ``AttributeError`` exception, only a
    ``KeyError`` if that is missing in the ``data`` dictionary.

    Related-Bug: #2013228
    Change-Id: I6a3249649dde58e666048a613640338ea8af7b36