From b05e52000b4eee764b383cc3031da0a3739e996e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Jul 2022 06:04:51 +0900 Subject: [PATCH 01/13] network: do not silently stop to process configuration on activation failure Previously, if activation failed, link did not enter the failed state, but still Link.activated flag not set. Hence, nothing processed even if the interface is manually brought up later. Partially fixes #23649. --- src/network/networkd-setlink.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 148277685e..97c6848717 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -951,17 +951,10 @@ static int link_up_or_down_handler(sd_netlink *rtnl, sd_netlink_message *m, Requ r = sd_netlink_message_get_errno(m); if (r == -ENETDOWN && up && link_up_dsa_slave(link) > 0) log_link_message_debug_errno(link, m, r, "Could not bring up dsa slave, retrying again after dsa master becomes up"); - else if (r < 0) { - const char *error_msg; - - error_msg = up ? - (on_activate ? "Could not bring up interface" : "Could not bring up interface, ignoring") : - (on_activate ? "Could not bring down interface" : "Could not bring down interface, ignoring"); - - log_link_message_warning_errno(link, m, r, error_msg); - if (on_activate) - return 0; - } + else if (r < 0) + log_link_message_warning_errno(link, m, r, up ? + "Could not bring up interface, ignoring" : + "Could not bring down interface, ignoring"); r = link_call_getlink(link, get_link_update_flag_handler); if (r < 0) { From 2829fca249bd58a8cd52013d625701e2b8e31737 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Jul 2022 09:08:10 +0900 Subject: [PATCH 02/13] sd-device: introduce device_get_sysattr_bool() --- src/libsystemd/sd-device/device-private.h | 1 + src/libsystemd/sd-device/sd-device.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 9602f9eda3..93c1d20a01 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -18,6 +18,7 @@ static inline int device_new_from_watch_handle(sd_device **ret, int wd) { } int device_get_property_bool(sd_device *device, const char *key); +int device_get_sysattr_bool(sd_device *device, const char *sysattr); int device_get_device_id(sd_device *device, const char **ret); int device_get_devlink_priority(sd_device *device, int *ret); int device_get_watch_handle(sd_device *device); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index a53f31e5ff..6c034626e7 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2166,6 +2166,20 @@ _public_ int sd_device_get_sysattr_value(sd_device *device, const char *sysattr, return 0; } +int device_get_sysattr_bool(sd_device *device, const char *sysattr) { + const char *value; + int r; + + assert(device); + assert(sysattr); + + r = sd_device_get_sysattr_value(device, sysattr, &value); + if (r < 0) + return r; + + return parse_boolean(value); +} + static void device_remove_cached_sysattr_value(sd_device *device, const char *_key) { _cleanup_free_ char *key = NULL; From 5eb83e9000413a0d1344476d44596397f57963b0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 21 Jul 2022 14:15:20 +0900 Subject: [PATCH 03/13] sd-device: introduce device_clear_sysattr_cache() --- src/libsystemd/sd-device/device-private.h | 1 + src/libsystemd/sd-device/sd-device.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/libsystemd/sd-device/device-private.h b/src/libsystemd/sd-device/device-private.h index 93c1d20a01..3d670a3afc 100644 --- a/src/libsystemd/sd-device/device-private.h +++ b/src/libsystemd/sd-device/device-private.h @@ -26,6 +26,7 @@ int device_get_devnode_mode(sd_device *device, mode_t *ret); int device_get_devnode_uid(sd_device *device, uid_t *ret); int device_get_devnode_gid(sd_device *device, gid_t *ret); +void device_clear_sysattr_cache(sd_device *device); 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); diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 6c034626e7..3e31beb1f2 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -2044,6 +2044,10 @@ _public_ int sd_device_get_trigger_uuid(sd_device *device, sd_id128_t *ret) { return 0; } +void device_clear_sysattr_cache(sd_device *device) { + device->sysattr_values = hashmap_free(device->sysattr_values); +} + 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; From d6985e76f2c3c40552e310e3e93fca86da5a3e3a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Jul 2022 08:46:51 +0900 Subject: [PATCH 04/13] sd-device: introduce device_unref_and_replace() --- src/basic/alloc-util.h | 11 +++++++++++ src/libsystemd/sd-device/device-util.h | 3 +++ 2 files changed, 14 insertions(+) diff --git a/src/basic/alloc-util.h b/src/basic/alloc-util.h index cf4b8b166a..b38db7d473 100644 --- a/src/basic/alloc-util.h +++ b/src/basic/alloc-util.h @@ -63,6 +63,17 @@ typedef void* (*mfree_func_t)(void *p); #define free_and_replace(a, b) \ free_and_replace_full(a, b, free) +/* This is similar to free_and_replace_full(), but NULL is not assigned to 'b', and its reference counter is + * increased. */ +#define unref_and_replace_full(a, b, ref_func, unref_func) \ + ({ \ + typeof(a)* _a = &(a); \ + typeof(b) _b = ref_func(b); \ + unref_func(*_a); \ + *_a = _b; \ + 0; \ + }) + void* memdup(const void *p, size_t l) _alloc_(2); void* memdup_suffix0(const void *p, size_t l); /* We can't use _alloc_() here, since we return a buffer one byte larger than the specified size */ diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index e2adfe8132..4eda2abaf3 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -10,6 +10,9 @@ #include "macro.h" #include "set.h" +#define device_unref_and_replace(a, b) \ + unref_and_replace_full(a, b, sd_device_ref, sd_device_unref) + #define FOREACH_DEVICE_PROPERTY(device, key, value) \ for (key = sd_device_get_property_first(device, &(value)); \ key; \ From 853188db9085e3dbb90cb263bbafce0276479f62 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Jul 2022 08:48:33 +0900 Subject: [PATCH 05/13] network: use device_unref_and_replace() --- src/network/networkd-link.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 94ef20b3ea..486ba3c161 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1433,9 +1433,7 @@ static int link_initialized(Link *link, sd_device *device) { /* Always replace with the new sd_device object. As the sysname (and possibly other properties * or sysattrs) may be outdated. */ - sd_device_ref(device); - sd_device_unref(link->sd_device); - link->sd_device = device; + device_unref_and_replace(link->sd_device, device); /* Do not ignore unamanaged state case here. If an interface is renamed after being once * configured, and the corresponding .network file has Name= in [Match] section, then the From 8de56fb3e04d1018aaac607ce435c61138b22bca Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 23 Jul 2022 20:00:44 +0900 Subject: [PATCH 06/13] network: rename Link.sd_device -> Link.dev --- src/network/networkd-dhcp4.c | 2 +- src/network/networkd-ipv4ll.c | 4 ++-- src/network/networkd-json.c | 4 ++-- src/network/networkd-link.c | 10 +++++----- src/network/networkd-link.h | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index e968773dbb..2c9efcc127 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -1324,7 +1324,7 @@ static bool link_needs_dhcp_broadcast(Link *link) { * If neither is set or a failure occurs, return false, which is the default for this flag. */ r = link->network->dhcp_broadcast; - if (r < 0 && link->sd_device && sd_device_get_property_value(link->sd_device, "ID_NET_DHCP_BROADCAST", &val) >= 0) { + if (r < 0 && link->dev && sd_device_get_property_value(link->dev, "ID_NET_DHCP_BROADCAST", &val) >= 0) { r = parse_boolean(val); if (r < 0) log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to parse ID_NET_DHCP_BROADCAST, ignoring: %m"); diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c index 6dfa60e08b..ac5bb217c3 100644 --- a/src/network/networkd-ipv4ll.c +++ b/src/network/networkd-ipv4ll.c @@ -187,8 +187,8 @@ int ipv4ll_configure(Link *link) { if (r < 0) return r; - if (link->sd_device && - net_get_unique_predictable_data(link->sd_device, true, &seed) >= 0) { + if (link->dev && + net_get_unique_predictable_data(link->dev, true, &seed) >= 0) { r = sd_ipv4ll_set_address_seed(link->ipv4ll, seed); if (r < 0) return r; diff --git a/src/network/networkd-json.c b/src/network/networkd-json.c index dc9823c375..9e2ff33d97 100644 --- a/src/network/networkd-json.c +++ b/src/network/networkd-json.c @@ -1158,7 +1158,7 @@ int link_build_json(Link *link, JsonVariant **ret) { assert(link); assert(ret); - r = net_get_type_string(link->sd_device, link->iftype, &type); + r = net_get_type_string(link->dev, link->iftype, &type); if (r == -ENOMEM) return r; @@ -1215,7 +1215,7 @@ int link_build_json(Link *link, JsonVariant **ret) { w = json_variant_unref(w); - r = device_build_json(link->sd_device, &w); + r = device_build_json(link->dev, &w); if (r < 0) return r; diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 486ba3c161..bf48331129 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -229,7 +229,7 @@ static Link *link_free(Link *link) { unlink_and_free(link->lldp_file); unlink_and_free(link->state_file); - sd_device_unref(link->sd_device); + sd_device_unref(link->dev); netdev_unref(link->netdev); hashmap_free(link->bound_to_links); @@ -1164,7 +1164,7 @@ static int link_get_network(Link *link, Network **ret) { r = net_match_config( &network->match, - link->sd_device, + link->dev, &link->hw_addr, &link->permanent_hw_addr, link->driver, @@ -1180,11 +1180,11 @@ static int link_get_network(Link *link, Network **ret) { if (r == 0) continue; - if (network->match.ifname && link->sd_device) { + if (network->match.ifname && link->dev) { uint8_t name_assign_type = NET_NAME_UNKNOWN; const char *attr; - if (sd_device_get_sysattr_value(link->sd_device, "name_assign_type", &attr) >= 0) + if (sd_device_get_sysattr_value(link->dev, "name_assign_type", &attr) >= 0) (void) safe_atou8(attr, &name_assign_type); warn = name_assign_type == NET_NAME_ENUM; @@ -1433,7 +1433,7 @@ static int link_initialized(Link *link, sd_device *device) { /* Always replace with the new sd_device object. As the sysname (and possibly other properties * or sysattrs) may be outdated. */ - device_unref_and_replace(link->sd_device, device); + device_unref_and_replace(link->dev, device); /* Do not ignore unamanaged state case here. If an interface is renamed after being once * configured, and the corresponding .network file has Name= in [Match] section, then the diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 5174127413..80c3c9240c 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -66,7 +66,7 @@ typedef struct Link { uint32_t min_mtu; uint32_t max_mtu; uint32_t original_mtu; - sd_device *sd_device; + sd_device *dev; char *driver; /* link-local addressing */ From 1fd3ac1eef8889d148f3a6e9da5e656e71f60af0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Jul 2022 08:32:39 +0900 Subject: [PATCH 07/13] network: introduce a tiny wrapper for manager_udev_process_link() Preparation for later commits. --- src/network/networkd-link.c | 29 ++++++++-------------------- src/network/networkd-link.h | 2 +- src/network/networkd-manager.c | 35 ++++++++++++++++++++++++++++++++-- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index bf48331129..8b0ef8f076 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1488,21 +1488,13 @@ static int link_check_initialized(Link *link) { return link_initialized(link, device); } -int manager_udev_process_link(sd_device_monitor *monitor, sd_device *device, void *userdata) { - sd_device_action_t action; - Manager *m = userdata; - Link *link = NULL; +int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t action) { int r, ifindex; + Link *link; assert(m); assert(device); - r = sd_device_get_action(device, &action); - if (r < 0) { - log_device_debug_errno(device, r, "Failed to get udev action, ignoring device: %m"); - return 0; - } - /* Ignore the "remove" uevent — let's remove a device only if rtnetlink says so. All other uevents * are "positive" events in some form, i.e. inform us about a changed or new network interface, that * still exists — and we are interested in that. */ @@ -1510,25 +1502,20 @@ int manager_udev_process_link(sd_device_monitor *monitor, sd_device *device, voi return 0; r = sd_device_get_ifindex(device, &ifindex); - if (r < 0) { - log_device_debug_errno(device, r, "Ignoring udev %s event for device without ifindex or with invalid ifindex: %m", - device_action_to_string(action)); - return 0; - } + if (r < 0) + return log_device_debug_errno(device, r, "Failed to get ifindex: %m"); r = device_is_renaming(device); - if (r < 0) { - log_device_debug_errno(device, r, "Failed to determine the device is renamed or not, ignoring '%s' uevent: %m", - device_action_to_string(action)); - return 0; - } + if (r < 0) + return log_device_debug_errno(device, r, "Failed to determine if the device is renaming or not: %m"); if (r > 0) { - log_device_debug(device, "Interface is under renaming, wait for the interface to be renamed."); + log_device_debug(device, "Device is renaming, waiting for the interface to be renamed."); return 0; } r = link_get_by_index(m, ifindex, &link); if (r < 0) { + /* This error is not critical, as the corresponding rtnl message may be received later. */ log_device_debug_errno(device, r, "Failed to get link from ifindex %i, ignoring: %m", ifindex); return 0; } diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 80c3c9240c..dfe43bb3a0 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -235,7 +235,7 @@ LinkState link_state_from_string(const char *s) _pure_; int link_reconfigure(Link *link, bool force); int link_reconfigure_after_sleep(Link *link); -int manager_udev_process_link(sd_device_monitor *monitor, sd_device *device, void *userdata); +int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t action); int manager_rtnl_process_link(sd_netlink *rtnl, sd_netlink_message *message, Manager *m); int link_flags_to_string_alloc(uint32_t flags, char **ret); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index d8a18006d3..57e972d0de 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -18,6 +18,8 @@ #include "bus-util.h" #include "conf-parser.h" #include "def.h" +#include "device-private.h" +#include "device-util.h" #include "dns-domain.h" #include "fd-util.h" #include "fileio.h" @@ -168,6 +170,35 @@ static int manager_connect_bus(Manager *m) { return 0; } +static int manager_process_uevent(sd_device_monitor *monitor, sd_device *device, void *userdata) { + Manager *m = ASSERT_PTR(userdata); + sd_device_action_t action; + const char *s; + int r; + + assert(device); + + r = sd_device_get_action(device, &action); + if (r < 0) + return log_device_warning_errno(device, r, "Failed to get udev action, ignoring: %m"); + + r = sd_device_get_subsystem(device, &s); + if (r < 0) + return log_device_warning_errno(device, r, "Failed to get subsystem, ignoring: %m"); + + if (streq(s, "net")) + r = manager_udev_process_link(m, device, action); + else { + log_device_debug(device, "Received device with unexpected subsystem \"%s\", ignoring.", s); + return 0; + } + if (r < 0) + log_device_warning_errno(device, r, "Failed to process \"%s\" uevent, ignoring: %m", + device_action_to_string(action)); + + return 0; +} + static int manager_connect_udev(Manager *m) { int r; @@ -186,13 +217,13 @@ static int manager_connect_udev(Manager *m) { r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "net", NULL); if (r < 0) - return log_error_errno(r, "Could not add device monitor filter: %m"); + return log_error_errno(r, "Could not add device monitor filter for net subsystem: %m"); r = sd_device_monitor_attach_event(m->device_monitor, m->event); if (r < 0) return log_error_errno(r, "Failed to attach event to device monitor: %m"); - r = sd_device_monitor_start(m->device_monitor, manager_udev_process_link, m); + r = sd_device_monitor_start(m->device_monitor, manager_process_uevent, m); if (r < 0) return log_error_errno(r, "Failed to start device monitor: %m"); From 75255a8d43d82d0893a699aab6b57ca95fddb791 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 23 Jul 2022 09:00:32 +0900 Subject: [PATCH 08/13] network: unref sd-device object assigned to Link on remove uevent Otherwise, outdated device information may be used in a short timespan. --- src/network/networkd-link.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 8b0ef8f076..838101a217 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1495,16 +1495,24 @@ int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t assert(m); assert(device); - /* Ignore the "remove" uevent — let's remove a device only if rtnetlink says so. All other uevents - * are "positive" events in some form, i.e. inform us about a changed or new network interface, that - * still exists — and we are interested in that. */ - if (action == SD_DEVICE_REMOVE) - return 0; - r = sd_device_get_ifindex(device, &ifindex); if (r < 0) return log_device_debug_errno(device, r, "Failed to get ifindex: %m"); + r = link_get_by_index(m, ifindex, &link); + if (r < 0) { + /* This error is not critical, as the corresponding rtnl message may be received later. */ + log_device_debug_errno(device, r, "Failed to get link from ifindex %i, ignoring: %m", ifindex); + return 0; + } + + /* Let's unref the sd-device object assigned to the corresponding Link object, but keep the Link + * object here. It will be removed only when rtnetlink says so. */ + if (action == SD_DEVICE_REMOVE) { + link->dev = sd_device_unref(link->dev); + return 0; + } + r = device_is_renaming(device); if (r < 0) return log_device_debug_errno(device, r, "Failed to determine if the device is renaming or not: %m"); @@ -1513,13 +1521,6 @@ int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t return 0; } - r = link_get_by_index(m, ifindex, &link); - if (r < 0) { - /* This error is not critical, as the corresponding rtnl message may be received later. */ - log_device_debug_errno(device, r, "Failed to get link from ifindex %i, ignoring: %m", ifindex); - return 0; - } - r = link_initialized(link, device); if (r < 0) link_enter_failed(link); From 71cdfe16aa7c854d1d0d50f34aff5701724e1b97 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 23 Jul 2022 09:11:54 +0900 Subject: [PATCH 09/13] network: add TODO about interface renaming --- src/network/networkd-link.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 838101a217..35d1e2f206 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -1518,6 +1518,9 @@ int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t return log_device_debug_errno(device, r, "Failed to determine if the device is renaming or not: %m"); if (r > 0) { log_device_debug(device, "Device is renaming, waiting for the interface to be renamed."); + /* TODO: + * What happens when a device is initialized, then soon renamed after that? When we detect + * such, maybe we should cancel or postpone all queued requests for the interface. */ return 0; } From fc85b3230273b2bac05faf653d67629d68a889af Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 23 Jul 2022 20:19:39 +0900 Subject: [PATCH 10/13] network: set wiphy name in wiphy_new() Then, we can assume all Wiphy objects managed by Manager has both valid index and name. --- src/network/networkd-wiphy.c | 63 ++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 861ebe6b69..41ce071edc 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -22,25 +22,42 @@ Wiphy *wiphy_free(Wiphy *w) { return mfree(w); } -static int wiphy_new(Manager *manager, uint32_t index, Wiphy **ret) { +static int wiphy_new(Manager *manager, sd_netlink_message *message, Wiphy **ret) { _cleanup_(wiphy_freep) Wiphy *w = NULL; + _cleanup_free_ char *name = NULL; + uint32_t index; int r; assert(manager); + assert(message); + + r = sd_netlink_message_read_u32(message, NL80211_ATTR_WIPHY, &index); + if (r < 0) + return r; + + r = sd_netlink_message_read_string_strdup(message, NL80211_ATTR_WIPHY_NAME, &name); + if (r < 0) + return r; w = new(Wiphy, 1); if (!w) return -ENOMEM; *w = (Wiphy) { + .manager = manager, .index = index, + .name = TAKE_PTR(name), }; r = hashmap_ensure_put(&manager->wiphy_by_index, NULL, UINT32_TO_PTR(w->index), w); if (r < 0) return r; - w->manager = manager; + r = hashmap_ensure_put(&w->manager->wiphy_by_name, &string_hash_ops, w->name, w); + if (r < 0) + return r; + + log_wiphy_debug(w, "Saved new wiphy: index=%"PRIu32, w->index); if (ret) *ret = w; @@ -94,30 +111,22 @@ static int wiphy_update_name(Wiphy *w, sd_netlink_message *message) { if (r < 0) return r; - if (streq_ptr(w->name, name)) + if (streq(w->name, name)) return 0; - if (w->name) - hashmap_remove_value(w->manager->wiphy_by_name, w->name, w); + log_wiphy_debug(w, "Wiphy name change detected, renamed to %s.", name); + + hashmap_remove_value(w->manager->wiphy_by_name, w->name, w); r = free_and_strdup(&w->name, name); if (r < 0) return r; - return hashmap_ensure_put(&w->manager->wiphy_by_name, &string_hash_ops, w->name, w); -} - -static int wiphy_update(Wiphy *w, sd_netlink_message *message) { - int r; - - assert(w); - assert(message); - - r = wiphy_update_name(w, message); + r = hashmap_ensure_put(&w->manager->wiphy_by_name, &string_hash_ops, w->name, w); if (r < 0) - return log_wiphy_debug_errno(w, r, "Failed to update wiphy name: %m"); + return r; - return 0; + return 1; /* updated */ } int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *message, Manager *manager) { @@ -166,23 +175,23 @@ int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *mes switch (cmd) { case NL80211_CMD_NEW_WIPHY: { - bool is_new = !w; if (!w) { - r = wiphy_new(manager, index, &w); + r = wiphy_new(manager, message, &w); if (r < 0) { - log_warning_errno(r, "Failed to allocate wiphy, ignoring: %m"); + log_warning_errno(r, "Failed to save new wiphy, ignoring: %m"); return 0; } + } else { + r = wiphy_update_name(w, message); + if (r < 0) { + log_wiphy_warning_errno(w, r, "Failed to update wiphy name, ignoring: %m"); + return 0; + } + if (r == 0) + return 0; } - r = wiphy_update(w, message); - if (r < 0) { - log_wiphy_warning_errno(w, r, "Failed to update wiphy, ignoring: %m"); - return 0; - } - - log_wiphy_debug(w, "Received %s phy.", is_new ? "new" : "updated"); break; } case NL80211_CMD_DEL_WIPHY: From c49d5362956d7f77e21889a9994076cc8ebe39c5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Jul 2022 08:48:55 +0900 Subject: [PATCH 11/13] network: assign corresponding sd-device object to Wiphy object As an analogy for Link object, let's each Wiphy object has the corresponding sd-device object. The sd-device object will be used in later commits. --- src/network/networkd-manager.c | 6 +++ src/network/networkd-wiphy.c | 72 ++++++++++++++++++++++++++++++++++ src/network/networkd-wiphy.h | 3 ++ 3 files changed, 81 insertions(+) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 57e972d0de..302ba729bd 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -188,6 +188,8 @@ static int manager_process_uevent(sd_device_monitor *monitor, sd_device *device, if (streq(s, "net")) r = manager_udev_process_link(m, device, action); + else if (streq(s, "ieee80211")) + r = manager_udev_process_wiphy(m, device, action); else { log_device_debug(device, "Received device with unexpected subsystem \"%s\", ignoring.", s); return 0; @@ -219,6 +221,10 @@ static int manager_connect_udev(Manager *m) { if (r < 0) return log_error_errno(r, "Could not add device monitor filter for net subsystem: %m"); + r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "ieee80211", NULL); + if (r < 0) + return log_error_errno(r, "Could not add device monitor filter for ieee80211 subsystem: %m"); + r = sd_device_monitor_attach_event(m->device_monitor, m->event); if (r < 0) return log_error_errno(r, "Failed to attach event to device monitor: %m"); diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 41ce071edc..113b671479 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -2,10 +2,12 @@ #include +#include "device-private.h" #include "device-util.h" #include "networkd-manager.h" #include "networkd-wiphy.h" #include "parse-util.h" +#include "udev-util.h" #include "wifi-util.h" Wiphy *wiphy_free(Wiphy *w) { @@ -18,6 +20,8 @@ Wiphy *wiphy_free(Wiphy *w) { hashmap_remove_value(w->manager->wiphy_by_name, w->name, w); } + sd_device_unref(w->dev); + free(w->name); return mfree(w); } @@ -129,6 +133,48 @@ static int wiphy_update_name(Wiphy *w, sd_netlink_message *message) { return 1; /* updated */ } +static int wiphy_update_device(Wiphy *w) { + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + int r; + + assert(w); + assert(w->name); + + if (!udev_available()) + return 0; + + w->dev = sd_device_unref(w->dev); + + r = sd_device_new_from_subsystem_sysname(&dev, "ieee80211", w->name); + if (r < 0) { + /* The corresponding syspath may not exist yet, and may appear later. */ + log_wiphy_debug_errno(w, r, "Failed to get wiphy device, ignoring: %m"); + return 0; + } + + if (DEBUG_LOGGING) { + const char *s = NULL; + + (void) sd_device_get_syspath(dev, &s); + log_wiphy_debug(w, "Found device: %s", strna(s)); + } + + w->dev = TAKE_PTR(dev); + return 0; +} + +static int wiphy_update(Wiphy *w) { + int r; + + assert(w); + + r = wiphy_update_device(w); + if (r < 0) + return log_wiphy_debug_errno(w, r, "Failed to update wiphy device: %m"); + + return 0; +} + int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *message, Manager *manager) { const char *family; uint32_t index; @@ -192,6 +238,10 @@ int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *mes return 0; } + r = wiphy_update(w); + if (r < 0) + log_wiphy_warning_errno(w, r, "Failed to update wiphy, ignoring: %m"); + break; } case NL80211_CMD_DEL_WIPHY: @@ -212,3 +262,25 @@ int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *mes return 0; } + +int manager_udev_process_wiphy(Manager *m, sd_device *device, sd_device_action_t action) { + const char *name; + Wiphy *w; + int r; + + assert(m); + assert(device); + + r = sd_device_get_sysname(device, &name); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to get sysname: %m"); + + r = wiphy_get_by_name(m, name, &w); + if (r < 0) { + /* This error is not critical, as the corresponding genl message may be received later. */ + log_device_debug_errno(device, r, "Failed to get Wiphy object, ignoring: %m"); + return 0; + } + + return device_unref_and_replace(w->dev, action == SD_DEVICE_REMOVE ? NULL : device); +} diff --git a/src/network/networkd-wiphy.h b/src/network/networkd-wiphy.h index b40354704b..9e8a3a5560 100644 --- a/src/network/networkd-wiphy.h +++ b/src/network/networkd-wiphy.h @@ -14,6 +14,8 @@ typedef struct Wiphy { uint32_t index; char *name; + + sd_device *dev; } Wiphy; Wiphy *wiphy_free(Wiphy *w); @@ -23,6 +25,7 @@ int wiphy_get_by_index(Manager *manager, uint32_t index, Wiphy **ret); int wiphy_get_by_name(Manager *manager, const char *name, Wiphy **ret); int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *message, Manager *manager); +int manager_udev_process_wiphy(Manager *m, sd_device *device, sd_device_action_t action); #define log_wiphy_full_errno_zerook(w, level, error, ...) \ ({ \ From 8642e04b78cd6856a7de6a25d5c78230bba8452b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 22 Jul 2022 09:46:52 +0900 Subject: [PATCH 12/13] network: also assign rfkill device to Wiphy object Preparation for later commits. --- src/network/networkd-manager.c | 6 +++ src/network/networkd-wiphy.c | 91 ++++++++++++++++++++++++++++++++++ src/network/networkd-wiphy.h | 2 + 3 files changed, 99 insertions(+) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 302ba729bd..265c7e62c6 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -190,6 +190,8 @@ static int manager_process_uevent(sd_device_monitor *monitor, sd_device *device, r = manager_udev_process_link(m, device, action); else if (streq(s, "ieee80211")) r = manager_udev_process_wiphy(m, device, action); + else if (streq(s, "rfkill")) + r = manager_udev_process_rfkill(m, device, action); else { log_device_debug(device, "Received device with unexpected subsystem \"%s\", ignoring.", s); return 0; @@ -225,6 +227,10 @@ static int manager_connect_udev(Manager *m) { if (r < 0) return log_error_errno(r, "Could not add device monitor filter for ieee80211 subsystem: %m"); + r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "rfkill", NULL); + if (r < 0) + return log_error_errno(r, "Could not add device monitor filter for rfkill subsystem: %m"); + r = sd_device_monitor_attach_event(m->device_monitor, m->event); if (r < 0) return log_error_errno(r, "Failed to attach event to device monitor: %m"); diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 113b671479..1c258e2416 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -7,6 +7,7 @@ #include "networkd-manager.h" #include "networkd-wiphy.h" #include "parse-util.h" +#include "path-util.h" #include "udev-util.h" #include "wifi-util.h" @@ -21,6 +22,7 @@ Wiphy *wiphy_free(Wiphy *w) { } sd_device_unref(w->dev); + sd_device_unref(w->rfkill); free(w->name); return mfree(w); @@ -163,6 +165,59 @@ static int wiphy_update_device(Wiphy *w) { return 0; } +static int wiphy_update_rfkill(Wiphy *w) { + _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + sd_device *rfkill; + int r; + + assert(w); + + if (!udev_available()) + return 0; + + w->rfkill = sd_device_unref(w->rfkill); + + if (!w->dev) + return 0; + + r = sd_device_enumerator_new(&e); + if (r < 0) + return r; + + r = sd_device_enumerator_allow_uninitialized(e); + if (r < 0) + return r; + + r = sd_device_enumerator_add_match_subsystem(e, "rfkill", true); + if (r < 0) + return r; + + r = sd_device_enumerator_add_match_parent(e, w->dev); + if (r < 0) + return r; + + rfkill = sd_device_enumerator_get_device_first(e); + if (!rfkill) { + /* rfkill device may not detected by the kernel yet, and may appear later. */ + log_wiphy_debug_errno(w, SYNTHETIC_ERRNO(ENODEV), "No rfkill device found, ignoring."); + return 0; + } + + if (sd_device_enumerator_get_device_next(e)) + return log_wiphy_debug_errno(w, SYNTHETIC_ERRNO(EEXIST), "Multiple rfkill devices found."); + + w->rfkill = sd_device_ref(rfkill); + + if (DEBUG_LOGGING) { + const char *s = NULL; + + (void) sd_device_get_syspath(rfkill, &s); + log_wiphy_debug(w, "Found rfkill device: %s", strna(s)); + } + + return 0; +} + static int wiphy_update(Wiphy *w) { int r; @@ -172,6 +227,10 @@ static int wiphy_update(Wiphy *w) { if (r < 0) return log_wiphy_debug_errno(w, r, "Failed to update wiphy device: %m"); + r = wiphy_update_rfkill(w); + if (r < 0) + return log_wiphy_debug_errno(w, r, "Failed to update rfkill device: %m"); + return 0; } @@ -284,3 +343,35 @@ int manager_udev_process_wiphy(Manager *m, sd_device *device, sd_device_action_t return device_unref_and_replace(w->dev, action == SD_DEVICE_REMOVE ? NULL : device); } + +int manager_udev_process_rfkill(Manager *m, sd_device *device, sd_device_action_t action) { + _cleanup_free_ char *parent_path = NULL, *parent_name = NULL; + const char *s; + Wiphy *w; + int r; + + assert(m); + assert(device); + + r = sd_device_get_syspath(device, &s); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to get syspath: %m"); + + /* Do not use sd_device_get_parent() here, as this might be a 'remove' uevent. */ + r = path_extract_directory(s, &parent_path); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to get parent syspath: %m"); + + r = path_extract_filename(parent_path, &parent_name); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to get parent name: %m"); + + r = wiphy_get_by_name(m, parent_name, &w); + if (r < 0) { + /* This error is not critical, as the corresponding genl message may be received later. */ + log_device_debug_errno(device, r, "Failed to get Wiphy object: %m"); + return 0; + } + + return device_unref_and_replace(w->rfkill, action == SD_DEVICE_REMOVE ? NULL : device); +} diff --git a/src/network/networkd-wiphy.h b/src/network/networkd-wiphy.h index 9e8a3a5560..5f0f47a230 100644 --- a/src/network/networkd-wiphy.h +++ b/src/network/networkd-wiphy.h @@ -16,6 +16,7 @@ typedef struct Wiphy { char *name; sd_device *dev; + sd_device *rfkill; } Wiphy; Wiphy *wiphy_free(Wiphy *w); @@ -26,6 +27,7 @@ int wiphy_get_by_name(Manager *manager, const char *name, Wiphy **ret); int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *message, Manager *manager); int manager_udev_process_wiphy(Manager *m, sd_device *device, sd_device_action_t action); +int manager_udev_process_rfkill(Manager *m, sd_device *device, sd_device_action_t action); #define log_wiphy_full_errno_zerook(w, level, error, ...) \ ({ \ From a9d2d037ee108327f7ac928139a0feb9a990c546 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 15 Jul 2022 06:26:57 +0900 Subject: [PATCH 13/13] network: do not try to bring up wifi interface if rfkill is active This fixes the following error: --- systemd-networkd[465]: wlan0: Could not bring up interface: Operation not possible due to RF-kill --- Fixes #23649. --- src/network/networkd-setlink.c | 11 ++- src/network/networkd-wiphy.c | 124 +++++++++++++++++++++++++++++++++ src/network/networkd-wiphy.h | 14 ++++ 3 files changed, 147 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-setlink.c b/src/network/networkd-setlink.c index 97c6848717..e2b5acc086 100644 --- a/src/network/networkd-setlink.c +++ b/src/network/networkd-setlink.c @@ -14,6 +14,7 @@ #include "networkd-manager.h" #include "networkd-queue.h" #include "networkd-setlink.h" +#include "networkd-wiphy.h" static int get_link_default_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { return link_getlink_handler_internal(rtnl, m, link, "Failed to sync link information"); @@ -998,7 +999,7 @@ static int link_up_or_down(Link *link, bool up, Request *req) { return request_call_netlink_async(link->manager->rtnl, m, req); } -static bool link_is_ready_to_activate(Link *link) { +static bool link_is_ready_to_activate(Link *link, bool up) { assert(link); if (!IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) @@ -1007,6 +1008,9 @@ static bool link_is_ready_to_activate(Link *link) { if (link->set_link_messages > 0) return false; + if (up && link_rfkilled(link) > 0) + return false; + return true; } @@ -1017,7 +1021,7 @@ static int link_process_activation(Request *req, Link *link, void *userdata) { assert(req); assert(link); - if (!link_is_ready_to_activate(link)) + if (!link_is_ready_to_activate(link, up)) return 0; r = link_up_or_down(link, up, req); @@ -1097,6 +1101,9 @@ static bool link_is_ready_to_bring_up_or_down(Link *link, bool up) { if (!link->activated) return false; + if (up && link_rfkilled(link) > 0) + return false; + return true; } diff --git a/src/network/networkd-wiphy.c b/src/network/networkd-wiphy.c index 1c258e2416..38d1ceeba1 100644 --- a/src/network/networkd-wiphy.c +++ b/src/network/networkd-wiphy.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include #include #include "device-private.h" @@ -103,6 +104,129 @@ int wiphy_get_by_name(Manager *manager, const char *name, Wiphy **ret) { return 0; } +static int link_get_wiphy(Link *link, Wiphy **ret) { + _cleanup_(sd_device_unrefp) sd_device *phy = NULL; + const char *s; + int r; + + assert(link); + assert(link->manager); + + if (link->iftype != ARPHRD_ETHER) + return -EOPNOTSUPP; + + if (!link->dev) + return -EOPNOTSUPP; + + r = sd_device_get_devtype(link->dev, &s); + if (r < 0) + return r; + + if (!streq_ptr(s, "wlan")) + return -EOPNOTSUPP; + + r = sd_device_get_syspath(link->dev, &s); + if (r < 0) + return r; + + s = strjoina(s, "/phy80211"); + r = sd_device_new_from_syspath(&phy, s); + if (r < 0) + return r; + + r = sd_device_get_sysname(phy, &s); + if (r < 0) + return r; + + /* TODO: + * Maybe, it is better to cache the found Wiphy object in the Link object. + * To support that, we need to investigate what happens when the _phy_ is renamed. */ + + return wiphy_get_by_name(link->manager, s, ret); +} + +static int rfkill_get_state(sd_device *dev) { + int r; + + assert(dev); + + /* The previous values may be outdated. Let's clear cache and re-read the values. */ + device_clear_sysattr_cache(dev); + + r = device_get_sysattr_bool(dev, "soft"); + if (r < 0 && r != -ENOENT) + return r; + if (r > 0) + return RFKILL_SOFT; + + r = device_get_sysattr_bool(dev, "hard"); + if (r < 0 && r != -ENOENT) + return r; + if (r > 0) + return RFKILL_HARD; + + return RFKILL_UNBLOCKED; +} + +static int wiphy_rfkilled(Wiphy *w) { + int r; + + assert(w); + + if (!udev_available()) { + if (w->rfkill_state != RFKILL_UNBLOCKED) { + log_wiphy_debug(w, "Running in container, assuming the radio transmitter is unblocked."); + w->rfkill_state = RFKILL_UNBLOCKED; /* To suppress the above log message, cache the state. */ + } + return false; + } + + if (!w->rfkill) { + if (w->rfkill_state != RFKILL_UNBLOCKED) { + log_wiphy_debug(w, "No rfkill device found, assuming the radio transmitter is unblocked."); + w->rfkill_state = RFKILL_UNBLOCKED; /* To suppress the above log message, cache the state. */ + } + return false; + } + + r = rfkill_get_state(w->rfkill); + if (r < 0) + return log_wiphy_debug_errno(w, r, "Could not get rfkill state: %m"); + + if (w->rfkill_state != r) + switch (r) { + case RFKILL_UNBLOCKED: + log_wiphy_debug(w, "The radio transmitter is unblocked."); + break; + case RFKILL_SOFT: + log_wiphy_debug(w, "The radio transmitter is turned off by software."); + break; + case RFKILL_HARD: + log_wiphy_debug(w, "The radio transmitter is forced off by something outside of the driver's control."); + break; + default: + assert_not_reached(); + } + + w->rfkill_state = r; /* Cache the state to suppress the above log messages. */ + return r != RFKILL_UNBLOCKED; +} + +int link_rfkilled(Link *link) { + Wiphy *w; + int r; + + assert(link); + + r = link_get_wiphy(link, &w); + if (IN_SET(r, -EOPNOTSUPP, -ENODEV)) + return false; /* Typically, non-wifi interface or running in container */ + if (r < 0) + return log_link_debug_errno(link, r, "Could not get phy: %m"); + + return wiphy_rfkilled(w); +} + static int wiphy_update_name(Wiphy *w, sd_netlink_message *message) { const char *name; int r; diff --git a/src/network/networkd-wiphy.h b/src/network/networkd-wiphy.h index 5f0f47a230..b9056e802c 100644 --- a/src/network/networkd-wiphy.h +++ b/src/network/networkd-wiphy.h @@ -7,8 +7,19 @@ #include "macro.h" +typedef struct Link Link; typedef struct Manager Manager; +/* The following values are different from the ones defined in linux/rfkill.h. */ +typedef enum RFKillState { + RFKILL_UNKNOWN, + RFKILL_UNBLOCKED, + RFKILL_SOFT, + RFKILL_HARD, + _RFKILL_STATE_MAX, + _RFKILL_STATE_INVALID = -EINVAL, +} RFKillState; + typedef struct Wiphy { Manager *manager; @@ -17,6 +28,7 @@ typedef struct Wiphy { sd_device *dev; sd_device *rfkill; + RFKillState rfkill_state; } Wiphy; Wiphy *wiphy_free(Wiphy *w); @@ -25,6 +37,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Wiphy*, wiphy_free); int wiphy_get_by_index(Manager *manager, uint32_t index, Wiphy **ret); int wiphy_get_by_name(Manager *manager, const char *name, Wiphy **ret); +int link_rfkilled(Link *link); + int manager_genl_process_nl80211_wiphy(sd_netlink *genl, sd_netlink_message *message, Manager *manager); int manager_udev_process_wiphy(Manager *m, sd_device *device, sd_device_action_t action); int manager_udev_process_rfkill(Manager *m, sd_device *device, sd_device_action_t action);