On 2/1/22 09:28, Dmitrii Shcherbakov wrote:
> 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 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.
>
> 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.
>
> In order to keep explicit clearing of VLAN ID working as it used to
> be passing a NULL pointer for VLAN ID is used.
>
> Signed-off-by: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
> ---
>  NEWS.rst              | 14 ++++++++++++++
>  src/util/virnetdev.c  | 11 ++++++++++-
>  tests/virnetdevtest.c |  2 +-
>  3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/NEWS.rst b/NEWS.rst
> index 666a593b58..f5453257ab 100644
> --- a/NEWS.rst
> +++ b/NEWS.rst
> @@ -34,6 +34,20 @@ v8.1.0 (unreleased)
>      to parse sysconfig files, in case they are created by the admin and filled
>      with the desired key=value pairs.
> 
> +  * virnetdev: Ignore EPERM on implicit clearing of VF VLAN ID
> +
> +    Libvirt will now ignore EPERM errors on attempts to implicitly clear a
> +    VLAN ID (when a VLAN is not explicitly provided via an interface XML
> +    using a 0 or a non-zero value) as SmartNIC DPUs do not expose VLAN
> +    programming capabilities to the hypervisor host. This allows Libvirt
> +    clients to avoid specifying a VLAN and expect VF configuration to work
> +    since Libvirt tries to clear a VLAN in the same operation
> +    as setting a MAC address for VIR_DOMAIN_NET_TYPE_HOSTDEV devices which
> +    is now split into two distinct operations. EPERM errors received while
> +    trying to program a non-zero VLAN ID or explicitly program a VLAN ID 0
> +    will still cause errors as before so there is no change in behavior
> +    in those cases.
> +
>  * **Bug fixes**
> 

Any NEWS.rst change MUST (in sense of RFC2119) be done in a separate
patch. The reason is that when somebody backports these patches onto one
of previous releases then they would get needless conflict only because
of this file.

> 
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
> index eacdc3bf1f..cf9056f1bd 100644
> --- a/src/util/virnetdev.c
> +++ b/src/util/virnetdev.c
> @@ -1613,12 +1613,21 @@ virNetDevSetVfVlan(const char *ifname, int vf, const int *vlanid)
>      requestError = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN,
>                                                   &ifla_vf_vlan, sizeof(ifla_vf_vlan));
> 

Again, nothing technically wrong here, I had to adopt it to the changes
I made to previous patches.

Michal