Merge pull request #20744 from yuwata/udev-netlink

udev: use netlink more aggressively

I'm pasting the comment from https://github.com/systemd/systemd/pull/20744#issuecomment-934485287
which is quite informative. The code wasn't changed significantly since then:

atenart commented 6 days ago:
> I ran tests without (93caec7) and with this PR (06735f2) on Fedora, having a few udev rules
> using attributes eligible to be cached and creating 50 veth on 4 CPUs. Although the time spent
> running the test is variable between runs, I generally saw an improvement when using this PR, e.g:
>
> 249-910-g93caec7:
> real	0m3.691s
> user	0m0.022s
> sys	0m1.338s
> 
> 249-920-g06735f2:
> real	0m2.950s
> user	0m0.005s
> sys	0m0.399s
> 
> On a different system than the one used above, I even saw a 40% improvement; results depend
> on many parameters (distro, udev rules, concurrent daemons accessing sysfs, etc.).
> 
> Because it's quite hard to measure the improvement here (as the kernel behave differently between
> the two test cases), I also ran tests using a modified kernel not hitting the trylock logic. There was
> an improvement with this PR as well. (Take this with a grain of salt though, as the kernel was
> modified not using patches approved upstream).

src/basic/ether-addr-util.c

@@ -7,6 +7,7 @@
 #include <sys/types.h>
 
 #include "ether-addr-util.h"
+#include "hexdecoct.h"
 #include "macro.h"
 #include "string-util.h"
 
@@ -15,12 +16,13 @@ char* hw_addr_to_string(const struct hw_addr_data *addr, char buffer[HW_ADDR_TO_
         assert(buffer);
         assert(addr->length <= HW_ADDR_MAX_SIZE);
 
-        for (size_t i = 0; i < addr->length; i++) {
-                sprintf(&buffer[3*i], "%02"PRIx8, addr->bytes[i]);
-                if (i < addr->length - 1)
-                        buffer[3*i + 2] = ':';
+        for (size_t i = 0, j = 0; i < addr->length; i++) {
+                buffer[j++] = hexchar(addr->bytes[i] >> 4);
+                buffer[j++] = hexchar(addr->bytes[i] & 0x0f);
+                buffer[j++] = ':';
         }
 
+        buffer[addr->length > 0 ? addr->length * 3 - 1 : 0] = '\0';
         return buffer;
 }
 

src/libsystemd/sd-device/device-private.h

@@ -18,13 +18,15 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) {
 }
 
 int device_get_device_id(sd_device *device, const char **ret);
-
 int device_get_devlink_priority(sd_device *device, int *priority);
 int device_get_watch_handle(sd_device *device);
 int device_get_devnode_mode(sd_device *device, mode_t *mode);
 int device_get_devnode_uid(sd_device *device, uid_t *uid);
 int device_get_devnode_gid(sd_device *device, gid_t *gid);
 
+int device_cache_sysattr_value(sd_device *device, const char *key, char *value);
+int device_get_cached_sysattr_value(sd_device *device, const char *key, const char **ret_value);
+
 void device_seal(sd_device *device);
 void device_set_is_initialized(sd_device *device);
 int device_set_watch_handle(sd_device *device, int wd);

src/libsystemd/sd-device/sd-device.c

@@ -1941,7 +1941,7 @@ _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) {
         return 0;
 }
 
