Switchdev VF Representor interface name on host is derived based on BDF of pci SR-IOV device in 'hostdev' and querying required net sysfs entries on host. --- v2 includes commented code cleanup in virnetdevhostdev.c po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 5 + src/util/virhostdev.c | 11 +++ src/util/virhostdev.h | 6 ++ src/util/virnetdevhostdev.c | 224 ++++++++++++++++++++++++++++++++++++++++++++ src/util/virnetdevhostdev.h | 33 +++++++ 7 files changed, 281 insertions(+) create mode 100644 src/util/virnetdevhostdev.c create mode 100644 src/util/virnetdevhostdev.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 285955469..73ce73397 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -237,6 +237,7 @@ src/util/virnetdevmacvlan.c src/util/virnetdevmidonet.c src/util/virnetdevopenvswitch.c src/util/virnetdevtap.c +src/util/virnetdevhostdev.c src/util/virnetdevveth.c src/util/virnetdevvportprofile.c src/util/virnetlink.c diff --git a/src/Makefile.am b/src/Makefile.am index db68e01db..0f3c3f1bc 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -148,6 +148,7 @@ UTIL_SOURCES = \ util/virnetdev.h util/virnetdev.c \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ + util/virnetdevhostdev.h util/virnetdevhostdev.c \ util/virnetdevip.h util/virnetdevip.c \ util/virnetdevmacvlan.c util/virnetdevmacvlan.h \ util/virnetdevmidonet.h util/virnetdevmidonet.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3b14d7d15..d9bc8ad72 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2288,6 +2288,11 @@ virNetDevBridgeSetSTPDelay; virNetDevBridgeSetVlanFiltering; +# util/virnetdevhostdev.h +virNetdevHostdevCheckVFRepIFName; +virNetdevHostdevGetVFRepIFName; + + # util/virnetdevip.h virNetDevIPAddrAdd; virNetDevIPAddrDel; diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index a12224c58..b6d824d07 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -351,6 +351,17 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, } +/* Non static wrapper for virHostdevNetDevice to use outside virhostdev */ +int +virHostdevNetDeviceWrapper(virDomainHostdevDefPtr hostdev, + int pfNetDevIdx, + char **linkdev, + int *vf) +{ + return virHostdevNetDevice(hostdev, pfNetDevIdx, linkdev, vf); +} + + static bool virHostdevIsPCINetDevice(virDomainHostdevDefPtr hostdev) { diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h index 54e1c66be..7dc860a85 100644 --- a/src/util/virhostdev.h +++ b/src/util/virhostdev.h @@ -202,5 +202,11 @@ int virHostdevPCINodeDeviceReAttach(virHostdevManagerPtr mgr, int virHostdevPCINodeDeviceReset(virHostdevManagerPtr mgr, virPCIDevicePtr pci) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +virHostdevNetDeviceWrapper(virDomainHostdevDefPtr hostdev, + int pfNetDevIdx, + char **linkdev, + int *vf) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4); #endif /* __VIR_HOSTDEV_H__ */ diff --git a/src/util/virnetdevhostdev.c b/src/util/virnetdevhostdev.c new file mode 100644 index 000000000..243c78a97 --- /dev/null +++ b/src/util/virnetdevhostdev.c @@ -0,0 +1,224 @@ +/* + * virnetdevhostdev.c: utilities to get/verify Switchdev VF Representor + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#include <config.h> + +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> +#include <dirent.h> + +#include "virhostdev.h" +#include "virnetdevhostdev.h" +#include "viralloc.h" +#include "virstring.h" +#include "virfile.h" +#include "virerror.h" +#include "virlog.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("util.netdevhostdev"); + +#ifndef IFNAMSIZ +#define IFNAMSIZ 16 +#endif + +#define IFSWITCHIDSIZ 20 + +#ifndef SYSFS_NET_DIR +#define SYSFS_NET_DIR "/sys/class/net/" +#endif + + +/** + * virNetdevHostdevNetSysfsPath + * + * @pf_name: netdev name of the physical function (PF) + * @vf: virtual function (VF) number for the device of interest + * @vf_representor: name of the VF representor interface + * + * Finds the VF representor name of VF# @vf of SRIOV PF @pfname, and + * puts it in @vf_representor. The caller must free @vf_representor + * when it's finished with it + * + * Returns 0 on success, -1 on failure + */ +static int +virNetdevHostdevNetSysfsPath(char *pf_name, int vf, char **vf_representor) +{ + char *pf_switch_id = NULL; + char *pf_switch_id_file = NULL; + char *pf_subsystem_device_file = NULL; + char *pf_subsystem_device_switch_id = NULL; + char *pf_subsystem_device_port_name_file = NULL; + char *pf_subsystem_dir = NULL; + char *vf_representor_name = NULL; + char *vf_num_str = NULL; + char *vf_suffix = NULL; + DIR *dirp = NULL; + struct dirent *dp; + int ret = -1; + + + if (virAsprintf(&pf_switch_id_file, SYSFS_NET_DIR "%s/phys_switch_id", + pf_name) < 0) + goto cleanup; + + if (virAsprintf(&pf_subsystem_dir, SYSFS_NET_DIR "%s/subsystem", + pf_name) < 0) + goto cleanup; + + if (virFileReadAllQuiet(pf_switch_id_file, IFSWITCHIDSIZ, + &pf_switch_id) <= 0) { + goto cleanup; + } + + if (virDirOpen(&dirp, pf_subsystem_dir) < 0) + goto cleanup; + + /* Iterate over the PFs subsystem devices to find entry with matching + * switch_id with that of PF. + */ + while (virDirRead(dirp, &dp, pf_subsystem_dir) > 0) { + if (STREQ(dp->d_name, pf_name)) + continue; + + if (virAsprintf(&pf_subsystem_device_file, "%s/%s/phys_switch_id", + pf_subsystem_dir, dp->d_name) < 0) + goto cleanup; + + if (virFileReadAllQuiet(pf_subsystem_device_file, IFSWITCHIDSIZ, + &pf_subsystem_device_switch_id) > 0) { + if (!STREQ(pf_switch_id, pf_subsystem_device_switch_id)) { + VIR_FREE(pf_subsystem_device_file); + VIR_FREE(pf_subsystem_device_switch_id); + continue; + } + } + + if (virAsprintf(&pf_subsystem_device_port_name_file, + "%s/%s/phys_port_name", pf_subsystem_dir, + dp->d_name) < 0) + goto cleanup; + + if (virFileReadAllQuiet + (pf_subsystem_device_port_name_file, IFNAMSIZ, + &vf_representor_name) <= 0) { + VIR_FREE(pf_subsystem_device_file); + VIR_FREE(pf_subsystem_device_switch_id); + VIR_FREE(pf_subsystem_device_port_name_file); + continue; + } + + if (virAsprintf(&vf_num_str, "%d", vf) < 0) + goto cleanup; + + /* phys_port_name may contain just VF number or string with + * 'vf' or 'VF' followed by VF number at the end. + */ + if (!(vf_suffix = strcasestr(vf_representor_name, "vf"))) + vf_suffix = vf_representor_name; + + if (strstr(vf_suffix, vf_num_str)) { + if (virAsprintf(vf_representor, "%s", dp->d_name) < 0) + goto cleanup; + + ret = 0; + break; + } + } + + cleanup: + VIR_DIR_CLOSE(dirp); + VIR_FREE(pf_switch_id); + VIR_FREE(pf_switch_id_file); + VIR_FREE(pf_subsystem_dir); + VIR_FREE(pf_subsystem_device_file); + VIR_FREE(pf_subsystem_device_switch_id); + VIR_FREE(pf_subsystem_device_port_name_file); + VIR_FREE(vf_num_str); + VIR_FREE(vf_representor_name); + return ret; +} + + +/** + * virNetdevHostdevGetVFRepIFName + * + * @hostdev: host device to check + * @ifname : Contains VF representor name upon successful return. + * + * Returns 0 on success, -1 on failure + */ +int +virNetdevHostdevGetVFRepIFName(virDomainHostdevDefPtr hostdev, + char **ifname) +{ + char *linkdev = NULL; + char *vf_representor = NULL; + int vf = -1; + int ret = -1; + + if (virHostdevNetDeviceWrapper(hostdev, -1, &linkdev, &vf) < 0) + goto cleanup; + + if (virNetdevHostdevNetSysfsPath(linkdev, vf, &vf_representor)) + goto cleanup; + + if (VIR_STRDUP(*ifname, vf_representor) > 0) + ret = 0; + + cleanup: + VIR_FREE(linkdev); + VIR_FREE(vf_representor); + return ret; +} + + +/** + * virNetdevHostdevCheckVFRepIFName + * + * @hostdev: host device to check + * @ifname : VF representor name to verify + * + * Returns 0 on success, -1 on failure + */ +int +virNetdevHostdevCheckVFRepIFName(virDomainHostdevDefPtr hostdev, + const char *ifname) +{ + char *linkdev = NULL; + char *vf_representor = NULL; + int vf = -1; + int ret = -1; + + if (virHostdevNetDeviceWrapper(hostdev, -1, &linkdev, &vf) < 0) + goto cleanup; + + if (virNetdevHostdevNetSysfsPath(linkdev, vf, &vf_representor)) + goto cleanup; + + if (STREQ(ifname, vf_representor)) + ret = 0; + + cleanup: + VIR_FREE(linkdev); + VIR_FREE(vf_representor); + return ret; +} diff --git a/src/util/virnetdevhostdev.h b/src/util/virnetdevhostdev.h new file mode 100644 index 000000000..c79c697fd --- /dev/null +++ b/src/util/virnetdevhostdev.h @@ -0,0 +1,33 @@ +/* + * virnetdevhostdev.h: utilities to get/verify Switchdev VF Representor + * + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + */ + +#ifndef __VIR_NETDEV_HOSTDEV_H__ +#define __VIR_NETDEV_HOSTDEV_H__ +#include "virnetdevtap.h" + +int +virNetdevHostdevGetVFRepIFName(virDomainHostdevDefPtr hostdev, + char **ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +int +virNetdevHostdevCheckVFRepIFName(virDomainHostdevDefPtr hostdev, + const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; +#define virNetdevHostdevVFRepInterfaceStats virNetDevTapInterfaceStats +#endif /* __VIR_NETDEV_HOSTDEV_H__ */ -- 2.13.6

Thanks John for a very detailed review and feedback for this patch set. As this is my first patch submission to libvirt, I was really looking for the review from the community and this review is really helpful and informing. I agree with concerns and suggestions highlighted and will prepare v3 to address them. -Jai On Wed, Feb 21, 2018 at 4:00 AM, John Ferlan <jferlan(a)redhat.com&gt; wrote: BDF refers to BUS:DEVICE:FUNCTION in pci address. Will surely prepare a more informed description in v3 for this feature. I agree this doesn't look good. I was not sure whether removing static from above mentioned function in virhostdev.c is allowed. Agreed. As this linux only feature,I should have taken care of this every where in the patch. Will take care of this in v3. No really. ifname can be returned as well. -Jai

On 02/12/2018 03:07 AM, Jai Singh Rana wrote: If there's 10 nnets and 1 nhostdev, things are not going to end well. Wait, what? Even w/ the correct usage - there's more than just one caller that could get an answer it wasn't expecting... Limit your usage to where you need this type of check... So you've hidden virNetdevHostdevVFRepInterfaceStats as a call to virNetDevTapInterfaceStats via the #define in virnetdevhostdev.h You need to figure out a different, better, more standard, non-hack mechanism for this. Rather than the virDomainNetFind adjustment above, this is where you should be more explicit. Can we go with just vf_ifname? Either this checks "< 0)" or if changed such that the called function returns some string... Note that QEMU_ADD_NAME_PARAM can goto cleanup, thus leaking your vf*_ifname. Just add the VIR_FREE at the cleanup label int rc; rc = virNetdevHostdevVFRepInterfaceStats(vf_iname, &tmp, !virDomainNetTypeSharesHostView(net)) VIR_FREE(vf_iname); if (rc < 0) { virResetLastError(); continue; } is a bit cleaner to me John