[libvirt] [PATCH v2 7/7] util: check for PF online status earlier in guest startup

[Laine Stump]

When using a VF from an SRIOV-capable network card in a guest (either
in macvtap passthrough mode, or via VFIO PCI device assignment), The
associated PF netdev must be online in order for the VF to be usable
by the guest. The guest, however, is not able to change the state of
the PF. And libvirt *could* set the PF online as needed, but that
could lead to the host receiving unexpected IPv6 traffic (since the
default for an unconfigured interface is to participate in IPv6
autoconf). For this reason, before assigning a VF to a guest, libvirt
verifies that the related PF netdev is online - if it isn't, then we
log an error and don't allow the guest startup to continue.

Until now, this check was done during virNetDevSetNetConfig(). This
works nicely because the same function is called both for macvtap
passthrough and for VFIO device assignment. But in the case of VFIO,
the VF has already been unbound from its netdev driver by the time we
get to virNetDevSetNetConfig(), and in the case of dual port Mellanox
NICs that have their VFs setup in single port mode, the *only* way to
determine the proper PF netdev to query for online status is via the
"phys_port_id" file that is in the VF netdev's sysfs directory. *BUT*
if we've unbound the VF from the netdev driver, then it doesn't *have*
a netdev sysfs directory.

So, in order to check the correct PF netdev for online status, this
patch moved the check earlier in the setup, into
virNetDevSaveNetConfig(), which is called *before* unbinding the VF
from its netdev driver.

(Note that this implies that if you are using VFIO device assignment
for the VFs of a Mellanox NIC that has the VFs programmed in single
port mode, you must let the VFs be bound to their net driver and use
"managed='yes'" in the device definition. To be more specific, this is
only true if the VFs in single port mode are using port *2* of the PF
- if the VFs are using only port 1, then the correct PF netdev will be
arrived at by default/chance))

---
New in V2.

 src/util/virnetdev.c | 59 +++++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 8fc643c93..4ea6d5de2 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1878,8 +1878,9 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
             goto cleanup;
 
         linkdev = vfDevOrig;
+        saveVlan = true;
 
-    } else if (saveVlan && virNetDevIsVirtualFunction(linkdev) == 1) {
+    } else if (virNetDevIsVirtualFunction(linkdev) == 1) {
         /* when vf is -1, linkdev might be a standard netdevice (not
          * SRIOV), or it might be an SRIOV VF. If it's a VF, normalize
          * it to PF + VFname
@@ -1894,6 +1895,34 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
             goto cleanup;
     }
 
+    if (pfDevName) {
+        bool pfIsOnline;
+
+        /* Assure that PF is online before trying to use it to set
+         * anything up for this VF. It *should* be online already,
+         * but if it isn't online the changes made to the VF via the
+         * PF won't take effect, yet there will be no error
+         * reported. In the case that the PF isn't online, we need to
+         * fail and report the error, rather than automatically
+         * setting it online, since setting an unconfigured interface
+         * online automatically turns on IPv6 autoconfig, which may
+         * not be what the admin expects, so we require them to
+         * explicitly enable the PF in the host system network config.
+         */
+        if (virNetDevGetOnline(pfDevName, &pfIsOnline) < 0)
+            goto cleanup;
+
+        if (!pfIsOnline) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to configure VF %d of PF '%s' "
+                             "because the PF is not online. Please "
+                             "change host network config to put the "
+                             "PF online."),
+                           vf, pfDevName);
+            goto cleanup;
+        }
+    }
+
     if (!(configJSON = virJSONValueNewObject()))
         goto cleanup;
 
@@ -1902,7 +1931,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
      * on the host)
      */
 
-    if (pfDevName) {
+    if (pfDevName && saveVlan) {
         if (virAsprintf(&filePath, "%s/%s_vf%d", stateDir, pfDevName, vf) < 0)
             goto cleanup;
 
@@ -2251,32 +2280,6 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
         }
 
     } else {
-        bool pfIsOnline;
-
-        /* Assure that PF is online before trying to use it to set
-         * anything up for this VF. It *should* be online already,
-         * but if it isn't online the changes made to the VF via the
-         * PF won't take effect, yet there will be no error
-         * reported. In the case that the PF isn't online, we need to
-         * fail and report the error, rather than automatically
-         * setting it online, since setting an unconfigured interface
-         * online automatically turns on IPv6 autoconfig, which may
-         * not be what the admin expects, so we require them to
-         * explicitly enable the PF in the host system network config.
-         */
-        if (virNetDevGetOnline(pfDevName, &pfIsOnline) < 0)
-            goto cleanup;
-
-        if (!pfIsOnline) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to configure VF %d of PF '%s' "
-                             "because the PF is not online. Please "
-                             "change host network config to put the "
-                             "PF online."),
-                           vf, pfDevName);
-            goto cleanup;
-        }
-
         if (vlan) {
             if (vlan->nTags != 1 || vlan->trunk) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-- 
2.13.3