-static int device_cache_sysattr_value(sd_device *device, const char *key, char *value) {
+int device_cache_sysattr_value(sd_device *device, const char *key, char *value) {
         _unused_ _cleanup_free_ char *old_value = NULL;
         _cleanup_free_ char *new_key = NULL;
         int r;
@@ -1969,18 +1969,19 @@ static int device_cache_sysattr_value(sd_device *device, const char *key, char *
         return 0;
 }
 
-static int device_get_cached_sysattr_value(sd_device *device, const char *_key, const char **_value) {
-        const char *key = NULL, *value;
+int device_get_cached_sysattr_value(sd_device *device, const char *key, const char **ret_value) {
+        const char *k = NULL, *value;
 
         assert(device);
-        assert(_key);
+        assert(key);
 
-        value = hashmap_get2(device->sysattr_values, _key, (void **) &key);
-        if (!key)
-                return -ENOENT;
-
-        if (_value)
-                *_value = value;
+        value = hashmap_get2(device->sysattr_values, key, (void **) &k);
+        if (!k)
+                return -ESTALE; /* We have not read the attribute. */
+        if (!value)
+                return -ENOENT; /* We have looked up the attribute before and it did not exist. */
+        if (ret_value)
+                *ret_value = value;
         return 0;
 }
 
@@ -1988,7 +1989,7 @@ static int device_get_cached_sysattr_value(sd_device *device, const char *_key,
  * with a NULL value in the cache, otherwise the returned string is stored */
 _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, const char **ret_value) {
         _cleanup_free_ char *value = NULL;
-        const char *path, *syspath, *cached_value = NULL;
+        const char *path, *syspath;
         struct stat statbuf;
         int r;
 
@@ -1996,20 +1997,9 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
         assert_return(sysattr, -EINVAL);
 
         /* look for possibly already cached result */
-        r = device_get_cached_sysattr_value(device, sysattr, &cached_value);
-        if (r != -ENOENT) {
-                if (r < 0)
-                        return r;
-
-                if (!cached_value)
-                        /* we looked up the sysattr before and it did not exist */
-                        return -ENOENT;
-
-                if (ret_value)
-                        *ret_value = cached_value;
-
-                return 0;
-        }
+        r = device_get_cached_sysattr_value(device, sysattr, ret_value);
+        if (r != -ESTALE)
+                return r;
 
         r = sd_device_get_syspath(device, &syspath);
         if (r < 0)

src/udev/meson.build

@@ -33,12 +33,12 @@ libudevd_core_sources = '''
         udev-builtin-hwdb.c
         udev-builtin-input_id.c
         udev-builtin-keyboard.c
-        udev-builtin-net_id-netlink.c
-        udev-builtin-net_id-netlink.h
         udev-builtin-net_id.c
         udev-builtin-net_setup_link.c
         udev-builtin-path_id.c
         udev-builtin-usb_id.c
+        udev-netlink.c
+        udev-netlink.h
         net/link-config.c
         net/link-config.h
 '''.split()
@@ -212,9 +212,9 @@ tests += [
          [threads,
           libacl]],
 
-        [['src/udev/test-udev-builtin-net_id-netlink.c',
-          'src/udev/udev-builtin-net_id-netlink.c',
-          'src/udev/udev-builtin-net_id-netlink.h']],
+        [['src/udev/test-udev-netlink.c',
+          'src/udev/udev-netlink.c',
+          'src/udev/udev-netlink.h']],
 
         [['src/udev/fido_id/test-fido-id-desc.c',
           'src/udev/fido_id/fido_id_desc.c']],

src/udev/test-udev-builtin-net_id-netlink.c

@@ -1,85 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-
-#include "sd-device.h"
-
-#include "arphrd-list.h"
-#include "ether-addr-util.h"
-#include "parse-util.h"
-#include "tests.h"
-#include "udev-builtin-net_id-netlink.h"
-
-static void test_link_info_one(sd_netlink *rtnl, int ifindex) {
-        _cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
-        _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
-        unsigned iftype, iflink;
-        const char *s;
-
-        log_debug("/* %s(ifindex=%i) */", __func__, ifindex);
-
-        assert_se(link_info_get(&rtnl, ifindex, &info) >= 0);
-        assert_se(sd_device_new_from_ifindex(&dev, ifindex) >= 0);
-
-        /* check iftype */
-        log_debug("iftype: %"PRIu16" (%s)", info.iftype, strna(arphrd_to_name(info.iftype)));
-        assert_se(sd_device_get_sysattr_value(dev, "type", &s) >= 0);
-        assert_se(safe_atou(s, &iftype) >= 0);
-        assert_se(iftype == info.iftype);
-
-        /* check hardware address */
-        log_debug("hardware address: %s", HW_ADDR_TO_STR(&info.hw_addr));
-        assert_se(sd_device_get_sysattr_value(dev, "address", &s) >= 0);
-        assert_se(streq(s, HW_ADDR_TO_STR(&info.hw_addr)));
-
-        /* check ifname */
-        log_debug("ifname: %s", info.ifname);
-        assert_se(sd_device_get_sysname(dev, &s) >= 0);
-        assert_se(streq(s, info.ifname));
-
-        /* check iflink */
-        log_debug("iflink: %"PRIu32, info.iflink);
-        assert_se(sd_device_get_sysattr_value(dev, "iflink", &s) >= 0);
-        assert_se(safe_atou(s, &iflink) >= 0);
-        assert_se(iflink == info.iflink);
-
-        /* check phys_port_name */
-        log_debug("phys_port_name: %s (%s)",
-                  strna(info.phys_port_name),
-                  info.support_phys_port_name ? "supported" : "unsupported");
-        if (info.support_phys_port_name) {
-                s = NULL;
-                (void) sd_device_get_sysattr_value(dev, "phys_port_name", &s);
-                assert_se(streq_ptr(s, info.phys_port_name));
-        }
-}
-
-static void test_link_info_get(void) {
-        _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;
-        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL;
-
-        log_debug("/* %s */", __func__);
-
-        assert_se(sd_netlink_open(&rtnl) >= 0);
-
-        assert_se(sd_rtnl_message_new_link(rtnl, &req, RTM_GETLINK, 0) >= 0);
-        assert_se(sd_netlink_message_request_dump(req, true) >= 0);
-        assert_se(sd_netlink_call(rtnl, req, 0, &reply) >= 0);
-
-        for (sd_netlink_message *reply_one = reply; reply_one; reply_one = sd_netlink_message_next(reply_one)) {
-                uint16_t nlmsg_type;
-                int ifindex;
-
-                assert_se(sd_netlink_message_get_type(reply_one, &nlmsg_type) >= 0);
-                assert_se(nlmsg_type == RTM_NEWLINK);
-                assert_se(sd_rtnl_message_link_get_ifindex(reply_one, &ifindex) >= 0);
-
-                test_link_info_one(rtnl, ifindex);
-        }
-}
-
-int main(int argc, char *argv[]) {
-        test_setup_logging(LOG_DEBUG);
-
-        test_link_info_get();
-
-        return 0;
-}

src/udev/test-udev-netlink.c

@@ -0,0 +1,165 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "sd-device.h"
+
+#include "arphrd-list.h"
+#include "ether-addr-util.h"
+#include "parse-util.h"
+#include "tests.h"
+#include "udev-netlink.h"
+
+static void test_link_info_one(sd_netlink *rtnl, int ifindex) {
+        _cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
+        _cleanup_(sd_device_unrefp) sd_device *dev = NULL, *dev_with_netlink = NULL;
+        const char *s, *t;
+        unsigned u;
+
+        log_debug("/* %s(ifindex=%i) */", __func__, ifindex);
+
+        assert_se(link_info_get(&rtnl, ifindex, &info) >= 0);
+        assert_se(sd_device_new_from_ifindex(&dev, ifindex) >= 0);
+        assert_se(sd_device_new_from_ifindex(&dev_with_netlink, ifindex) >= 0);
+
+        /* check iftype */
+        log_debug("iftype: %"PRIu16" (%s)", info.iftype, strna(arphrd_to_name(info.iftype)));
+        assert_se(sd_device_get_sysattr_value(dev, "type", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.iftype);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "type", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.iftype);
+
+        /* check hardware address length */
+        log_debug("hardware address length: %zu", info.hw_addr.length);
+        assert_se(sd_device_get_sysattr_value(dev, "addr_len", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.hw_addr.length);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "addr_len", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.hw_addr.length);
+
+        /* check hardware address */
+        log_debug("hardware address: %s", HW_ADDR_TO_STR(&info.hw_addr));
+        assert_se(sd_device_get_sysattr_value(dev, "address", &s) >= 0);
+        assert_se(streq(s, HW_ADDR_TO_STR(&info.hw_addr)));
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "address", &s) >= 0);
+        assert_se(streq(s, HW_ADDR_TO_STR(&info.hw_addr)));
+
+        /* check broadcast address */
+        log_debug("broadcast address: %s", HW_ADDR_TO_STR(&info.broadcast));
+        assert_se(sd_device_get_sysattr_value(dev, "broadcast", &s) >= 0);
+        assert_se(streq(s, HW_ADDR_TO_STR(&info.broadcast)));
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "broadcast", &s) >= 0);
+        assert_se(streq(s, HW_ADDR_TO_STR(&info.broadcast)));
+
+        /* check ifname */
+        log_debug("ifname: %s", info.ifname);
+        assert_se(sd_device_get_sysname(dev, &s) >= 0);
+        assert_se(streq(s, info.ifname));
+        assert_se(sd_device_get_sysname(dev_with_netlink, &s) >= 0);
+        assert_se(streq(s, info.ifname));
+
+        /* check mtu */
+        log_debug("mtu: %"PRIu32, info.mtu);
+        assert_se(sd_device_get_sysattr_value(dev, "mtu", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.mtu);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "mtu", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.mtu);
+
+        /* check iflink */
+        log_debug("iflink: %"PRIu32, info.iflink);
+        assert_se(sd_device_get_sysattr_value(dev, "iflink", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.iflink);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "iflink", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.iflink);
+
+        /* check link_mode */
+        log_debug("link_mode: %"PRIu8, info.link_mode);
+        assert_se(sd_device_get_sysattr_value(dev, "link_mode", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.link_mode);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "link_mode", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.link_mode);
+
+        /* check ifalias */
+        log_debug("ifalias: %s", strna(info.ifalias));
+        assert_se(sd_device_get_sysattr_value(dev, "ifalias", &s) >= 0);
+        assert_se(streq(s, strempty(info.ifalias)));
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "ifalias", &s) >= 0);
+        assert_se(streq(s, strempty(info.ifalias)));
+
+        /* check netdev_group */
+        log_debug("netdev_group: %"PRIu32, info.group);
+        assert_se(sd_device_get_sysattr_value(dev, "netdev_group", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.group);
+        assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "netdev_group", &s) >= 0);
+        assert_se(safe_atou(s, &u) >= 0);
+        assert_se(u == info.group);
+
+        /* check phys_port_id */
+        log_debug("phys_port_id: (%s)",
+                  info.phys_port_id_supported ? "supported" : "unsupported");
+        s = t = NULL;
+        (void) sd_device_get_sysattr_value(dev, "phys_port_id", &s);
+        (void) device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "phys_port_id", &t);
+        assert_se(streq_ptr(s, t));
+
+        /* check phys_switch_id */
+        log_debug("phys_switch_id: (%s)",
+                  info.phys_switch_id_supported ? "supported" : "unsupported");
+        s = t = NULL;
+        (void) sd_device_get_sysattr_value(dev, "phys_switch_id", &s);
+        (void) device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "phys_switch_id", &t);
+        assert_se(streq_ptr(s, t));
+
+        /* check phys_port_name */
+        log_debug("phys_port_name: %s (%s)",
+                  strna(info.phys_port_name),
+                  info.phys_port_name_supported ? "supported" : "unsupported");
+        s = t = NULL;
+        (void) sd_device_get_sysattr_value(dev, "phys_port_name", &s);
+        (void) device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "phys_port_name", &t);
+        assert_se(streq_ptr(s, t));
+        if (info.phys_port_name_supported) {
+                assert_se(streq_ptr(s, info.phys_port_name));
+                assert_se(streq_ptr(t, info.phys_port_name));
+        }
+}
+
+static void test_link_info_get(void) {
+        _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;
+        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL;
+
+        log_debug("/* %s */", __func__);
+
+        assert_se(sd_netlink_open(&rtnl) >= 0);
+
+        assert_se(sd_rtnl_message_new_link(rtnl, &req, RTM_GETLINK, 0) >= 0);
+        assert_se(sd_netlink_message_request_dump(req, true) >= 0);
+        assert_se(sd_netlink_call(rtnl, req, 0, &reply) >= 0);
+
+        for (sd_netlink_message *reply_one = reply; reply_one; reply_one = sd_netlink_message_next(reply_one)) {
+                uint16_t nlmsg_type;
+                int ifindex;
+
+                assert_se(sd_netlink_message_get_type(reply_one, &nlmsg_type) >= 0);
+                assert_se(nlmsg_type == RTM_NEWLINK);
+                assert_se(sd_rtnl_message_link_get_ifindex(reply_one, &ifindex) >= 0);
+
+                test_link_info_one(rtnl, ifindex);
+        }
+}
+
+int main(int argc, char *argv[]) {
+        test_setup_logging(LOG_DEBUG);
+
+        test_link_info_get();
+
+        return 0;
+}

