Don't fail when the IPv6 link-local address is removed (rh #2209353)

Resolves: #2209353

1005-ipv6ll-don-t-regenerate-the-address-when-removed-rh2209353.patch

@@ -0,0 +1,140 @@
+From 3fcb1a072f230b53c6fdf6e106e0972293a2f742 Mon Sep 17 00:00:00 2001
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Thu, 11 May 2023 13:32:13 +0200
+Subject: [PATCH] ipv6ll: don't regenerate the address when it's removed
+ externally
+
+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.
+
+[bgalvani@redhat.com: since the branch is missing commit 7ca95cee15b3
+('platform: always reconfigure IP routes even if removed externally'),
+we need to set flag NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE when committing
+the address, otherwise it's not re-added]
+
+Fixes: aa070fb82190 ('core: add NML3IPv6LL helper')
+https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1622
+(cherry picked from commit 53ba9f4701f30b12637df2c7215a0b7da845b34c)
+(cherry picked from commit 2976e4c3b7fcee06051ce83c9a7fa911ad192dc4)
+(cherry picked from commit 4a13b5f52217c81ddf2329ba343796bfa4ed5ef9)
+---
+ src/core/nm-l3-ipv6ll.c | 34 ++++++++++++++++++++++------------
+ 1 file changed, 22 insertions(+), 12 deletions(-)
+
+diff --git a/src/core/nm-l3-ipv6ll.c b/src/core/nm-l3-ipv6ll.c
+index 2640c07554..6e5e460258 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;
+@@ -420,7 +420,9 @@ _lladdr_handle_changed(NML3IPv6LL *self)
+                                 NM_DNS_PRIORITY_DEFAULT_NORMAL,
+                                 NM_L3_ACD_DEFEND_TYPE_ALWAYS,
+                                 0,
+-                                NM_L3CFG_CONFIG_FLAGS_NONE,
++                                /* Even if the address was removed from platform, it must
++                                 * be re-added, hence FORCE_ONCE. */
++                                NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE,
+                                 NM_L3_CONFIG_MERGE_FLAGS_NONE))
+             changed = TRUE;
+     } else {
+@@ -434,7 +436,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 +517,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 +529,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_utils_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_utils_inet6_ntop(&self->cur_lladdr, sbuf));
+-            _lladdr_handle_changed(self);
++            _lladdr_handle_changed(self, FALSE);
+         }
+         return;
+     }
+@@ -543,11 +546,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_utils_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_utils_inet6_ntop(&self->cur_lladdr, sbuf));
++        } else {
+             _LOGT("changed: address %s is gone", _nm_utils_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,19 +578,20 @@ _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_utils_inet6_ntop(&self->cur_lladdr, sbuf));
+-        _lladdr_handle_changed(self);
++        _lladdr_handle_changed(self, restarted);
+     }
+     return;
+ }
+-- 
+2.39.2
+

NetworkManager.spec

@@ -6,7 +6,7 @@
 %global epoch_version 1
 %global real_version 1.40.16
 %global rpm_version %{real_version}
-%global release_version 5
+%global release_version 6
 %global snapshot %{nil}
 %global git_sha %{nil}
 %global bcond_default_debug 0
@@ -200,6 +200,7 @@ Patch1001: 1001-cloud-setup-IMDSv2-rh2151987.patch
 Patch1002: 1002-dns-add-support-to-no-aaaa-option-rh2144521.patch
 Patch1003: 1003-suppport-bond-port-prio-rh1920398.patch
 Patch1004: 1004-team-don-t-try-to-connect-to-teamd-in-update_connect-rh2182029.patch
+Patch1005: 1005-ipv6ll-don-t-regenerate-the-address-when-removed-rh2209353.patch
 
 Requires(post): systemd
 %if 0%{?fedora} || 0%{?rhel} >= 8
@@ -1235,6 +1236,9 @@ fi
 
 
 %changelog
+* Wed May 24 2023 Beniamino Galvani <bgalvani@redhat.com> - 1:1.40.16-6
+- don't fail when the IPv6 link-local address is removed (rh #2209353)
+
 * Wed May 17 2023 Fernando Fernandez Mancera <ferferna@redhat.com> - 1:1.40.16-5
 - support bond port prio property (rh #1920398)
 - team: don't try to connect to teamd in update_connection() (rh #2182029)