network: make 'networkctl reconfigure' work safely even when KeepConfiguration=dhcp or yes

Previously, even if KeepConfiguration=dhcp or yes is specified in the
new .network file, dynamic configurations like DHCP address and routes
were dropped when 'networkctl reconfigure INTERFACE' is invoked.

If the setting is specified, let's gracefully handle the dynamic
configurations. Then, 'networkctl reconfigure' can be also used for
an interface that has critical connections.

7 files changed, 99 insertions, 57 deletions

diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c
index 7a5d0e0415..16426de981 100644
--- a/src/network/networkd-dhcp-prefix-delegation.c
+++ b/src/network/networkd-dhcp-prefix-delegation.c
@@ -1294,14 +1294,21 @@ int dhcp_request_prefix_delegation(Link *link) {
 
 int link_drop_dhcp_pd_config(Link *link, Network *network) {
         assert(link);
-        assert(network);
+        assert(link->network);
 
-        if (!link_dhcp_pd_is_enabled(link))
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
+
+        if (!link_dhcp_pd_is_enabled(link)) /* Disabled now, drop all configs. */
+                return dhcp_pd_remove(link, /* only_marked = */ false);
+
+        /* If previously explicitly disabled, then there is nothing we need to drop.
+         * If this is called on start up, we do not know the previous settings, assume nothing changed. */
+        if (!network || !network->dhcp_pd)
                 return 0;
 
-        /* If will be disabled or at least one config is changed, then drop all configurations. */
-        if (!network->dhcp_pd ||
-            link->network->dhcp_pd_assign != network->dhcp_pd_assign ||
+        /* If at least one setting is changed, then drop all configurations. */
+        if (link->network->dhcp_pd_assign != network->dhcp_pd_assign ||
             (link->network->dhcp_pd_assign &&
              (link->network->dhcp_pd_manage_temporary_address != network->dhcp_pd_manage_temporary_address ||
               !set_equal(link->network->dhcp_pd_tokens, network->dhcp_pd_tokens))) ||
diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c
index dae1a7b7f4..684580f5b6 100644
--- a/src/network/networkd-dhcp4.c
+++ b/src/network/networkd-dhcp4.c
@@ -1837,15 +1837,22 @@ int link_drop_dhcp4_config(Link *link, Network *network) {
         int ret = 0;
 
         assert(link);
-        assert(network);
+        assert(link->network);
 
-        if (!link_dhcp4_enabled(link))
-                return 0; /* Currently DHCPv4 client is not enabled, there is nothing we need to drop. */
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
 
-        if (!FLAGS_SET(network->dhcp, ADDRESS_FAMILY_IPV4))
-                /* Currently enabled but will be disabled. Stop the client and drop the lease. */
+        if (!link_dhcp4_enabled(link)) {
+                /* DHCP client is disabled. Stop the client if it is running and drop the lease. */
                 ret = sd_dhcp_client_stop(link->dhcp_client);
 
+                /* Also explicitly drop DHCPv4 address and routes. Why? This is for the case when the DHCPv4
+                 * client was enabled on the previous invocation of networkd, but when it is restarted, a new
+                 * .network file may match to the interface, and DHCPv4 client may be disabled. In that case,
+                 * the DHCPv4 client is not running, hence sd_dhcp_client_stop() in the above does nothing. */
+                RET_GATHER(ret, dhcp4_remove_address_and_routes(link, /* only_marked = */ false));
+        }
+
         /* Even if the client is currently enabled and also enabled in the new .network file, detailed
          * settings for the client may be different. Let's unref() the client. But do not unref() the lease.
          * it will be unref()ed later when a new lease is acquired. */
diff --git a/src/network/networkd-dhcp6.c b/src/network/networkd-dhcp6.c
index a6808fdb75..31a3fbc8e8 100644
--- a/src/network/networkd-dhcp6.c
+++ b/src/network/networkd-dhcp6.c
@@ -844,15 +844,19 @@ int link_drop_dhcp6_config(Link *link, Network *network) {
         int ret = 0;
 
         assert(link);
-        assert(network);
+        assert(link->network);
 
-        if (!link_dhcp6_enabled(link))
-                return 0; /* Currently DHCPv6 client is not enabled, there is nothing we need to drop. */
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
 
-        if (!FLAGS_SET(network->dhcp, ADDRESS_FAMILY_IPV6))
-                /* Currently enabled but will be disabled. Stop the client and drop the lease. */
+        if (!link_dhcp6_enabled(link)) {
+                /* DHCPv6 client is disabled. Stop the client if it is running and drop the lease. */
                 ret = sd_dhcp6_client_stop(link->dhcp6_client);
 
+                /* Also explicitly drop DHCPv6 addresses and routes. See also link_drop_dhcp4_config(). */
+                RET_GATHER(ret, dhcp6_remove(link, /* only_marked = */ false));
+        }
+
         /* Even if the client is currently enabled and also enabled in the new .network file, detailed
          * settings for the client may be different. Let's unref() the client. But do not unref() the lease.
          * it will be unref()ed later when a new lease is acquired. */
diff --git a/src/network/networkd-ipv4ll.c b/src/network/networkd-ipv4ll.c
index 094cee6a14..7398cefe77 100644
--- a/src/network/networkd-ipv4ll.c
+++ b/src/network/networkd-ipv4ll.c
@@ -206,7 +206,7 @@ int ipv4ll_configure(Link *link) {
                 return 0;
 
         if (link->ipv4ll)
-                return -EBUSY;
+                return 0;
 
         r = sd_ipv4ll_new(&link->ipv4ll);
         if (r < 0)
@@ -246,19 +246,27 @@ int link_drop_ipv4ll_config(Link *link, Network *network) {
         int ret = 0;
 
         assert(link);
-        assert(network);
-
-        if (!link_ipv4ll_enabled(link))
-                return 0;
+        assert(link->network);
 
-        Network *saved = link->network;
-        link->network = network;
-        bool enabled = link_ipv4ll_enabled(link);
-        link->network = saved;
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
 
-        if (!enabled)
+        if (!link_ipv4ll_enabled(link)) {
+                /* The client is disabled. Stop if it is running, and drop the address. */
                 ret = sd_ipv4ll_stop(link->ipv4ll);
 
+                /* Also, explicitly drop the address for the case that this is called on start up.
+                 * See also comments in link_drop_dhcp4_config(). */
+                Address *a;
+                SET_FOREACH(a, link->addresses) {
+                        if (a->source != NETWORK_CONFIG_SOURCE_IPV4LL)
+                                continue;
+
+                        assert(a->family == AF_INET);
+                        RET_GATHER(ret, address_remove_and_cancel(a, link));
+                }
+        }
+
         link->ipv4ll = sd_ipv4ll_unref(link->ipv4ll);
         return ret;
 }
diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 72765eebd8..ef314975de 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -1121,7 +1121,7 @@ static int link_drop_dynamic_config(Link *link, Network *network) {
         int r;
 
         assert(link);
-        assert(network);
+        assert(link->network);
 
         /* Drop unnecessary dynamic configurations gracefully, e.g. drop DHCP lease in the case that
          * previously DHCP=yes and now DHCP=no, but keep DHCP lease when DHCP setting is unchanged. */
@@ -1394,22 +1394,7 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) {
                               joined,
                               isempty(joined) ? "" : ")");
 
-        /* Dropping old .network file */
-
-        if (FLAGS_SET(flags, LINK_RECONFIGURE_CLEANLY)) {
-                /* Stop DHCP client and friends, and drop dynamic configurations like DHCP address. */
-                r = link_stop_engines(link, /* may_keep_dhcp = */ false);
-                if (r < 0)
-                        return r;
-
-                /* Free DHCP client and friends. */
-                link_free_engines(link);
-        } else {
-                r = link_drop_dynamic_config(link, network);
-                if (r < 0)
-                        return r;
-        }
-
+        /* Dropping configurations based on the old .network file. */
         r = link_drop_requests(link);
         if (r < 0)
                 return r;
@@ -1420,10 +1405,30 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) {
          * map here, as it depends on .network files assigned to other links. */
         link_free_bound_to_list(link);
 
-        link->network = network_unref(link->network);
+        _cleanup_(network_unrefp) Network *old_network = TAKE_PTR(link->network);
 
         /* Then, apply new .network file */
         link->network = network_ref(network);
+
+        if (FLAGS_SET(network->keep_configuration, KEEP_CONFIGURATION_DHCP) ||
+            !FLAGS_SET(flags, LINK_RECONFIGURE_CLEANLY)) {
+                /* To make 'networkctl reconfigure INTERFACE' work safely for an interface whose new .network
+                 * file has KeepConfiguration=dhcp or yes, even if a clean reconfiguration is requested,
+                 * drop only unnecessary or possibly being changed dynamic configurations here. */
+                r = link_drop_dynamic_config(link, old_network);
+                if (r < 0)
+                        return r;
+        } else {
+                /* Otherwise, stop DHCP client and friends unconditionally, and drop all dynamic
+                 * configurations like DHCP address and routes. */
+                r = link_stop_engines(link, /* may_keep_dhcp = */ false);
+                if (r < 0)
+                        return r;
+
+                /* Free DHCP client and friends. */
+                link_free_engines(link);
+        }
+
         link_update_operstate(link, true);
         link_dirty(link);
 
diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c
index 5ab9c881f2..c1d2373f65 100644
--- a/src/network/networkd-ndisc.c
+++ b/src/network/networkd-ndisc.c
@@ -2672,28 +2672,39 @@ int link_drop_ndisc_config(Link *link, Network *network) {
         int r, ret = 0;
 
         assert(link);
-        assert(network);
+        assert(link->network);
 
-        if (!link_ndisc_enabled(link))
-                return 0; /* Currently DHCPv4 client is not enabled, there is nothing we need to drop. */
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
 
-        Network *current = link->network;
-        link->network = network;
-        bool enabled = link_ndisc_enabled(link);
-        link->network = current;
-
-        if (!enabled) {
-                /* Currently enabled but will be disabled. Stop the client and flush configs. */
+        if (!link_ndisc_enabled(link)) {
+                /* NDisc is disabled. Stop the client if it is running and flush configs. */
                 ret = ndisc_stop(link);
                 ndisc_flush(link);
                 link->ndisc = sd_ndisc_unref(link->ndisc);
                 return ret;
         }
 
-        /* Even if the client is currently enabled and also enabled in the new .network file, detailed
+        /* Even if the client was previously enabled and also enabled in the new .network file, detailed
          * settings for the client may be different. Let's unref() the client. */
         link->ndisc = sd_ndisc_unref(link->ndisc);
 
+        /* Get if NDisc was enabled or not. */
+        Network *current = link->network;
+        link->network = network;
+        bool enabled = link_ndisc_enabled(link);
+        link->network = current;
+
+        /* If previously explicitly disabled, there should be nothing to drop.
+         * If we do not know the previous setting of the client, e.g. when networkd is restarted, in that
+         * case we do not have the previous .network file assigned to the interface, then  let's assume no
+         * detailed configuration is changed. Hopefully, unmatching configurations will be dropped after
+         * their lifetime. */
+        if (!enabled)
+                return 0;
+
+        assert(network);
+
         /* Redirect messages will be ignored. Drop configurations based on the previously received redirect
          * messages. */
         if (!network->ndisc_use_redirect)
diff --git a/src/network/networkd-radv.c b/src/network/networkd-radv.c
index a284e2c47e..deb37c4850 100644
--- a/src/network/networkd-radv.c
+++ b/src/network/networkd-radv.c
@@ -649,10 +649,10 @@ int link_drop_radv_config(Link *link, Network *network) {
         int ret = 0;
 
         assert(link);
-        assert(network);
+        assert(link->network);
 
-        if (!link_radv_enabled(link))
-                return 0;
+        if (link->network == network)
+                return 0; /* .network file is unchanged. It is not necessary to reconfigure the server. */
 
         // FIXME: check detailed settings and do not stop if nothing changed.
         // FIXME: save dynamic prefixes acquired by DHCP-PD.