src/udev/udev-builtin-net_id-netlink.c

@@ -1,87 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-
-#include "netlink-util.h"
-#include "udev-builtin-net_id-netlink.h"
-
-void link_info_clear(LinkInfo *info) {
-        if (!info)
-                return;
-
-        info->ifname = mfree(info->ifname);
-        info->phys_port_name = mfree(info->phys_port_name);
-}
-
-int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret) {
-        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL;
-        _cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
-        uint16_t nlmsg_type;
-        int r;
-
-        assert(rtnl);
-        assert(ifindex > 0);
-        assert(ret);
-
-        if (!*rtnl) {
-                r = sd_netlink_open(rtnl);
-                if (r < 0)
-                        return r;
-        }
-
-        r = sd_rtnl_message_new_link(*rtnl, &message, RTM_GETLINK, ifindex);
-        if (r < 0)
-                return r;
-
-        r = sd_netlink_call(*rtnl, message, 0, &reply);
-        if (r == -EINVAL)
-                return -ENODEV; /* The device does not exist */
-        if (r < 0)
-                return r;
-
-        r = sd_netlink_message_get_type(reply, &nlmsg_type);
-        if (r < 0)
-                return r;
-        if (nlmsg_type != RTM_NEWLINK)
-                return -ENXIO;
-
-        r = sd_rtnl_message_link_get_ifindex(reply, &info.ifindex);
-        if (r < 0)
-                return r;
-        if (info.ifindex != ifindex)
-                return -ENXIO;
-
-        r = sd_rtnl_message_link_get_type(reply, &info.iftype);
-        if (r < 0)
-                return r;
-
-        r = netlink_message_read_hw_addr(reply, IFLA_ADDRESS, &info.hw_addr);
-        if (r < 0 && r != -ENODATA)
-                return r;
-
-        r = sd_netlink_message_read_string_strdup(reply, IFLA_IFNAME, &info.ifname);
-        if (r < 0)
-                return r;
-
-        r = sd_netlink_message_read_u32(reply, IFLA_LINK, &info.iflink);
-        if (r == -ENODATA)
-                info.iflink = info.ifindex;
-        else if (r < 0)
-                return r;
-
-        r = sd_netlink_message_read_string_strdup(reply, IFLA_PHYS_PORT_NAME, &info.phys_port_name);
-        if (r == -ENODATA) {
-                uint16_t max_attr;
-
-                r = sd_netlink_message_get_max_attribute(reply, &max_attr);
-                if (r < 0)
-                        return r;
-
-                info.support_phys_port_name = max_attr >= IFLA_PHYS_PORT_NAME;
-        } else if (r >= 0)
-                info.support_phys_port_name = true;
-        else
-                return r;
-
-        *ret = info;
-        info = LINK_INFO_NULL;
-        return 0;
-}

