network/dhcp4: send release message before stopping the client

Otherwise, the socket is already closed and sending release will be
anyway skipped.

With this patch, release message is sent before stopping the client.
```
Jan 20 18:29:41 systemd[1]: Stopping systemd-networkd.service - Network Management...
Jan 20 18:29:41 systemd-networkd[3821255]: wlp59s0: DHCPv4 client: RELEASE
Jan 20 18:29:41 systemd-networkd[3821255]: wlp59s0: DHCPv4 client: STOPPED
Jan 20 18:29:41 systemd-networkd[3821255]: wlp59s0: DHCP lease lost
```

Fixes #39299.

diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c
index 48215e6d21f245072952096cfdc2e6d613988602..8c373146f61850f2dcc7dd95e71ca1342761d4cd 100644 (file)
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -103,6 +103,7 @@ struct sd_dhcp_client {
         bool socket_priority_set;
         bool ipv6_acquired;
         bool bootp;
+        bool send_release;
 };
 
 static const uint8_t default_req_opts[] = {
@@ -663,6 +664,14 @@ int sd_dhcp_client_set_bootp(sd_dhcp_client *client, int bootp) {
         return 0;
 }
 
+int sd_dhcp_client_set_send_release(sd_dhcp_client *client, int enable) {
+        assert_return(client, -EINVAL);
+
+        client->send_release = enable;
+
+        return 0;
+}
+
 static void client_set_state(sd_dhcp_client *client, DHCPState state) {
         assert(client);
 
@@ -2347,13 +2356,18 @@ int sd_dhcp_client_start(sd_dhcp_client *client) {
         return r;
 }
 
-int sd_dhcp_client_send_release(sd_dhcp_client *client) {
+static int client_send_release(sd_dhcp_client *client) {
         _cleanup_free_ DHCPPacket *release = NULL;
         size_t optoffset, optlen;
         int r;
 
-        if (!sd_dhcp_client_is_running(client) || !client->lease || client->bootp)
-                return 0; /* do nothing */
+        assert(client);
+
+        if (!client->send_release)
+                return 0; /* disabled */
+
+        if (!client->lease || client->bootp)
+                return 0; /* there is nothing to be released */
 
         r = client_message_init(client, DHCP_RELEASE, &release, &optlen, &optoffset);
         if (r < 0)
@@ -2377,12 +2391,7 @@ int sd_dhcp_client_send_release(sd_dhcp_client *client) {
                 return r;
 
         log_dhcp_client(client, "RELEASE");
-
-        /* This function is mostly called when stopping daemon. Hence, do not call client_stop() or
-         * client_restart(). Otherwise, the notification callback will be called again and we may easily
-         * enter an infinite loop. */
-        client_initialize(client);
-        return 1; /* sent and stopped. */
+        return 0;
 }
 
 int sd_dhcp_client_send_decline(sd_dhcp_client *client) {
@@ -2425,11 +2434,18 @@ int sd_dhcp_client_send_decline(sd_dhcp_client *client) {
 }
 
 int sd_dhcp_client_stop(sd_dhcp_client *client) {
+        int r;
+
         if (!client)
                 return 0;
 
         DHCP_CLIENT_DONT_DESTROY(client);
 
+        r = client_send_release(client);
+        if (r < 0)
+                log_dhcp_client_errno(client, r,
+                                      "Failed to send DHCP release message, ignoring: %m");
+
         client_stop(client, SD_DHCP_CLIENT_EVENT_STOP);
 
         return 0;

diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c
index 6ec8f8b2417e5dff8db9d64b171790834e7f584b..102047a4e4303ea21e374fef3c3247279feda739 100644 (file)
--- a/src/network/networkd-dhcp4.c
+++ b/src/network/networkd-dhcp4.c
@@ -1206,14 +1206,6 @@ static int dhcp4_handler(sd_dhcp_client *client, int event, void *userdata) {
                                 log_link_debug(link, "DHCP client is stopped. Acquiring IPv4 link-local address.");
 
                         if (link->dhcp_lease) {
-                                if (link->network->dhcp_send_release) {
-                                        r = sd_dhcp_client_send_release(client);
-                                        if (r < 0)
-                                                log_link_full_errno(link,
-                                                                    ERRNO_IS_DISCONNECT(r) ? LOG_DEBUG : LOG_WARNING,
-                                                                    r, "Failed to send DHCP RELEASE, ignoring: %m");
-                                }
-
                                 r = dhcp4_lease_lost(link);
                                 if (r < 0) {
                                         link_enter_failed(link);
@@ -1503,6 +1495,11 @@ static int dhcp4_configure(Link *link) {
                 return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to %s BOOTP: %m",
                                             enable_disable(link->network->dhcp_use_bootp));
 
+        r = sd_dhcp_client_set_send_release(link->dhcp_client, link->network->dhcp_send_release);
+        if (r < 0)
+                return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to %s sending release message on stop: %m",
+                                            enable_disable(link->network->dhcp_send_release));
+
         r = sd_dhcp_client_attach_event(link->dhcp_client, link->manager->event, 0);
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to attach event to DHCPv4 client: %m");
@@ -1865,8 +1862,27 @@ int link_request_dhcp4_client(Link *link) {
         return 0;
 }
 
+static bool link_should_drop_dhcp4_config(Link *link, Network *network) {
+        assert(link);
+        assert(link->network);
+
+        if (!link_dhcp4_enabled(link))
+                 /* DHCP client is now disabled. */
+                return true;
+
+        if (link->dhcp_client && link->network->dhcp_use_bootp &&
+            network && !network->dhcp_use_bootp && network->dhcp_send_release)
+                /* The client was enabled as a DHCP client and sending release message is requested, and now
+                 * the client is enabled as a BOOTP client. In this case, we need to release the previous
+                 * lease, and hence all DHCPv4 configurations (address, routes, DNS servers, and so on) needs
+                 * to be dropped. */
+                return true;
+
+        return false;
+}
+
 int link_drop_dhcp4_config(Link *link, Network *network) {
-        int r, ret = 0;
+        int ret = 0;
 
         assert(link);
         assert(link->network);
@@ -1874,8 +1890,8 @@ int link_drop_dhcp4_config(Link *link, Network *network) {
         if (link->network == network)
                 return 0; /* .network file is unchanged. It is not necessary to reconfigure the client. */
 
-        if (!link_dhcp4_enabled(link)) {
-                /* DHCP client is disabled. Stop the client if it is running and drop the lease. */
+        if (link_should_drop_dhcp4_config(link, network)) {
+                /* 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
@@ -1885,17 +1901,6 @@ int link_drop_dhcp4_config(Link *link, Network *network) {
                 RET_GATHER(ret, dhcp4_remove_address_and_routes(link, /* only_marked= */ false));
         }
 
-        if (link->dhcp_client && link->network->dhcp_use_bootp &&
-            network && !network->dhcp_use_bootp && network->dhcp_send_release) {
-                /* If the client was enabled as a DHCP client, and is now enabled as a BOOTP client, release
-                 * the previous lease. Note, this can be easily fail, e.g. when the interface is down. Hence,
-                 * ignore any failures here. */
-                r = sd_dhcp_client_send_release(link->dhcp_client);
-                if (r < 0)
-                        log_link_full_errno(link, ERRNO_IS_DISCONNECT(r) ? LOG_DEBUG : LOG_WARNING, r,
-                                            "Failed to send DHCP RELEASE, ignoring: %m");
-        }
-
         /* 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/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
index 9bcc00e145e0f2c68e7964e8d10d5584407f6899..b2995a961f32035668f15cf50ccf7496b675d062 100644 (file)
--- a/src/systemd/sd-dhcp-client.h
+++ b/src/systemd/sd-dhcp-client.h
@@ -148,6 +148,7 @@ int sd_dhcp_client_set_fallback_lease_lifetime(
 int sd_dhcp_client_set_bootp(
                 sd_dhcp_client *client,
                 int bootp);
+int sd_dhcp_client_set_send_release(sd_dhcp_client *client, int enable);
 
 int sd_dhcp_client_add_option(sd_dhcp_client *client, sd_dhcp_option *v);
 int sd_dhcp_client_add_vendor_option(sd_dhcp_client *client, sd_dhcp_option *v);
@@ -155,7 +156,6 @@ int sd_dhcp_client_add_vendor_option(sd_dhcp_client *client, sd_dhcp_option *v);
 int sd_dhcp_client_is_running(sd_dhcp_client *client);
 int sd_dhcp_client_stop(sd_dhcp_client *client);
 int sd_dhcp_client_start(sd_dhcp_client *client);
-int sd_dhcp_client_send_release(sd_dhcp_client *client);
 int sd_dhcp_client_send_decline(sd_dhcp_client *client);
 int sd_dhcp_client_send_renew(sd_dhcp_client *client);
 int sd_dhcp_client_set_ipv6_connectivity(sd_dhcp_client *client, int have);