]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
udev-builtin-net_id: split out get_dev_port() and make its failure critical
authorYu Watanabe <watanabe.yu+github@gmail.com>
Thu, 22 Sep 2022 06:53:35 +0000 (15:53 +0900)
committerYu Watanabe <watanabe.yu+github@gmail.com>
Tue, 1 Aug 2023 01:56:26 +0000 (10:56 +0900)
As dev_port and dev_id sysfs attributes are fundamental properties for
network interfaces. Hence, it should not fail.

Also, the type of dev_port is changed to unsigned. The kernel internally
uses 'unsigned short' for dev_port and dev_id. Hence, unsigned (that is,
32 bits) is still overkill, but should be enough.

src/udev/udev-builtin-net_id.c

index 38afe6cf334b46b386e699bbe4f82bde1c80c85c..57eccb349dbcdace0c7e84fa9a6f2aa7d900809f 100644 (file)
@@ -138,6 +138,46 @@ static int get_virtfn_info(sd_device *pcidev, sd_device **ret_physfn_pcidev, cha
         return -ENOENT;
 }
 
+static int get_dev_port(sd_device *dev, bool fallback_to_dev_id, unsigned *ret) {
+        unsigned v;
+        int r;
+
+        assert(dev);
+        assert(ret);
+
+        /* Get kernel provided port index for the case when multiple ports on a single PCI function. */
+
+        r = device_get_sysattr_unsigned(dev, "dev_port", &v);
+        if (r < 0)
+                return r;
+        if (r > 0) {
+                /* Found a positive index. Let's use it. */
+                *ret = v;
+                return 1; /* positive */
+        }
+        assert(v == 0);
+
+        /* With older kernels IP-over-InfiniBand network interfaces sometimes erroneously provide the port
+         * number in the 'dev_id' sysfs attribute instead of 'dev_port', which thus stays initialized as 0. */
+
+        if (fallback_to_dev_id) {
+                unsigned iftype;
+
+                r = device_get_sysattr_unsigned(dev, "type", &iftype);
+                if (r < 0)
+                        return r;
+
+                fallback_to_dev_id = (iftype == ARPHRD_INFINIBAND);
+        }
+
+        if (fallback_to_dev_id)
+                return device_get_sysattr_unsigned(dev, "dev_id", ret);
+
+        /* Otherwise, return the original index 0. */
+        *ret = 0;
+        return 0; /* zero */
+}
+
 static bool is_valid_onboard_index(unsigned long idx) {
         /* Some BIOSes report rubbish indexes that are excessively high (2^24-1 is an index VMware likes to
          * report for example). Let's define a cut-off where we don't consider the index reliable anymore. We
@@ -150,7 +190,8 @@ static bool is_valid_onboard_index(unsigned long idx) {
 
 /* retrieve on-board index number and label from firmware */
 static int dev_pci_onboard(sd_device *dev, const LinkInfo *info, NetNames *names) {
-        unsigned long idx, dev_port = 0;
+        unsigned long idx;
+        unsigned dev_port;
         const char *attr;
         size_t l;
         char *s;
@@ -182,13 +223,9 @@ static int dev_pci_onboard(sd_device *dev, const LinkInfo *info, NetNames *names
                 return log_device_debug_errno(names->pcidev, SYNTHETIC_ERRNO(ENOENT),
                                               "Not a valid onboard index: %lu", idx);
 
-        /* kernel provided port index for multiple ports on a single PCI function */
-        if (sd_device_get_sysattr_value(dev, "dev_port", &attr) >= 0) {
-                r = safe_atolu_full(attr, 10, &dev_port);
-                if (r < 0)
-                        log_device_debug_errno(dev, r, "Failed to parse dev_port, ignoring: %m");
-                log_device_debug(dev, "dev_port=%lu", dev_port);
-        }
+        r = get_dev_port(dev, /* fallback_to_dev_id = */ false, &dev_port);
+        if (r < 0)
+                return r;
 
         s = names->pci_onboard;
         l = sizeof(names->pci_onboard);
@@ -200,10 +237,10 @@ static int dev_pci_onboard(sd_device *dev, const LinkInfo *info, NetNames *names
                 /* kernel provided front panel port name for multiple port PCI device */
                 l = strpcpyf(&s, l, "n%s", info->phys_port_name);
         else if (dev_port > 0)
-                l = strpcpyf(&s, l, "d%lu", dev_port);
+                l = strpcpyf(&s, l, "d%u", dev_port);
         if (l == 0)
                 names->pci_onboard[0] = '\0';
-        log_device_debug(dev, "Onboard index identifier: index=%lu phys_port=%s dev_port=%lu %s %s",
+        log_device_debug(dev, "Onboard index identifier: index=%lu phys_port=%s dev_port=%u %s %s",
                          idx, strempty(info->phys_port_name), dev_port,
                          special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(names->pci_onboard));
 
@@ -431,9 +468,8 @@ static int pci_get_hotplug_slot(sd_device *dev, uint32_t *ret) {
 }
 
 static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
-        const char *sysname, *attr;
-        unsigned domain, bus, slot, func;
-        unsigned long dev_port = 0;
+        const char *sysname;
+        unsigned domain, bus, slot, func, dev_port;
         uint32_t hotplug_slot = 0;  /* avoid false maybe-uninitialized warning */
         size_t l;
         char *s;
@@ -460,27 +496,9 @@ static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
                  * where the slot makes up the upper 5 bits. */
                 func += slot * 8;
 
-        /* kernel provided port index for multiple ports on a single PCI function */
-        if (sd_device_get_sysattr_value(dev, "dev_port", &attr) >= 0) {
-                log_device_debug(dev, "dev_port=%s", attr);
-
-                r = safe_atolu_full(attr, 10, &dev_port);
-                if (r < 0)
-                        log_device_debug_errno(dev, r, "Failed to parse attribute dev_port, ignoring: %m");
-
-                /* With older kernels IP-over-InfiniBand network interfaces sometimes erroneously
-                 * provide the port number in the 'dev_id' sysfs attribute instead of 'dev_port',
-                 * which thus stays initialized as 0. */
-                if (dev_port == 0 &&
-                    info->iftype == ARPHRD_INFINIBAND &&
-                    sd_device_get_sysattr_value(dev, "dev_id", &attr) >= 0) {
-                        log_device_debug(dev, "dev_id=%s", attr);
-
-                        r = safe_atolu_full(attr, 10, &dev_port);
-                        if (r < 0)
-                                log_device_debug_errno(dev, r, "Failed to parse attribute dev_id, ignoring: %m");
-                }
-        }
+        r = get_dev_port(dev, /* fallback_to_dev_id = */ true, &dev_port);
+        if (r < 0)
+                return r;
 
         /* compose a name based on the raw kernel's PCI bus, slot numbers */
         s = names->pci_path;
@@ -497,11 +515,11 @@ static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
                 /* kernel provided front panel port name for multi-port PCI device */
                 l = strpcpyf(&s, l, "n%s", info->phys_port_name);
         else if (dev_port > 0)
-                l = strpcpyf(&s, l, "d%lu", dev_port);
+                l = strpcpyf(&s, l, "d%u", dev_port);
         if (l == 0)
                 names->pci_path[0] = '\0';
 
-        log_device_debug(dev, "PCI path identifier: domain=%u bus=%u slot=%u func=%u phys_port=%s dev_port=%lu %s %s",
+        log_device_debug(dev, "PCI path identifier: domain=%u bus=%u slot=%u func=%u phys_port=%s dev_port=%u %s %s",
                          domain, bus, slot, func, strempty(info->phys_port_name), dev_port,
                          special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(names->pci_path));
 
@@ -526,11 +544,11 @@ static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
         else if (!isempty(info->phys_port_name))
                 l = strpcpyf(&s, l, "n%s", info->phys_port_name);
         else if (dev_port > 0)
-                l = strpcpyf(&s, l, "d%lu", dev_port);
+                l = strpcpyf(&s, l, "d%u", dev_port);
         if (l == 0)
                 names->pci_slot[0] = '\0';
 
-        log_device_debug(dev, "Slot identifier: domain=%u slot=%"PRIu32" func=%u phys_port=%s dev_port=%lu %s %s",
+        log_device_debug(dev, "Slot identifier: domain=%u slot=%"PRIu32" func=%u phys_port=%s dev_port=%u %s %s",
                          domain, hotplug_slot, func, strempty(info->phys_port_name), dev_port,
                          special_glyph(SPECIAL_GLYPH_ARROW_RIGHT), empty_to_na(names->pci_slot));