src/udev/udev-builtin-net_id-netlink.h

@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: LGPL-2.1-or-later */
-#pragma once
-
-#include "sd-netlink.h"
-
-#include "ether-addr-util.h"
-
-typedef struct LinkInfo {
-        int ifindex;
-        uint16_t iftype;             /* ARPHRD_* */
-
-        struct hw_addr_data hw_addr; /* IFLA_ADDRESS */
-        char *ifname;                /* IFLA_IFNAME */
-        uint32_t iflink;             /* IFLA_LINK */
-        char *phys_port_name;        /* IFLA_PHYS_PORT_NAME */
-
-        bool support_phys_port_name;
-} LinkInfo;
-
-#define LINK_INFO_NULL ((LinkInfo) {})
-
-void link_info_clear(LinkInfo *info);
-int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret);

src/udev/udev-builtin-net_id.c

@@ -35,8 +35,8 @@
 #include "string-util.h"
 #include "strv.h"
 #include "strxcpyx.h"
-#include "udev-builtin-net_id-netlink.h"
 #include "udev-builtin.h"
+#include "udev-netlink.h"
 
 #define ONBOARD_14BIT_INDEX_MAX ((1U << 14) - 1)
 #define ONBOARD_16BIT_INDEX_MAX ((1U << 16) - 1)
