Merge pull request #24812 from yuwata/udev-drop-netlink

udev: drop workaround for slow read of phys_port_name sysattr
This commit is contained in:
Yu Watanabe
2022-09-27 14:10:02 +09:00
committed by GitHub
8 changed files with 74 additions and 233 deletions

View File

@@ -18,6 +18,7 @@ int device_new_from_strv(sd_device **ret, char **strv);
int device_opendir(sd_device *device, const char *subdir, DIR **ret);
int device_get_property_bool(sd_device *device, const char *key);
int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value);
int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value);
int device_get_sysattr_bool(sd_device *device, const char *sysattr);
int device_get_device_id(sd_device *device, const char **ret);

View File

@@ -2339,6 +2339,25 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr,
return 0;
}
int device_get_sysattr_int(sd_device *device, const char *sysattr, int *ret_value) {
const char *value;
int r;
r = sd_device_get_sysattr_value(device, sysattr, &value);
if (r < 0)
return r;
int v;
r = safe_atoi(value, &v);
if (r < 0)
return log_device_debug_errno(device, r, "Failed to parse '%s' attribute: %m", sysattr);
if (ret_value)
*ret_value = v;
/* We return "true" if the value is positive. */
return v > 0;
}
int device_get_sysattr_unsigned(sd_device *device, const char *sysattr, unsigned *ret_value) {
const char *value;
int r;

View File

@@ -39,8 +39,6 @@ libudevd_core_sources = files(
'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',
)
@@ -214,10 +212,6 @@ tests += [
[threads,
libacl]],
[files('test-udev-netlink.c',
'udev-netlink.c',
'udev-netlink.h')],
[files('fido_id/test-fido-id-desc.c',
'fido_id/fido_id_desc.c')],
]

View File

@@ -1,157 +0,0 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */
#include "sd-device.h"
#include "arphrd-util.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));
}
}
TEST(link_info_get) {
_cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL;
_cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL;
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_set_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);
}
}
DEFINE_TEST_MAIN(LOG_DEBUG);

View File

@@ -27,6 +27,7 @@
#include "device-private.h"
#include "device-util.h"
#include "dirent-util.h"
#include "ether-addr-util.h"
#include "fd-util.h"
#include "fileio.h"
#include "glyph-util.h"
@@ -38,7 +39,6 @@
#include "strv.h"
#include "strxcpyx.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)
@@ -76,6 +76,15 @@ typedef struct NetNames {
char devicetree_onboard[ALTIFNAMSIZ];
} NetNames;
typedef struct LinkInfo {
int ifindex;
int iflink;
int iftype;
const char *devtype;
const char *phys_port_name;
struct hw_addr_data hw_addr;
} LinkInfo;
/* skip intermediate virtio devices */
static sd_device *skip_virtio(sd_device *dev) {
/* there can only ever be one virtio bus per parent device, so we can
@@ -1050,37 +1059,57 @@ static int ieee_oui(sd_device *dev, const LinkInfo *info, bool test) {
return udev_builtin_hwdb_lookup(dev, NULL, str, NULL, test);
}
static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) {
_cleanup_(link_info_clear) LinkInfo info = LINK_INFO_NULL;
const char *devtype, *prefix = "en";
NetNames names = {};
int ifindex, r;
static int get_link_info(sd_device *dev, LinkInfo *info) {
const char *s;
int r;
r = sd_device_get_ifindex(dev, &ifindex);
assert(dev);
assert(info);
r = sd_device_get_ifindex(dev, &info->ifindex);
if (r < 0)
return r;
r = link_info_get(rtnl, ifindex, &info);
r = device_get_sysattr_int(dev, "iflink", &info->iflink);
if (r < 0)
return r;
if (!info.phys_port_name_supported) {
const char *s;
r = device_get_sysattr_int(dev, "type", &info->iftype);
if (r < 0)
return r;
r = sd_device_get_sysattr_value(dev, "phys_port_name", &s);
if (r >= 0) {
info.phys_port_name = strdup(s);
if (!info.phys_port_name)
return log_oom();
}
r = sd_device_get_devtype(dev, &info->devtype);
if (r < 0 && r != -ENOENT)
return r;
r = sd_device_get_sysattr_value(dev, "phys_port_name", &info->phys_port_name);
if (r < 0 && r != -ENOENT)
return r;
r = sd_device_get_sysattr_value(dev, "address", &s);
if (r < 0 && r != -ENOENT)
return r;
if (r > 0) {
r = parse_hw_addr(s, &info->hw_addr);
if (r < 0)
log_device_debug_errno(dev, r, "Failed to parse 'address' sysattr, ignoring: %m");
}
r = device_cache_sysattr_from_link_info(dev, &info);
return 0;
}
static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) {
const char *prefix;
NetNames names = {};
LinkInfo info = {};
int r;
r = get_link_info(dev, &info);
if (r < 0)
return r;
/* skip stacked devices, like VLANs, ... */
if (info.ifindex != (int) info.iflink)
if (info.ifindex != info.iflink)
return 0;
/* handle only ARPHRD_ETHER, ARPHRD_SLIP and ARPHRD_INFINIBAND devices */
@@ -1101,12 +1130,10 @@ static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *arg
return 0;
}
if (sd_device_get_devtype(dev, &devtype) >= 0) {
if (streq("wlan", devtype))
prefix = "wl";
else if (streq("wwan", devtype))
prefix = "ww";
}
if (streq_ptr("wlan", info.devtype))
prefix = "wl";
else if (streq_ptr("wwan", info.devtype))
prefix = "ww";
udev_builtin_add_property(dev, test, "ID_NET_NAMING_SCHEME", naming_scheme()->name);

View File

@@ -30,7 +30,6 @@
#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"
@@ -357,11 +356,11 @@ static ssize_t udev_event_subst_format(
/* try to read the attribute the device */
if (!val)
(void) device_get_sysattr_value_maybe_from_netlink(dev, &event->rtnl, attr, &val);
(void) sd_device_get_sysattr_value(dev, 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) device_get_sysattr_value_maybe_from_netlink(event->dev_parent, &event->rtnl, attr, &val);
(void) sd_device_get_sysattr_value(event->dev_parent, attr, &val);
if (!val)
goto null_terminate;

View File

@@ -1,41 +0,0 @@
/* 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);

View File

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