From 407d753a5a70bc12b2e1426ef2833c1abe945de9 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 11 Jul 2025 15:47:05 +0200 Subject: [PATCH 1/3] l3cfg: don't reset the ACD probe timestamp during timer events acd_data->probing_timestamp_msec indicates when the probing started. It is used in different places to calculate the timeout for certain operations. In particular, it is used to detect that the probe creation took too long when handling the ACD_STATE_CHANGE_MODE_TIMEOUT event. If we reset this timestamp at every timer event, we'll never hit the probe creation timeout. Therefore, the l3cfg will keep trying forever to create the probe. See: https://lists.freedesktop.org/archives/networkmanager/2025-July/000418.html Fix this by not updating the timestamp during a timeout event. Fixes: a09f9cc61629 ('l3cfg: ensure the probing timeout is initialized on probe start') --- src/core/nm-l3cfg.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 8a69a28483..1797cd0f93 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -2751,9 +2751,8 @@ handle_init: goto handle_start_defending; } - acd_data->probing_timestamp_msec = (*p_now_msec); - acd_data->probing_timeout_msec = acd_timeout_msec; - log_reason = "retry probing on timeout"; + acd_data->probing_timeout_msec = acd_timeout_msec; + log_reason = "retry probing on timeout"; goto handle_start_probing; } From 127f73a5c2d1816eb4f40d740940854414eae23c Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 11 Jul 2025 21:09:10 +0200 Subject: [PATCH 2/3] l3cfg: fix the interval of the ACD restart timer After ACD_WAIT_PROBING_EXTRA_TIME_MSEC has elapsed, _l3_acd_data_timeout_schedule_probing_restart() keeps rescheduling the timer with a zero interval, resulting in 100% CPU usage. This continues until the probe is destroyed after ACD_WAIT_PROBING_EXTRA_TIME2_MSEC. When computing the interval, we need to use (ACD_WAIT_PROBING_EXTRA_TIME_MSEC + ACD_WAIT_PROBING_EXTRA_TIME2_MSEC) as the expiry time. --- src/core/nm-l3cfg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 1797cd0f93..54ca2cdd31 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -2322,7 +2322,8 @@ _l3_acd_data_timeout_schedule_probing_restart(AcdData *acd_data, gint64 now_msec nm_assert(acd_data->probing_timeout_msec > 0); nm_assert(acd_data->probing_timestamp_msec > 0); - expiry_msec = acd_data->probing_timestamp_msec + ACD_WAIT_PROBING_EXTRA_TIME_MSEC; + expiry_msec = acd_data->probing_timestamp_msec + ACD_WAIT_PROBING_EXTRA_TIME_MSEC + + ACD_WAIT_PROBING_EXTRA_TIME2_MSEC; timeout_msec = NM_MAX(0, expiry_msec - now_msec); From eb0a41ce1f9066c90576bd3bafecfde245237fe4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 14 Jul 2025 09:47:13 +0200 Subject: [PATCH 3/3] l3cfg: simplify the ACD timeouts ACD_WAIT_PROBING_EXTRA_TIME_MSEC and ACD_WAIT_PROBING_EXTRA_TIME2_MSEC now are always used together. Consolidate them into a single constant. --- src/core/nm-l3cfg.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 54ca2cdd31..35dc81cff9 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -40,8 +40,7 @@ G_STATIC_ASSERT(NM_ACD_TIMEOUT_RFC5227_MSEC == N_ACD_TIMEOUT_RFC5227); #define ACD_SUPPORTED_ETH_ALEN ETH_ALEN #define ACD_ENSURE_RATELIMIT_MSEC ((guint32) 4000u) -#define ACD_WAIT_PROBING_EXTRA_TIME_MSEC ((guint32) (1000u + ACD_ENSURE_RATELIMIT_MSEC)) -#define ACD_WAIT_PROBING_EXTRA_TIME2_MSEC ((guint32) 1000u) +#define ACD_WAIT_PROBING_EXTRA_TIME_MSEC ((guint32) (2000u + ACD_ENSURE_RATELIMIT_MSEC)) #define ACD_WAIT_TIME_PROBING_FULL_RESTART_MSEC ((guint32) 30000u) #define ACD_WAIT_TIME_CONFLICT_RESTART_MSEC ((guint32) 120000u) #define ACD_WAIT_TIME_ANNOUNCE_RESTART_MSEC ((guint32) 30000u) @@ -2322,8 +2321,7 @@ _l3_acd_data_timeout_schedule_probing_restart(AcdData *acd_data, gint64 now_msec nm_assert(acd_data->probing_timeout_msec > 0); nm_assert(acd_data->probing_timestamp_msec > 0); - expiry_msec = acd_data->probing_timestamp_msec + ACD_WAIT_PROBING_EXTRA_TIME_MSEC - + ACD_WAIT_PROBING_EXTRA_TIME2_MSEC; + expiry_msec = acd_data->probing_timestamp_msec + ACD_WAIT_PROBING_EXTRA_TIME_MSEC; timeout_msec = NM_MAX(0, expiry_msec - now_msec); @@ -2741,9 +2739,8 @@ handle_init: nm_utils_get_monotonic_timestamp_msec_cached(p_now_msec); if (acd_data->info.state == NM_L3_ACD_ADDR_STATE_PROBING) { - if ((*p_now_msec) > acd_data->probing_timestamp_msec - + ACD_WAIT_PROBING_EXTRA_TIME_MSEC - + ACD_WAIT_PROBING_EXTRA_TIME2_MSEC) { + if ((*p_now_msec) + > acd_data->probing_timestamp_msec + ACD_WAIT_PROBING_EXTRA_TIME_MSEC) { /* hm. We failed to create a new probe too long. Something is really wrong * internally, but let's ignore the issue and assume the address is good. What * else would we do? Assume the address is USED? */ @@ -2949,7 +2946,7 @@ handle_init: nm_utils_get_monotonic_timestamp_msec_cached(p_now_msec); if (acd_data->probing_timestamp_msec + acd_data->probing_timeout_msec - + ACD_WAIT_PROBING_EXTRA_TIME_MSEC + ACD_WAIT_PROBING_EXTRA_TIME2_MSEC + + ACD_WAIT_PROBING_EXTRA_TIME_MSEC >= (*p_now_msec)) { /* The probing already started quite a while ago. We ignore the link event * and let the probe come to it's natural end. */