@@ -342,22 +342,16 @@ static int dev_pci_slot(sd_device *dev, const LinkInfo *info, NetNames *names) {
                 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 &&
-                    sd_device_get_sysattr_value(dev, "type", &attr) >= 0) {
-                        unsigned long type;
-
-                        r = safe_atolu_full(attr, 10, &type);
+                    info->iftype == ARPHRD_INFINIBAND &&
+                    sd_device_get_sysattr_value(dev, "dev_id", &attr) >= 0) {
+                        r = safe_atolu_full(attr, 10, &dev_port);
                         if (r < 0)
-                                log_device_debug_errno(dev, r, "Failed to parse attribute type, ignoring: %m");
-                        else if (type == ARPHRD_INFINIBAND &&
-                                 sd_device_get_sysattr_value(dev, "dev_id", &attr) >= 0) {
-                                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");
-                        }
+                                log_device_debug_errno(dev, r, "Failed to parse attribute dev_id, ignoring: %m");
                 }
         }
 
@@ -859,7 +853,7 @@ static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *arg
         if (r < 0)
                 return r;
 
-        if (!info.support_phys_port_name) {
+        if (!info.phys_port_name_supported) {
                 const char *s;
 
                 r = sd_device_get_sysattr_value(dev, "phys_port_name", &s);
@@ -870,6 +864,10 @@ static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *arg
                 }
         }
 
+        r = device_cache_sysattr_from_link_info(dev, &info);
+        if (r < 0)
+                return r;
+
         /* skip stacked devices, like VLANs, ... */
         if (info.ifindex != (int) info.iflink)
                 return 0;

