From d0b31efc1ab7f6826ad834cf6b9e371bf73776aa Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Wed, 2 Nov 2022 11:05:01 -0400 Subject: [PATCH 1/6] udev/net: allow new link name as an altname before renaming happens When configuring a link's alternative names, the link's new name to-be is not allowed to be included because interface renaming will fail if the new name is already present as an alternative name. However, rtnl_set_link_name will delete the conflicting alternative name before renaming the device, if necessary. Allow the new link name to be set as an alternative name before the device is renamed. This means that if the rename is later skipped (i.e. because the link is already up), then the name can at least still be present as an alternative name. --- src/udev/net/link-config.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 1fbfc58c0d..25edb49d28 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -841,8 +841,6 @@ static int link_apply_alternative_names(Link *link, sd_netlink **rtnl) { } } - if (link->new_name) - strv_remove(altnames, link->new_name); strv_remove(altnames, link->ifname); r = rtnl_get_link_alternative_names(rtnl, link->ifindex, ¤t_altnames); From 080afbb57c4b2d592c5cf77ab10c6e0be74f0732 Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Fri, 2 Dec 2022 15:26:18 -0500 Subject: [PATCH 2/6] sd-netlink: do not swap old name and alternative name Commit 434a348380 ("netlink: do not fail when new interface name is already used as an alternative name") added logic to set the old interface name as an alternative name, but only when the new name is currently an alternative name. This is not the desired outcome in most cases, and the important part of this commit was to delete the new name from the list of alternative names if necessary. --- src/libsystemd/sd-netlink/netlink-util.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index c6091542d2..a53ffb2616 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -3,7 +3,6 @@ #include "sd-netlink.h" #include "fd-util.h" -#include "format-util.h" #include "io-util.h" #include "memory-util.h" #include "netlink-internal.h" @@ -15,7 +14,6 @@ int rtnl_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; - char old_name[IF_NAMESIZE] = {}; int r; assert(rtnl); @@ -35,10 +33,6 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { if (r < 0) return log_debug_errno(r, "Failed to remove '%s' from alternative names on network interface %i: %m", name, ifindex); - - r = format_ifname(ifindex, old_name); - if (r < 0) - return log_debug_errno(r, "Failed to get current name of network interface %i: %m", ifindex); } r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); @@ -53,13 +47,6 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { if (r < 0) return r; - if (!isempty(old_name)) { - r = rtnl_set_link_alternative_names(rtnl, ifindex, STRV_MAKE(old_name)); - if (r < 0) - log_debug_errno(r, "Failed to set '%s' as an alternative name on network interface %i, ignoring: %m", - old_name, ifindex); - } - return 0; } From 4d600667f8af2985850b03a46357e068d3fb8570 Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Wed, 2 Nov 2022 05:36:14 -0400 Subject: [PATCH 3/6] sd-netlink: restore altname on error in rtnl_set_link_name If a current alternative name is to be used to rename a network interface, the alternative name must be removed first. If interface renaming fails, restore the alternative name that was deleted if necessary. --- src/libsystemd/sd-netlink/netlink-util.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index a53ffb2616..af601bd347 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -14,6 +14,7 @@ int rtnl_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); @@ -33,21 +34,33 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *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; } r = sd_rtnl_message_new_link(*rtnl, &message, RTM_SETLINK, ifindex); if (r < 0) - return r; + goto fail; r = sd_netlink_message_append_string(message, IFLA_IFNAME, name); if (r < 0) - return r; + goto fail; r = sd_netlink_call(*rtnl, message, 0, NULL); if (r < 0) - return r; + goto fail; return 0; + +fail: + if (altname_deleted) { + int q = rtnl_set_link_alternative_names(rtnl, ifindex, STRV_MAKE(name)); + if (q < 0) + log_debug_errno(q, "Failed to restore '%s' as an alternative name on network interface %i, ignoring: %m", + name, ifindex); + } + + return r; } int rtnl_set_link_properties( From 53584e7b61373c26635b906eb64e98fbd3fd3ba4 Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Fri, 2 Dec 2022 15:35:25 -0500 Subject: [PATCH 4/6] udev: attempt device rename even if interface is up Currently rename_netif() will not attempt to rename a device if it is already up, because the kernel will return -EBUSY unless live renaming is allowed on the device. This restriction will be removed in a future kernel version [1]. To cover both cases, always attempt to rename the interface and return 0 if we get -EBUSY. [1] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=bd039b5ea2a9 --- src/udev/udev-event.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 30326e4d1c..63be5275e4 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -862,7 +862,6 @@ int udev_event_spawn( static int rename_netif(UdevEvent *event) { const char *oldname; sd_device *dev; - unsigned flags; int ifindex, r; assert(event); @@ -896,16 +895,6 @@ static int rename_netif(UdevEvent *event) { return 0; } - r = rtnl_get_link_info(&event->rtnl, ifindex, NULL, &flags, NULL, NULL, NULL); - if (r < 0) - return log_device_warning_errno(dev, r, "Failed to get link flags: %m"); - - if (FLAGS_SET(flags, IFF_UP)) { - log_device_info(dev, "Network interface '%s' is already up, refusing to rename to '%s'.", - oldname, event->name); - return 0; - } - /* Set ID_RENAMING boolean property here. It will be dropped when the corresponding move uevent is processed. */ r = device_add_property(dev, "ID_RENAMING", "1"); if (r < 0) @@ -927,6 +916,11 @@ static int rename_netif(UdevEvent *event) { return log_device_debug_errno(event->dev_db_clone, r, "Failed to update database under /run/udev/data/: %m"); r = rtnl_set_link_name(&event->rtnl, ifindex, event->name); + if (r == -EBUSY) { + log_device_info(dev, "Network interface '%s' is already up, cannot rename to '%s'.", + oldname, event->name); + return 0; + } if (r < 0) return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m", ifindex, oldname, event->name); From b338a8bb402a3ab241a617e096b21ae6a7b7badf Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Tue, 22 Nov 2022 17:01:47 -0500 Subject: [PATCH 5/6] sd-netlink: add a test for rtnl_set_link_name() Add a test that verifies a deleted alternative name is restored on error in rtnl_set_link_name(). --- src/libsystemd/sd-netlink/test-netlink.c | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/libsystemd/sd-netlink/test-netlink.c b/src/libsystemd/sd-netlink/test-netlink.c index 3f74ecc068..2d93f9eba0 100644 --- a/src/libsystemd/sd-netlink/test-netlink.c +++ b/src/libsystemd/sd-netlink/test-netlink.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "sd-netlink.h" @@ -16,6 +17,7 @@ #include "macro.h" #include "netlink-genl.h" #include "netlink-internal.h" +#include "netlink-util.h" #include "socket-util.h" #include "stdio-util.h" #include "string-util.h" @@ -667,6 +669,30 @@ static void test_genl(void) { } } +static void test_rtnl_set_link_name(sd_netlink *rtnl, int ifindex) { + _cleanup_strv_free_ char **alternative_names = NULL; + int r; + + log_debug("/* %s */", __func__); + + if (geteuid() != 0) + return (void) log_tests_skipped("not root"); + + /* Test that the new name (which is currently an alternative name) is + * restored as an alternative name on error. Create an error by using + * an invalid device name, namely one that exceeds IFNAMSIZ + * (alternative names can exceed IFNAMSIZ, but not regular names). */ + r = rtnl_set_link_alternative_names(&rtnl, ifindex, STRV_MAKE("testlongalternativename")); + if (r == -EPERM) + return (void) log_tests_skipped("missing required capabilities"); + + assert_se(r >= 0); + assert_se(rtnl_set_link_name(&rtnl, ifindex, "testlongalternativename") == -EINVAL); + assert_se(rtnl_get_link_alternative_names(&rtnl, ifindex, &alternative_names) >= 0); + assert_se(strv_contains(alternative_names, "testlongalternativename")); + assert_se(rtnl_delete_link_alternative_names(&rtnl, ifindex, STRV_MAKE("testlongalternativename")) >= 0); +} + int main(void) { sd_netlink *rtnl; sd_netlink_message *m; @@ -698,6 +724,7 @@ int main(void) { test_pipe(if_loopback); test_event_loop(if_loopback); test_link_configure(rtnl, if_loopback); + test_rtnl_set_link_name(rtnl, if_loopback); test_get_addresses(rtnl); test_message_link_bridge(rtnl); From f68f644a167af3452be853b631fa9144c6716c28 Mon Sep 17 00:00:00 2001 From: Nick Rosbrook Date: Wed, 7 Dec 2022 12:28:28 -0500 Subject: [PATCH 6/6] test-network: add a test for renaming device to current altname --- .../test-network/conf/12-dummy-rename-to-altname.link | 7 +++++++ test/test-network/systemd-networkd-tests.py | 11 +++++++++++ 2 files changed, 18 insertions(+) create mode 100644 test/test-network/conf/12-dummy-rename-to-altname.link diff --git a/test/test-network/conf/12-dummy-rename-to-altname.link b/test/test-network/conf/12-dummy-rename-to-altname.link new file mode 100644 index 0000000000..bef4bf3dc5 --- /dev/null +++ b/test/test-network/conf/12-dummy-rename-to-altname.link @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +[Match] +OriginalName=dummy98 + +[Link] +Name=dummyalt +AlternativeName=dummyalt hogehogehogehogehogehoge diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 7e17cac10b..dec1d085b9 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -933,6 +933,17 @@ class NetworkctlTests(unittest.TestCase, Utilities): output = check_output(*networkctl_cmd, '-n', '0', 'status', 'dummy98', env=env) self.assertRegex(output, 'hogehogehogehogehogehoge') + @expectedFailureIfAlternativeNameIsNotAvailable() + def test_rename_to_altname(self): + copy_network_unit('26-netdev-link-local-addressing-yes.network', + '12-dummy.netdev', '12-dummy-rename-to-altname.link') + start_networkd() + self.wait_online(['dummyalt:degraded']) + + output = check_output(*networkctl_cmd, '-n', '0', 'status', 'dummyalt', env=env) + self.assertIn('hogehogehogehogehogehoge', output) + self.assertNotIn('dummy98', output) + def test_reconfigure(self): copy_network_unit('25-address-static.network', '12-dummy.netdev') start_networkd()