From cfe170216accf60938ff4ea9440a4ac78b0bd83f Mon Sep 17 00:00:00 2001
Message-Id: <cfe170216accf60938ff4ea9440a4ac78b0bd83f@dist-git>
From: Dmytro Linkin <dlinkin@nvidia.com>
Date: Thu, 28 Jan 2021 23:17:29 -0500
Subject: [PATCH] util: Add phys_port_name support on virPCIGetNetName
virPCIGetNetName is used to get the name of the netdev associated with
a particular PCI device. This is used when we have a VF name, but need
the PF name in order to send a netlink command (e.g. in order to
get/set the MAC address of the VF).
In simple cases there is a single netdev associated with any PCI
device, so it is easy to figure out the PF netdev for a VF - just look
for the PCI device that has the VF listed in its "virtfns" directory;
the only name in the "net" subdirectory of that PCI device's sysfs
directory is the PF netdev that is upstream of the VF in question.
In some cases there can be more than one netdev in a PCI device's net
directory though. In the past, the only case of this was for SR-IOV
NICs that could have multiple PF's per PCI device. In this case, all
PF netdevs associated with a PCI address would be listed in the "net"
subdirectory of the PCI device's directory in sysfs. At the same time,
all VF netdevs and all PF netdevs have a phys_port_id in their sysfs,
so the way to learn the correct PF netdev for a particular VF netdev
is to search through the list of devices in the net subdirectory of
the PF's PCI device, looking for the one netdev with a "phys_port_id"
matching that of the VF netdev.
But starting in kernel 5.8, the NVIDIA Mellanox driver began linking
the VFs' representor netdevs to the PF PCI address [1], and so the VF
representor netdevs would also show up in the net
subdirectory. However, all of the devices that do so also only have a
single PF netdev for any given PCI address.
This means that the net directory of the PCI device can still hold
multiple net devices, but only one of them will be the PF netdev (the
others are VF representors):
$ ls '/sys/bus/pci/devices/0000:82:00.0/net'
ens1f0 eth0 eth1
In this case the way to find the PF device is to look at the
"phys_port_name" attribute of each netdev in sysfs. All PF devices
have a phys_port_name matching a particular regex
(p[0-9]+$)|(p[0-9]+s[0-9]+$)
Since there can only be one PF in the entire list of devices, once we
match that regex, we've found the PF netdev.
[1] - https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/
commit/?id=123f0f53dd64b67e34142485fe866a8a581f12f1
Resolves: https://bugzilla.redhat.com/1918708
Co-Authored-by: Moshe Levi <moshele@nvidia.com>
Signed-off-by: Dmytro Linkin <dlinkin@nvidia.com>
Reviewed-by: Adrian Chiris <adrianc@nvidia.com>
Reviewed-by: Laine Stump <laine@redhat.com>
(cherry picked from commit 5b1c525b1f3608156884aed0dc5e925306c1e260)
Conflicts: src/util/virpci.c - upstream all DIR* were converted to use
g_autoptr, which permitted virPCIGetNetName() to be
simplified. Unfortunately, backporting this refactor would require
backporting an ever-ballooning set of patches, making the
possibility of causing a regression a very real danger. Instead,
one small refactor of virPCIGetName() that didn't affect any other
functions was backported, and this patch (adding phys_port_name
support) resolved the remaining conflicts by mimicking the current
upstream version of the function, but with all "return 0" replaced
by "ret = 0; goto cleanup;" and all "return -1" replaced by "goto
cleanup;" (the code at cleanup: just closes the DIR* and returns
the current value of ret). This will assure identical behavior to
upstream.
Signed-off-by: Laine Stump <laine@redhat.com>
Message-Id: <20210129041729.1076345-4-laine@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
src/util/virpci.c | 93 ++++++++++++++++++++++++++++-------------------
src/util/virpci.h | 5 +++
2 files changed, 61 insertions(+), 37 deletions(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 00377eed31..d5c038b7fe 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2424,9 +2424,9 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr addr,
* 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)
+ * (ignored if physPortID is set or physPortName is available)
* @physPortID: match this string in the netdev's phys_port_id
- * (or NULL to ignore and use idx instead)
+ * (or NULL to ignore and use phys_port_name or idx instead)
* @netname: used to return the name of the netdev
* (set to NULL (but returns success) if there is no netdev)
*
@@ -2460,6 +2460,14 @@ virPCIGetNetName(const char *device_link_sysfs_path,
}
while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
+ /* save the first entry we find to use as a failsafe
+ * in case we don't match the phys_port_id. This is
+ * needed because some NIC drivers (e.g. i40e)
+ * implement phys_port_id for PFs, but not for VFs
+ */
+ if (!firstEntryName)
+ firstEntryName = g_strdup(entry->d_name);
+
/* if the caller sent a physPortID, compare it to the
* physportID of this netdev. If not, look for entry[idx].
*/
@@ -2470,50 +2478,61 @@ virPCIGetNetName(const char *device_link_sysfs_path,
goto cleanup;
/* if this one doesn't match, keep looking */
- if (STRNEQ_NULLABLE(physPortID, thisPhysPortID)) {
- /* save the first entry we find to use as a failsafe
- * in case we don't match the phys_port_id. This is
- * needed because some NIC drivers (e.g. i40e)
- * implement phys_port_id for PFs, but not for VFs
- */
- if (!firstEntryName)
- firstEntryName = g_strdup(entry->d_name);
-
+ if (STRNEQ_NULLABLE(physPortID, thisPhysPortID))
continue;
- }
+
} else {
- if (i++ < idx)
- continue;
- }
+ /* Most switch devices use phys_port_name instead of
+ * phys_port_id.
+ * NOTE: VFs' representors net devices can be linked to PF's PCI
+ * device, which mean that there'll be multiple net devices
+ * instances and to get a proper net device need to match on
+ * specific regex.
+ * To get PF netdev, for ex., used following regex:
+ * "(p[0-9]+$)|(p[0-9]+s[0-9]+$)"
+ * or to get exact VF's netdev next regex is used:
+ * "pf0vf1$"
+ */
+ g_autofree char *thisPhysPortName = NULL;
- *netname = g_strdup(entry->d_name);
+ if (virNetDevGetPhysPortName(entry->d_name, &thisPhysPortName) < 0)
+ goto cleanup;
- ret = 0;
- break;
- }
+ if (thisPhysPortName) {
+
+ /* if this one doesn't match, keep looking */
+ if (!virStringMatch(thisPhysPortName, VIR_PF_PHYS_PORT_NAME_REGEX))
+ continue;
- if (ret < 0) {
- if (physPortID) {
- if (firstEntryName) {
- /* we didn't match the provided phys_port_id, but this
- * is probably because phys_port_id isn't implemented
- * for this NIC driver, so just return the first
- * (probably only) netname we found.
- */
- *netname = firstEntryName;
- firstEntryName = NULL;
- ret = 0;
} else {
- 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);
+
+ if (i++ < idx)
+ continue;
}
- } else {
- ret = 0; /* no netdev at the given index is *not* an error */
}
+
+ *netname = g_strdup(entry->d_name);
+ ret = 0;
+ goto cleanup;
}
- cleanup:
+
+ if (firstEntryName) {
+ /* we didn't match the provided phys_port_id / find a
+ * phys_port_name matching VIR_PF_PHYS_PORT_NAME_REGEX / find
+ * as many net devices as the value of idx, but this is
+ * probably because phys_port_id / phys_port_name isn't
+ * implemented for this NIC driver, so just return the first
+ * (probably only) netname we found.
+ */
+ *netname = g_steal_pointer(&firstEntryName);
+ ret = 0;
+ goto cleanup;
+ }
+
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find any network device under PCI device at %s"),
+ device_link_sysfs_path);
+cleanup:
VIR_DIR_CLOSE(dir);
return ret;
}
diff --git a/src/util/virpci.h b/src/util/virpci.h
index f6796fc422..e47c766918 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -49,6 +49,11 @@ struct _virZPCIDeviceAddress {
#define VIR_PCI_DEVICE_ADDRESS_FMT "%04x:%02x:%02x.%d"
+/* Represents format of PF's phys_port_name in switchdev mode:
+ * 'p%u' or 'p%us%u'. New line checked since value is readed from sysfs file.
+ */
+#define VIR_PF_PHYS_PORT_NAME_REGEX "(p[0-9]+$)|(p[0-9]+s[0-9]+$)"
+
struct _virPCIDeviceAddress {
unsigned int domain;
unsigned int bus;
--
2.30.0