src/udev/udev-event.c

@@ -30,6 +30,7 @@
 #include "strxcpyx.h"
 #include "udev-builtin.h"
 #include "udev-event.h"
+#include "udev-netlink.h"
 #include "udev-node.h"
 #include "udev-util.h"
 #include "udev-watch.h"
@@ -351,11 +352,11 @@ static ssize_t udev_event_subst_format(
 
                 /* try to read the attribute the device */
                 if (!val)
-                        (void) sd_device_get_sysattr_value(dev, attr, &val);
+                        (void) device_get_sysattr_value_maybe_from_netlink(dev, &event->rtnl, attr, &val);
 
                 /* try to read the attribute of the parent device, other matches have selected */
                 if (!val && event->dev_parent && event->dev_parent != dev)
-                        (void) sd_device_get_sysattr_value(event->dev_parent, attr, &val);
+                        (void) device_get_sysattr_value_maybe_from_netlink(event->dev_parent, &event->rtnl, attr, &val);
 
                 if (!val)
                         goto null_terminate;

src/udev/udev-netlink.c

@@ -0,0 +1,330 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "device-private.h"
+#include "hexdecoct.h"
+#include "netlink-util.h"
+#include "strv.h"
+#include "udev-netlink.h"
+
+void link_info_clear(LinkInfo *info) {
+        if (!info)
+                return;
+
+        info->ifname = mfree(info->ifname);
+        info->ifalias = mfree(info->ifalias);
+        info->phys_port_id = mfree(info->phys_port_id);
+        info->phys_switch_id = mfree(info->phys_switch_id);
+        info->phys_port_name = mfree(info->phys_port_name);
+}
+
+int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret) {
+        _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL, *reply = NULL;
+        _cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
+        uint16_t nlmsg_type, max_attr;
+        int r;
+
+        assert(rtnl);
+        assert(ifindex > 0);
+        assert(ret);
+
+        if (!*rtnl) {
+                r = sd_netlink_open(rtnl);
+                if (r < 0)
+                        return r;
+        }
+
+        r = sd_rtnl_message_new_link(*rtnl, &message, RTM_GETLINK, ifindex);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_call(*rtnl, message, 0, &reply);
+        if (r == -EINVAL)
+                return -ENODEV; /* The device does not exist */
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_message_get_type(reply, &nlmsg_type);
+        if (r < 0)
+                return r;
+        if (nlmsg_type != RTM_NEWLINK)
+                return -ENXIO;
+
+        r = sd_rtnl_message_link_get_ifindex(reply, &info.ifindex);
+        if (r < 0)
+                return r;
+        if (info.ifindex != ifindex)
+                return -ENXIO;
+
+        r = sd_rtnl_message_link_get_type(reply, &info.iftype);
+        if (r < 0)
+                return r;
+
+        r = netlink_message_read_hw_addr(reply, IFLA_ADDRESS, &info.hw_addr);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = netlink_message_read_hw_addr(reply, IFLA_BROADCAST, &info.broadcast);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = sd_netlink_message_read_string_strdup(reply, IFLA_IFNAME, &info.ifname);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_message_read_u32(reply, IFLA_MTU, &info.mtu);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_message_read_u32(reply, IFLA_LINK, &info.iflink);
+        if (r == -ENODATA)
+                info.iflink = info.ifindex;
+        else if (r < 0)
+                return r;
+
+        r = sd_netlink_message_read_u8(reply, IFLA_LINKMODE, &info.link_mode);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_message_read_string_strdup(reply, IFLA_IFALIAS, &info.ifalias);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = sd_netlink_message_read_u32(reply, IFLA_GROUP, &info.group);
+        if (r < 0)
+                return r;
+
+        r = sd_netlink_message_read_data(reply, IFLA_PHYS_PORT_ID,
+                                         &info.phys_port_id_len, (void**) &info.phys_port_id);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = sd_netlink_message_read_data(reply, IFLA_PHYS_SWITCH_ID,
+                                         &info.phys_switch_id_len, (void**) &info.phys_switch_id);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = sd_netlink_message_read_string_strdup(reply, IFLA_PHYS_PORT_NAME, &info.phys_port_name);
+        if (r < 0 && r != -ENODATA)
+                return r;
+
+        r = sd_netlink_message_get_max_attribute(reply, &max_attr);
+        if (r < 0)
+                return r;
+
+        info.phys_port_id_supported = max_attr >= IFLA_PHYS_PORT_ID;
+        info.phys_switch_id_supported = max_attr >= IFLA_PHYS_SWITCH_ID;
+        info.phys_port_name_supported = max_attr >= IFLA_PHYS_PORT_NAME;
+
+        *ret = info;
+        info = LINK_INFO_NULL;
+        return 0;
+}
+
+static int cache_unsigned(sd_device *device, const char *attr, uint64_t val) {
+        _cleanup_free_ char *str = NULL;
+        int r;
+
+        assert(device);
+        assert(attr);
+
+        if (device_get_cached_sysattr_value(device, attr, NULL) != -ESTALE)
+                return 0;
+
+        if (asprintf(&str, "%"PRIu64, val) < 0)
+                return -ENOMEM;
+
+        r = device_cache_sysattr_value(device, attr, str);
+        if (r < 0)
+                return r;
+
+        TAKE_PTR(str);
+        return 0;
+}
+
+static int cache_hw_addr(sd_device *device, const char *attr, const struct hw_addr_data *hw_addr) {
+        _cleanup_free_ char *str = NULL;
+        int r;
+
+        assert(device);
+        assert(attr);
+        assert(hw_addr);
+
+        if (device_get_cached_sysattr_value(device, attr, NULL) != -ESTALE)
+                return 0;
+
+        str = new(char, HW_ADDR_TO_STRING_MAX);
+        if (!str)
+                return -ENOMEM;
+
+        r = device_cache_sysattr_value(device, attr, hw_addr_to_string(hw_addr, str));
+        if (r < 0)
+                return r;
+
+        TAKE_PTR(str);
+        return 0;
+}
+
+static int cache_binary(sd_device *device, const char *attr, size_t len, const uint8_t *data) {
+        _cleanup_free_ char *str = NULL;
+        int r;
+
+        assert(device);
+        assert(attr);
+
+        if (device_get_cached_sysattr_value(device, attr, NULL) != -ESTALE)
+                return 0;
+
+        if (data) {
+                size_t j = 0;
+
+                str = new(char, len * 2 + 1);
+                if (!str)
+                        return -ENOMEM;
+
+                for (size_t i = 0; i < len; i++) {
+                        str[j++] = hexchar(data[i] >> 4);
+                        str[j++] = hexchar(data[i] & 0x0f);
+                }
+
+                str[j] = '\0';
+        }
+
+        r = device_cache_sysattr_value(device, attr, str);
+        if (r < 0)
+                return r;
+
+        TAKE_PTR(str);
+        return 0;
+}
+
+static int cache_string(sd_device *device, const char *attr, const char *val) {
+        _cleanup_free_ char *str = NULL;
+        int r;
+
+        assert(device);
+        assert(attr);
+
+        if (device_get_cached_sysattr_value(device, attr, NULL) != -ESTALE)
+                return 0;
+
+        if (val) {
+                str = strdup(val);
+                if (!str)
+                        return -ENOMEM;
+        }
+
+        r = device_cache_sysattr_value(device, attr, str);
+        if (r < 0)
+                return r;
+
+        TAKE_PTR(str);
+        return 0;
+}
+
+int device_cache_sysattr_from_link_info(sd_device *device, LinkInfo *info) {
+        int ifindex, r;
+
+        assert(device);
+        assert(info);
+
+        r = sd_device_get_ifindex(device, &ifindex);
+        if (r < 0)
+                return r;
+
+        if (ifindex != info->ifindex)
+                return -EINVAL;
+
+        r = cache_unsigned(device, "type", info->iftype);
+        if (r < 0)
+                return r;
+
+        r = cache_unsigned(device, "addr_len", info->hw_addr.length);
+        if (r < 0)
+                return r;
+
+        r = cache_hw_addr(device, "address", &info->hw_addr);
+        if (r < 0)
+                return r;
+
+        r = cache_hw_addr(device, "broadcast", &info->broadcast);
+        if (r < 0)
+                return r;
+
+        r = cache_unsigned(device, "mtu", info->mtu);
+        if (r < 0)
+                return r;
+
+        r = cache_unsigned(device, "iflink", info->iflink);
+        if (r < 0)
+                return r;
+
+        r = cache_unsigned(device, "link_mode", info->link_mode);
+        if (r < 0)
+                return r;
+
+        r = cache_string(device, "ifalias", strempty(info->ifalias));
+        if (r < 0)
+                return r;
+
+        r = cache_unsigned(device, "netdev_group", info->group);
+        if (r < 0)
+                return r;
+
+        if (info->phys_port_id_supported) {
+                r = cache_binary(device, "phys_port_id", info->phys_port_id_len, info->phys_port_id);
+                if (r < 0)
+                        return r;
+        }
+
+        if (info->phys_switch_id_supported) {
+                r = cache_binary(device, "phys_switch_id", info->phys_switch_id_len, info->phys_switch_id);
+                if (r < 0)
+                        return r;
+        }
+
+        if (info->phys_port_name_supported) {
+                r = cache_string(device, "phys_port_name", info->phys_port_name);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
+
+int device_get_sysattr_value_maybe_from_netlink(
+                sd_device *device,
+                sd_netlink **rtnl,
+                const char *sysattr,
+                const char **ret_value) {
+
+        _cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
+        int ifindex, r;
+
+        assert(device);
+        assert(rtnl);
+        assert(sysattr);
+
+        if (sd_device_get_ifindex(device, &ifindex) < 0)
+                return sd_device_get_sysattr_value(device, sysattr, ret_value);
+
+        if (!STR_IN_SET(sysattr,
+                        "type", "addr_len", "address", "broadcast", "mtu", "iflink", "linkmode",
+                        "ifalias", "group", "phys_port_id", "phys_switch_id", "phys_port_name"))
+                return sd_device_get_sysattr_value(device, sysattr, ret_value);
+
+        r = device_get_cached_sysattr_value(device, sysattr, ret_value);
+        if (r != -ESTALE)
+                return r;
+
+        r = link_info_get(rtnl, ifindex, &info);
+        if (r < 0)
+                return r;
+
+        r = device_cache_sysattr_from_link_info(device, &info);
+        if (r < 0)
+                return r;
+
+        /* Do not use device_get_cached_sysattr_value() here, as kernel may not support
+         * IFLA_PHYS_PORT_NAME, and in that case we need to read the value from sysfs. */
+        return sd_device_get_sysattr_value(device, sysattr, ret_value);
+}

src/udev/udev-netlink.h

@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include "sd-device.h"
+#include "sd-netlink.h"
+
+#include "ether-addr-util.h"
+
+typedef struct LinkInfo {
+        int ifindex;
+        uint16_t iftype;             /* ARPHRD_* (type) */
+
+        struct hw_addr_data hw_addr;   /* IFLA_ADDRESS (address, addr_len) */
+        struct hw_addr_data broadcast; /* IFLA_BROADCAST (broadcast) */
+        char *ifname;                  /* IFLA_IFNAME */
+        uint32_t mtu;                  /* IFLA_MTU (mtu) */
+        uint32_t iflink;               /* IFLA_LINK (iflink) */
+        uint8_t link_mode;             /* IFLA_LINKMODE (link_mode) */
+        char *ifalias;                 /* IFLA_IFALIAS (ifalias) */
+        uint32_t group;                /* IFLA_GROUP (netdev_group) */
+        uint8_t *phys_port_id;         /* IFLA_PHYS_PORT_ID (phys_port_id) */
+        size_t phys_port_id_len;
+        uint8_t *phys_switch_id;       /* IFLA_PHYS_SWITCH_ID (phys_switch_id) */
+        size_t phys_switch_id_len;
+        char *phys_port_name;          /* IFLA_PHYS_PORT_NAME (phys_port_name) */
+
+        bool phys_port_id_supported;
+        bool phys_switch_id_supported;
+        bool phys_port_name_supported;
+} LinkInfo;
+
+#define LINK_INFO_NULL ((LinkInfo) {})
+
+void link_info_clear(LinkInfo *info);
+int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret);
+int device_cache_sysattr_from_link_info(sd_device *device, LinkInfo *info);
+int device_get_sysattr_value_maybe_from_netlink(
+                sd_device *device,
+                sd_netlink **rtnl,
+                const char *sysattr,
+                const char **ret_value);

src/udev/udev-rules.c

@@ -29,6 +29,7 @@
 #include "syslog-util.h"
 #include "udev-builtin.h"
 #include "udev-event.h"
+#include "udev-netlink.h"
 #include "udev-rules.h"
 #include "udev-util.h"
 #include "user-util.h"
@@ -1396,7 +1397,7 @@ static bool token_match_attr(UdevRuleToken *token, sd_device *dev, UdevEvent *ev
                 name = nbuf;
                 _fallthrough_;
         case SUBST_TYPE_PLAIN:
-                if (sd_device_get_sysattr_value(dev, name, &value) < 0)
+                if (device_get_sysattr_value_maybe_from_netlink(dev, &event->rtnl, name, &value) < 0)
                         return false;
                 break;
         case SUBST_TYPE_SUBSYS: