From 489a937eb427e5e342d84e5f72ae8aa81ca91c2c Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Thu, 10 Aug 2017 15:46:38 -0400 Subject: [PATCH] util: check for PF online status earlier in guest startup 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)) This resolves: https://bugzilla.redhat.com/267191 --- 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 66be9451e1..ae7da53425 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", -- GitLab