From 2c8ec0095e6fd2e72879d4915ff8a9e5c0664d0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Sekleta=CC=81r?= <msekleta@redhat.com>
Date: Mon, 19 Oct 2020 11:10:31 +0200
Subject: [PATCH] udev/net_id: don't generate slot based names if multiple
 devices might claim the same slot

---
 man/systemd.net-naming-scheme.xml | 14 ++++++++++++++
 src/udev/net/naming-scheme.c      |  1 +
 src/udev/net/naming-scheme.h      |  2 ++
 src/udev/udev-builtin-net_id.c    | 29 ++++++++++++++++++++++++++++-
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/man/systemd.net-naming-scheme.xml b/man/systemd.net-naming-scheme.xml
index 91ad57d..40d7e58 100644
--- a/man/systemd.net-naming-scheme.xml
+++ b/man/systemd.net-naming-scheme.xml
@@ -182,6 +182,9 @@
           <para>SR-IOV virtual devices are named based on the name of the parent interface, with a suffix of
           <constant>v</constant> and the virtual device number, with any leading zeros removed. The bus
           number is ignored. This device type is found in IBM PowerVMs.</para>
+
+          <para>In some configurations a parent PCI bridge of a given network controller may be associated
+          with a slot. In such case we don't generate this device property to avoid possible naming conflicts.</para>
         </varlistentry>
 
         <varlistentry>
@@ -329,6 +332,17 @@
           <para>Previously two-letter interface type prefix was prepended to
           <varname>ID_NET_LABEL_ONBOARD=</varname>. This is not done anymore.</para></listitem>
         </varlistentry>
+
+        <varlistentry>
+          <term><constant>v246</constant></term>
+
+          <listitem><para>If the PCI slot is assocated with PCI bridge and that has multiple child network
+          controllers then all of them might derive the same value of <varname>ID_NET_NAME_SLOT</varname>
+          property. That could cause naming conflict if the property is selected as a device name. Now, we detect the
+          situation, slot - bridge relation, and we don't produce the <varname>ID_NET_NAME_SLOT</varname> property to
+          avoid possible naming conflict.</para></listitem>
+        </varlistentry>
+
     </variablelist>
 
     <para>Note that <constant>latest</constant> may be used to denote the latest scheme known (to this
diff --git a/src/udev/net/naming-scheme.c b/src/udev/net/naming-scheme.c
index 0d7f413..ebab2ff 100644
--- a/src/udev/net/naming-scheme.c
+++ b/src/udev/net/naming-scheme.c
@@ -10,6 +10,7 @@ static const NamingScheme naming_schemes[] = {
         { "v240", NAMING_V240 },
         { "v241", NAMING_V241 },
         { "v243", NAMING_V243 },
+        { "v246", NAMING_V246 },
         /* … add more schemes here, as the logic to name devices is updated … */
 };
 
diff --git a/src/udev/net/naming-scheme.h b/src/udev/net/naming-scheme.h
index 38dfa75..d744f3e 100644
--- a/src/udev/net/naming-scheme.h
+++ b/src/udev/net/naming-scheme.h
@@ -30,6 +30,7 @@ typedef enum NamingSchemeFlags {
         NAMING_STABLE_VIRTUAL_MACS = 1 << 5, /* Use device name to generate MAC, see 6d3646406560 */
         NAMING_NETDEVSIM           = 1 << 6, /* Generate names for netdevsim devices, see eaa9d507d855 */
         NAMING_LABEL_NOPREFIX      = 1 << 7, /* Don't prepend ID_NET_LABEL_ONBOARD with interface type prefix */
+        NAMING_BRIDGE_NO_SLOT      = 1 << 9, /* Don't use PCI hotplug slot information if the corresponding device is a PCI bridge */
 
         /* And now the masks that combine the features above */
         NAMING_V238 = 0,
@@ -37,6 +38,7 @@ typedef enum NamingSchemeFlags {
         NAMING_V240 = NAMING_V239 | NAMING_INFINIBAND | NAMING_ZERO_ACPI_INDEX | NAMING_ALLOW_RERENAMES,
         NAMING_V241 = NAMING_V240 | NAMING_STABLE_VIRTUAL_MACS,
         NAMING_V243 = NAMING_V241 | NAMING_NETDEVSIM | NAMING_LABEL_NOPREFIX,
+        NAMING_V246 = NAMING_V243 | NAMING_BRIDGE_NO_SLOT,
 
         _NAMING_SCHEME_FLAGS_INVALID = -1,
 } NamingSchemeFlags;
diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
index 99d44a2..af0d2a7 100644
--- a/src/udev/udev-builtin-net_id.c
+++ b/src/udev/udev-builtin-net_id.c
@@ -244,6 +244,25 @@ static bool is_pci_ari_enabled(sd_device *dev) {
         return streq(a, "1");
 }
 
+static bool is_pci_bridge(sd_device *dev) {
+        const char *v, *p;
+
+        if (sd_device_get_sysattr_value(dev, "modalias", &v) < 0)
+                return false;
+
+        if (!startswith(v, "pci:"))
+                return false;
+
+        p = strrchr(v, 's');
+        if (!p)
+                return false;
+        if (p[1] != 'c')
+                return false;
+
+        /* PCI device subclass 04 corresponds to PCI bridge */
+        return strneq(p + 2, "04", 2);
+}
+
 static int dev_pci_slot(sd_device *dev, struct netnames *names) {
         unsigned long dev_port = 0;
         unsigned domain, bus, slot, func;
@@ -343,10 +362,18 @@ static int dev_pci_slot(sd_device *dev, struct netnames *names) {
                             read_one_line_file(str, &address) >= 0 &&
                             startswith(sysname, address)) {
                                 hotplug_slot = i;
+
+                                /* We found the match between PCI device and slot. However, we won't use the
+                                 * slot index if the device is a PCI bridge, because it can have other child
+                                 * devices that will try to claim the same index and that would create name
+                                 * collision. */
+                                if (naming_scheme_has(NAMING_BRIDGE_NO_SLOT) && is_pci_bridge(hotplug_slot_dev))
+                                        hotplug_slot = 0;
+
                                 break;
                         }
                 }
-                if (hotplug_slot > 0)
+                if (hotplug_slot >= 0)
                         break;
                 if (sd_device_get_parent_with_subsystem_devtype(hotplug_slot_dev, "pci", NULL, &hotplug_slot_dev) < 0)
                         break;
-- 
2.23.0

