From b3cfe5900108df81fbf547b297d51ac8c7359a9b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 16:34:31 +0900 Subject: [PATCH 1/7] udev: use SYNTHETIC_ERRNO() at one more place --- src/udev/udevadm-test-builtin.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/udev/udevadm-test-builtin.c b/src/udev/udevadm-test-builtin.c index 81b633611e..5570eec789 100644 --- a/src/udev/udevadm-test-builtin.c +++ b/src/udev/udevadm-test-builtin.c @@ -87,8 +87,7 @@ int builtin_main(int argc, char *argv[], void *userdata) { cmd = udev_builtin_lookup(arg_command); if (cmd < 0) { - log_error("Unknown command '%s'", arg_command); - r = -EINVAL; + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown command '%s'", arg_command); goto finish; } From 5668f3a7cfccca704ea8e8bdc84ca7e17a5f101e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 11:29:49 +0900 Subject: [PATCH 2/7] udev: make udev_builtin_run() take UdevEvent* No functional change, preparation for later commits. --- src/udev/udev-builtin-blkid.c | 3 ++- src/udev/udev-builtin-btrfs.c | 3 ++- src/udev/udev-builtin-hwdb.c | 3 ++- src/udev/udev-builtin-input_id.c | 6 ++---- src/udev/udev-builtin-keyboard.c | 3 ++- src/udev/udev-builtin-kmod.c | 5 ++--- src/udev/udev-builtin-net_id.c | 3 ++- src/udev/udev-builtin-net_setup_link.c | 7 ++++--- src/udev/udev-builtin-path_id.c | 5 ++--- src/udev/udev-builtin-uaccess.c | 3 ++- src/udev/udev-builtin-usb_id.c | 5 ++--- src/udev/udev-builtin.c | 7 ++++--- src/udev/udev-builtin.h | 5 +++-- src/udev/udev-event.c | 2 +- src/udev/udev-rules.c | 2 +- src/udev/udevadm-test-builtin.c | 10 ++++++++-- 16 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/udev/udev-builtin-blkid.c b/src/udev/udev-builtin-blkid.c index 2b2205513b..11419a3e61 100644 --- a/src/udev/udev-builtin-blkid.c +++ b/src/udev/udev-builtin-blkid.c @@ -315,7 +315,8 @@ notloop: return 0; } -static int builtin_blkid(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_blkid(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); const char *devnode, *root_partition = NULL, *data, *name; _cleanup_(blkid_free_probep) blkid_probe pr = NULL; _cleanup_free_ char *backing_fname = NULL; diff --git a/src/udev/udev-builtin-btrfs.c b/src/udev/udev-builtin-btrfs.c index 79f91ea2ae..9b12aebb3a 100644 --- a/src/udev/udev-builtin-btrfs.c +++ b/src/udev/udev-builtin-btrfs.c @@ -12,7 +12,8 @@ #include "strxcpyx.h" #include "udev-builtin.h" -static int builtin_btrfs(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_btrfs(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); struct btrfs_ioctl_vol_args args = {}; _cleanup_close_ int fd = -EBADF; int r; diff --git a/src/udev/udev-builtin-hwdb.c b/src/udev/udev-builtin-hwdb.c index 8d652e46fe..19e07e734f 100644 --- a/src/udev/udev-builtin-hwdb.c +++ b/src/udev/udev-builtin-hwdb.c @@ -118,7 +118,7 @@ next: return r; } -static int builtin_hwdb(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_hwdb(UdevEvent *event, int argc, char *argv[], bool test) { static const struct option options[] = { { "filter", required_argument, NULL, 'f' }, { "device", required_argument, NULL, 'd' }, @@ -131,6 +131,7 @@ static int builtin_hwdb(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[ const char *subsystem = NULL; const char *prefix = NULL; _cleanup_(sd_device_unrefp) sd_device *srcdev = NULL; + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); int r; if (!hwdb) diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c index 540f390530..295e8d2159 100644 --- a/src/udev/udev-builtin-input_id.c +++ b/src/udev/udev-builtin-input_id.c @@ -369,8 +369,8 @@ static bool test_key(sd_device *dev, return found; } -static int builtin_input_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { - sd_device *pdev; +static int builtin_input_id(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *pdev, *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); unsigned long bitmask_ev[NBITS(EV_MAX)]; unsigned long bitmask_abs[NBITS(ABS_MAX)]; unsigned long bitmask_key[NBITS(KEY_MAX)]; @@ -380,8 +380,6 @@ static int builtin_input_id(sd_device *dev, sd_netlink **rtnl, int argc, char *a bool is_pointer; bool is_key; - assert(dev); - /* walk up the parental chain until we find the real input device; the * argument is very likely a subdevice of this, like eventN */ for (pdev = dev; pdev; ) { diff --git a/src/udev/udev-builtin-keyboard.c b/src/udev/udev-builtin-keyboard.c index 80cfdee0c1..da67c2be1c 100644 --- a/src/udev/udev-builtin-keyboard.c +++ b/src/udev/udev-builtin-keyboard.c @@ -159,7 +159,8 @@ static int set_trackpoint_sensitivity(sd_device *dev, const char *value) { return 0; } -static int builtin_keyboard(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_keyboard(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); unsigned release[1024]; unsigned release_count = 0; _cleanup_close_ int fd = -EBADF; diff --git a/src/udev/udev-builtin-kmod.c b/src/udev/udev-builtin-kmod.c index eade042f35..3ab5c485f8 100644 --- a/src/udev/udev-builtin-kmod.c +++ b/src/udev/udev-builtin-kmod.c @@ -22,11 +22,10 @@ _printf_(6,0) static void udev_kmod_log(void *data, int priority, const char *fi log_internalv(priority, 0, file, line, fn, format, args); } -static int builtin_kmod(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_kmod(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); int r; - assert(dev); - if (!ctx) return 0; diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index 5e2b69d6f7..998478ec4e 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -1096,7 +1096,8 @@ static int get_link_info(sd_device *dev, LinkInfo *info) { return 0; } -static int builtin_net_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_net_id(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); const char *prefix; NetNames names = {}; LinkInfo info = {}; diff --git a/src/udev/udev-builtin-net_setup_link.c b/src/udev/udev-builtin-net_setup_link.c index 18450536b5..84a0a6ef3f 100644 --- a/src/udev/udev-builtin-net_setup_link.c +++ b/src/udev/udev-builtin-net_setup_link.c @@ -12,7 +12,8 @@ static LinkConfigContext *ctx = NULL; -static int builtin_net_setup_link(sd_device *dev, sd_netlink **rtnl, int argc, char **argv, bool test) { +static int builtin_net_setup_link(UdevEvent *event, int argc, char **argv, bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); _cleanup_(link_freep) Link *link = NULL; _cleanup_free_ char *joined = NULL; int r; @@ -20,7 +21,7 @@ static int builtin_net_setup_link(sd_device *dev, sd_netlink **rtnl, int argc, c if (argc > 1) return log_device_error_errno(dev, SYNTHETIC_ERRNO(EINVAL), "This program takes no arguments."); - r = link_new(ctx, rtnl, dev, &link); + r = link_new(ctx, &event->rtnl, dev, &link); if (r == -ENODEV) { log_device_debug_errno(dev, r, "Link vanished while getting information, ignoring."); return 0; @@ -41,7 +42,7 @@ static int builtin_net_setup_link(sd_device *dev, sd_netlink **rtnl, int argc, c return log_device_error_errno(dev, r, "Failed to get link config: %m"); } - r = link_apply_config(ctx, rtnl, link); + r = link_apply_config(ctx, &event->rtnl, link); if (r == -ENODEV) log_device_debug_errno(dev, r, "Link vanished while applying configuration, ignoring."); else if (r < 0) diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index ff6682cad9..404196f409 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -589,15 +589,14 @@ static int find_real_nvme_parent(sd_device *dev, sd_device **ret) { return 0; } -static int builtin_path_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_path_id(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); _cleanup_(sd_device_unrefp) sd_device *dev_other_branch = NULL; _cleanup_free_ char *path = NULL, *compat_path = NULL; bool supported_transport = false, supported_parent = false; const char *subsystem; int r; - assert(dev); - /* walk up the chain of devices and compose path */ for (sd_device *parent = dev; parent; ) { const char *subsys, *sysname; diff --git a/src/udev/udev-builtin-uaccess.c b/src/udev/udev-builtin-uaccess.c index 6e73d99375..36c993cbb0 100644 --- a/src/udev/udev-builtin-uaccess.c +++ b/src/udev/udev-builtin-uaccess.c @@ -16,7 +16,8 @@ #include "log.h" #include "udev-builtin.h" -static int builtin_uaccess(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_uaccess(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); const char *path = NULL, *seat; bool changed_acl = false; uid_t uid; diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c index 91deb64292..8e83c9c342 100644 --- a/src/udev/udev-builtin-usb_id.c +++ b/src/udev/udev-builtin-usb_id.c @@ -224,7 +224,8 @@ static int dev_if_packed_info(sd_device *dev, char *ifs_str, size_t len) { * 6.) If the device supplies a serial number, this number * is concatenated with the identification with an underscore '_'. */ -static int builtin_usb_id(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test) { +static int builtin_usb_id(UdevEvent *event, int argc, char *argv[], bool test) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); char vendor_str[64] = ""; char vendor_str_enc[256]; const char *vendor_id; @@ -250,8 +251,6 @@ static int builtin_usb_id(sd_device *dev, sd_netlink **rtnl, int argc, char *arg const char *syspath, *sysname, *devtype, *interface_syspath; int r; - assert(dev); - r = sd_device_get_syspath(dev, &syspath); if (r < 0) return r; diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c index 566641e400..a8dd656b00 100644 --- a/src/udev/udev-builtin.c +++ b/src/udev/udev-builtin.c @@ -98,11 +98,12 @@ UdevBuiltinCommand udev_builtin_lookup(const char *command) { return _UDEV_BUILTIN_INVALID; } -int udev_builtin_run(sd_device *dev, sd_netlink **rtnl, UdevBuiltinCommand cmd, const char *command, bool test) { +int udev_builtin_run(UdevEvent *event, UdevBuiltinCommand cmd, const char *command, bool test) { _cleanup_strv_free_ char **argv = NULL; int r; - assert(dev); + assert(event); + assert(event->dev); assert(cmd >= 0 && cmd < _UDEV_BUILTIN_MAX); assert(command); @@ -115,7 +116,7 @@ int udev_builtin_run(sd_device *dev, sd_netlink **rtnl, UdevBuiltinCommand cmd, /* we need '0' here to reset the internal state */ optind = 0; - return builtins[cmd]->cmd(dev, rtnl, strv_length(argv), argv, test); + return builtins[cmd]->cmd(event, strv_length(argv), argv, test); } int udev_builtin_add_property(sd_device *dev, bool test, const char *key, const char *val) { diff --git a/src/udev/udev-builtin.h b/src/udev/udev-builtin.h index 490b2dbe96..b2cf81ad31 100644 --- a/src/udev/udev-builtin.h +++ b/src/udev/udev-builtin.h @@ -7,6 +7,7 @@ #include "sd-netlink.h" #include "macro.h" +#include "udev-event.h" typedef enum UdevBuiltinCommand { #if HAVE_BLKID @@ -32,7 +33,7 @@ typedef enum UdevBuiltinCommand { typedef struct UdevBuiltin { const char *name; - int (*cmd)(sd_device *dev, sd_netlink **rtnl, int argc, char *argv[], bool test); + int (*cmd)(UdevEvent *event, int argc, char *argv[], bool test); const char *help; int (*init)(void); void (*exit)(void); @@ -76,7 +77,7 @@ void udev_builtin_exit(void); UdevBuiltinCommand udev_builtin_lookup(const char *command); const char *udev_builtin_name(UdevBuiltinCommand cmd); bool udev_builtin_run_once(UdevBuiltinCommand cmd); -int udev_builtin_run(sd_device *dev, sd_netlink **rtnl, UdevBuiltinCommand cmd, const char *command, bool test); +int udev_builtin_run(UdevEvent *event, UdevBuiltinCommand cmd, const char *command, bool test); void udev_builtin_list(void); bool udev_builtin_should_reload(void); int udev_builtin_add_property(sd_device *dev, bool test, const char *key, const char *val); diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index a8d7db40b4..88b01f4e1b 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -1185,7 +1185,7 @@ void udev_event_execute_run(UdevEvent *event, usec_t timeout_usec, int timeout_s if (builtin_cmd != _UDEV_BUILTIN_INVALID) { log_device_debug(event->dev, "Running built-in command \"%s\"", command); - r = udev_builtin_run(event->dev, &event->rtnl, builtin_cmd, command, false); + r = udev_builtin_run(event, builtin_cmd, command, false); if (r < 0) log_device_debug_errno(event->dev, r, "Failed to run built-in command \"%s\", ignoring: %m", command); } else { diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index d6e701f3cc..fc1ef34964 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -2193,7 +2193,7 @@ static int udev_rule_apply_token_to_event( log_event_debug(dev, token, "Importing properties from results of builtin command '%s'", buf); - r = udev_builtin_run(dev, &event->rtnl, cmd, buf, false); + r = udev_builtin_run(event, cmd, buf, false); if (r < 0) { /* remember failure */ log_event_debug_errno(dev, token, r, "Failed to run builtin '%s': %m", buf); diff --git a/src/udev/udevadm-test-builtin.c b/src/udev/udevadm-test-builtin.c index 5570eec789..5d1fafbd03 100644 --- a/src/udev/udevadm-test-builtin.c +++ b/src/udev/udevadm-test-builtin.c @@ -72,7 +72,7 @@ static int parse_argv(int argc, char *argv[]) { } int builtin_main(int argc, char *argv[], void *userdata) { - _cleanup_(sd_netlink_unrefp) sd_netlink *rtnl = NULL; + _cleanup_(udev_event_freep) UdevEvent *event = NULL; _cleanup_(sd_device_unrefp) sd_device *dev = NULL; UdevBuiltinCommand cmd; int r; @@ -97,7 +97,13 @@ int builtin_main(int argc, char *argv[], void *userdata) { goto finish; } - r = udev_builtin_run(dev, &rtnl, cmd, arg_command, true); + event = udev_event_new(dev, 0, NULL, LOG_DEBUG); + if (!event) { + r = log_oom(); + goto finish; + } + + r = udev_builtin_run(event, cmd, arg_command, true); if (r < 0) log_debug_errno(r, "Builtin command '%s' fails: %m", arg_command); From e65c6c1baa8ea905f7e5bad3b8486d509775ec6a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 11:46:11 +0900 Subject: [PATCH 3/7] udev/net: verify ID_NET_XYZ before trying to assign it as an alternative name --- src/udev/net/link-config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 4c83eede7a..e5f1220928 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -836,7 +836,7 @@ static int link_apply_alternative_names(Link *link, sd_netlink **rtnl) { default: assert_not_reached(); } - if (!isempty(n)) { + if (ifname_valid_full(n, IFNAME_VALID_ALTERNATIVE)) { r = strv_extend(&altnames, n); if (r < 0) return log_oom(); From cd941e6596adba6bb139c387ae596f26b35701f7 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 13:05:09 +0900 Subject: [PATCH 4/7] udev/net: generate new network interface name only on add uevent On other uevents, the name will be anyway ignored in rename_netif() in udev-event.c. --- src/udev/net/link-config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index e5f1220928..28571319fc 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -724,7 +724,7 @@ static int link_generate_new_name(Link *link) { config = link->config; device = link->device; - if (link->action == SD_DEVICE_MOVE) { + if (link->action != SD_DEVICE_ADD) { log_link_debug(link, "Skipping to apply Name= and NamePolicy= on '%s' uevent.", device_action_to_string(link->action)); goto no_rename; From 81824455008070253c62bf5c27187028ba8e7e99 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 13:29:37 +0900 Subject: [PATCH 5/7] sd-netlink: make rtnl_set_link_name() optionally append alternative names --- src/libsystemd/sd-netlink/netlink-util.c | 89 ++++++++++++++++++------ src/libsystemd/sd-netlink/netlink-util.h | 5 +- src/libsystemd/sd-netlink/test-netlink.c | 6 +- src/udev/udev-event.c | 2 +- 4 files changed, 78 insertions(+), 24 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 6cd916a209..6ded66b935 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -11,44 +11,93 @@ #include "process-util.h" #include "strv.h" -int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { +static int set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *message = NULL; - _cleanup_strv_free_ char **alternative_names = NULL; - bool altname_deleted = false; int r; assert(rtnl); assert(ifindex > 0); assert(name); - if (!ifname_valid(name)) + /* Assign the requested name. */ + r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); + if (r < 0) + return r; + + r = sd_netlink_message_append_string(message, IFLA_IFNAME, name); + if (r < 0) + return r; + + return sd_netlink_call(*rtnl, message, 0, NULL); +} + +int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name, char* const *alternative_names) { + _cleanup_strv_free_ char **original_altnames = NULL, **new_altnames = NULL; + bool altname_deleted = false; + int r; + + assert(rtnl); + assert(ifindex > 0); + + if (isempty(name) && strv_isempty(alternative_names)) + return 0; + + if (name && !ifname_valid(name)) return -EINVAL; - r = rtnl_get_link_alternative_names(rtnl, ifindex, &alternative_names); + /* If the requested name is already assigned as an alternative name, then first drop it. */ + r = rtnl_get_link_alternative_names(rtnl, ifindex, &original_altnames); if (r < 0) log_debug_errno(r, "Failed to get alternative names on network interface %i, ignoring: %m", ifindex); - if (strv_contains(alternative_names, name)) { - r = rtnl_delete_link_alternative_names(rtnl, ifindex, STRV_MAKE(name)); - if (r < 0) - return log_debug_errno(r, "Failed to remove '%s' from alternative names on network interface %i: %m", - name, ifindex); + if (name) { + if (strv_contains(original_altnames, name)) { + r = rtnl_delete_link_alternative_names(rtnl, ifindex, STRV_MAKE(name)); + if (r < 0) + return log_debug_errno(r, "Failed to remove '%s' from alternative names on network interface %i: %m", + name, ifindex); - altname_deleted = true; + altname_deleted = true; + } + + r = set_link_name(rtnl, ifindex, name); + if (r < 0) + goto fail; } - r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); - if (r < 0) - goto fail; + /* Filter out already assigned names from requested alternative names. Also, dedup the request. */ + STRV_FOREACH(a, alternative_names) { + if (streq_ptr(name, *a)) + continue; - r = sd_netlink_message_append_string(message, IFLA_IFNAME, name); - if (r < 0) - goto fail; + if (strv_contains(original_altnames, *a)) + continue; - r = sd_netlink_call(*rtnl, message, 0, NULL); - if (r < 0) - goto fail; + if (strv_contains(new_altnames, *a)) + continue; + + if (!ifname_valid_full(*a, IFNAME_VALID_ALTERNATIVE)) + continue; + + r = strv_extend(&new_altnames, *a); + if (r < 0) + return r; + } + + strv_sort(new_altnames); + + /* Finally, assign alternative names. */ + r = rtnl_set_link_alternative_names(rtnl, ifindex, new_altnames); + if (r == -EEXIST) /* Already assigned to another interface? */ + STRV_FOREACH(a, new_altnames) { + r = rtnl_set_link_alternative_names(rtnl, ifindex, STRV_MAKE(*a)); + if (r < 0) + log_debug_errno(r, "Failed to assign '%s' as an alternative name on network interface %i, ignoring: %m", + *a, ifindex); + } + else if (r < 0) + log_debug_errno(r, "Failed to assign alternative names on network interface %i, ignoring: %m", ifindex); return 0; diff --git a/src/libsystemd/sd-netlink/netlink-util.h b/src/libsystemd/sd-netlink/netlink-util.h index 72d16fa9a9..2f9f7e9676 100644 --- a/src/libsystemd/sd-netlink/netlink-util.h +++ b/src/libsystemd/sd-netlink/netlink-util.h @@ -28,7 +28,10 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(MultipathRoute*, multipath_route_free); int multipath_route_dup(const MultipathRoute *m, MultipathRoute **ret); -int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name); +int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name, char* const* alternative_names); +static inline int rtnl_append_link_alternative_names(sd_netlink **rtnl, int ifindex, char* const *alternative_names) { + return rtnl_set_link_name(rtnl, ifindex, NULL, alternative_names); +} int rtnl_set_link_properties( sd_netlink **rtnl, int ifindex, diff --git a/src/libsystemd/sd-netlink/test-netlink.c b/src/libsystemd/sd-netlink/test-netlink.c index 9ad8ecf320..43124b99ae 100644 --- a/src/libsystemd/sd-netlink/test-netlink.c +++ b/src/libsystemd/sd-netlink/test-netlink.c @@ -662,12 +662,13 @@ TEST(rtnl_set_link_name) { assert_se(strv_contains(alternative_names, "testlongalternativename")); assert_se(strv_contains(alternative_names, "test-shortname")); - assert_se(rtnl_set_link_name(&rtnl, ifindex, "testlongalternativename") == -EINVAL); - assert_se(rtnl_set_link_name(&rtnl, ifindex, "test-shortname") >= 0); + assert_se(rtnl_set_link_name(&rtnl, ifindex, "testlongalternativename", NULL) == -EINVAL); + assert_se(rtnl_set_link_name(&rtnl, ifindex, "test-shortname", STRV_MAKE("testlongalternativename", "test-shortname", "test-additional-name")) >= 0); alternative_names = strv_free(alternative_names); assert_se(rtnl_get_link_alternative_names(&rtnl, ifindex, &alternative_names) >= 0); assert_se(strv_contains(alternative_names, "testlongalternativename")); + assert_se(strv_contains(alternative_names, "test-additional-name")); assert_se(!strv_contains(alternative_names, "test-shortname")); assert_se(rtnl_delete_link_alternative_names(&rtnl, ifindex, STRV_MAKE("testlongalternativename")) >= 0); @@ -675,6 +676,7 @@ TEST(rtnl_set_link_name) { alternative_names = strv_free(alternative_names); assert_se(rtnl_get_link_alternative_names(&rtnl, ifindex, &alternative_names) >= 0); assert_se(!strv_contains(alternative_names, "testlongalternativename")); + assert_se(strv_contains(alternative_names, "test-additional-name")); assert_se(!strv_contains(alternative_names, "test-shortname")); } diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 88b01f4e1b..7f6d8e4a75 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -976,7 +976,7 @@ static int rename_netif(UdevEvent *event) { goto revert; } - r = rtnl_set_link_name(&event->rtnl, ifindex, event->name); + r = rtnl_set_link_name(&event->rtnl, ifindex, event->name, NULL); if (r < 0) { if (r == -EBUSY) { log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.", From 9094ae52caca0c19ff6abdbd95d17d8e401ea3b1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 12:28:23 +0900 Subject: [PATCH 6/7] udev/net: assign alternative names only on add uevent Previously, we first assign alternative names to a network interface, then later change its main name if requested. So, we could not assign the name that currently assigned as the main name of an interface as an alternative name. So, we retry to assign the previous main name as an alternative name on later move uevent. However, that causes some confusing situation. E.g. if a .link file has ``` Name=foo AlternativeNames=foo baz ``` then even if the interface is renamed by a user e.g. by invoking 'ip link' command manually, the interface can be still referenced as 'foo', as the name is now assigned as an alternative name. This makes the order of name assignment inverse: the main name is first changed, and then the requested alternative names are assigned. And udevd do not assign alternative names on move uevent. Replaces #27506. --- src/udev/net/link-config.c | 37 ++++++++------------- src/udev/net/link-config.h | 1 + src/udev/udev-builtin-net_setup_link.c | 2 ++ src/udev/udev-event.c | 45 ++++++++++++++++++++++---- src/udev/udev-event.h | 1 + 5 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 28571319fc..b554a6827d 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -365,6 +365,7 @@ Link *link_free(Link *link) { sd_device_unref(link->device); free(link->kind); free(link->driver); + strv_free(link->altnames); return mfree(link); } @@ -793,19 +794,22 @@ no_rename: return 0; } -static int link_apply_alternative_names(Link *link, sd_netlink **rtnl) { - _cleanup_strv_free_ char **altnames = NULL, **current_altnames = NULL; +static int link_generate_alternative_names(Link *link) { + _cleanup_strv_free_ char **altnames = NULL; LinkConfig *config; sd_device *device; int r; assert(link); - assert(link->config); - assert(link->device); - assert(rtnl); + config = ASSERT_PTR(link->config); + device = ASSERT_PTR(link->device); + assert(!link->altnames); - config = link->config; - device = link->device; + if (link->action != SD_DEVICE_ADD) { + log_link_debug(link, "Skipping to apply AlternativeNames= and AlternativeNamesPolicy= on '%s' uevent.", + device_action_to_string(link->action)); + return 0; + } if (config->alternative_names) { altnames = strv_copy(config->alternative_names); @@ -843,22 +847,7 @@ static int link_apply_alternative_names(Link *link, sd_netlink **rtnl) { } } - strv_remove(altnames, link->ifname); - - r = rtnl_get_link_alternative_names(rtnl, link->ifindex, ¤t_altnames); - if (r < 0) - log_link_debug_errno(link, r, "Failed to get alternative names, ignoring: %m"); - - STRV_FOREACH(p, current_altnames) - strv_remove(altnames, *p); - - strv_uniq(altnames); - strv_sort(altnames); - r = rtnl_set_link_alternative_names(rtnl, link->ifindex, altnames); - if (r < 0) - log_link_full_errno(link, r == -EOPNOTSUPP ? LOG_DEBUG : LOG_WARNING, r, - "Could not set AlternativeName= or apply AlternativeNamesPolicy=, ignoring: %m"); - + link->altnames = TAKE_PTR(altnames); return 0; } @@ -960,7 +949,7 @@ int link_apply_config(LinkConfigContext *ctx, sd_netlink **rtnl, Link *link) { if (r < 0) return r; - r = link_apply_alternative_names(link, rtnl); + r = link_generate_alternative_names(link); if (r < 0) return r; diff --git a/src/udev/net/link-config.h b/src/udev/net/link-config.h index ed0896e9de..713d41040a 100644 --- a/src/udev/net/link-config.h +++ b/src/udev/net/link-config.h @@ -27,6 +27,7 @@ typedef struct Link { int ifindex; const char *ifname; const char *new_name; + char **altnames; LinkConfig *config; sd_device *device; diff --git a/src/udev/udev-builtin-net_setup_link.c b/src/udev/udev-builtin-net_setup_link.c index 84a0a6ef3f..5bc965f6d8 100644 --- a/src/udev/udev-builtin-net_setup_link.c +++ b/src/udev/udev-builtin-net_setup_link.c @@ -52,6 +52,8 @@ static int builtin_net_setup_link(UdevEvent *event, int argc, char **argv, bool if (link->new_name) udev_builtin_add_property(dev, test, "ID_NET_NAME", link->new_name); + event->altnames = TAKE_PTR(link->altnames); + STRV_FOREACH(d, link->config->dropins) { _cleanup_free_ char *escaped = NULL; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 7f6d8e4a75..b3cdb32a63 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -87,6 +87,7 @@ UdevEvent *udev_event_free(UdevEvent *event) { ordered_hashmap_free_free_free(event->seclabel_list); free(event->program_result); free(event->name); + strv_free(event->altnames); return mfree(event); } @@ -914,9 +915,6 @@ static int rename_netif(UdevEvent *event) { dev = ASSERT_PTR(event->dev); - if (!device_for_action(dev, SD_DEVICE_ADD)) - return 0; /* Rename the interface only when it is added. */ - r = sd_device_get_ifindex(dev, &ifindex); if (r == -ENOENT) return 0; /* Device is not a network interface. */ @@ -976,7 +974,7 @@ static int rename_netif(UdevEvent *event) { goto revert; } - r = rtnl_set_link_name(&event->rtnl, ifindex, event->name, NULL); + r = rtnl_set_link_name(&event->rtnl, ifindex, event->name, event->altnames); if (r < 0) { if (r == -EBUSY) { log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.", @@ -1007,6 +1005,35 @@ revert: return r; } +static int assign_altnames(UdevEvent *event) { + sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); + int ifindex, r; + const char *s; + + if (strv_isempty(event->altnames)) + return 0; + + r = sd_device_get_ifindex(dev, &ifindex); + if (r == -ENOENT) + return 0; /* Device is not a network interface. */ + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to get ifindex: %m"); + + r = sd_device_get_sysname(dev, &s); + if (r < 0) + return log_device_warning_errno(dev, r, "Failed to get sysname: %m"); + + /* Filter out the current interface name. */ + strv_remove(event->altnames, s); + + r = rtnl_append_link_alternative_names(&event->rtnl, ifindex, event->altnames); + if (r < 0) + log_device_full_errno(dev, r == -EOPNOTSUPP ? LOG_DEBUG : LOG_WARNING, r, + "Could not set AlternativeName= or apply AlternativeNamesPolicy=, ignoring: %m"); + + return 0; +} + static int update_devnode(UdevEvent *event) { sd_device *dev = ASSERT_PTR(ASSERT_PTR(event)->dev); int r; @@ -1148,9 +1175,13 @@ int udev_event_execute_rules( DEVICE_TRACE_POINT(rules_finished, dev); - r = rename_netif(event); - if (r < 0) - return r; + if (action == SD_DEVICE_ADD) { + r = rename_netif(event); + if (r < 0) + return r; + if (r == 0) + (void) assign_altnames(event); + } r = update_devnode(event); if (r < 0) diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h index c54c7891b6..b99559ca2e 100644 --- a/src/udev/udev-event.h +++ b/src/udev/udev-event.h @@ -22,6 +22,7 @@ typedef struct UdevEvent { sd_device *dev_parent; sd_device *dev_db_clone; char *name; + char **altnames; char *program_result; mode_t mode; uid_t uid; From 40b6b448bda5294582e685091123952fbcd43502 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 May 2023 16:28:54 +0900 Subject: [PATCH 7/7] test: add tests for renaming network interface --- test/units/testsuite-17.12.sh | 86 +++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100755 test/units/testsuite-17.12.sh diff --git a/test/units/testsuite-17.12.sh b/test/units/testsuite-17.12.sh new file mode 100755 index 0000000000..df74d356ee --- /dev/null +++ b/test/units/testsuite-17.12.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -ex +set -o pipefail + +# shellcheck source=test/units/assert.sh +. "$(dirname "$0")"/assert.sh + +create_link_file() { + name=${1?} + + mkdir -p /run/systemd/network/ + cat >/run/systemd/network/10-test.link <