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 cb114929faf838b1a637f4eb3fac0c3a1c953aed..654aea36a2796bd0c9192e1ed664b1f5c066fd9c 100644 (file)
--- 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 44a6efb62670b1af4efe1b5883870148987b7c4f..40b65c52eeec754a9a419683cfa087ff2d3dd2d1 100644 (file)
--- 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 1c5c9ec7096a561896ec99529bbb019dd5921ed0..9ae7e1c8da4f7750c8cc27aa485fef99ae908206 100644 (file)
--- 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 476e02fd28f9672ca2eb257ab392cc0ba738206b..f8c0da4b42802af5f1acc3e57253a561be1b51c9 100644 (file)
--- 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 ee63c3ec9649558e4f446283b164d4cf5a1b0e58..bc1d79adcef430d6ca6e57ab49c81a1d09c9bab2 100644 (file)
--- 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);