l3cfg: fix pruning of ACD data (take 2)bg/l3cfg-acd-pruning

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.

This is a respin of commit ed565f914699 ('l3cfg: fix pruning of ACD
data') that was reverted because it was causing a crash. The crash was
caused by unconditionally clearing `acd_data_pruning_needed` in
_l3cfg_update_combined_config(), while we need to do it only when
actually committing the configuration.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1749

1 files changed, 9 insertions, 1 deletions

diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c
index 456b5906e4..752febfaaf 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,9 @@ _l3cfg_update_combined_config(NML3Cfg               *self,
 
     NM_SET_OUT(out_changed_combined_l3cd, FALSE);
 
+    if (to_commit)
+        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 +3843,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;
     }