udev-builtin-net_id: use firmware_node/sun for ID_NET_NAME_SLOT

pci_get_hotplug_slot() has the following limitations:
- if slots are not hotpluggable, they are not in /sys/bus/pci/slots.
- the address at /sys/bus/pci/slots/X/addr doesn't contains the function part,
  so on some system, 2 different slots with different _SUN end up with the same
  hotplug_slot, leading to naming conflicts.
- it tries all parent devices until it finds a slot number, which is incorrect,
  and what led to NAMING_BRIDGE_MULTIFUNCTION_SLOT being disabled.

The use of PCI hotplug to find the slot (ACPI _SUN) was introduced in
https://github.com/systemd/systemd/commit/0035597a30d120f70df2dd7da3d6128fb8ba6051
"udev: net_id - export PCI hotplug slot names" on 2012/11/26.
At the same time on the kernel side we got
https://github.com/torvalds/linux/commit/bb74ac23b10820d8722c3e1f4add9ef59e703f63
"ACPI: create _SUN sysfs file" on 2012/11/16.

Using PCI hotplug was the only way at the time, but now 12 years later we can use
firmware_node/sun sysfs file.
Looking at a small selection of server HW, for HPE (Gen10 DL325), the _SUN is attached
to the NIC device, whereas for Dell (R640/R6515/R6615) and Cisco (UCSC-C220-M5SX),
the _SUN is on the first parent pcieport.

We still fallback to pci_get_hotplug_slot() to handle the s390 case and
maybe some other coner cases (_SUN on grand parent device that is not a
bridge ?).

diff --git a/man/systemd.net-naming-scheme.xml b/man/systemd.net-naming-scheme.xml
index ff811c2bd714e86fa3e12123fe23daf336ece1ba..905351005c6883f2cdb8e95201336e112ea80677 100644 (file)
--- a/man/systemd.net-naming-scheme.xml
+++ b/man/systemd.net-naming-scheme.xml
@@ -525,6 +525,14 @@
           </listitem>
         </varlistentry>
 
