Merge pull request #17342 from yuwata/network-dhcp-ipv4-acd-fixes

network: fixes several issues in IPv4 DAD for DHCP4

src/network/networkd-dhcp4.c

@@ -563,6 +563,9 @@ static int dhcp_lease_lost(Link *link) {
         link->dhcp_lease = sd_dhcp_lease_unref(link->dhcp_lease);
         link_dirty(link);
 
+        if (link->dhcp_acd)
+                (void) sd_ipv4acd_stop(link->dhcp_acd);
+
         return r;
 }
 
@@ -617,26 +620,61 @@ static void dhcp_address_on_acd(sd_ipv4acd *acd, int event, void *userdata) {
         return;
 }
 
-static int configure_dhcpv4_duplicate_address_detection(Link *link) {
+static int dhcp4_configure_dad(Link *link) {
+        int r;
+
+        assert(link);
+        assert(link->manager);
+        assert(link->network);
+
+        if (!link->network->dhcp_send_decline)
+                return 0;
+
+        if (!link->dhcp_acd) {
+                r = sd_ipv4acd_new(&link->dhcp_acd);
+                if (r < 0)
+                        return r;
+
+                r = sd_ipv4acd_attach_event(link->dhcp_acd, link->manager->event, 0);
+                if (r < 0)
+                        return r;
+        }
+
+        r = sd_ipv4acd_set_ifindex(link->dhcp_acd, link->ifindex);
+        if (r < 0)
+                return r;
+
+        r = sd_ipv4acd_set_mac(link->dhcp_acd, &link->mac);
+        if (r < 0)
+                return r;
+
+        return 0;
+}
+
+static int dhcp4_dad_update_mac(Link *link) {
+        bool running;
         int r;
 
         assert(link);
 
-        r = sd_ipv4acd_new(&link->network->dhcp_acd);
+        if (!link->dhcp_acd)
+                return 0;
+
+        running = sd_ipv4acd_is_running(link->dhcp_acd);
+
+        r = sd_ipv4acd_stop(link->dhcp_acd);
         if (r < 0)
                 return r;
 
-        r = sd_ipv4acd_attach_event(link->network->dhcp_acd, link->manager->event, 0);
+        r = sd_ipv4acd_set_mac(link->dhcp_acd, &link->mac);
         if (r < 0)
                 return r;
 
-        r = sd_ipv4acd_set_ifindex(link->network->dhcp_acd, link->ifindex);
-        if (r < 0)
-                return r;
-
-        r = sd_ipv4acd_set_mac(link->network->dhcp_acd, &link->mac);
-        if (r < 0)
-                return r;
+        if (running) {
+                r = sd_ipv4acd_start(link->dhcp_acd, true);
+                if (r < 0)
+                        return r;
+        }
 
         return 0;
 }
@@ -652,7 +690,7 @@ static int dhcp4_start_acd(Link *link) {
         if (!link->dhcp_lease)
                 return 0;
 
-        (void) sd_ipv4acd_stop(link->network->dhcp_acd);
+        (void) sd_ipv4acd_stop(link->dhcp_acd);
 
         link->dhcp4_address_bind = false;
 
@@ -660,15 +698,15 @@ static int dhcp4_start_acd(Link *link) {
         if (r < 0)
                 return r;
 
-        r = sd_ipv4acd_get_address(link->network->dhcp_acd, &old);
+        r = sd_ipv4acd_get_address(link->dhcp_acd, &old);
         if (r < 0)
                 return r;
 
-        r = sd_ipv4acd_set_address(link->network->dhcp_acd, &addr.in);
+        r = sd_ipv4acd_set_address(link->dhcp_acd, &addr.in);
         if (r < 0)
                 return r;
 
-        r = sd_ipv4acd_set_callback(link->network->dhcp_acd, dhcp_address_on_acd, link);
+        r = sd_ipv4acd_set_callback(link->dhcp_acd, dhcp_address_on_acd, link);
         if (r < 0)
                 return r;
 
@@ -679,7 +717,7 @@ static int dhcp4_start_acd(Link *link) {
                 log_link_debug(link, "Starting IPv4ACD client. Probing DHCPv4 address %s", strna(pretty));
         }
 
-        r = sd_ipv4acd_start(link->network->dhcp_acd, !in4_addr_equal(&addr.in, &old));
+        r = sd_ipv4acd_start(link->dhcp_acd, !in4_addr_equal(&addr.in, &old));
         if (r < 0)
                 return r;
 
@@ -1425,11 +1463,9 @@ int dhcp4_configure(Link *link) {
                         return log_link_error_errno(link, r, "DHCP4 CLIENT: Failed set to lease lifetime: %m");
         }
 
-        if (link->network->dhcp_send_decline) {
-                r = configure_dhcpv4_duplicate_address_detection(link);
-                if (r < 0)
-                        return log_link_error_errno(link, r, "DHCP4 CLIENT: Failed to configure service type: %m");
-        }
+        r = dhcp4_configure_dad(link);
+        if (r < 0)
+                return log_link_error_errno(link, r, "DHCP4 CLIENT: Failed to configure service type: %m");
 
         return dhcp4_set_client_identifier(link);
 }
@@ -1450,6 +1486,10 @@ int dhcp4_update_mac(Link *link) {
         if (r < 0)
                 return r;
 
+        r = dhcp4_dad_update_mac(link);
+        if (r < 0)
+                return r;
+
         return 0;
 }
 

src/network/networkd-link.c

@@ -503,6 +503,7 @@ static void link_free_engines(Link *link) {
         link->dhcp6_lease = sd_dhcp6_lease_unref(link->dhcp6_lease);
         link->ndisc = sd_ndisc_unref(link->ndisc);
         link->radv = sd_radv_unref(link->radv);
+        link->dhcp_acd = sd_ipv4acd_unref(link->dhcp_acd);
 }
 
 static Link *link_free(Link *link) {
@@ -622,6 +623,12 @@ int link_stop_clients(Link *link, bool may_keep_dhcp) {
                         r = log_link_warning_errno(link, k, "Could not stop DHCPv4 client: %m");
         }
 
+        if (link->dhcp_acd) {
+                k = sd_ipv4acd_stop(link->dhcp_acd);
+                if (k < 0)
+                        r = log_link_warning_errno(link, k, "Could not stop IPv4 ACD client for DHCPv4: %m");
+        }
+
         if (link->ipv4ll) {
                 k = sd_ipv4ll_stop(link->ipv4ll);
                 if (k < 0)
@@ -668,7 +675,7 @@ void link_enter_failed(Link *link) {
 
         link_set_state(link, LINK_STATE_FAILED);
 
-        link_stop_clients(link, false);
+        (void) link_stop_clients(link, false);
 
         link_dirty(link);
 }

src/network/networkd-link.h

@@ -9,6 +9,7 @@
 #include "sd-dhcp-client.h"
 #include "sd-dhcp-server.h"
 #include "sd-dhcp6-client.h"
+#include "sd-ipv4acd.h"
 #include "sd-ipv4ll.h"
 #include "sd-lldp.h"
 #include "sd-ndisc.h"
@@ -105,6 +106,7 @@ typedef struct Link {
         uint32_t original_mtu;
         unsigned dhcp4_messages;
         unsigned dhcp4_remove_messages;
+        sd_ipv4acd *dhcp_acd;
         bool dhcp4_route_failed:1;
         bool dhcp4_route_retrying:1;
         bool dhcp4_configured:1;

src/network/networkd-network.c

@@ -636,9 +636,6 @@ static Network *network_free(Network *network) {
         strv_free(network->dhcp6_user_class);
         strv_free(network->dhcp6_vendor_class);
 
-        if (network->dhcp_acd)
-                sd_ipv4acd_unref(network->dhcp_acd);
-
         strv_free(network->ntp);
         for (unsigned i = 0; i < network->n_dns; i++)
                 in_addr_full_free(network->dns[i]);

src/network/networkd-network.h

@@ -5,7 +5,6 @@
 
 #include "sd-bus.h"
 #include "sd-device.h"
-#include "sd-ipv4acd.h"
 
 #include "bridge.h"
 #include "condition.h"
@@ -123,7 +122,6 @@ struct Network {
         bool dhcp_send_release;
         bool dhcp_send_decline;
         DHCPUseDomains dhcp_use_domains;
-        sd_ipv4acd *dhcp_acd;
         Set *dhcp_deny_listed_ip;
         Set *dhcp_allow_listed_ip;
         Set *dhcp_request_options;