l3cfg: schedule an update after every commit-type/config-data register/unregisterff/fix_ecmp_empty_lst

When we register/unregister a commit-type or when we add/remove a
config-data to NML3Cfg, that act only does the registration/addition.
Only on the next commit, are the changes actually done. The purpose
of that is to add/register multiple configurations and commit them later
when ready.

However, it would be wrong to not do the commit a short time after. The
configuration state is dirty with need to be committed, and that should
happen soon.

Worse, when a interface disappears, NMDevice will clear the ifindex and
the NML3Cfg instance, thereby unregistering all config data and commit
type. If we previously commited something, we need to do another follow-up
commit to cleanup that state.

That is for example important with ECMP routes, which are registered in
NMNetns. When NML3Cfg goes down, it always must unregister to properly
cleanup. Failure to do so, causes an assertion failure and crash. This
change fixes that.

Fix that by automatically schedule and idle commit on
register/unregister/add/remove of commit-type/config-data.
It should *always* be permissible to call a AUTO commit from
an idle handler, because various parties cannot use NML3Cfg
independently, and they cannot know when somebody else does a
commit.

Note that NML3Cfg remembers if it presiouvly did a commit
("commit_type_update_sticky"), so even if the last commit-type gets
unregistered, the next commit will still do a sticky update (one more
time).

The only remaining question is what happens during quitting. When
quitting, NetworkManager we may want to leave some interfaces up and
configured. If we were to properly cleanup the NML3Cfg we might need a
mechanism to handle that. However, currently we just leak everything
during quit, so that is not a concern now. It is something that needs
to be addressed in the future.

https://bugzilla.redhat.com/show_bug.cgi?id=2158394
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1505

2 files changed, 21 insertions, 9 deletions

diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c
index 74787166b1..81dabc0433 100644
--- a/src/core/nm-l3cfg.c
+++ b/src/core/nm-l3cfg.c
@@ -3475,8 +3475,10 @@ nm_l3cfg_add_config(NML3Cfg              *self,
 
     nm_assert(l3_config_data->acd_defend_type_confdata == acd_defend_type);
 
-    if (changed)
+    if (changed) {
         _l3_changed_configs_set_dirty(self);
+        nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
+    }
 
     return changed;
 }
@@ -3520,6 +3522,9 @@ _l3cfg_remove_config(NML3Cfg              *self,
         }
     }
 
+    if (changed)
+        nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
+
     if (self->priv.p->l3_config_datas->len == 0) {
         nm_assert(changed);
         nm_clear_pointer(&self->priv.p->l3_config_datas, g_array_unref);
@@ -4582,7 +4587,7 @@ _l3_commit_one(NML3Cfg              *self,
     gs_unref_ptrarray GPtrArray *addresses_prune                    = NULL;
     gs_unref_ptrarray GPtrArray *routes_prune                       = NULL;
     gs_unref_ptrarray GPtrArray *routes_temporary_not_available_arr = NULL;
-    NMIPRouteTableSyncMode       route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_NONE;
+    NMIPRouteTableSyncMode       route_table_sync;
     gboolean                     final_failure_for_temporary_not_available = FALSE;
     char                         sbuf_commit_type[50];
     gboolean                     success = TRUE;
@@ -4599,15 +4604,15 @@ _l3_commit_one(NML3Cfg              *self,
           nm_utils_addr_family_to_char(addr_family),
           _l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type)));
 
-    if (self->priv.p->combined_l3cd_commited) {
-        addresses = _commit_collect_addresses(self, addr_family, commit_type);
+    addresses = _commit_collect_addresses(self, addr_family, commit_type);
 
-        _commit_collect_routes(self, addr_family, commit_type, &routes, &routes_nodev);
+    _commit_collect_routes(self, addr_family, commit_type, &routes, &routes_nodev);
 
-        route_table_sync =
-            nm_l3_config_data_get_route_table_sync(self->priv.p->combined_l3cd_commited,
-                                                   addr_family);
-    }
+    route_table_sync =
+        self->priv.p->combined_l3cd_commited
+            ? nm_l3_config_data_get_route_table_sync(self->priv.p->combined_l3cd_commited,
+                                                     addr_family)
+            : NM_IP_ROUTE_TABLE_SYNC_MODE_NONE;
 
     if (!IS_IPv4) {
         _l3_commit_ip6_privacy(self, commit_type);
@@ -4942,6 +4947,9 @@ nm_l3cfg_commit_type_register(NML3Cfg                 *self,
         c_list_link_tail(&self->priv.p->commit_type_lst_head, &handle->commit_type_lst);
 
     ret = handle;
+
+    nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
+
 out:
     _LOGT("commit type register (type \"%s\", source \"%s\", existing " NM_HASH_OBFUSCATE_PTR_FMT
           ") -> " NM_HASH_OBFUSCATE_PTR_FMT "",
@@ -4964,6 +4972,8 @@ nm_l3cfg_commit_type_unregister(NML3Cfg *self, NML3CfgCommitTypeHandle *handle)
 
     _LOGT("commit type unregister " NM_HASH_OBFUSCATE_PTR_FMT "", NM_HASH_OBFUSCATE_PTR(handle));
 
+    nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO);
+
     c_list_unlink_stale(&handle->commit_type_lst);
     if (c_list_is_empty(&self->priv.p->commit_type_lst_head))
         g_object_unref(self);
diff --git a/src/core/nm-netns.c b/src/core/nm-netns.c
index e6c72651d0..b5c150c0f3 100644
--- a/src/core/nm-netns.c
+++ b/src/core/nm-netns.c
@@ -638,6 +638,8 @@ nm_netns_ip_route_ecmp_commit(NMNetns *self, NML3Cfg *l3cfg, GPtrArray **out_sin
 
     nm_assert_l3cfg(self, l3cfg);
 
+    _LOGT("ecmp-route: committing IPv4 ECMP routes");
+
     /* First, delete all dirty entries, and mark the survivors as dirty, so that on the
      * next update they must be touched again. */
     c_list_for_each_entry_safe (track_obj,