[libvirt PATCH v5 0/3] Ignore EPERM on implicit clearing of VF VLAN ID

Dmitrii Shcherbakov posted 3 patches 2 years, 4 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211118084326.29204-1-dmitrii.shcherbakov@canonical.com
There is a newer version of this series
NEWS.rst                    |  15 +++
src/hypervisor/virhostdev.c |   4 +-
src/libvirt_private.syms    |   7 ++
src/util/virnetdev.c        | 218 ++++++++++++++++++++------------
src/util/virnetdevpriv.h    |  44 +++++++
tests/virnetdevtest.c       | 241 +++++++++++++++++++++++++++++++++++-
6 files changed, 451 insertions(+), 78 deletions(-)
create mode 100644 src/util/virnetdevpriv.h
[libvirt PATCH v5 0/3] Ignore EPERM on implicit clearing of VF VLAN ID
Posted by Dmitrii Shcherbakov 2 years, 4 months ago
SmartNIC DPUs may not expose some privileged eswitch operations
to the hypervisor hosts. For example, this happens with Bluefield
devices running in the ECPF (default) mode [1] for security reasons. While
VF MAC address programming is possible via an RTM_SETLINK operation,
trying to set a VLAN ID in the same operation will fail with EPERM.

In the kernel a relevant call chain may look like

do_setlink -> do_setvfinfo -> dev->netdev_ops->set_vf_vlan

which calls a driver-specific function like [2] eventually.

The equivalent ip link commands below provide an illustration:

1. This works:

sudo ip link set enp130s0f0 vf 2 mac de:ad:be:ef:ca:fe

2. Setting (or clearing) a VLAN fails with EPERM:

sudo ip link set enp130s0f0 vf 2 vlan 0
RTNETLINK answers: Operation not permitted

3. This is what Libvirt attempts to do today (when trying to clear a
   VF VLAN at the same time as programming a VF MAC).

sudo ip link set enp130s0f0 vf 2 vlan 0 mac de:ad:be:ef:ca:fe
RTNETLINK answers: Operation not permitted

If setting an explicit VLAN ID results in an EPERM, clearing a VLAN
(setting a VLAN ID to 0) can be handled gracefully by ignoring the
EPERM error with the rationale being that if we cannot set this state
in the first place, we cannot clear it either.

Thus, virNetDevSetVfConfig is split into two distinct functions. If
clearing a VLAN ID fails with EPERM when clearing is implicit, the
error is simply ignored. For explicit clearing EPERM is still a
fatal error.

Both new functions rely virNetDevSendVfSetLinkRequest that implements
common functionality related to formatting a request, sending it and
handling error conditions and returns 0 or an error since in both cases
the payload is either NLMSG_DONE (no error) or NLMSG_ERROR where an
error message is needed by the caller to handle known cases
appropriately. This function allows the conditional code to be unit tested.

An alternative to this could be providing a higher level control plane
mechanism that would provide metadata about a device being remotely
managed in which case Libvirt would avoid trying to set or clear a
VLAN ID. This would be more complicated since other software (like Nova
in the OpenStack case) would have to annotate every guest device with an
attribute indicating whether a device is remotely managed or not based
on operator provided configuration so that Libvirt can act on this and
avoid VLAN programming.

https://gitlab.com/dmitriis/libvirt/-/pipelines/411334205

v4 changes:

* Split the change into several patches;
* Added a missing test case;
* Reworked error handling;
* Added a way to pass a NULL vlan id to preserve the old behavior
  when needed.

v5 changes:

* added the news file entry;
* fixed vlan handling in virNetDevSetVfVlan (for an issue introduced in v4).

[1] https://docs.mellanox.com/display/BlueFieldSWv35111601/Modes+of+Operation#ModesofOperation-SmartNICmode
[2] https://github.com/torvalds/linux/blob/v5.15/drivers/net/ethernet/mellanox/mlx5/core/esw/legacy.c#L427-L434

Dmitrii Shcherbakov (3):
  Set VF MAC and VLAN ID in two different operations
  Allow VF vlanid to be passed as a pointer
  Ignore EPERM on implicit clearing of VF VLAN ID

 NEWS.rst                    |  15 +++
 src/hypervisor/virhostdev.c |   4 +-
 src/libvirt_private.syms    |   7 ++
 src/util/virnetdev.c        | 218 ++++++++++++++++++++------------
 src/util/virnetdevpriv.h    |  44 +++++++
 tests/virnetdevtest.c       | 241 +++++++++++++++++++++++++++++++++++-
 6 files changed, 451 insertions(+), 78 deletions(-)
 create mode 100644 src/util/virnetdevpriv.h

-- 
2.32.0