From: Etienne Champetier Date: Tue, 9 Jul 2024 15:53:50 +0000 (-0400) Subject: udev-builtin-net_id: use firmware_node/sun for ID_NET_NAME_SLOT X-Git-Tag: v257-rc1~861 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0a4ecc54cb9f2d3418b970c51bfadb69c34ae9eb;p=thirdparty%2Fsystemd.git 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 ff811c2bd71..905351005c6 100644 --- a/man/systemd.net-naming-scheme.xml +++ b/man/systemd.net-naming-scheme.xml @@ -525,6 +525,14 @@ + + v257 + + PCI slot number is now read from firmware_node/sun sysfs file + + + + Note that latest may be used to denote the latest scheme known (to this @@ -597,7 +605,7 @@ ID_NET_NAME_ONBOARD_LABEL=Ethernet Port 1 - PCI Ethernet card in hotplug slot with firmware index number + PCI Ethernet card in slot with firmware index number # /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 2955b6e8d57..50335668784 100644 --- 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 27461f4ce99..45a53291855 100644 --- 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 384a1f31cbc..d34357fdb21 100644 --- 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;