ipv6ll: don't regenerate the address when it's removed externallybg/ipv6ll-removed

Currently if the IPv6 link-local address is removed after it passed
DAD, NetworkManager tries to generate a new link-local address. If
this fails, which is always the case for EUI64, ipv6ll is considered
as failed and the connection can go down (depending on may-fail).

This is particularly bad for virtual interfaces because if somebody
removes the link-local address, the activation can fail and destroy
the interface, breaking all services that require it. Also, it's a
change in behavior introduced in 1.36.0.

It seems that a better approach here is to re-add the address that was
removed externally.

Fixes: aa070fb82190 ('core: add NML3IPv6LL helper')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1622

1 files changed, 19 insertions, 11 deletions

diff --git a/src/core/nm-l3-ipv6ll.c b/src/core/nm-l3-ipv6ll.c
index 2e2a6a0a5a..9becd07caf 100644
--- a/src/core/nm-l3-ipv6ll.c
+++ b/src/core/nm-l3-ipv6ll.c
@@ -391,7 +391,7 @@ _pladdr_find_ll(NML3IPv6LL *self, gboolean *out_cur_addr_failed)
 /*****************************************************************************/
 
 static void
-_lladdr_handle_changed(NML3IPv6LL *self)
+_lladdr_handle_changed(NML3IPv6LL *self, gboolean force_commit)
 {
     const NML3ConfigData *l3cd;
     gboolean              changed = FALSE;
@@ -434,7 +434,7 @@ _lladdr_handle_changed(NML3IPv6LL *self)
                                                               self->l3cfg_commit_handle,
                                                               "ipv6ll");
 
-    if (changed)
+    if (changed || force_commit)
         nm_l3cfg_commit_on_idle_schedule(self->l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO);
 
     if (!self->emit_changed_idle_source) {
@@ -515,6 +515,7 @@ _check(NML3IPv6LL *self)
     const NMPlatformIP6Address *pladdr;
     char                        sbuf[INET6_ADDRSTRLEN];
     gboolean                    cur_addr_failed;
+    gboolean                    restarted = FALSE;
     struct in6_addr             lladdr;
 
     pladdr = _pladdr_find_ll(self, &cur_addr_failed);
@@ -526,14 +527,14 @@ _check(NML3IPv6LL *self)
             if (_set_cur_lladdr_obj(self, NM_L3_IPV6LL_STATE_DAD_IN_PROGRESS, pladdr)) {
                 _LOGT("changed: waiting for address %s to complete DAD",
                       nm_inet6_ntop(&self->cur_lladdr, sbuf));
-                _lladdr_handle_changed(self);
+                _lladdr_handle_changed(self, FALSE);
             }
             return;
         }
 
         if (_set_cur_lladdr_obj(self, NM_L3_IPV6LL_STATE_READY, pladdr)) {
             _LOGT("changed: address %s is ready", nm_inet6_ntop(&self->cur_lladdr, sbuf));
-            _lladdr_handle_changed(self);
+            _lladdr_handle_changed(self, FALSE);
         }
         return;
     }
@@ -543,11 +544,17 @@ _check(NML3IPv6LL *self)
          * Prematurely abort DAD to generate a new address below. */
         nm_assert(
             NM_IN_SET(self->state, NM_L3_IPV6LL_STATE_DAD_IN_PROGRESS, NM_L3_IPV6LL_STATE_READY));
-        if (self->state == NM_L3_IPV6LL_STATE_DAD_IN_PROGRESS)
-            _LOGT("changed: address %s did not complete DAD",
-                  nm_inet6_ntop(&self->cur_lladdr, sbuf));
-        else {
+
+        if (cur_addr_failed) {
+            /* On DAD failure, we always try to regenerate a new address. */
+            _LOGT("changed: address %s failed", nm_inet6_ntop(&self->cur_lladdr, sbuf));
+        } else {
             _LOGT("changed: address %s is gone", nm_inet6_ntop(&self->cur_lladdr, sbuf));
+            /* When the address is removed, we always try to re-add it. */
+            nm_clear_g_source_inst(&self->wait_for_addr_source);
+            lladdr    = self->cur_lladdr;
+            restarted = TRUE;
+            goto commit;
         }
 
         /* reset the state here, so that we are sure that the following
@@ -569,18 +576,19 @@ _check(NML3IPv6LL *self)
         if (_set_cur_lladdr_bin(self, NM_L3_IPV6LL_STATE_DAD_FAILED, NULL)) {
             _LOGW("changed: no IPv6 link local address to retry after Duplicate Address Detection "
                   "failures (back off)");
-            _lladdr_handle_changed(self);
+            _lladdr_handle_changed(self, FALSE);
         }
         return;
     }
 
+commit:
     /* we give NML3Cfg 2 seconds to configure the address on the interface. We
      * thus very soon expect to see this address configured (and kernel started DAD).
      * If that does not happen within timeout, we assume that this address failed DAD. */
     self->wait_for_addr_source = nm_g_timeout_add_source(2000, _wait_for_addr_timeout_cb, self);
-    if (_set_cur_lladdr_bin(self, NM_L3_IPV6LL_STATE_DAD_IN_PROGRESS, &lladdr)) {
+    if (_set_cur_lladdr_bin(self, NM_L3_IPV6LL_STATE_DAD_IN_PROGRESS, &lladdr) || restarted) {
         _LOGT("changed: starting DAD for address %s", nm_inet6_ntop(&self->cur_lladdr, sbuf));
-        _lladdr_handle_changed(self);
+        _lladdr_handle_changed(self, restarted);
     }
     return;
 }