From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415761074549.6608307713323; Thu, 10 Aug 2017 18:42:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F329880465; Fri, 11 Aug 2017 01:42:38 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 160B25C6D7; Fri, 11 Aug 2017 01:42:38 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 37A2D4A467; Fri, 11 Aug 2017 01:42:35 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gVZa003125 for ; Thu, 10 Aug 2017 21:42:31 -0400 Received: by smtp.corp.redhat.com (Postfix) id E5E429814B; Fri, 11 Aug 2017 01:42:31 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7564E71C33; Fri, 11 Aug 2017 01:42:31 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F329880465 Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com F329880465 From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:16 -0400 Message-Id: <20170811014222.29548-2-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 1/7] util: new function virNetDevGetPhysPortID() X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 11 Aug 2017 01:42:39 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" On Linux each network device *can* (but not necessarily *does*) have an attribute called phys_port_id which can be read from the file of that name in the netdev's sysfs directory. The examples I've seen have been a many-digit hexadecimal number (as an ASCII string). This value can be useful when a single PCI device is associated with multiple netdevs (e.g a dual port Mellanox SR-IOV NIC - this card has a single PCI Physical Function (PF), and that PF has two netdevs associated with it (the "net" subdirectory of the PF in sysfs has two links rather than the usual single link to a netdev directory). Each of the PF netdevs has a different phys_port_id. The Virtual Functions (VF) are similar - the PF (a PCI device) has "n" VFs (also each of these is a PCI device), each VF has two netdevs, and each of the VF netdevs points back to the VF PCI device (with the "device" entry in its sysfs directory) as well as having a phys_port_id matching the PF netdev it is associated with. virNetDevGetPhysPortID() simply attempts to read the phys_port_id for the given netdev and return it to the caller. If this particular netdev driver doesn't support phys_port_id, it returns NULL (*not* a NULL-terminated string, but a NULL pointer) but still counts it as a success. --- No change from V1. Already ACKed. src/libvirt_private.syms | 1 + src/util/virnetdev.c | 51 ++++++++++++++++++++++++++++++++++++++++++++= ++++ src/util/virnetdev.h | 5 +++++ 3 files changed, 57 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 183a9194d..e6868b55f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2113,6 +2113,7 @@ virNetDevGetMTU; virNetDevGetName; virNetDevGetOnline; virNetDevGetPhysicalFunction; +virNetDevGetPhysPortID; virNetDevGetPromiscuous; virNetDevGetRcvAllMulti; virNetDevGetRcvMulti; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 90b7bee34..a2664de78 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1170,6 +1170,46 @@ virNetDevGetPCIDevice(const char *devName) =20 =20 /** + * virNetDevGetPhysPortID: + * + * @ifname: name of a netdev + * + * @physPortID: pointer to char* that will receive @ifname's + * phys_port_id from sysfs (null terminated + * string). Could be NULL if @ifname's net driver doesn't + * support phys_port_id (most netdev drivers + * don't). Caller is responsible for freeing the string + * when finished. + * + * Returns 0 on success or -1 on failure. + */ +int +virNetDevGetPhysPortID(const char *ifname, + char **physPortID) +{ + int ret =3D -1; + char *physPortIDFile =3D NULL; + + *physPortID =3D NULL; + + if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0) + goto cleanup; + + /* a failure to read just means the driver doesn't support + * phys_port_id, so set success now and ignore the return from + * virFileReadAllQuiet(). + */ + ret =3D 0; + + ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID)); + + cleanup: + VIR_FREE(physPortIDFile); + return ret; +} + + +/** * virNetDevGetVirtualFunctions: * * @pfname : name of the physical function interface name @@ -1433,6 +1473,17 @@ virNetDevGetVirtualFunctionInfo(const char *vfname, = char **pfname, =20 #else /* !__linux__ */ int +virNetDevGetPhysPortID(const char *ifname ATTRIBUTE_UNUSED, + char **physPortID ATTRIBUTE_UNUSED) +{ + /* this actually should never be called, and is just here to + * satisfy the linker. + */ + *physPortID =3D NULL; + return 0; +} + +int virNetDevGetVirtualFunctions(const char *pfname ATTRIBUTE_UNUSED, char ***vfname ATTRIBUTE_UNUSED, virPCIDeviceAddressPtr **virt_fns ATTRIBUTE_U= NUSED, diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 51fcae544..9205c0e86 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -226,6 +226,11 @@ int virNetDevGetPhysicalFunction(const char *ifname, c= har **pfname) int virNetDevPFGetVF(const char *pfname, int vf, char **vfname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; =20 +int virNetDevGetPhysPortID(const char *ifname, + char **physPortID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + int virNetDevGetVirtualFunctions(const char *pfname, char ***vfname, virPCIDeviceAddressPtr **virt_fns, --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415954505371.5499249476479; Thu, 10 Aug 2017 18:45:54 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0E0C6C014164; Fri, 11 Aug 2017 01:45:53 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D94A471C33; Fri, 11 Aug 2017 01:45:52 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 90ADC14B1E; Fri, 11 Aug 2017 01:45:52 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gWBb003147 for ; Thu, 10 Aug 2017 21:42:32 -0400 Received: by smtp.corp.redhat.com (Postfix) id 87D8F9814B; Fri, 11 Aug 2017 01:42:32 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1945598145; Fri, 11 Aug 2017 01:42:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 0E0C6C014164 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 0E0C6C014164 From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:17 -0400 Message-Id: <20170811014222.29548-3-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 2/7] util: Fix const'ness of 1st arg to virPCIGetNetName() X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 11 Aug 2017 01:45:53 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The first arg isn't modified in the function, so it should be const. --- New for V2. src/util/virpci.c | 4 ++-- src/util/virpci.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 2c1b75855..110d9741c 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2857,7 +2857,7 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPt= r addr, * Returns the network device name of a pci device */ int -virPCIGetNetName(char *device_link_sysfs_path, char **netname) +virPCIGetNetName(const char *device_link_sysfs_path, char **netname) { char *pcidev_sysfs_net_path =3D NULL; int ret =3D -1; @@ -2991,7 +2991,7 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPt= r dev ATTRIBUTE_UNUSED, } =20 int -virPCIGetNetName(char *device_link_sysfs_path ATTRIBUTE_UNUSED, +virPCIGetNetName(const char *device_link_sysfs_path ATTRIBUTE_UNUSED, char **netname ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); diff --git a/src/util/virpci.h b/src/util/virpci.h index 570684e75..82d4ddc61 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -207,7 +207,7 @@ int virPCIGetVirtualFunctionIndex(const char *pf_sysfs_= device_link, int virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr addr, char **pci_sysfs_device_link); =20 -int virPCIGetNetName(char *device_link_sysfs_path, char **netname); +int virPCIGetNetName(const char *device_link_sysfs_path, char **netname); =20 int virPCIGetSysfsFile(char *virPCIDeviceName, char **pci_sysfs_device_link) --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415784467296.7433364053353; Thu, 10 Aug 2017 18:43:04 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3B596A49FF; Fri, 11 Aug 2017 01:43:02 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 12C7698159; Fri, 11 Aug 2017 01:43:02 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id AF259388B; Fri, 11 Aug 2017 01:43:01 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gXuk003151 for ; Thu, 10 Aug 2017 21:42:33 -0400 Received: by smtp.corp.redhat.com (Postfix) id 308979814C; Fri, 11 Aug 2017 01:42:33 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id B447A71C33; Fri, 11 Aug 2017 01:42:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3B596A49FF Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3B596A49FF From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:18 -0400 Message-Id: <20170811014222.29548-4-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 3/7] util: make virPCIGetNetName() more versatile X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Fri, 11 Aug 2017 01:43:03 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" A single PCI device may have multiple netdevs associated with it. Each of those netdevs will have a different phys_port_id entry in sysfs. This patch modifies virPCIGetNetName() to allow selecting one of the potential many netdevs in two different ways: 1) by setting the "idx" argument, the caller can select the 1st (0), 2nd (1), etc. netdev from the PCI device's net subdirectory. 2) If the physPortID arg is set (to a null-terminated string) then virPCIGetNetName() returns the netdev that has that phys_port_id in the sysfs file of the same name in the netdev's directory. --- Change from V1 - in V1 I had only added the physPortID arg. In V2 I also added the idx arg to allow choosing port 1 or 2 regardless of physPortID (because in some situations you can't know the physPortID). src/util/virhostdev.c | 2 +- src/util/virnetdev.c | 9 ++++--- src/util/virpci.c | 66 ++++++++++++++++++++++++++++++++++++++++++-----= ---- src/util/virpci.h | 5 +++- 4 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 579563c3f..580f0fac0 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -326,7 +326,7 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, cha= r **linkdev, * type=3D'hostdev'>, and it is only those devices that should * end up calling this function. */ - if (virPCIGetNetName(sysfs_path, linkdev) < 0) + if (virPCIGetNetName(sysfs_path, 0, NULL, linkdev) < 0) goto cleanup; =20 if (!linkdev) { diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index a2664de78..b6ef00e2e 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1262,8 +1262,10 @@ virNetDevGetVirtualFunctions(const char *pfname, goto cleanup; } =20 - if (virPCIGetNetName(pci_sysfs_device_link, &((*vfname)[i])) < 0) + if (virPCIGetNetName(pci_sysfs_device_link, 0, + NULL, &((*vfname)[i])) < 0) { goto cleanup; + } =20 if (!(*vfname)[i]) VIR_INFO("VF does not have an interface name"); @@ -1362,7 +1364,8 @@ virNetDevGetPhysicalFunction(const char *ifname, char= **pfname) if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0) return ret; =20 - if (virPCIGetNetName(physfn_sysfs_path, pfname) < 0) + if (virPCIGetNetName(physfn_sysfs_path, 0, + NULL, pfname) < 0) goto cleanup; =20 if (!*pfname) { @@ -1422,7 +1425,7 @@ virNetDevPFGetVF(const char *pfname, int vf, char **v= fname) * isn't bound to a netdev driver, it won't have a netdev name, * and vfname will be NULL). */ - ret =3D virPCIGetNetName(virtfnSysfsPath, vfname); + ret =3D virPCIGetNetName(virtfnSysfsPath, 0, NULL, vfname); =20 cleanup: VIR_FREE(virtfnName); diff --git a/src/util/virpci.c b/src/util/virpci.c index 110d9741c..62a36b380 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -24,6 +24,7 @@ #include =20 #include "virpci.h" +#include "virnetdev.h" =20 #include #include @@ -2853,16 +2854,30 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddress= Ptr addr, return 0; } =20 -/* - * Returns the network device name of a pci device +/** + * virPCIGetNetName: + * @device_link_sysfs_path: sysfs path to the PCI device + * @idx: used to choose which netdev when there are several + * (ignored if physPortID is set) + * @physPortID: match this string in the netdev's phys_port_id + * (or NULL to ignore and use idx instead) + * @netname: used to return the name of the netdev + * (set to NULL (but returns success) if there is no netdev) + * + * Returns 0 on success, -1 on error (error has been logged) */ int -virPCIGetNetName(const char *device_link_sysfs_path, char **netname) +virPCIGetNetName(const char *device_link_sysfs_path, + size_t idx, + char *physPortID, + char **netname) { char *pcidev_sysfs_net_path =3D NULL; int ret =3D -1; DIR *dir =3D NULL; struct dirent *entry =3D NULL; + char *thisPhysPortID =3D NULL; + size_t i =3D 0; =20 if (virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, "net") =3D=3D -1) { @@ -2873,21 +2888,48 @@ virPCIGetNetName(const char *device_link_sysfs_path= , char **netname) if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) { /* this *isn't* an error - caller needs to check for netname =3D= =3D NULL */ ret =3D 0; - goto out; + goto cleanup; } =20 while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) { - /* Assume a single directory entry */ - if (VIR_STRDUP(*netname, entry->d_name) > 0) - ret =3D 0; + /* if the caller sent a physPortID, compare it to the + * physportID of this netdev. If not, look for entry[idx]. + */ + if (physPortID) { + if (virNetDevGetPhysPortID(entry->d_name, &thisPhysPortID) < 0) + goto cleanup; + + /* if this one doesn't match, keep looking */ + if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) { + VIR_FREE(thisPhysPortID); + continue; + } + } else { + if (i++ < idx) + continue; + } + + if (VIR_STRDUP(*netname, entry->d_name) < 0) + goto cleanup; + + ret =3D 0; break; } =20 + if (ret < 0) { + if (physPortID) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not find network device with " + "phys_port_id '%s' under PCI device at %s"), + physPortID, device_link_sysfs_path); + } else { + ret =3D 0; /* no netdev at the given index is *not* an error */ + } + } + cleanup: VIR_DIR_CLOSE(dir); - - out: VIR_FREE(pcidev_sysfs_net_path); - + VIR_FREE(thisPhysPortID); return ret; } =20 @@ -2915,7 +2957,7 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_dev= ice_path, goto cleanup; } =20 - if (virPCIGetNetName(pf_sysfs_device_path, pfname) < 0) + if (virPCIGetNetName(pf_sysfs_device_path, 0, NULL, pfname) < 0) goto cleanup; =20 if (!*pfname) { @@ -2992,6 +3034,8 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPt= r dev ATTRIBUTE_UNUSED, =20 int virPCIGetNetName(const char *device_link_sysfs_path ATTRIBUTE_UNUSED, + size_t idx ATTRIBUTE_UNUSED, + char *physPortID ATTRIBUTE_UNUSED, char **netname ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); diff --git a/src/util/virpci.h b/src/util/virpci.h index 82d4ddc61..adf336706 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -207,7 +207,10 @@ int virPCIGetVirtualFunctionIndex(const char *pf_sysfs= _device_link, int virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr addr, char **pci_sysfs_device_link); =20 -int virPCIGetNetName(const char *device_link_sysfs_path, char **netname); +int virPCIGetNetName(const char *device_link_sysfs_path, + size_t idx, + char *physPortID, + char **netname); =20 int virPCIGetSysfsFile(char *virPCIDeviceName, char **pci_sysfs_device_link) --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415780383599.2071697399072; Thu, 10 Aug 2017 18:43:00 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C4B91C098D1F; Fri, 11 Aug 2017 01:42:58 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D4F2600C6; Fri, 11 Aug 2017 01:42:58 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 4B72F3FCFB; Fri, 11 Aug 2017 01:42:58 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gXmk003154 for ; Thu, 10 Aug 2017 21:42:33 -0400 Received: by smtp.corp.redhat.com (Postfix) id CD09F98150; Fri, 11 Aug 2017 01:42:33 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5933F98145; Fri, 11 Aug 2017 01:42:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com C4B91C098D1F Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com C4B91C098D1F From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:19 -0400 Message-Id: <20170811014222.29548-5-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 4/7] util: match phys_port_id when converting PF-netdev to/from VF-netdev X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 11 Aug 2017 01:42:59 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" This patch updates functions in netdev.c to pay attention to phys_port_id. It uses the new function virNetDevGetPhysPortID() to learn the phys_port_id of a VF or PF, then sends that info to virPCIGetNetName(), which has newly been modified to take an optional phys_port_id. --- Change from V1 - Minor changes due to differences in patch 3. src/util/virnetdev.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index b6ef00e2e..5738a8936 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1231,13 +1231,17 @@ virNetDevGetVirtualFunctions(const char *pfname, char *pf_sysfs_device_link =3D NULL; char *pci_sysfs_device_link =3D NULL; char *pciConfigAddr =3D NULL; + char *pfPhysPortID =3D NULL; =20 *virt_fns =3D NULL; *n_vfname =3D 0; *max_vfs =3D 0; =20 + if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) + goto cleanup; + if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) - return ret; + goto cleanup; =20 if (virPCIGetVirtualFunctions(pf_sysfs_device_link, virt_fns, n_vfname, max_vfs) < 0) @@ -1263,7 +1267,7 @@ virNetDevGetVirtualFunctions(const char *pfname, } =20 if (virPCIGetNetName(pci_sysfs_device_link, 0, - NULL, &((*vfname)[i])) < 0) { + pfPhysPortID, &((*vfname)[i])) < 0) { goto cleanup; } =20 @@ -1278,6 +1282,7 @@ virNetDevGetVirtualFunctions(const char *pfname, VIR_FREE(*vfname); VIR_FREE(*virt_fns); } + VIR_FREE(pfPhysPortID); VIR_FREE(pf_sysfs_device_link); VIR_FREE(pci_sysfs_device_link); VIR_FREE(pciConfigAddr); @@ -1359,14 +1364,19 @@ int virNetDevGetPhysicalFunction(const char *ifname, char **pfname) { char *physfn_sysfs_path =3D NULL; + char *vfPhysPortID =3D NULL; int ret =3D -1; =20 + if (virNetDevGetPhysPortID(ifname, &vfPhysPortID) < 0) + goto cleanup; + if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0) - return ret; + goto cleanup; =20 if (virPCIGetNetName(physfn_sysfs_path, 0, - NULL, pfname) < 0) + vfPhysPortID, pfname) < 0) { goto cleanup; + } =20 if (!*pfname) { /* this shouldn't be possible. A VF can't exist unless its @@ -1380,6 +1390,7 @@ virNetDevGetPhysicalFunction(const char *ifname, char= **pfname) =20 ret =3D 0; cleanup: + VIR_FREE(vfPhysPortID); VIR_FREE(physfn_sysfs_path); return ret; } @@ -1407,8 +1418,16 @@ virNetDevPFGetVF(const char *pfname, int vf, char **= vfname) { char *virtfnName =3D NULL; char *virtfnSysfsPath =3D NULL; + char *pfPhysPortID =3D NULL; int ret =3D -1; =20 + /* a VF may have multiple "ports", each one having its own netdev, + * and each netdev having a different phys_port_id. Be sure we get + * the VF netdev with a phys_port_id matchine that of pfname + */ + if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) + goto cleanup; + if (virAsprintf(&virtfnName, "virtfn%d", vf) < 0) goto cleanup; =20 @@ -1425,11 +1444,12 @@ virNetDevPFGetVF(const char *pfname, int vf, char *= *vfname) * isn't bound to a netdev driver, it won't have a netdev name, * and vfname will be NULL). */ - ret =3D virPCIGetNetName(virtfnSysfsPath, 0, NULL, vfname); + ret =3D virPCIGetNetName(virtfnSysfsPath, 0, pfPhysPortID, vfname); =20 cleanup: VIR_FREE(virtfnName); VIR_FREE(virtfnSysfsPath); + VIR_FREE(pfPhysPortID); =20 return ret; } --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415782521220.27037988916572; Thu, 10 Aug 2017 18:43:02 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EC614F6886; Fri, 11 Aug 2017 01:42:59 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C692967C72; Fri, 11 Aug 2017 01:42:59 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 6C895180B467; Fri, 11 Aug 2017 01:42:59 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gYYa003171 for ; Thu, 10 Aug 2017 21:42:34 -0400 Received: by smtp.corp.redhat.com (Postfix) id 77DA798145; Fri, 11 Aug 2017 01:42:34 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 039E59814B; Fri, 11 Aug 2017 01:42:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com EC614F6886 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com EC614F6886 From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:20 -0400 Message-Id: <20170811014222.29548-6-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 5/7] util: save the correct VF's info when using a dual port SRIOV NIC in single port mode X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 11 Aug 2017 01:43:01 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Mellanox ConnectX-3 dual port SRIOV NICs present a bit of a challenge when assigning one of their VFs to a guest using VFIO device assignment. These NICs have only a single PCI PF device, and that single PF has two netdevs sharing the single PCI address - one for port 1 and one for port 2. When a VF is created it can also have 2 netdevs, or it can be setup in "single port" mode, where the VF has only a single netdev, and that netdev is connected either to port 1 or to port 2. When the VF is created in dual port mode, you get/set the MAC address/vlan tag for the port 1 VF by sending a netlink message to the PF's port1 netdev, and you get/set the MAC address/vlan tag for the port 2 VF by sending a netlink message to the PF's port 2 netdev. (Of course libvirt doesn't have any way to describe MAC/vlan info for 2 ports in a single hostdev interface, so that's a bit of a moot point) When the VF is created in single port mode, you can *set* the MAC/vlan info by sending a netlink message to *either* PF netdev - the driver is smart enough to understand that there's only a single netdev, and set the MAC/vlan for that netdev. When you want to *get* it, however, the driver is more accurate - it will return 00:00:00:00:00:00 for the MAC if you request it from the port 1 PF netdev when the VF was configured to be single port on port 2, or if you request if from the port 2 PF netdev when the VF was configured to be single port on port 1. Based on this information, when *getting* the MAC/vlan info (to save the original setting prior to assignment), we determine the correct PF netdev by matching phys_port_id between VF and PF. (IMPORTANT NOTE: this implies that to do PCI device assignment of the VFs on dual port Mellanox cards using (i.e. if you want the MAC address/vlan tag to be set), not only must the VFs be configured in single port mode, but also the VFs *must* be bound to the host VF net driver, and libvirt must use managed=3D'yes') By the time libvirt is ready to set the new MAC/vlan tag, the VF has already been unbound from the host net driver and bound to vfio-pci. This isn't problematic though because, as stated earlier, when a VF is created in single port mode, commands to configure it can be sent to either the port 1 PF netdev or the port 2 PF netdev. When it is time to restore the original MAC/vlan tag, again the VF will *not* be bound to a host net driver, so it won't be possible to learn from sysfs whether to use the port 1 or port 2 PF netdev for the netlink commands. And again, it doesn't matter which netdev you use. However, we must keep in mind that we saved the original settings to a file called "${PF}_${VFNUM}". To solve this problem, we just check for the existence of ${PF1}_${VFNUM} and ${PF2}_${VFNUM}, and use whichever one we find (since we know that only one can be there) --- New in V2 src/util/virhostdev.c | 27 +++++++++++++++++++++------ src/util/virpci.c | 31 +++++++++++++++++++++++++++++-- src/util/virpci.h | 4 +++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 580f0fac0..102fd85c1 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -307,7 +307,9 @@ virHostdevIsVirtualFunction(virDomainHostdevDefPtr host= dev) =20 =20 static int -virHostdevNetDevice(virDomainHostdevDefPtr hostdev, char **linkdev, +virHostdevNetDevice(virDomainHostdevDefPtr hostdev, + int pfNetDevIdx, + char **linkdev, int *vf) { int ret =3D -1; @@ -317,9 +319,10 @@ virHostdevNetDevice(virDomainHostdevDefPtr hostdev, ch= ar **linkdev, return ret; =20 if (virPCIIsVirtualFunction(sysfs_path) =3D=3D 1) { - if (virPCIGetVirtualFunctionInfo(sysfs_path, linkdev, - vf) < 0) + if (virPCIGetVirtualFunctionInfo(sysfs_path, pfNetDevIdx, + linkdev, vf) < 0) { goto cleanup; + } } else { /* In practice this should never happen, since we currently * only support assigning SRIOV VFs via parent.data.net); @@ -545,7 +548,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostd= ev, return ret; } =20 - if (virHostdevNetDevice(hostdev, &linkdev, &vf) < 0) + if (virHostdevNetDevice(hostdev, 0, &linkdev, &vf) < 0) return ret; =20 virtPort =3D virDomainNetGetActualVirtPortProfile( @@ -565,6 +568,18 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr host= dev, ret =3D virNetDevReadNetConfig(linkdev, vf, oldStateDir, &adminMAC, &vlan, &MAC); =20 + if (ret < 0) { + /* see if the config was saved using the PF's "port 2" + * netdev for the file name. + */ + VIR_FREE(linkdev); + + if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) >=3D 0) { + ret =3D virNetDevReadNetConfig(linkdev, vf, stateDir, + &adminMAC, &vlan, &MAC); + } + } + if (ret =3D=3D 0) { /* if a MAC was stored for the VF, we should now restore * that as the adminMAC. We have to do it this way because diff --git a/src/util/virpci.c b/src/util/virpci.c index 62a36b380..5ded77087 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2935,10 +2935,14 @@ virPCIGetNetName(const char *device_link_sysfs_path, =20 int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, - char **pfname, int *vf_index) + int pfNetDevIdx, + char **pfname, + int *vf_index) { virPCIDeviceAddressPtr pf_config_address =3D NULL; char *pf_sysfs_device_path =3D NULL; + char *vfname =3D NULL; + char *vfPhysPortID =3D NULL; int ret =3D -1; =20 if (virPCIGetPhysicalFunction(vf_sysfs_device_path, &pf_config_address= ) < 0) @@ -2957,8 +2961,28 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_de= vice_path, goto cleanup; } =20 - if (virPCIGetNetName(pf_sysfs_device_path, 0, NULL, pfname) < 0) + /* If the caller hasn't asked for a specific pfNetDevIdx, and VF + * is bound to a netdev, learn that netdev's phys_port_id (if + * available). This can be used to disambiguate when the PF has + * multiple netdevs. If the VF isn't bound to a netdev, then we + * return netdev[pfNetDevIdx] on the PF, which may or may not be + * correct. + */ + if (pfNetDevIdx =3D=3D -1) { + if (virPCIGetNetName(vf_sysfs_device_path, 0, NULL, &vfname) < 0) + goto cleanup; + + if (vfname) { + if (virNetDevGetPhysPortID(vfname, &vfPhysPortID) < 0) + goto cleanup; + } + pfNetDevIdx =3D 0; + } + + if (virPCIGetNetName(pf_sysfs_device_path, + pfNetDevIdx, vfPhysPortID, pfname) < 0) { goto cleanup; + } =20 if (!*pfname) { /* this shouldn't be possible. A VF can't exist unless its @@ -2974,6 +2998,8 @@ virPCIGetVirtualFunctionInfo(const char *vf_sysfs_dev= ice_path, cleanup: VIR_FREE(pf_config_address); VIR_FREE(pf_sysfs_device_path); + VIR_FREE(vfname); + VIR_FREE(vfPhysPortID); =20 return ret; } @@ -3044,6 +3070,7 @@ virPCIGetNetName(const char *device_link_sysfs_path A= TTRIBUTE_UNUSED, =20 int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path ATTRIBUTE_UN= USED, + int pfNetDevIdx ATTRIBUTE_UNUSED, char **pfname ATTRIBUTE_UNUSED, int *vf_index ATTRIBUTE_UNUSED) { diff --git a/src/util/virpci.h b/src/util/virpci.h index adf336706..f1fbe39e6 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -226,7 +226,9 @@ int virPCIGetAddrString(unsigned int domain, int virPCIDeviceAddressParse(char *address, virPCIDeviceAddressPtr bdf); =20 int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, - char **pfname, int *vf_index); + int pfNetDevIdx, + char **pfname, + int *vf_index); =20 int virPCIDeviceUnbind(virPCIDevicePtr dev); int virPCIDeviceRebind(virPCIDevicePtr dev); --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415785948403.1050997729169; Thu, 10 Aug 2017 18:43:05 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED84BC07D1C4; Fri, 11 Aug 2017 01:43:03 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BB90AA1033; Fri, 11 Aug 2017 01:43:03 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 5761A388C; Fri, 11 Aug 2017 01:43:03 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gZpl003177 for ; Thu, 10 Aug 2017 21:42:35 -0400 Received: by smtp.corp.redhat.com (Postfix) id 220689814B; Fri, 11 Aug 2017 01:42:35 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id A318B71C33; Fri, 11 Aug 2017 01:42:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ED84BC07D1C4 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com ED84BC07D1C4 From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:21 -0400 Message-Id: <20170811014222.29548-7-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 6/7] util: restructure virNetDevReadNetConfig() to eliminate false error logs X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 11 Aug 2017 01:43:05 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" virHostdevRestoreNetConfig() calls virNetDevReadNetConfig() to try and read the "original config" of a netdev, and if that fails, it tries again with a different directory/netdev name. This achieves the desired effect (we end up finding the config wherever it may be), but for each failure, virNetDevReadNetConfig() places a nice error message in the system logs. Experience has shown that false-positive error logs like this lead to erroneous bug reports, and can often mislead those searching for *real* bugs. This patch changes virNetDevReadNetConfig() to explicitly check if the file exists before calling virFileReadAll(); if it doesn't exist, virNetDevReadNetConfig() returns a success, but leaves all the variables holding the results as NULL. (This makes sense if you define the purpose of the function as "read a netdev's config from its config file *if that file exists*). To take advantage of that change, the caller, virHostdevRestoreNetConfig() is modified to fail immediately if virNetDevReadNetConfig() returns an error, and otherwise to try the different directory/netdev name if adminMAC & vlan & MAC are all NULL after the preceding attempt. --- New in V2. src/util/virhostdev.c | 126 ++++++++++++++++++++++++++++------------= ---- src/util/virnetdev.c | 16 ++++-- src/util/virnetdevmacvlan.c | 5 +- 3 files changed, 96 insertions(+), 51 deletions(-) diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index 102fd85c1..cca9d81a4 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -534,6 +534,10 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr host= dev, int ret =3D -1; int vf =3D -1; bool port_profile_associate =3D false; + virMacAddrPtr MAC =3D NULL; + virMacAddrPtr adminMAC =3D NULL; + virNetDevVlanPtr vlan =3D NULL; + =20 /* This is only needed for PCI devices that have been defined * using . For all others, it is a NOP. @@ -559,62 +563,92 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hos= tdev, NULL, port_profile_associate); } else { - virMacAddrPtr MAC =3D NULL; - virMacAddrPtr adminMAC =3D NULL; - virNetDevVlanPtr vlan =3D NULL; - - ret =3D virNetDevReadNetConfig(linkdev, vf, stateDir, &adminMAC, &= vlan, &MAC); - if (ret < 0 && oldStateDir) - ret =3D virNetDevReadNetConfig(linkdev, vf, oldStateDir, - &adminMAC, &vlan, &MAC); - - if (ret < 0) { - /* see if the config was saved using the PF's "port 2" - * netdev for the file name. - */ - VIR_FREE(linkdev); + /* we need to try 3 different places for the config file: + * 1) ${stateDir}/${PF}_vf${vf} + * This is almost always where the saved config is + * + * 2) ${oldStateDir/${PF}_vf${vf} + * saved config is only here if this machine was running a + * (by now *very*) old version of libvirt that saved the + * file in a different directory + * + * 3) ${stateDir}${PF[1]}_vf${VF} + * PF[1] means "the netdev for port 2 of the PF device", and + * is only valid when the PF is a Mellanox dual port NIC with + * a VF that was created in "single port" mode. + * + * NB: if virNetDevReadNetConfig() returns < 0, then it found + * the file, but there was a problem, so we should + * immediately return an error to our caller. If it returns + * 0, but all of the interesting stuff is NULL, that means + * the file wasn't found, so we can/should check other + * locations for it. + */ =20 - if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) >=3D 0) { - ret =3D virNetDevReadNetConfig(linkdev, vf, stateDir, - &adminMAC, &vlan, &MAC); - } + /* 1) standard location */ + if (virNetDevReadNetConfig(linkdev, vf, stateDir, + &adminMAC, &vlan, &MAC) < 0) { + goto cleanup; } =20 - if (ret =3D=3D 0) { - /* if a MAC was stored for the VF, we should now restore - * that as the adminMAC. We have to do it this way because - * the VF is still not bound to the host's net driver, so - * we can't directly set its MAC (and even after it is - * re-bound to the host net driver, it will still have its - * "administratively set" flag on, and that prohibits the - * VF's net driver from directly setting the MAC - * anyway). But it we set the desired VF MAC as the "admin - * MAC" *now*, then when the VF is re-bound to the host - * net driver (which will happen soon after returning from - * this function), that adminMAC will be set (by the PF) - * as the VF's new initial MAC. - * - * If no MAC was stored for the VF, that means it wasn't - * bound to a net driver before we used it anyway, so the - * adminMAC is all we have, and we can just restore it - * directly. - */ - if (MAC) { - VIR_FREE(adminMAC); - adminMAC =3D MAC; - MAC =3D NULL; + /* 2) "old" (pre-1.2.3 circa 2014) location - whenever we get + * to the point that nobody will ever upgrade directly from + * 1.2.3 (or older) directly to current libvirt, we can + * eliminate this clause + **/ + if (!(adminMAC || vlan || MAC) && oldStateDir && + virNetDevReadNetConfig(linkdev, vf, oldStateDir, + &adminMAC, &vlan, &MAC) < 0) { + goto cleanup; + } + + /* 3) try using the PF's "port 2" netdev as the name of the + * config file + */ + if (!(adminMAC || vlan || MAC)) { + VIR_FREE(linkdev); + + if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) < 0 || + virNetDevReadNetConfig(linkdev, vf, stateDir, + &adminMAC, &vlan, &MAC) < 0) { + goto cleanup; } + } =20 - ignore_value(virNetDevSetNetConfig(linkdev, vf, - adminMAC, vlan, MAC, true)); + /* if a MAC was stored for the VF, we should now restore + * that as the adminMAC. We have to do it this way because + * the VF is still not bound to the host's net driver, so + * we can't directly set its MAC (and even after it is + * re-bound to the host net driver, it will still have its + * "administratively set" flag on, and that prohibits the + * VF's net driver from directly setting the MAC + * anyway). But it we set the desired VF MAC as the "admin + * MAC" *now*, then when the VF is re-bound to the host + * net driver (which will happen soon after returning from + * this function), that adminMAC will be set (by the PF) + * as the VF's new initial MAC. + * + * If no MAC was stored for the VF, that means it wasn't + * bound to a net driver before we used it anyway, so the + * adminMAC is all we have, and we can just restore it + * directly. + */ + if (MAC) { + VIR_FREE(adminMAC); + adminMAC =3D MAC; + MAC =3D NULL; } =20 - VIR_FREE(MAC); - VIR_FREE(adminMAC); - virNetDevVlanFree(vlan); + ignore_value(virNetDevSetNetConfig(linkdev, vf, + adminMAC, vlan, MAC, true)); } =20 + ret =3D 0; + cleanup: VIR_FREE(linkdev); + VIR_FREE(MAC); + VIR_FREE(adminMAC); + virNetDevVlanFree(vlan); =20 return ret; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 5738a8936..8fc643c93 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1971,7 +1971,9 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, * @linkdev:@vf from a file in @stateDir. (see virNetDevSaveNetConfig * for details of file name and format). * - * Returns 0 on success, -1 on failure. + * Returns 0 on success, -1 on failure. It is *NOT* considered failure + * if no file is found to read. In that case, adminMAC, vlan, and MAC + * are set to NULL, and success is returned. * * The caller MUST free adminMAC, vlan, and MAC when it is finished * with them (they will be NULL if they weren't found in the file) @@ -2036,8 +2038,8 @@ virNetDevReadNetConfig(const char *linkdev, int vf, if (linkdev && !virFileExists(filePath)) { /* the device may have been stored in a file named for the * VF due to saveVlan =3D=3D false (or an older version of - * libvirt), so reset filePath so we'll try the other - * filename before failing. + * libvirt), so reset filePath and pfDevName so we'll try + * the other filename. */ VIR_FREE(filePath); pfDevName =3D NULL; @@ -2049,6 +2051,14 @@ virNetDevReadNetConfig(const char *linkdev, int vf, goto cleanup; } =20 + if (!virFileExists(filePath)) { + /* having no file to read is not necessarily an error, so we + * just return success, but with MAC, adminMAC, and vlan set to NU= LL + */ + ret =3D 0; + goto cleanup; + } + if (virFileReadAll(filePath, 128, &fileStr) < 0) goto cleanup; =20 diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c index 97c87701c..fb41bf934 100644 --- a/src/util/virnetdevmacvlan.c +++ b/src/util/virnetdevmacvlan.c @@ -1187,8 +1187,9 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char= *ifname, virMacAddrPtr adminMAC =3D NULL; virNetDevVlanPtr vlan =3D NULL; =20 - if (virNetDevReadNetConfig(linkdev, -1, stateDir, - &adminMAC, &vlan, &MAC) =3D=3D 0) { + if ((virNetDevReadNetConfig(linkdev, -1, stateDir, + &adminMAC, &vlan, &MAC) =3D=3D 0) && + (adminMAC || vlan || MAC)) { =20 ignore_value(virNetDevSetNetConfig(linkdev, -1, adminMAC, vlan, MAC, !!vlan= )); --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Fri Apr 26 19:30:28 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1502415962733222.66598267852237; Thu, 10 Aug 2017 18:46:02 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 60DBC28A2AB; Fri, 11 Aug 2017 01:46:01 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 33CA85C6D7; Fri, 11 Aug 2017 01:46:01 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id C521C4BB79; Fri, 11 Aug 2017 01:46:00 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v7B1gZOa003182 for ; Thu, 10 Aug 2017 21:42:35 -0400 Received: by smtp.corp.redhat.com (Postfix) id B9E7198145; Fri, 11 Aug 2017 01:42:35 +0000 (UTC) Received: from vhost2.laine.org (ovpn-117-36.phx2.redhat.com [10.3.117.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4683E9814B; Fri, 11 Aug 2017 01:42:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 60DBC28A2AB Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=laine.org Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 60DBC28A2AB From: Laine Stump To: libvir-list@redhat.com Date: Thu, 10 Aug 2017 21:42:22 -0400 Message-Id: <20170811014222.29548-8-laine@laine.org> In-Reply-To: <20170811014222.29548-1-laine@laine.org> References: <20170811014222.29548-1-laine@laine.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: mprivozn@redhat.com, moshele@mellanox.com Subject: [libvirt] [PATCH v2 7/7] util: check for PF online status earlier in guest startup X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 11 Aug 2017 01:46:01 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" 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=3D'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; =20 linkdev =3D vfDevOrig; + saveVlan =3D true; =20 - } else if (saveVlan && virNetDevIsVirtualFunction(linkdev) =3D=3D 1) { + } else if (virNetDevIsVirtualFunction(linkdev) =3D=3D 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; } =20 + 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 =3D virJSONValueNewObject())) goto cleanup; =20 @@ -1902,7 +1931,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf, * on the host) */ =20 - if (pfDevName) { + if (pfDevName && saveVlan) { if (virAsprintf(&filePath, "%s/%s_vf%d", stateDir, pfDevName, vf) = < 0) goto cleanup; =20 @@ -2251,32 +2280,6 @@ virNetDevSetNetConfig(const char *linkdev, int vf, } =20 } 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 !=3D 1 || vlan->trunk) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", --=20 2.13.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list