From a5bbfe997a19c6d0041e165c2f407cfcb0061c63 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 1 Sep 2023 09:42:46 +0200 Subject: [PATCH 01/11] device: wait DAD before starting dnsmasq in IPv4 shared mode Currently, IPv4 shared mode fails to start when DAD is enabled because dnsmasq tries to bind to an address that is not yet configured on the interface. Delay the start of dnsmasq until the shared4 l3cd is ready. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') (cherry picked from commit e97ebb2441e8c2ea46bc05b8481e84b16c109dc8) --- src/core/devices/nm-device.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index d1212560db..8d002d703e 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -4284,7 +4284,6 @@ _dev_l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, N if (priv->ipshared_data_4.state == NM_DEVICE_IP_STATE_PENDING && !priv->ipshared_data_4.v4.dnsmasq_manager && priv->ipshared_data_4.v4.l3cd) { _dev_ipshared4_spawn_dnsmasq(self); - nm_clear_l3cd(&priv->ipshared_data_4.v4.l3cd); } _dev_ip_state_check_async(self, AF_UNSPEC); _dev_ipmanual_check_ready(self); @@ -12726,18 +12725,32 @@ out_fail: static void _dev_ipshared4_spawn_dnsmasq(NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - const char *ip_iface; - gs_free_error GError *error = NULL; - NMSettingConnection *s_con; - gboolean announce_android_metered; - NMConnection *applied; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + const char *ip_iface; + gs_free_error GError *error = NULL; + NMSettingConnection *s_con; + gboolean announce_android_metered; + NMConnection *applied; + gs_unref_array GArray *conflicts = NULL; + gboolean ready; nm_assert(priv->ipshared_data_4.v4.firewall_config); nm_assert(priv->ipshared_data_4.v4.dnsmasq_state_id == 0); nm_assert(!priv->ipshared_data_4.v4.dnsmasq_manager); nm_assert(priv->ipshared_data_4.v4.l3cd); + ready = nm_l3cfg_check_ready(priv->l3cfg, + priv->l3cds[L3_CONFIG_DATA_TYPE_SHARED_4].d, + AF_INET, + NM_L3CFG_CHECK_READY_FLAGS_IP4_ACD_READY, + &conflicts); + if (!ready) { + _LOGT_ipshared(AF_INET, "address not ready, wait"); + return; + } + if (conflicts) + goto out_fail; + ip_iface = nm_device_get_ip_iface(self); g_return_if_fail(ip_iface); @@ -12782,9 +12795,11 @@ _dev_ipshared4_spawn_dnsmasq(NMDevice *self) _dev_ipsharedx_set_state(self, AF_INET, NM_DEVICE_IP_STATE_READY); _dev_ip_state_check_async(self, AF_INET); + nm_clear_l3cd(&priv->ipshared_data_4.v4.l3cd); return; out_fail: + nm_clear_l3cd(&priv->ipshared_data_4.v4.l3cd); _dev_ipsharedx_set_state(self, AF_INET, NM_DEVICE_IP_STATE_FAILED); _dev_ip_state_check_async(self, AF_INET); } From e2a7e9c32f69013f36d7ca27c5da6b40833c4a47 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 1 Sep 2023 16:40:29 +0200 Subject: [PATCH 02/11] n-acd: use separate seed state for each probe of the same acd Currently, all the probes of an acd instance share the same seed state. This means that the state is updated by all the probes, and as a consequence they get different jitters for the wait timeouts; therefore the order in which addresses become available (and are configured on the interface) is not deterministic. Keep a separate seed state for each probe, initialized from the acd seed. This ensures that all the probes use the same timeouts when sending probe requests, and that in case of no collision, addresses are available in the order of probe start. n-acd pull request: https://github.com/nettools/n-acd/pull/10 (cherry picked from commit 23727917b234203bd98b874d7200e31b844685bd) --- src/n-acd/src/n-acd-private.h | 1 + src/n-acd/src/n-acd-probe.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/n-acd/src/n-acd-private.h b/src/n-acd/src/n-acd-private.h index 4583c018e2..db28b1a560 100644 --- a/src/n-acd/src/n-acd-private.h +++ b/src/n-acd/src/n-acd-private.h @@ -105,6 +105,7 @@ struct NAcdProbe { void *userdata; /* state */ + unsigned int seed; unsigned int state; unsigned int n_iteration; unsigned int defend; diff --git a/src/n-acd/src/n-acd-probe.c b/src/n-acd/src/n-acd-probe.c index c1ed59ae9e..d32e0103b4 100644 --- a/src/n-acd/src/n-acd-probe.c +++ b/src/n-acd/src/n-acd-probe.c @@ -172,7 +172,7 @@ static void n_acd_probe_schedule(NAcdProbe *probe, uint64_t n_timeout, unsigned if (n_jitter) { uint64_t random; - random = ((uint64_t)rand_r(&probe->acd->seed) << 32) | (uint64_t)rand_r(&probe->acd->seed); + random = ((uint64_t)rand_r(&probe->seed) << 32) | (uint64_t)rand_r(&probe->seed); n_time += random % n_jitter; } @@ -283,6 +283,7 @@ int n_acd_probe_new(NAcdProbe **probep, NAcd *acd, NAcdProbeConfig *config) { *probe = (NAcdProbe)N_ACD_PROBE_NULL(*probe); probe->acd = n_acd_ref(acd); probe->ip = config->ip; + probe->seed = acd->seed; /* * We use the provided timeout-length as multiplier for all our From 97792b34e60951544f6e9954fed4cc82a7047817 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 4 Sep 2023 10:18:23 +0200 Subject: [PATCH 03/11] l3cfg: schedule a commit when ACD is not supported On interfaces not supporting ACD (for example, layer3 interfaces), the probe fails to be created with message: l3cfg[...,ifindex=2]: acd[172.25.17.1, init]: probe-good (interface does not support acd, initial post-commit) l3cfg[...,ifindex=2]: acd[172.25.17.1, ready]: set state to ready (probe is ready, waiting for address to be configured) During the post-commit event, if the address is not yet configured, we need to schedule a new commit to actually add it. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') (cherry picked from commit 687051368ffd51381d8e58685a43d3b0f4f945db) --- src/core/nm-l3cfg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index ce9d584901..76f4209688 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -2972,8 +2972,7 @@ handle_start_defending: NM_L3_ACD_ADDR_STATE_READY, !NM_IN_SET(state_change_mode, ACD_STATE_CHANGE_MODE_INIT, - ACD_STATE_CHANGE_MODE_INIT_REAPPLY, - ACD_STATE_CHANGE_MODE_POST_COMMIT), + ACD_STATE_CHANGE_MODE_INIT_REAPPLY), "probe is ready, waiting for address to be configured"); } return; From ea36338d07889f9eb16e3cfbe9caf2459c64bd76 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 5 Sep 2023 14:47:53 +0200 Subject: [PATCH 04/11] l3cfg: skip ACD for interfaces with IFF_NOARP Interfaces with IFF_NOARP don't support Address Conflict Detection, which is based on ARP. Trying to start ACD on them would result in ENOBUFS always being returned by send(), and n-acd handles such error by retrying indefinitely. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') (cherry picked from commit 7548ff57d3750bfe73294e59421fc7b22b26a866) --- src/core/nm-l3cfg.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 76f4209688..2d0c372b72 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -394,7 +394,8 @@ static void _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is static void _nm_l3cfg_emit_signal_notify_acd_event_all(NML3Cfg *self); -static gboolean _acd_has_valid_link(const NMPObject *obj, +static gboolean _acd_has_valid_link(NML3Cfg *self, + const NMPObject *obj, const guint8 **out_addr_bin, gboolean *out_acd_not_supported); @@ -1403,8 +1404,8 @@ _load_link(NML3Cfg *self, gboolean initial) nacd_link_now_up = FALSE; nacd_changed = FALSE; - nacd_old_valid = _acd_has_valid_link(obj_old, &nacd_old_addr, NULL); - nacd_new_valid = _acd_has_valid_link(obj, &nacd_new_addr, NULL); + nacd_old_valid = _acd_has_valid_link(self, obj_old, &nacd_old_addr, NULL); + nacd_new_valid = _acd_has_valid_link(self, obj, &nacd_new_addr, NULL); if (self->priv.p->nacd_instance_ensure_retry) { if (nacd_new_valid && (!nacd_old_valid @@ -1614,7 +1615,8 @@ _acd_data_find_track(const AcdData *acd_data, /*****************************************************************************/ static gboolean -_acd_has_valid_link(const NMPObject *obj, +_acd_has_valid_link(NML3Cfg *self, + const NMPObject *obj, const guint8 **out_addr_bin, gboolean *out_acd_not_supported) { @@ -1635,6 +1637,11 @@ _acd_has_valid_link(const NMPObject *obj, return FALSE; } + if (nm_platform_link_get_ifi_flags(self->priv.platform, self->priv.ifindex, IFF_NOARP)) { + NM_SET_OUT(out_acd_not_supported, TRUE); + return FALSE; + } + NM_SET_OUT(out_acd_not_supported, FALSE); NM_SET_OUT(out_addr_bin, addr_bin); return TRUE; @@ -1864,7 +1871,7 @@ again: return NULL; } - valid = _acd_has_valid_link(self->priv.plobj, &addr_bin, &acd_not_supported); + valid = _acd_has_valid_link(self, self->priv.plobj, &addr_bin, &acd_not_supported); if (!valid) goto failed_create_acd; From b4f0b504d403749a29c80f5727694dd6001ac857 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 9 Sep 2023 13:53:25 +0200 Subject: [PATCH 05/11] l3cfg: fix pruning of ACD data If a commit is invoked without any change to the l3cd or to the ACD data, in _l3cfg_update_combined_config() we skip calling _l3_acd_data_add_all(), which should clear the dirty flag from ACDs. Therefore, in case of such no-op commits the ACDs still marked as dirty - but valid - are removed via: _l3_commit() _l3_acd_data_process_changes() _l3_acd_data_prune() _l3_acd_data_prune_one() Invoking a l3cfg commit without any actual changes is allowed, see the explanation in commit e773559d9d92 ('device: schedule an idle commit when setting device's sys-iface-state'). The bug is visible by running test 'bond_addreses_restart_persistence' with IPv4 ACD/DAD is enabled by default: after restart IPv6 completes immediately, the devices becomes ACTIVATED, the sys-iface-state transitions from ASSUME to MANAGED, a commit is done, and it incorrectly prunes the ACD data. The result is that the IPv4 address is never added again. Fix this by doing the pruning only when we update the dirty flags. (cherry picked from commit ed565f91469987e1b6eaf0e0263fe2b02853ed7b) --- src/core/nm-l3cfg.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 2d0c372b72..42f18a6982 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -334,6 +334,7 @@ typedef struct _NML3CfgPrivate { bool nacd_acd_not_supported : 1; bool acd_ipv4_addresses_on_link_has : 1; + bool acd_data_pruning_needed : 1; bool changed_configs_configs : 1; bool changed_configs_acd_state : 1; @@ -3059,7 +3060,10 @@ _l3_acd_data_process_changes(NML3Cfg *self) AcdData *acd_data; gint64 now_msec = 0; - _l3_acd_data_prune(self, FALSE); + if (self->priv.p->acd_data_pruning_needed) + _l3_acd_data_prune(self, FALSE); + + self->priv.p->acd_data_pruning_needed = FALSE; c_list_for_each_entry (acd_data, &self->priv.p->acd_lst_head, acd_lst) { _l3_acd_data_state_change(self, @@ -3789,6 +3793,8 @@ _l3cfg_update_combined_config(NML3Cfg *self, NM_SET_OUT(out_changed_combined_l3cd, FALSE); + self->priv.p->acd_data_pruning_needed = FALSE; + if (!self->priv.p->changed_configs_configs) { if (!self->priv.p->changed_configs_acd_state) goto out; @@ -3836,6 +3842,7 @@ _l3cfg_update_combined_config(NML3Cfg *self, self->priv.p->changed_configs_acd_state = TRUE; } else { _l3_acd_data_add_all(self, l3_config_datas_arr, l3_config_datas_len, reapply); + self->priv.p->acd_data_pruning_needed = TRUE; self->priv.p->changed_configs_acd_state = FALSE; } From 233d89da4bf21dc812b89780692e4d8503bbb9ba Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 5 Sep 2023 17:53:33 +0200 Subject: [PATCH 06/11] dhcp: don't schedule commit of type "update" when clearing acd We don't know the reason why the DHCP client is being stopped. It is wrong to schedule a commit of type "update" because the device could be now unmanaged. Schedule instead a commit of type "auto", which automatically determines the type of commit based on registered handles. (cherry picked from commit a49913504d4ce1b62d84679085912ba1ca4b1e0e) --- src/core/dhcp/nm-dhcp-client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 6978bd3cb0..4be03f4bab 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -527,7 +527,7 @@ _acd_reglist_data_remove(NMDhcpClient *self, guint idx, gboolean do_log) nm_clear_l3cd(®list_data->l3cd); - nm_l3cfg_commit_on_idle_schedule(priv->config.l3cfg, NM_L3_CFG_COMMIT_TYPE_UPDATE); + nm_l3cfg_commit_on_idle_schedule(priv->config.l3cfg, NM_L3_CFG_COMMIT_TYPE_AUTO); g_array_remove_index(priv->v4.acd.reglist, idx); From 707ddcfaacb0205b53dbdd9950d4e47288140948 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 8 Sep 2023 11:45:58 +0200 Subject: [PATCH 07/11] device: check DAD result for manual method even without carrier IPv4 and IPv6 DAD work slightly differently: for IPv4 the presence or absence of carrier doesn't have any effect on the duration of the probe; for IPv6, DAD never completes without carrier because kernel never removes the tentative flag. In both cases, we shouldn't ignore the DAD result because that would mean that we complete the ipmanual method without addresses actually configured. (cherry picked from commit 1f730347196362f303cad17e8bcbc7559861d07e) --- src/core/devices/nm-device.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 8d002d703e..e8c25bc6ef 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -10392,11 +10392,8 @@ _dev_ipmanual_check_ready(NMDevice *self) } } - flags = NM_L3CFG_CHECK_READY_FLAGS_NONE; - if (has_carrier) { - flags |= NM_L3CFG_CHECK_READY_FLAGS_IP4_ACD_READY; - flags |= NM_L3CFG_CHECK_READY_FLAGS_IP6_DAD_READY; - } + flags = NM_L3CFG_CHECK_READY_FLAGS_IP4_ACD_READY; + flags |= NM_L3CFG_CHECK_READY_FLAGS_IP6_DAD_READY; for (IS_IPv4 = 0; IS_IPv4 < 2; IS_IPv4++) { const int addr_family = IS_IPv4 ? AF_INET : AF_INET6; From 906caf96f5cf2bd3c9a9c9b9f7a43ff1e007df97 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 8 Sep 2023 22:16:33 +0200 Subject: [PATCH 08/11] l3cfg: improve logging - avoid "update" as it is also a commit type - make clear that the commit is not happening now (cherry picked from commit e83e8b73f4e5ce2c9e77d54df7191daf21aa256c) --- src/core/nm-l3cfg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 42f18a6982..abf53aab5d 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -3281,7 +3281,7 @@ nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self, NML3CfgCommitType commit_type) if (self->priv.p->commit_on_idle_source) { if (self->priv.p->commit_on_idle_type < commit_type) { /* For multiple calls, we collect the maximum "commit-type". */ - _LOGT("commit on idle (scheduled) (update to %s)", + _LOGT("schedule commit on idle (upgrade type to %s)", _l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type))); @@ -3290,7 +3290,7 @@ nm_l3cfg_commit_on_idle_schedule(NML3Cfg *self, NML3CfgCommitType commit_type) return FALSE; } - _LOGT("commit on idle (scheduled) (%s)", + _LOGT("schedule commit on idle (%s)", _l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type))); self->priv.p->commit_on_idle_source = nm_g_idle_add_source(_l3_commit_on_idle_cb, self); self->priv.p->commit_on_idle_type = commit_type; From 5bf855c7c1d4ace10f60db5b1ab4c96d6396c7b5 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 8 Sep 2023 22:15:56 +0200 Subject: [PATCH 09/11] l3cfg: log the reason when marking IP configuration dirty (cherry picked from commit 6ebf2c6ba15669309e044f073d3627377ba6af2d) --- src/core/nm-l3cfg.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index abf53aab5d..42f34f989f 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -676,9 +676,9 @@ _nm_l3cfg_emit_signal_notify_l3cd_changed(NML3Cfg *self, /*****************************************************************************/ static void -_l3_changed_configs_set_dirty(NML3Cfg *self) +_l3_changed_configs_set_dirty(NML3Cfg *self, const char *reason) { - _LOGT("IP configuration changed (mark dirty)"); + _LOGT("IP configuration changed (mark dirty): %s", reason); self->priv.p->changed_configs_configs = TRUE; self->priv.p->changed_configs_acd_state = TRUE; } @@ -1795,7 +1795,7 @@ _l3_acd_nacd_instance_ensure_retry_cb(gpointer user_data) nm_clear_g_source_inst(&self->priv.p->nacd_instance_ensure_retry); - _l3_changed_configs_set_dirty(self); + _l3_changed_configs_set_dirty(self, "nacd retry"); nm_l3cfg_commit(self, NM_L3_CFG_COMMIT_TYPE_AUTO); return G_SOURCE_REMOVE; } @@ -1818,7 +1818,7 @@ _l3_acd_nacd_instance_reset(NML3Cfg *self, NMTernary start_timer, gboolean acd_d switch (start_timer) { case NM_TERNARY_FALSE: - _l3_changed_configs_set_dirty(self); + _l3_changed_configs_set_dirty(self, "nacd reset"); nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); break; case NM_TERNARY_TRUE: @@ -2333,7 +2333,7 @@ _nm_printf(5, 6) static void _l3_acd_data_state_set_full(NML3Cfg *self, if (changed && allow_commit) { /* The availability of an address just changed (and we are instructed to * trigger a new commit). Do it. */ - _l3_changed_configs_set_dirty(self); + _l3_changed_configs_set_dirty(self, "acd state changed"); nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); } } @@ -3563,7 +3563,7 @@ nm_l3cfg_add_config(NML3Cfg *self, nm_assert(l3_config_data->acd_defend_type_confdata == acd_defend_type); if (changed) { - _l3_changed_configs_set_dirty(self); + _l3_changed_configs_set_dirty(self, "configuration added"); nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); } @@ -3600,7 +3600,7 @@ _l3cfg_remove_config(NML3Cfg *self, continue; } - _l3_changed_configs_set_dirty(self); + _l3_changed_configs_set_dirty(self, "configuration removed"); _l3_config_datas_remove_index_fast(self->priv.p->l3_config_datas, idx); changed = TRUE; if (l3cd) { From 3ddd1d6e7be54998fb09d7b4dec2f27733a81abc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 4 Sep 2023 15:40:05 +0200 Subject: [PATCH 10/11] libnm: better document IPv4 DAD property Clarify that the value is the *maximum* interval; the actual value is randomized and can be as low as half the specified one. (cherry picked from commit 536805231ac2de02aee43dac164c74be7cf86bd3) --- src/libnm-core-impl/nm-setting-ip-config.c | 16 +++++++++------- src/libnmc-setting/settings-docs.h.in | 4 ++-- src/nmcli/gen-metadata-nm-settings-nmcli.xml.in | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 727a4bae1e..ac852d8dec 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -6655,14 +6655,16 @@ nm_setting_ip_config_class_init(NMSettingIPConfigClass *klass) /** * NMSettingIPConfig:dad-timeout: * - * Timeout in milliseconds used to check for the presence of duplicate IP - * addresses on the network. If an address conflict is detected, the - * activation will fail. A zero value means that no duplicate address - * detection is performed, -1 means the default value (either configuration - * ipvx.dad-timeout override or zero). A value greater than zero is a - * timeout in milliseconds. + * Maximum timeout in milliseconds used to check for the presence of duplicate + * IP addresses on the network. If an address conflict is detected, the + * activation will fail. The property is currently implemented only for IPv4. * - * The property is currently implemented only for IPv4. + * A zero value means that no duplicate address detection is performed, -1 means + * the default value (either the value configured globally in NetworkManger.conf + * or zero). A value greater than zero is a timeout in milliseconds. Note that + * the time intervals are subject to randomization as per RFC 5227 and so the + * actual duration can be between half and the full time specified in this + * property. * * Since: 1.2 **/ diff --git a/src/libnmc-setting/settings-docs.h.in b/src/libnmc-setting/settings-docs.h.in index bdda8e502e..ad0c71a14d 100644 --- a/src/libnmc-setting/settings-docs.h.in +++ b/src/libnmc-setting/settings-docs.h.in @@ -160,7 +160,7 @@ #define DESCRIBE_DOC_NM_SETTING_INFINIBAND_TRANSPORT_MODE N_("The IP-over-InfiniBand transport mode. Either \"datagram\" or \"connected\".") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_ADDRESSES N_("A list of IPv4 addresses and their prefix length. Multiple addresses can be separated by comma. For example \"192.168.1.5/24, 10.1.0.5/24\". The addresses are listed in decreasing priority, meaning the first address will be the primary address.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_AUTO_ROUTE_EXT_GW N_("VPN connections will default to add the route automatically unless this setting is set to FALSE. For other connection types, adding such an automatic route is currently not supported and setting this to TRUE has no effect.") -#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DAD_TIMEOUT N_("Timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. A zero value means that no duplicate address detection is performed, -1 means the default value (either configuration ipvx.dad-timeout override or zero). A value greater than zero is a timeout in milliseconds. The property is currently implemented only for IPv4.") +#define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DAD_TIMEOUT N_("Maximum timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. The property is currently implemented only for IPv4. A zero value means that no duplicate address detection is performed, -1 means the default value (either the value configured globally in NetworkManger.conf or zero). A value greater than zero is a timeout in milliseconds. Note that the time intervals are subject to randomization as per RFC 5227 and so the actual duration can be between half and the full time specified in this property.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_CLIENT_ID N_("A string sent to the DHCP server to identify the local machine which the DHCP server may use to customize the DHCP lease and options. When the property is a hex string ('aa:bb:cc') it is interpreted as a binary client ID, in which case the first byte is assumed to be the 'type' field as per RFC 2132 section 9.14 and the remaining bytes may be an hardware address (e.g. '01:xx:xx:xx:xx:xx:xx' where 1 is the Ethernet ARP type and the rest is a MAC address). If the property is not a hex string it is considered as a non-hardware-address client ID and the 'type' field is set to 0. The special values \"mac\" and \"perm-mac\" are supported, which use the current or permanent MAC address of the device to generate a client identifier with type ethernet (01). Currently, these options only work for ethernet type of links. The special value \"ipv6-duid\" uses the DUID from \"ipv6.dhcp-duid\" property as an RFC4361-compliant client identifier. As IAID it uses \"ipv4.dhcp-iaid\" and falls back to \"ipv6.dhcp-iaid\" if unset. The special value \"duid\" generates a RFC4361-compliant client identifier based on \"ipv4.dhcp-iaid\" and uses a DUID generated by hashing /etc/machine-id. The special value \"stable\" is supported to generate a type 0 client identifier based on the stable-id (see connection.stable-id) and a per-host key. If you set the stable-id, you may want to include the \"${DEVICE}\" or \"${MAC}\" specifier to get a per-device key. If unset, a globally configured default is used. If still unset, the default depends on the DHCP plugin.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_FQDN N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified FQDN will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-hostname\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP4_CONFIG_DHCP_HOSTNAME N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified name will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-fqdn\" are mutually exclusive and cannot be set at the same time.") @@ -190,7 +190,7 @@ #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE N_("Configure method for creating the IPv6 interface identifer of addresses with RFC4862 IPv6 Stateless Address Autoconfiguration and Link Local addresses. The permitted values are: \"eui64\" (0), \"stable-privacy\" (1), \"default\" (3) or \"default-or-eui64\" (2). If the property is set to \"eui64\", the addresses will be generated using the interface token derived from hardware address. This makes the host part of the address to stay constant, making it possible to track the host's presence when it changes networks. The address changes when the interface hardware is replaced. If a duplicate address is detected, there is also no fallback to generate another address. When configured, the \"ipv6.token\" is used instead of the MAC address to generate addresses for stateless autoconfiguration. If the property is set to \"stable-privacy\", the interface identifier is generated as specified by RFC7217. This works by hashing a host specific key (see NetworkManager(8) manual), the interface name, the connection's \"connection.stable-id\" property and the address prefix. This improves privacy by making it harder to use the address to track the host's presence and the address is stable when the network interface hardware is replaced. The special values \"default\" and \"default-or-eui64\" will fallback to the global connection default as documented in the NetworkManager.conf(5) manual. If the global default is not specified, the fallback value is \"stable-privacy\" or \"eui64\", respectively. If not specified, when creating a new profile the default is \"default\". Note that this setting is distinct from the Privacy Extensions as configured by \"ip6-privacy\" property and it does not affect the temporary addresses configured with this option.") #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_ADDRESSES N_("A list of IPv6 addresses and their prefix length. Multiple addresses can be separated by comma. For example \"2001:db8:85a3::8a2e:370:7334/64, 2001:db8:85a3::5/64\". The addresses are listed in decreasing priority, meaning the first address will be the primary address. This can make a difference with IPv6 source address selection (RFC 6724, section 5).") #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_AUTO_ROUTE_EXT_GW N_("VPN connections will default to add the route automatically unless this setting is set to FALSE. For other connection types, adding such an automatic route is currently not supported and setting this to TRUE has no effect.") -#define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_DAD_TIMEOUT N_("Timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. A zero value means that no duplicate address detection is performed, -1 means the default value (either configuration ipvx.dad-timeout override or zero). A value greater than zero is a timeout in milliseconds. The property is currently implemented only for IPv4.") +#define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_DAD_TIMEOUT N_("Maximum timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. The property is currently implemented only for IPv4. A zero value means that no duplicate address detection is performed, -1 means the default value (either the value configured globally in NetworkManger.conf or zero). A value greater than zero is a timeout in milliseconds. Note that the time intervals are subject to randomization as per RFC 5227 and so the actual duration can be between half and the full time specified in this property.") #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_DHCP_DUID N_("A string containing the DHCPv6 Unique Identifier (DUID) used by the dhcp client to identify itself to DHCPv6 servers (RFC 3315). The DUID is carried in the Client Identifier option. If the property is a hex string ('aa:bb:cc') it is interpreted as a binary DUID and filled as an opaque value in the Client Identifier option. The special value \"lease\" will retrieve the DUID previously used from the lease file belonging to the connection. If no DUID is found and \"dhclient\" is the configured dhcp client, the DUID is searched in the system-wide dhclient lease file. If still no DUID is found, or another dhcp client is used, a global and permanent DUID-UUID (RFC 6355) will be generated based on the machine-id. The special values \"llt\" and \"ll\" will generate a DUID of type LLT or LL (see RFC 3315) based on the current MAC address of the device. In order to try providing a stable DUID-LLT, the time field will contain a constant timestamp that is used globally (for all profiles) and persisted to disk. The special values \"stable-llt\", \"stable-ll\" and \"stable-uuid\" will generate a DUID of the corresponding type, derived from the connection's stable-id and a per-host unique key. You may want to include the \"${DEVICE}\" or \"${MAC}\" specifier in the stable-id, in case this profile gets activated on multiple devices. So, the link-layer address of \"stable-ll\" and \"stable-llt\" will be a generated address derived from the stable id. The DUID-LLT time value in the \"stable-llt\" option will be picked among a static timespan of three years (the upper bound of the interval is the same constant timestamp used in \"llt\"). When the property is unset, the global value provided for \"ipv6.dhcp-duid\" is used. If no global value is provided, the default \"lease\" value is assumed.") #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_DHCP_HOSTNAME N_("If the \"dhcp-send-hostname\" property is TRUE, then the specified name will be sent to the DHCP server when acquiring a lease. This property and \"dhcp-fqdn\" are mutually exclusive and cannot be set at the same time.") #define DESCRIBE_DOC_NM_SETTING_IP6_CONFIG_DHCP_HOSTNAME_FLAGS N_("Flags for the DHCP hostname and FQDN. Currently, this property only includes flags to control the FQDN flags set in the DHCP FQDN option. Supported FQDN flags are NM_DHCP_HOSTNAME_FLAG_FQDN_SERV_UPDATE (0x1), NM_DHCP_HOSTNAME_FLAG_FQDN_ENCODED (0x2) and NM_DHCP_HOSTNAME_FLAG_FQDN_NO_UPDATE (0x4). When no FQDN flag is set and NM_DHCP_HOSTNAME_FLAG_FQDN_CLEAR_FLAGS (0x8) is set, the DHCP FQDN option will contain no flag. Otherwise, if no FQDN flag is set and NM_DHCP_HOSTNAME_FLAG_FQDN_CLEAR_FLAGS (0x8) is not set, the standard FQDN flags are set in the request: NM_DHCP_HOSTNAME_FLAG_FQDN_SERV_UPDATE (0x1), NM_DHCP_HOSTNAME_FLAG_FQDN_ENCODED (0x2) for IPv4 and NM_DHCP_HOSTNAME_FLAG_FQDN_SERV_UPDATE (0x1) for IPv6. When this property is set to the default value NM_DHCP_HOSTNAME_FLAG_NONE (0x0), a global default is looked up in NetworkManager configuration. If that value is unset or also NM_DHCP_HOSTNAME_FLAG_NONE (0x0), then the standard FQDN flags described above are sent in the DHCP requests.") diff --git a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in index 1e0797abe0..a374621f32 100644 --- a/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in +++ b/src/nmcli/gen-metadata-nm-settings-nmcli.xml.in @@ -702,7 +702,7 @@ + nmcli-description="Maximum timeout in milliseconds used to check for the presence of duplicate IP addresses on the network. If an address conflict is detected, the activation will fail. The property is currently implemented only for IPv4. A zero value means that no duplicate address detection is performed, -1 means the default value (either the value configured globally in NetworkManger.conf or zero). A value greater than zero is a timeout in milliseconds. Note that the time intervals are subject to randomization as per RFC 5227 and so the actual duration can be between half and the full time specified in this property." /> Date: Thu, 31 Aug 2023 14:06:09 +0200 Subject: [PATCH 11/11] core: don't fail if at least one static address passes DAD It seems more useful to have a best effort approach and configure everything we can; in that way we achieve at least some connectivity, and then sysadmin can check the logs in case something is missing. Currently instead, the whole activation fails (so, no address is configured) if just one of the addresses fails DAD. Ideally, we should have a way to make this configurable; but for now, implement the more useful behavior as default. (cherry picked from commit a45024714f457e38f268f9aec5f6e4fbeab508ed) --- src/core/devices/nm-device.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index e8c25bc6ef..14d85b80e2 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -10403,12 +10403,24 @@ _dev_ipmanual_check_ready(NMDevice *self) addr_family, flags, &conflicts); - if (conflicts) { - _dev_ipmanual_set_state(self, addr_family, NM_DEVICE_IP_STATE_FAILED); - _dev_ip_state_check_async(self, AF_UNSPEC); - } else if (ready) { - _dev_ipmanual_set_state(self, addr_family, NM_DEVICE_IP_STATE_READY); - _dev_ip_state_check_async(self, AF_UNSPEC); + if (ready) { + guint num_addrs = 0; + + num_addrs = + nm_l3_config_data_get_num_addresses(priv->l3cds[L3_CONFIG_DATA_TYPE_MANUALIP].d, + addr_family); + + if (conflicts && conflicts->len == num_addrs) { + _LOGD_ipmanual(addr_family, "all manual addresses failed DAD, failing"); + _dev_ipmanual_set_state(self, addr_family, NM_DEVICE_IP_STATE_FAILED); + _dev_ip_state_check_async(self, AF_UNSPEC); + } else { + if (conflicts) { + _LOGD_ipmanual(addr_family, "some manual addresses passed DAD, continuing"); + } + _dev_ipmanual_set_state(self, addr_family, NM_DEVICE_IP_STATE_READY); + _dev_ip_state_check_async(self, AF_UNSPEC); + } } } }