From: Yu Watanabe Date: Mon, 4 Nov 2024 19:04:33 +0000 (+0900) Subject: network: several cleanups for link_reconfigure() X-Git-Tag: v257-rc1~18^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2b07a3211ba8b1b81d6cebb9650d5cb24554b08a;p=thirdparty%2Fsystemd.git network: several cleanups for link_reconfigure() Effectively no functional changes, just refactoring and preparation for later changes. - convert boolean flag 'force' to LinkReconfigurationFlag enum, - merge link_reconfigure() and reconfigure_handler_on_bus_method_reload() as link_reconfigure_full(), - Rename ReconfigureData -> LinkReconfigurationData, - make Reconfigure() DBus message wait for reconfiguration being started before sending reply. --- diff --git a/src/network/networkd-link-bus.c b/src/network/networkd-link-bus.c index cb114929faf..654aea36a27 100644 --- a/src/network/networkd-link-bus.c +++ b/src/network/networkd-link-bus.c @@ -665,11 +665,9 @@ int bus_link_method_reconfigure(sd_bus_message *message, void *userdata, sd_bus_ if (r == 0) return 1; /* Polkit will call us back */ - r = link_reconfigure(l, /* force = */ true); - if (r > 0) - r = link_save_and_clean_full(l, /* also_save_manager = */ true); - if (r < 0) - return r; + r = link_reconfigure_full(l, LINK_RECONFIGURE_UNCONDITIONALLY, message, /* counter = */ NULL); + if (r != 0) + return r; /* Will reply later when r > 0. */ return sd_bus_reply_method_return(message, NULL); } diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 44a6efb6267..40b65c52eee 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -445,7 +445,7 @@ void link_enter_failed(Link *link) { } log_link_info(link, "Trying to reconfigure the interface."); - if (link_reconfigure(link, /* force = */ true) > 0) + if (link_reconfigure(link, LINK_RECONFIGURE_UNCONDITIONALLY) > 0) return; stop: @@ -1344,7 +1344,7 @@ static void link_enter_unmanaged(Link *link) { link_set_state(link, LINK_STATE_UNMANAGED); } -int link_reconfigure_impl(Link *link, bool force) { +int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) { Network *network = NULL; int r; @@ -1367,7 +1367,7 @@ int link_reconfigure_impl(Link *link, bool force) { if (r < 0) return r; - if (link->network == network && !force) + if (link->network == network && !FLAGS_SET(flags, LINK_RECONFIGURE_UNCONDITIONALLY)) return 0; _cleanup_free_ char *joined = strv_join(network->dropins, ", "); @@ -1393,7 +1393,7 @@ int link_reconfigure_impl(Link *link, bool force) { if (r < 0) return r; - if (!force && network->keep_configuration != KEEP_CONFIGURATION_YES) + if (!FLAGS_SET(flags, LINK_RECONFIGURE_UNCONDITIONALLY) && network->keep_configuration != KEEP_CONFIGURATION_YES) /* When a new/updated .network file is assigned, first make all configs (addresses, * routes, and so on) foreign, and then drop unnecessary configs later by * link_drop_foreign_config() in link_configure(). @@ -1433,63 +1433,14 @@ int link_reconfigure_impl(Link *link, bool force) { return 1; } -static int link_reconfigure_handler_internal(sd_netlink *rtnl, sd_netlink_message *m, Link *link, bool force) { - int r; - - assert(link); - - r = link_getlink_handler_internal(rtnl, m, link, "Failed to update link state"); - if (r <= 0) - return r; - - r = link_reconfigure_impl(link, force); - if (r < 0) { - link_enter_failed(link); - return 0; - } - - return r; -} - -static int link_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return link_reconfigure_handler_internal(rtnl, m, link, /* force = */ false); -} - -static int link_force_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { - return link_reconfigure_handler_internal(rtnl, m, link, /* force = */ true); -} - -int link_reconfigure(Link *link, bool force) { - int r; - - assert(link); - - /* When link in pending or initialized state, then link_configure() will be called. To prevent - * the function from being called multiple times simultaneously, refuse to reconfigure the - * interface in these cases. */ - if (IN_SET(link->state, LINK_STATE_PENDING, LINK_STATE_INITIALIZED, LINK_STATE_LINGER)) - return 0; /* 0 means no-op. */ - - r = link_call_getlink(link, force ? link_force_reconfigure_handler : link_reconfigure_handler); - if (r < 0) { - log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); - link_enter_failed(link); - return r; - } - - if (force || link->state == LINK_STATE_FAILED) - link_set_state(link, LINK_STATE_INITIALIZED); - - return 1; /* 1 means the interface will be reconfigured. */ -} - -typedef struct ReconfigureData { +typedef struct LinkReconfigurationData { Link *link; - Manager *manager; + LinkReconfigurationFlag flags; sd_bus_message *message; -} ReconfigureData; + unsigned *counter; +} LinkReconfigurationData; -static ReconfigureData* reconfigure_data_free(ReconfigureData *data) { +static LinkReconfigurationData* link_reconfiguration_data_free(LinkReconfigurationData *data) { if (!data) return NULL; @@ -1499,56 +1450,72 @@ static ReconfigureData* reconfigure_data_free(ReconfigureData *data) { return mfree(data); } -DEFINE_TRIVIAL_CLEANUP_FUNC(ReconfigureData*, reconfigure_data_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(LinkReconfigurationData*, link_reconfiguration_data_free); -static void reconfigure_data_destroy_callback(ReconfigureData *data) { +static void link_reconfiguration_data_destroy_callback(LinkReconfigurationData *data) { int r; assert(data); - assert(data->manager); - assert(data->manager->reloading > 0); - assert(data->message); - data->manager->reloading--; - if (data->manager->reloading <= 0) { - r = sd_bus_reply_method_return(data->message, NULL); - if (r < 0) - log_warning_errno(r, "Failed to send reply for 'Reload' DBus method, ignoring: %m"); + if (data->message) { + if (data->counter) { + assert(*data->counter > 0); + (*data->counter)--; + } + + if (!data->counter || *data->counter <= 0) { + r = sd_bus_reply_method_return(data->message, NULL); + if (r < 0) + log_warning_errno(r, "Failed to reply for DBus method, ignoring: %m"); + } } - reconfigure_data_free(data); + link_reconfiguration_data_free(data); } -static int reconfigure_handler_on_bus_method_reload(sd_netlink *rtnl, sd_netlink_message *m, ReconfigureData *data) { - assert(data); - assert(data->link); - return link_reconfigure_handler_internal(rtnl, m, data->link, /* force = */ false); +static int link_reconfigure_handler(sd_netlink *rtnl, sd_netlink_message *m, LinkReconfigurationData *data) { + Link *link = ASSERT_PTR(ASSERT_PTR(data)->link); + int r; + + r = link_getlink_handler_internal(rtnl, m, link, "Failed to update link state"); + if (r <= 0) + return r; + + r = link_reconfigure_impl(link, data->flags); + if (r < 0) { + link_enter_failed(link); + return 0; + } + + return r; } -int link_reconfigure_on_bus_method_reload(Link *link, sd_bus_message *message) { +int link_reconfigure_full(Link *link, LinkReconfigurationFlag flags, sd_bus_message *message, unsigned *counter) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; - _cleanup_(reconfigure_data_freep) ReconfigureData *data = NULL; + _cleanup_(link_reconfiguration_data_freep) LinkReconfigurationData *data = NULL; int r; assert(link); assert(link->manager); assert(link->manager->rtnl); - assert(message); - /* See comments in link_reconfigure() above. */ + /* When the link is in the pending or initialized state, link_reconfigure_impl() will be called later + * by link_initialized() or link_initialized_and_synced(). To prevent the function from being called + * multiple times simultaneously, let's refuse to reconfigure the interface here in such cases. */ if (IN_SET(link->state, LINK_STATE_PENDING, LINK_STATE_INITIALIZED, LINK_STATE_LINGER)) - return 0; + return 0; /* 0 means no-op. */ - data = new(ReconfigureData, 1); + data = new(LinkReconfigurationData, 1); if (!data) { r = -ENOMEM; goto failed; } - *data = (ReconfigureData) { + *data = (LinkReconfigurationData) { .link = link_ref(link), - .manager = link->manager, - .message = sd_bus_message_ref(message), + .flags = flags, + .message = sd_bus_message_ref(message), /* message may be NULL, but _ref() works fine. */ + .counter = counter, }; r = sd_rtnl_message_new_link(link->manager->rtnl, &req, RTM_GETLINK, link->ifindex); @@ -1556,18 +1523,19 @@ int link_reconfigure_on_bus_method_reload(Link *link, sd_bus_message *message) { goto failed; r = netlink_call_async(link->manager->rtnl, NULL, req, - reconfigure_handler_on_bus_method_reload, - reconfigure_data_destroy_callback, data); + link_reconfigure_handler, + link_reconfiguration_data_destroy_callback, data); if (r < 0) goto failed; TAKE_PTR(data); - link->manager->reloading++; + if (counter) + (*counter)++; if (link->state == LINK_STATE_FAILED) link_set_state(link, LINK_STATE_INITIALIZED); - return 0; + return 1; /* 1 means the interface will be reconfigured, and bus method will be replied later. */ failed: log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); @@ -1594,7 +1562,7 @@ static int link_initialized_and_synced(Link *link) { return r; } - return link_reconfigure_impl(link, /* force = */ false); + return link_reconfigure_impl(link, /* flags = */ 0); } static int link_initialized_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *link) { @@ -1638,7 +1606,7 @@ static int link_initialized(Link *link, sd_device *device) { log_link_warning_errno(link, r, "Failed to manage SR-IOV PF and VF ports, ignoring: %m"); if (link->state != LINK_STATE_PENDING) - return link_reconfigure(link, /* force = */ false); + return link_reconfigure(link, /* flags = */ 0); log_link_debug(link, "udev initialized link"); @@ -1740,7 +1708,7 @@ int manager_udev_process_link(Manager *m, sd_device *device, sd_device_action_t } static int link_carrier_gained(Link *link) { - bool force_reconfigure; + LinkReconfigurationFlag flags = 0; int r; assert(link); @@ -1766,11 +1734,12 @@ static int link_carrier_gained(Link *link) { * already assigned to the interface (in that case, the .network file does not have the SSID= * setting in the [Match] section), and the interface is already being configured. Of course, * there may exist another .network file with higher priority and a matching SSID= setting. But - * in that case, link_reconfigure_impl() can handle that without the force_reconfigure flag. + * in that case, link_reconfigure_impl() can handle that without any flags. * * For non-wireless interfaces, we have no way to detect the connected network change. So, - * setting force_reconfigure = false. Note, both ssid and previous_ssid are NULL in that case. */ - force_reconfigure = link->previous_ssid && !streq_ptr(link->previous_ssid, link->ssid); + * we do not set any flags here. Note, both ssid and previous_ssid are NULL in that case. */ + if (link->previous_ssid && !streq_ptr(link->previous_ssid, link->ssid)) + flags |= LINK_RECONFIGURE_UNCONDITIONALLY; link->previous_ssid = mfree(link->previous_ssid); /* AP and P2P-GO interfaces may have a new SSID - update the link properties in case a new .network @@ -1785,7 +1754,7 @@ static int link_carrier_gained(Link *link) { /* At this stage, both wlan and link information should be up-to-date. Hence, it is not necessary to * call RTM_GETLINK, NL80211_CMD_GET_INTERFACE, or NL80211_CMD_GET_STATION commands, and simply call * link_reconfigure_impl(). Note, link_reconfigure_impl() returns 1 when the link is reconfigured. */ - r = link_reconfigure_impl(link, force_reconfigure); + r = link_reconfigure_impl(link, flags); if (r != 0) return r; @@ -2880,7 +2849,7 @@ int manager_rtnl_process_link(sd_netlink *rtnl, sd_netlink_message *message, Man if (manager->enumerating) return 0; - r = link_reconfigure_impl(link, /* force = */ false); + r = link_reconfigure_impl(link, /* flags = */ 0); if (r < 0) { log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); link_enter_failed(link); diff --git a/src/network/networkd-link.h b/src/network/networkd-link.h index 1c5c9ec7096..9ae7e1c8da4 100644 --- a/src/network/networkd-link.h +++ b/src/network/networkd-link.h @@ -42,6 +42,10 @@ typedef enum LinkState { _LINK_STATE_INVALID = -EINVAL, } LinkState; +typedef enum LinkReconfigurationFlag { + LINK_RECONFIGURE_UNCONDITIONALLY = 1 << 0, /* Reconfigure an interface even if .network file is unchanged. */ +} LinkReconfigurationFlag; + typedef struct Manager Manager; typedef struct Network Network; typedef struct NetDev NetDev; @@ -259,9 +263,11 @@ LinkState link_state_from_string(const char *s) _pure_; int link_request_stacked_netdevs(Link *link, NetDevLocalAddressType type); -int link_reconfigure_impl(Link *link, bool force); -int link_reconfigure(Link *link, bool force); -int link_reconfigure_on_bus_method_reload(Link *link, sd_bus_message *message); +int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags); +int link_reconfigure_full(Link *link, LinkReconfigurationFlag flags, sd_bus_message *message, unsigned *counter); +static inline int link_reconfigure(Link *link, LinkReconfigurationFlag flags) { + return link_reconfigure_full(link, flags, NULL, NULL); +} int link_check_initialized(Link *link); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 476e02fd28f..f8c0da4b428 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -89,7 +89,7 @@ static int match_prepare_for_sleep(sd_bus_message *message, void *userdata, sd_b log_debug("Coming back from suspend, reconfiguring all connections..."); HASHMAP_FOREACH(link, m->links_by_index) - (void) link_reconfigure(link, /* force = */ true); + (void) link_reconfigure(link, LINK_RECONFIGURE_UNCONDITIONALLY); return 0; } @@ -1219,12 +1219,9 @@ int manager_reload(Manager *m, sd_bus_message *message) { goto finish; } - HASHMAP_FOREACH(link, m->links_by_index) { - if (message) - (void) link_reconfigure_on_bus_method_reload(link, message); - else - (void) link_reconfigure(link, /* force = */ false); - } + HASHMAP_FOREACH(link, m->links_by_index) + (void) link_reconfigure_full(link, /* flags = */ 0, message, + /* counter = */ message ? &m->reloading : NULL); log_debug("Reloaded."); r = 0; diff --git a/src/network/networkd-wifi.c b/src/network/networkd-wifi.c index ee63c3ec964..bc1d79adcef 100644 --- a/src/network/networkd-wifi.c +++ b/src/network/networkd-wifi.c @@ -287,7 +287,7 @@ int manager_genl_process_nl80211_mlme(sd_netlink *genl, sd_netlink_message *mess * To make SSID= or other WiFi related settings in [Match] section work, let's try to * reconfigure the interface. */ if (link->ssid && link_has_carrier(link)) { - r = link_reconfigure_impl(link, /* force = */ false); + r = link_reconfigure_impl(link, /* flags = */ 0); if (r < 0) { log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); link_enter_failed(link); @@ -326,7 +326,7 @@ int manager_genl_process_nl80211_mlme(sd_netlink *genl, sd_netlink_message *mess } /* If necessary, reconfigure based on those new properties */ - r = link_reconfigure_impl(link, /* force = */ false); + r = link_reconfigure_impl(link, /* flags = */ 0); if (r < 0) { log_link_warning_errno(link, r, "Failed to reconfigure interface: %m"); link_enter_failed(link);