From b63dd5aad82cb7abdbfc8ec7425743abe18ad842 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 15:21:44 +0900 Subject: [PATCH 01/10] sd-device: make device_get_cached_sysattr_value() distinguish if we have looked up the attribute --- src/libsystemd/sd-device/sd-device.c | 38 ++++++++++------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 1c0dce7b07..5dbf2d627e 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -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; +static 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) From 18b113797757ef401be75fda1ea50b2db1ec65a7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 12:14:46 +0900 Subject: [PATCH 02/10] sd-device: expose device_cache_sysattr_value() and device_get_cached_sysattr_value() --- src/libsystemd/sd-device/device-private.h | 4 +++- src/libsystemd/sd-device/sd-device.c | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 9bb5eff208..04b932309c 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/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); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 5dbf2d627e..141e10a18a 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/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,7 +1969,7 @@ 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 **ret_value) { +int device_get_cached_sysattr_value(sd_device *device, const char *key, const char **ret_value) { const char *k = NULL, *value; assert(device); From 914ac555cd40f9c09e655a737214bfb7de21b8d9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 22:59:52 +0900 Subject: [PATCH 03/10] ether-addr-util: make hw_addr_to_string() return valid string even if hardware address is null Previously, when the length of the hardware address is zero, then the buffer was not nul-terminated. This also replaces sprintf() with hexchar(). --- src/basic/ether-addr-util.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/basic/ether-addr-util.c b/src/basic/ether-addr-util.c index e660ac2c6f..dc5b5b833d 100644 --- a/src/basic/ether-addr-util.c +++ b/src/basic/ether-addr-util.c @@ -7,6 +7,7 @@ #include #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; } From 6ebc6a45602966e32c48643fba17a03e0f8dc569 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 12:20:04 +0900 Subject: [PATCH 04/10] udev: rename udev-builtin-net_id-netlink.[ch] --- src/udev/meson.build | 10 +++++----- ...ev-builtin-net_id-netlink.c => test-udev-netlink.c} | 2 +- src/udev/udev-builtin-net_id.c | 2 +- .../{udev-builtin-net_id-netlink.c => udev-netlink.c} | 2 +- .../{udev-builtin-net_id-netlink.h => udev-netlink.h} | 0 5 files changed, 8 insertions(+), 8 deletions(-) rename src/udev/{test-udev-builtin-net_id-netlink.c => test-udev-netlink.c} (98%) rename src/udev/{udev-builtin-net_id-netlink.c => udev-netlink.c} (98%) rename src/udev/{udev-builtin-net_id-netlink.h => udev-netlink.h} (100%) diff --git a/src/udev/meson.build b/src/udev/meson.build index 7de080348f..3423d6de94 100644 --- a/src/udev/meson.build +++ b/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']], diff --git a/src/udev/test-udev-builtin-net_id-netlink.c b/src/udev/test-udev-netlink.c similarity index 98% rename from src/udev/test-udev-builtin-net_id-netlink.c rename to src/udev/test-udev-netlink.c index ee9fd675f1..5f09a489d6 100644 --- a/src/udev/test-udev-builtin-net_id-netlink.c +++ b/src/udev/test-udev-netlink.c @@ -6,7 +6,7 @@ #include "ether-addr-util.h" #include "parse-util.h" #include "tests.h" -#include "udev-builtin-net_id-netlink.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; diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index db155421b2..ec30df51b9 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/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) diff --git a/src/udev/udev-builtin-net_id-netlink.c b/src/udev/udev-netlink.c similarity index 98% rename from src/udev/udev-builtin-net_id-netlink.c rename to src/udev/udev-netlink.c index 7fcd2ecd17..4c93934e3e 100644 --- a/src/udev/udev-builtin-net_id-netlink.c +++ b/src/udev/udev-netlink.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "netlink-util.h" -#include "udev-builtin-net_id-netlink.h" +#include "udev-netlink.h" void link_info_clear(LinkInfo *info) { if (!info) diff --git a/src/udev/udev-builtin-net_id-netlink.h b/src/udev/udev-netlink.h similarity index 100% rename from src/udev/udev-builtin-net_id-netlink.h rename to src/udev/udev-netlink.h From d9ea90598fe99efbf97fd2b9f7d8ae7539cde1a3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 13:00:21 +0900 Subject: [PATCH 05/10] udev: introduce device_get_sysattr_value_maybe_from_netlink() --- src/udev/udev-netlink.c | 140 ++++++++++++++++++++++++++++++++++++++++ src/udev/udev-netlink.h | 7 ++ 2 files changed, 147 insertions(+) diff --git a/src/udev/udev-netlink.c b/src/udev/udev-netlink.c index 4c93934e3e..d9afdabb15 100644 --- a/src/udev/udev-netlink.c +++ b/src/udev/udev-netlink.c @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "device-private.h" #include "netlink-util.h" +#include "strv.h" #include "udev-netlink.h" void link_info_clear(LinkInfo *info) { @@ -85,3 +87,141 @@ int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret) { 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_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_hw_addr(device, "address", &info->hw_addr); + if (r < 0) + return r; + + r = cache_unsigned(device, "iflink", info->iflink); + if (r < 0) + return r; + + if (info->support_phys_port_name) { + 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", "address", "iflink", "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); +} diff --git a/src/udev/udev-netlink.h b/src/udev/udev-netlink.h index 286ac198b9..a6ea0d5fe2 100644 --- a/src/udev/udev-netlink.h +++ b/src/udev/udev-netlink.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-device.h" #include "sd-netlink.h" #include "ether-addr-util.h" @@ -21,3 +22,9 @@ typedef struct 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); From 552b1699e2b0a47853a89ce96fc71c7ccb374096 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 13:20:31 +0900 Subject: [PATCH 06/10] test: add tests for device_get_sysattr_value_maybe_from_netlink() --- src/udev/test-udev-netlink.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/udev/test-udev-netlink.c b/src/udev/test-udev-netlink.c index 5f09a489d6..bda713ecdc 100644 --- a/src/udev/test-udev-netlink.c +++ b/src/udev/test-udev-netlink.c @@ -10,45 +10,59 @@ 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; + _cleanup_(sd_device_unrefp) sd_device *dev = NULL, *dev_with_netlink = NULL; unsigned iftype, iflink; - const char *s; + const char *s, *t; 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, &iftype) >= 0); assert_se(iftype == info.iftype); + assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "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))); + 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 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 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); + assert_se(device_get_sysattr_value_maybe_from_netlink(dev_with_netlink, &rtnl, "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"); + 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.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)); + assert_se(streq_ptr(t, info.phys_port_name)); } } From 78ae4d449eb44cf5ad72803087a7ad4aa0e338a2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 13:27:59 +0900 Subject: [PATCH 07/10] udev: cache obtained sysattr from netlink into sd_device object --- src/udev/udev-builtin-net_id.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index ec30df51b9..bc174ad965 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -870,6 +870,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; From e44b0fbce78fdf44f0c632a8cc308be77062ef18 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 13:36:34 +0900 Subject: [PATCH 08/10] udev: use LinkInfo::iftype at one more place --- src/udev/udev-builtin-net_id.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index bc174ad965..a3cbbe50d5 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -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"); } } From 1c2ad7a66c2e8945cc26c9376b264a241da354fb Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 13:30:16 +0900 Subject: [PATCH 09/10] udev: replace sd_device_get_sysattr_value() with device_get_sysattr_value_maybe_from_netlink() --- src/udev/udev-event.c | 5 +++-- src/udev/udev-rules.c | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 11531236ce..145204b226 100644 --- a/src/udev/udev-event.c +++ b/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; diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 3759ac8400..693c743c57 100644 --- a/src/udev/udev-rules.c +++ b/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: From 1321f675e778c6f36d75054c9fd1b4c921bab222 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 15 Sep 2021 19:57:38 +0900 Subject: [PATCH 10/10] udev: read more attributes through netlink and cache them --- src/udev/test-udev-netlink.c | 88 +++++++++++++++++++--- src/udev/udev-builtin-net_id.c | 2 +- src/udev/udev-netlink.c | 133 +++++++++++++++++++++++++++++---- src/udev/udev-netlink.h | 23 ++++-- 4 files changed, 213 insertions(+), 33 deletions(-) diff --git a/src/udev/test-udev-netlink.c b/src/udev/test-udev-netlink.c index bda713ecdc..d177b09567 100644 --- a/src/udev/test-udev-netlink.c +++ b/src/udev/test-udev-netlink.c @@ -11,8 +11,8 @@ 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; - unsigned iftype, iflink; const char *s, *t; + unsigned u; log_debug("/* %s(ifindex=%i) */", __func__, ifindex); @@ -23,11 +23,20 @@ static void test_link_info_one(sd_netlink *rtnl, int ifindex) { /* 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); + 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, &iftype) >= 0); - assert_se(iftype == info.iftype); + 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)); @@ -36,6 +45,13 @@ static void test_link_info_one(sd_netlink *rtnl, int ifindex) { 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); @@ -43,24 +59,74 @@ static void test_link_info_one(sd_netlink *rtnl, int ifindex) { 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, &iflink) >= 0); - assert_se(iflink == info.iflink); + 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, &iflink) >= 0); - assert_se(iflink == info.iflink); + 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.support_phys_port_name ? "supported" : "unsupported"); + 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.support_phys_port_name) { + if (info.phys_port_name_supported) { assert_se(streq_ptr(s, info.phys_port_name)); assert_se(streq_ptr(t, info.phys_port_name)); } diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index a3cbbe50d5..7c404d783c 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -853,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); diff --git a/src/udev/udev-netlink.c b/src/udev/udev-netlink.c index d9afdabb15..3c41f15963 100644 --- a/src/udev/udev-netlink.c +++ b/src/udev/udev-netlink.c @@ -1,6 +1,7 @@ /* 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" @@ -10,13 +11,16 @@ void link_info_clear(LinkInfo *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; + uint16_t nlmsg_type, max_attr; int r; assert(rtnl); @@ -59,30 +63,58 @@ int link_info_get(sd_netlink **rtnl, int ifindex, LinkInfo *ret) { 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_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 + 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; @@ -132,6 +164,39 @@ static int cache_hw_addr(sd_device *device, const char *attr, const struct hw_ad 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; @@ -173,15 +238,51 @@ int device_cache_sysattr_from_link_info(sd_device *device, LinkInfo *info) { 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; - if (info->support_phys_port_name) { + 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; @@ -206,7 +307,9 @@ int device_get_sysattr_value_maybe_from_netlink( if (sd_device_get_ifindex(device, &ifindex) < 0) return sd_device_get_sysattr_value(device, sysattr, ret_value); - if (!STR_IN_SET(sysattr, "type", "address", "iflink", "phys_port_name")) + 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); diff --git a/src/udev/udev-netlink.h b/src/udev/udev-netlink.h index a6ea0d5fe2..a82352dd61 100644 --- a/src/udev/udev-netlink.h +++ b/src/udev/udev-netlink.h @@ -8,14 +8,25 @@ typedef struct LinkInfo { int ifindex; - uint16_t iftype; /* ARPHRD_* */ + uint16_t iftype; /* ARPHRD_* (type) */ - 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 */ + 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 support_phys_port_name; + bool phys_port_id_supported; + bool phys_switch_id_supported; + bool phys_port_name_supported; } LinkInfo; #define LINK_INFO_NULL ((LinkInfo) {})