+        <varlistentry>
+          <term><constant>v257</constant></term>
+
+          <listitem><para>PCI slot number is now read from <constant>firmware_node/sun</constant> sysfs file</para>
+
+          <xi:include href="version-info.xml" xpointer="v257"/>
+          </listitem>
+        </varlistentry>
       </variablelist>
 
     <para>Note that <constant>latest</constant> may be used to denote the latest scheme known (to this
@@ -597,7 +605,7 @@ ID_NET_NAME_ONBOARD_LABEL=Ethernet Port 1
     </example>
 
     <example>
-      <title>PCI Ethernet card in hotplug slot with firmware index number</title>
+      <title>PCI Ethernet card in slot with firmware index number</title>
 
       <programlisting># /sys/devices/pci0000:00/0000:00:1c.3/0000:05:00.0/net/ens1
 ID_NET_NAME_MAC=enx000000000466

diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c
index 2955b6e8d57757db2fad01ee1a4d5c659bdaa2b2..5033566878437e260a641d825dfc2f422de6817d 100644 (file)
--- a/src/shared/netif-naming-scheme.c
+++ b/src/shared/netif-naming-scheme.c
@@ -24,6 +24,7 @@ static const NamingScheme naming_schemes[] = {
         { "v253", NAMING_V253 },
         { "v254", NAMING_V254 },
         { "v255", NAMING_V255 },
+        { "v257", NAMING_V257 },
         /* … add more schemes here, as the logic to name devices is updated … */
 
         EXTRA_NET_NAMING_MAP

diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h
index 27461f4ce99ea18b62642d6840d80779d6a9c929..45a53291855f67c324b0c4d6dd25804646d09498 100644 (file)
--- a/src/shared/netif-naming-scheme.h
+++ b/src/shared/netif-naming-scheme.h
@@ -43,6 +43,7 @@ typedef enum NamingSchemeFlags {
         NAMING_DEVICETREE_ALIASES        = 1 << 15, /* Generate names from devicetree aliases */
         NAMING_USB_HOST                  = 1 << 16, /* Generate names for usb host */
         NAMING_SR_IOV_R                  = 1 << 17, /* Use "r" suffix for SR-IOV VF representors */
+        NAMING_FIRMWARE_NODE_SUN         = 1 << 18, /* Use firmware_node/sun to get PCI slot number */
 
         /* And now the masks that combine the features above */
         NAMING_V238 = 0,
@@ -62,6 +63,7 @@ typedef enum NamingSchemeFlags {
                                                        * patch later. NAMING_SR_IOV_R is enabled by default in
                                                        * systemd version 255, naming scheme "v255". */
         NAMING_V255 = NAMING_V254 & ~NAMING_BRIDGE_MULTIFUNCTION_SLOT,
+        NAMING_V257 = NAMING_V255 | NAMING_FIRMWARE_NODE_SUN,
 
         EXTRA_NET_NAMING_SCHEMES
 

diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c
index 384a1f31cbc46801524b05b1fce9b031d07a37a3..d34357fdb214b903acf93f9a202b0d9414a38606 100644 (file)
--- a/src/udev/udev-builtin-net_id.c
+++ b/src/udev/udev-builtin-net_id.c
@@ -566,6 +566,51 @@ static int pci_get_hotplug_slot(sd_device *dev, uint32_t *ret) {
         return -ENOENT;
 }
 
+static int get_device_firmware_node_sun(sd_device *dev, uint32_t *ret) {
+        const char *attr;
+        int r;
+
+        assert(dev);
+        assert(ret);
+
+        r = device_get_sysattr_value_filtered(dev, "firmware_node/sun", &attr);
+        if (r < 0)
+                return log_device_debug_errno(dev, r, "Failed to read firmware_node/sun, ignoring: %m");
+
+        r = safe_atou32(attr, ret);
+        if (r < 0)
+                return log_device_warning_errno(dev, r, "Failed to parse firmware_node/sun '%s', ignoring: %m", attr);
+
+        return 0;
+}
+
+static int pci_get_slot_from_firmware_node_sun(sd_device *dev, uint32_t *ret) {
+        int r;
+        sd_device *slot_dev;
+
+        assert(dev);
+        assert(ret);
+
+        /* Try getting the ACPI _SUN for the device */
+        if (get_device_firmware_node_sun(dev, ret) >= 0)
+                return 0;
+
+        r = sd_device_get_parent_with_subsystem_devtype(dev, "pci", NULL, &slot_dev);
+        if (r < 0)
+                return log_device_debug_errno(dev, r, "Failed to find pci parent, ignoring: %m");
+
+        if (is_pci_bridge(slot_dev) && is_pci_multifunction(dev) <= 0)
+                return log_device_debug_errno(dev, SYNTHETIC_ERRNO(ESTALE),
+                                              "Not using slot information because the parent pcieport "
+                                              "is a bridge and the PCI device is not multifunction.");
+
+        /* Try getting the ACPI _SUN from the parent pcieport */
+        if (get_device_firmware_node_sun(slot_dev, ret) >= 0)
+                return 0;
+
+        return -ENOENT;
+}
+
 static int get_pci_slot_specifiers(
                 sd_device *dev,
                 char **ret_domain,
@@ -616,7 +661,7 @@ static int get_pci_slot_specifiers(
 
 static int names_pci_slot(sd_device *dev, sd_device *pci_dev, const char *prefix, const char *suffix, EventMode mode) {
         _cleanup_free_ char *domain = NULL, *bus_and_slot = NULL, *func = NULL, *port = NULL;
-        uint32_t hotplug_slot = 0;  /* avoid false maybe-uninitialized warning */
+        uint32_t slot = 0;  /* avoid false maybe-uninitialized warning */
         char str[ALTIFNAMSIZ];
         int r;
 
@@ -641,20 +686,27 @@ static int names_pci_slot(sd_device *dev, sd_device *pci_dev, const char *prefix
                          strna(domain), bus_and_slot, strna(func), strna(port),
                          special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(str));
 
-        r = pci_get_hotplug_slot(pci_dev, &hotplug_slot);
-        if (r < 0)
-                return r;
-        if (r > 0)
-                /* If the hotplug slot is found through the function ID, then drop the domain from the name.
-                 * See comments in parse_hotplug_slot_from_function_id(). */
-                domain = mfree(domain);
+        if (naming_scheme_has(NAMING_FIRMWARE_NODE_SUN))
+                r = pci_get_slot_from_firmware_node_sun(pci_dev, &slot);
+        else
+                r = -1;
+        /* If we don't find a slot using firmware_node/sun, fallback to hotplug_slot */
+        if (r < 0) {
+                r = pci_get_hotplug_slot(pci_dev, &slot);
+                if (r < 0)
+                        return r;
+                if (r > 0)
+                        /* If the hotplug slot is found through the function ID, then drop the domain from the name.
+                        * See comments in parse_hotplug_slot_from_function_id(). */
+                        domain = mfree(domain);
+        }
 
         if (snprintf_ok(str, sizeof str, "%s%ss%"PRIu32"%s%s%s",
-                        prefix, strempty(domain), hotplug_slot, strempty(func), strempty(port), strempty(suffix)))
+                        prefix, strempty(domain), slot, strempty(func), strempty(port), strempty(suffix)))
                 udev_builtin_add_property(dev, mode, "ID_NET_NAME_SLOT", str);
 
         log_device_debug(dev, "Slot identifier: domain=%s slot=%"PRIu32" func=%s port=%s %s %s",
-                         strna(domain), hotplug_slot, strna(func), strna(port),
+                         strna(domain), slot, strna(func), strna(port),
                          special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(str));
 
         return 0;