From 4d64309955fe6543b5bbf9c2c7da81a7ea8da9d3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 11:24:33 +0900 Subject: [PATCH 1/8] netlink: check new interface name is valid or not before sending request --- src/libsystemd/sd-netlink/netlink-util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsystemd/sd-netlink/netlink-util.c b/src/libsystemd/sd-netlink/netlink-util.c index 3928dfbabf..628ce507a0 100644 --- a/src/libsystemd/sd-netlink/netlink-util.c +++ b/src/libsystemd/sd-netlink/netlink-util.c @@ -13,6 +13,9 @@ int rtnl_set_link_name(sd_netlink **rtnl, int ifindex, const char *name) { assert(ifindex > 0); assert(name); + if (!ifname_valid(name)) + return -EINVAL; + if (!*rtnl) { r = sd_netlink_open(rtnl); if (r < 0) From 589384be8dadfa146eefd9ae0a95b297ca14ad45 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 11:26:23 +0900 Subject: [PATCH 2/8] udev: drop unnecessary copy of new interface name --- src/udev/udev-event.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 6f90516ff6..b37411889f 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -696,7 +696,6 @@ int udev_event_spawn(UdevEvent *event, static int rename_netif(UdevEvent *event) { sd_device *dev = event->dev; const char *action, *oldname; - char name[IFNAMSIZ]; int ifindex, r; if (!event->name) @@ -722,16 +721,18 @@ static int rename_netif(UdevEvent *event) { if (r < 0) return log_device_error_errno(dev, r, "Failed to get ifindex: %m"); - strscpy(name, IFNAMSIZ, event->name); - r = rtnl_set_link_name(&event->rtnl, ifindex, name); + r = rtnl_set_link_name(&event->rtnl, ifindex, event->name); if (r < 0) - return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m", ifindex, oldname, name); + return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m", + ifindex, oldname, event->name); r = device_rename(dev, event->name); if (r < 0) - return log_warning_errno(r, "Network interface %i is renamed from '%s' to '%s', but could not update sd_device object: %m", ifindex, oldname, name); + return log_device_warning_errno(dev, r, "Network interface %i is renamed from '%s' to '%s', " + "but could not update sd_device object: %m", + ifindex, oldname, event->name); - log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, name); + log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, event->name); return 1; } From 6d0fdf45136e459208591142aa1a843b94af1e36 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 5 Mar 2019 10:31:20 +0900 Subject: [PATCH 3/8] udev: do not read UdevEvent object before checking it is non-NULL --- src/udev/udev-event.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index b37411889f..a124601419 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -821,13 +821,15 @@ int udev_event_execute_rules(UdevEvent *event, usec_t timeout_usec, Hashmap *properties_list, UdevRules *rules) { - sd_device *dev = event->dev; const char *subsystem, *action; + sd_device *dev; int r; assert(event); assert(rules); + dev = event->dev; + r = sd_device_get_subsystem(dev, &subsystem); if (r < 0) return log_device_error_errno(dev, r, "Failed to get subsystem: %m"); From a4055a608e3fde96074a33863439409ea2851ef2 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 11:45:58 +0900 Subject: [PATCH 4/8] udev: set ID_RENAMING property when interface renaming is requested And drop the property on the corresponding 'move' uevent. --- src/udev/udev-event.c | 47 +++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index a124601419..46b2d9ceb9 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -726,11 +726,14 @@ static int rename_netif(UdevEvent *event) { return log_device_error_errno(dev, r, "Failed to rename network interface %i from '%s' to '%s': %m", ifindex, oldname, event->name); + /* Set ID_RENAMING boolean property here, and drop it in the corresponding move uevent later. */ + r = device_add_property(dev, "ID_RENAMING", "1"); + if (r < 0) + log_device_warning_errno(dev, r, "Failed to add 'ID_RENAMING' property: %m"); + r = device_rename(dev, event->name); if (r < 0) - return log_device_warning_errno(dev, r, "Network interface %i is renamed from '%s' to '%s', " - "but could not update sd_device object: %m", - ifindex, oldname, event->name); + log_device_warning_errno(dev, r, "Failed to update properties with new name '%s': %m", event->name); log_device_debug(dev, "Network interface %i is renamed from '%s' to '%s'", ifindex, oldname, event->name); @@ -817,6 +820,25 @@ static void event_execute_rules_on_remove( (void) udev_node_remove(dev); } +static int udev_event_on_move(UdevEvent *event) { + sd_device *dev = event->dev; + int r; + + if (event->dev_db_clone && + sd_device_get_devnum(dev, NULL) < 0) { + r = device_copy_properties(dev, event->dev_db_clone); + if (r < 0) + log_device_debug_errno(dev, r, "Failed to copy properties from cloned sd_device object, ignoring: %m"); + } + + /* Drop previously added property */ + r = device_add_property(dev, "ID_RENAMING", NULL); + if (r < 0) + return log_device_debug_errno(dev, r, "Failed to remove 'ID_RENAMING' property, ignoring: %m"); + + return 0; +} + int udev_event_execute_rules(UdevEvent *event, usec_t timeout_usec, Hashmap *properties_list, @@ -847,21 +869,12 @@ int udev_event_execute_rules(UdevEvent *event, if (r < 0) log_device_debug_errno(dev, r, "Failed to clone sd_device object, ignoring: %m"); - if (event->dev_db_clone) { - r = sd_device_get_devnum(dev, NULL); - if (r < 0) { - if (r != -ENOENT) - log_device_debug_errno(dev, r, "Failed to get devnum, ignoring: %m"); + if (event->dev_db_clone && sd_device_get_devnum(dev, NULL) >= 0) + /* Disable watch during event processing. */ + (void) udev_watch_end(event->dev_db_clone); - if (streq(action, "move")) { - r = device_copy_properties(dev, event->dev_db_clone); - if (r < 0) - log_device_debug_errno(dev, r, "Failed to copy properties from cloned device, ignoring: %m"); - } - } else - /* Disable watch during event processing. */ - (void) udev_watch_end(event->dev_db_clone); - } + if (streq(action, "move")) + (void) udev_event_on_move(event); (void) udev_rules_apply_to_event(rules, event, timeout_usec, properties_list); From 30de2b89d125a8692c22579ef805b03f2054b30b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 12:01:26 +0900 Subject: [PATCH 5/8] network: always drop configs when interface is renamed Before the renaming, wrong .network file may be assigned to the link. So, let's always drop link configuration. --- src/network/networkd-link.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 1738a0d944..a5d4ede374 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -3767,20 +3767,14 @@ int link_update(Link *link, sd_netlink_message *m) { r = sd_netlink_message_read_string(m, IFLA_IFNAME, &ifname); if (r >= 0 && !streq(ifname, link->ifname)) { + Manager *manager = link->manager; + log_link_info(link, "Interface name change detected, %s has been renamed to %s.", link->ifname, ifname); - if (link->state == LINK_STATE_PENDING) { - r = free_and_strdup(&link->ifname, ifname); - if (r < 0) - return r; - } else { - Manager *manager = link->manager; - - link_drop(link); - r = link_add(manager, m, &link); - if (r < 0) - return r; - } + link_drop(link); + r = link_add(manager, m, &link); + if (r < 0) + return r; } r = sd_netlink_message_read_u32(m, IFLA_MTU, &mtu); From 90ba130f005b968685a90c9cc309d1d09815bf83 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 12:16:46 +0900 Subject: [PATCH 6/8] util: introduce device_is_renaming() It will be used in the later commit. --- src/shared/udev-util.c | 12 ++++++++++++ src/shared/udev-util.h | 1 + 2 files changed, 13 insertions(+) diff --git a/src/shared/udev-util.c b/src/shared/udev-util.c index 4200032b3b..6847d715f6 100644 --- a/src/shared/udev-util.c +++ b/src/shared/udev-util.c @@ -169,3 +169,15 @@ int device_wait_for_initialization(sd_device *device, const char *subsystem, sd_ *ret = TAKE_PTR(data.device); return 0; } + +int device_is_renaming(sd_device *dev) { + int r; + + assert(dev); + + r = sd_device_get_property_value(dev, "ID_RENAMING", NULL); + if (r < 0 && r != -ENOENT) + return r; + + return r >= 0; +} diff --git a/src/shared/udev-util.h b/src/shared/udev-util.h index 932c4a9cd5..c45d6a11fd 100644 --- a/src/shared/udev-util.h +++ b/src/shared/udev-util.h @@ -27,3 +27,4 @@ static inline int udev_parse_config(void) { } int device_wait_for_initialization(sd_device *device, const char *subsystem, sd_device **ret); +int device_is_renaming(sd_device *dev); From 299ad32d48c01d1529affddc612198c60733f23c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 12:19:05 +0900 Subject: [PATCH 7/8] network: do not configure interfaces under renaming --- src/network/networkd-link.c | 11 +++++++++++ src/network/networkd-manager.c | 13 ++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index a5d4ede374..9e3cd71a09 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -31,6 +31,7 @@ #include "strv.h" #include "sysctl-util.h" #include "tmpfile-util.h" +#include "udev-util.h" #include "util.h" #include "virt.h" @@ -3624,6 +3625,16 @@ int link_add(Manager *m, sd_netlink_message *message, Link **ret) { return 0; } + r = device_is_renaming(device); + if (r < 0) { + log_link_warning_errno(link, r, "Failed to determine the device is renamed or not: %m"); + goto failed; + } + if (r > 0) { + log_link_debug(link, "Interface is under renaming, pending initialization."); + return 0; + } + r = link_initialized(link, device); if (r < 0) goto failed; diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 079cd3e1d1..c87f907957 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -26,6 +26,7 @@ #include "strv.h" #include "sysctl-util.h" #include "tmpfile-util.h" +#include "udev-util.h" #include "virt.h" /* use 8 MB for receive socket kernel queue. */ @@ -194,7 +195,7 @@ static int manager_udev_process_link(sd_device_monitor *monitor, sd_device *devi return 0; } - if (!STR_IN_SET(action, "add", "change")) { + if (!STR_IN_SET(action, "add", "change", "move")) { log_device_debug(device, "Ignoring udev %s event for device.", action); return 0; } @@ -205,6 +206,16 @@ static int manager_udev_process_link(sd_device_monitor *monitor, sd_device *devi return 0; } + r = device_is_renaming(device); + if (r < 0) { + log_device_error_errno(device, r, "Failed to determine the device is renamed or not, ignoring '%s' uevent: %m", action); + return 0; + } + if (r > 0) { + log_device_debug(device, "Interface is under renaming, wait for the interface to be renamed: %m"); + return 0; + } + r = link_get(m, ifindex, &link); if (r < 0) { if (r != -ENODEV) From 23041689ca511dd72dda13dab0c0699f1377bf70 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 4 Mar 2019 12:20:51 +0900 Subject: [PATCH 8/8] dhcp: refuse to configure DHCP IAID if the interface is under renaming systemd-networkd itself does not start dhcp client, but the code may be used in other projects. So, check that the interface is under renaming or not. --- src/libsystemd-network/dhcp-identifier.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index 04bf64cce5..0c1ff34e5d 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -12,6 +12,7 @@ #include "siphash24.h" #include "sparse-endian.h" #include "stdio-util.h" +#include "udev-util.h" #include "virt.h" #define SYSTEMD_PEN 43793 @@ -182,6 +183,13 @@ int dhcp_identifier_set_iaid( /* not yet ready */ return -EBUSY; + r = device_is_renaming(device); + if (r < 0) + return r; + if (r > 0) + /* device is under renaming */ + return -EBUSY; + name = net_get_name(device); } }