<div dir="ltr"><div dir="ltr"><div>> The reason is that when somebody backports these patches onto one</div>of previous releases then they would get needless conflict only because<br>of this file.<span class="gmail-im" style="color:rgb(80,0,80)"><br></span><div><br></div>Ack, I'll make a note of that for the future changes, thanks guiding me with this!<div><div><br clear="all"><div><div dir="ltr" data-smartmail="gmail_signature"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr">Best Regards,<div>Dmitrii Shcherbakov</div><div><span style="color:rgb(136,136,136);font-size:12.8px">LP/MM/oftc: dmitriis</span><br></div></div></div></div></div></div></div></div><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Feb 2, 2022 at 9:04 PM Michal Prívozník <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/1/22 09:28, Dmitrii Shcherbakov wrote:<br>
> SmartNIC DPUs may not expose some privileged eswitch operations<br>
> to the hypervisor hosts. For example, this happens with Bluefield<br>
> devices running in the ECPF (default) mode for security reasons. While<br>
> VF MAC address programming is possible via an RTM_SETLINK operation,<br>
> trying to set a VLAN ID in the same operation will fail with EPERM.<br>
> <br>
> The equivalent ip link commands below provide an illustration:<br>
> <br>
> 1. This works:<br>
> <br>
> sudo ip link set enp130s0f0 vf 2 mac de:ad:be:ef:ca:fe<br>
> <br>
> 2. Setting (or clearing) a VLAN fails with EPERM:<br>
> <br>
> sudo ip link set enp130s0f0 vf 2 vlan 0<br>
> RTNETLINK answers: Operation not permitted<br>
> <br>
> 3. This is what Libvirt attempts to do today (when trying to clear a<br>
> VF VLAN at the same time as programming a VF MAC).<br>
> <br>
> sudo ip link set enp130s0f0 vf 2 vlan 0 mac de:ad:be:ef:ca:fe<br>
> RTNETLINK answers: Operation not permitted<br>
> <br>
> If setting an explicit VLAN ID results in an EPERM, clearing a VLAN<br>
> (setting a VLAN ID to 0) can be handled gracefully by ignoring the<br>
> EPERM error with the rationale being that if we cannot set this state<br>
> in the first place, we cannot clear it either.<br>
> <br>
> In order to keep explicit clearing of VLAN ID working as it used to<br>
> be passing a NULL pointer for VLAN ID is used.<br>
> <br>
> Signed-off-by: Dmitrii Shcherbakov <<a href="mailto:dmitrii.shcherbakov@canonical.com" target="_blank">dmitrii.shcherbakov@canonical.com</a>><br>
> ---<br>
> NEWS.rst | 14 ++++++++++++++<br>
> src/util/virnetdev.c | 11 ++++++++++-<br>
> tests/virnetdevtest.c | 2 +-<br>
> 3 files changed, 25 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/NEWS.rst b/NEWS.rst<br>
> index 666a593b58..f5453257ab 100644<br>
> --- a/NEWS.rst<br>
> +++ b/NEWS.rst<br>
> @@ -34,6 +34,20 @@ v8.1.0 (unreleased)<br>
> to parse sysconfig files, in case they are created by the admin and filled<br>
> with the desired key=value pairs.<br>
> <br>
> + * virnetdev: Ignore EPERM on implicit clearing of VF VLAN ID<br>
> +<br>
> + Libvirt will now ignore EPERM errors on attempts to implicitly clear a<br>
> + VLAN ID (when a VLAN is not explicitly provided via an interface XML<br>
> + using a 0 or a non-zero value) as SmartNIC DPUs do not expose VLAN<br>
> + programming capabilities to the hypervisor host. This allows Libvirt<br>
> + clients to avoid specifying a VLAN and expect VF configuration to work<br>
> + since Libvirt tries to clear a VLAN in the same operation<br>
> + as setting a MAC address for VIR_DOMAIN_NET_TYPE_HOSTDEV devices which<br>
> + is now split into two distinct operations. EPERM errors received while<br>
> + trying to program a non-zero VLAN ID or explicitly program a VLAN ID 0<br>
> + will still cause errors as before so there is no change in behavior<br>
> + in those cases.<br>
> +<br>
> * **Bug fixes**<br>
> <br>
<br>
Any NEWS.rst change MUST (in sense of RFC2119) be done in a separate<br>
patch. The reason is that when somebody backports these patches onto one<br>
of previous releases then they would get needless conflict only because<br>
of this file.<br>
<br>
> <br>
> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c<br>
> index eacdc3bf1f..cf9056f1bd 100644<br>
> --- a/src/util/virnetdev.c<br>
> +++ b/src/util/virnetdev.c<br>
> @@ -1613,12 +1613,21 @@ virNetDevSetVfVlan(const char *ifname, int vf, const int *vlanid)<br>
> requestError = virNetDevSendVfSetLinkRequest(ifname, IFLA_VF_VLAN,<br>
> &ifla_vf_vlan, sizeof(ifla_vf_vlan));<br>
> <br>
<br>
Again, nothing technically wrong here, I had to adopt it to the changes<br>
I made to previous patches.<br>
<br>
Michal<br>
<br>
</blockquote></div>
</div>