From cd5a1dfc012480c49d954320b4d6bff94bc6c661 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 24 May 2023 10:45:13 +0200 Subject: [PATCH] Don't fail when the IPv6 link-local address is removed (rh #2209353) Resolves: #2209353 --- ...e-the-address-when-removed-rh2209353.patch | 140 ++++++++++++++++++ NetworkManager.spec | 6 +- 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 1005-ipv6ll-don-t-regenerate-the-address-when-removed-rh2209353.patch diff --git a/1005-ipv6ll-don-t-regenerate-the-address-when-removed-rh2209353.patch b/1005-ipv6ll-don-t-regenerate-the-address-when-removed-rh2209353.patch new file mode 100644 index 0000000..b322c77 --- /dev/null +++ b/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 +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 + diff --git a/NetworkManager.spec b/NetworkManager.spec index e3d4b35..6187117 100644 --- a/NetworkManager.spec +++ b/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 - 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 - 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)