[v3,1/4] util: fixing wrong assumption that PF has to have netdev assigned

Message ID 20190122192615.9256-2-dann.frazier@canonical.com
State New
Headers show
Series
  • [v3,1/4] util: fixing wrong assumption that PF has to have netdev assigned
Related show

Commit Message

dann frazier Jan. 22, 2019, 7:26 p.m.
From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>


libvirt wrongly assumes that VF netdev has to have the
netdev assigned to PF. There is no such requirement in SRIOV standard.
This patch change the virNetDevSwitchdevFeature() function to deal
with SRIOV devices which does not have netdev on PF. Also corrects
one comment about PF netdev assumption.

One example of such devices is ThunderX VNIC.
By applying this change, VF device is used for virNetlinkCommand() as
it is the only netdev assigned to VNIC.

Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>

[ dannf: Reinstated error path in virNetDevGetPhysicalFunction() as
  suggested by Michal Privoznik ]
Signed-off-by: dann frazier <dann.frazier@canonical.com>

---
 src/util/virnetdev.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Michal Prívozník Jan. 23, 2019, 9:28 a.m. | #1
On 1/22/19 8:26 PM, dann frazier wrote:
> From: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>

> 

> libvirt wrongly assumes that VF netdev has to have the

> netdev assigned to PF. There is no such requirement in SRIOV standard.

> This patch change the virNetDevSwitchdevFeature() function to deal

> with SRIOV devices which does not have netdev on PF. Also corrects

> one comment about PF netdev assumption.

> 

> One example of such devices is ThunderX VNIC.

> By applying this change, VF device is used for virNetlinkCommand() as

> it is the only netdev assigned to VNIC.

> 

> Signed-off-by: Radoslaw Biernacki <radoslaw.biernacki@linaro.org>

> [ dannf: Reinstated error path in virNetDevGetPhysicalFunction() as

>   suggested by Michal Privoznik ]

> Signed-off-by: dann frazier <dann.frazier@canonical.com>

> ---

>  src/util/virnetdev.c | 19 +++++++++++++------

>  1 file changed, 13 insertions(+), 6 deletions(-)

> 

> diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c

> index 2111b3ada9..82823c0dfc 100644

> --- a/src/util/virnetdev.c

> +++ b/src/util/virnetdev.c

> @@ -1355,9 +1355,8 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)

>      }

>  

>      if (!*pfname) {

> -        /* this shouldn't be possible. A VF can't exist unless its

> -         * PF device is bound to a network driver

> -         */

> +        /* The SRIOV standard does not require VF netdevs to have the

> +           netdev assigned to a PF */

>          virReportError(VIR_ERR_INTERNAL_ERROR,

>                         _("The PF device for VF %s has no network device name"),

>                         ifname);

> @@ -1449,8 +1448,12 @@ virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,

>  

>      *pfname = NULL;

>  

> -    if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)

> +    if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) {

> +        virReportError(VIR_ERR_INTERNAL_ERROR,

> +                       _("Cannot get PF netdev name for VF %s"),

> +                       vfname);

>          return ret;


This is not needed. It overwrites (probably more accurate) error
reported by virNetDevGetPhysicalFunction().


ACK with that removed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 2111b3ada9..82823c0dfc 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1355,9 +1355,8 @@  virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
     }
 
     if (!*pfname) {
-        /* this shouldn't be possible. A VF can't exist unless its
-         * PF device is bound to a network driver
-         */
+        /* The SRIOV standard does not require VF netdevs to have the
+           netdev assigned to a PF */
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("The PF device for VF %s has no network device name"),
                        ifname);
@@ -1449,8 +1448,12 @@  virNetDevGetVirtualFunctionInfo(const char *vfname, char **pfname,
 
     *pfname = NULL;
 
-    if (virNetDevGetPhysicalFunction(vfname, pfname) < 0)
+    if (virNetDevGetPhysicalFunction(vfname, pfname) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Cannot get PF netdev name for VF %s"),
+                       vfname);
         return ret;
+    }
 
     if (virNetDevSysfsFile(&pf_sysfs_path, *pfname, "device") < 0)
         goto cleanup;
@@ -3178,8 +3181,12 @@  virNetDevSwitchdevFeature(const char *ifname,
     if ((is_vf = virNetDevIsVirtualFunction(ifname)) < 0)
         return ret;
 
-    if (is_vf == 1 && virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
-        goto cleanup;
+    if (is_vf == 1) {
+        /* ignore error if PF does noto have netdev assigned
+         * in that case pfname == NULL */
+        if (virNetDevGetPhysicalFunction(ifname, &pfname) < 0)
+            virResetLastError();
+    }
 
     pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
                               virNetDevGetPCIDevice(ifname);