Re: [libvirt PATCH v8 3/3] Ignore EPERM on implicit clearing of VF VLAN ID

The reason is that when somebody backports these patches onto one

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(a)canonical.com&gt; > --- > 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