From 3dbe8234dffa8718250461cb79334f91de6f86c0 Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Mon, 27 Nov 2023 20:23:20 +0100 Subject: [PATCH] l3cfg: handle dynamic added routes tracking and deletion Dynamic added routes i.e ECMP single-hop routes, are not managed by l3cd as the other ones. Therefore, they need to be tracked properly and marked as dirty when they are. This way, NetworkManager makes sure there are no leftovers from merging ECMP multi-hop routes. --- src/core/nm-l3cfg.c | 100 +++++++++++++++++++++++++++++++------------- 1 file changed, 70 insertions(+), 30 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 9961eae820..fe5a8df908 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -166,6 +166,12 @@ typedef struct { /* This flag is only used temporarily to do a bulk update and * clear all the ones that are no longer in used. */ bool os_dirty : 1; + + /* Indicates whether this is configured dynamically or statically on l3cd. */ + bool os_dynamic : 1; + + /* Indicates that this dynamic obj-state is marked as dirty. */ + bool os_dynamic_dirty : 1; } ObjStateData; G_STATIC_ASSERT(G_STRUCT_OFFSET(ObjStateData, obj) == 0); @@ -804,7 +810,7 @@ _nm_n_acd_data_probe_new(NML3Cfg *self, in_addr_t addr, guint32 timeout_msec, gp NMP_CACHE_ID_TYPE_OBJECT_TYPE, \ _obj_state->obj)); \ nm_assert( \ - c_list_is_empty(&obj_state->os_zombie_lst) \ + c_list_is_empty(&obj_state->os_zombie_lst) && !obj_state->os_dynamic \ ? (_obj_state->obj \ == nm_dedup_multi_entry_get_obj(nm_l3_config_data_lookup_obj( \ _self->priv.p->combined_l3cd_commited, \ @@ -831,6 +837,8 @@ _obj_state_data_new(const NMPObject *obj, const NMPObject *plobj) .os_was_in_platform = !!plobj, .os_nm_configured = FALSE, .os_dirty = FALSE, + .os_dynamic = FALSE, + .os_dynamic_dirty = FALSE, .os_failedobj_expiry_msec = 0, .os_failedobj_prioq_idx = NM_PRIOQ_IDX_NULL, .os_zombie_lst = C_LIST_INIT(obj_state->os_zombie_lst), @@ -1001,7 +1009,7 @@ out: } static void -_obj_states_track_new(NML3Cfg *self, const NMPObject *obj) +_obj_states_track_new(NML3Cfg *self, const NMPObject *obj, bool is_dynamic) { char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; ObjStateData *obj_state; @@ -1009,12 +1017,51 @@ _obj_states_track_new(NML3Cfg *self, const NMPObject *obj) obj_state = _obj_state_data_new( obj, nm_platform_lookup_obj(self->priv.platform, NMP_CACHE_ID_TYPE_OBJECT_TYPE, obj)); + obj_state->os_dynamic = is_dynamic; c_list_link_tail(&self->priv.p->obj_state_lst_head, &obj_state->os_lst); g_hash_table_add(self->priv.p->obj_state_hash, obj_state); _LOGD("obj-state: track: %s", _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); nm_assert_obj_state(self, obj_state); } +static void +_obj_states_remove_track(NML3Cfg *self, bool dynamics) +{ + GHashTableIter h_iter; + ObjStateData *obj_state; + char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; + + g_hash_table_iter_init(&h_iter, self->priv.p->obj_state_hash); + while (g_hash_table_iter_next(&h_iter, (gpointer *) &obj_state, NULL)) { + if (!c_list_is_empty(&obj_state->os_zombie_lst)) + continue; + if (!dynamics && !obj_state->os_dirty) + continue; + if (dynamics && !obj_state->os_dynamic) + continue; + if (dynamics && !obj_state->os_dynamic_dirty) + continue; + + if (obj_state->os_plobj && obj_state->os_nm_configured) { + nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); + c_list_link_tail(&self->priv.p->obj_state_zombie_lst_head, &obj_state->os_zombie_lst); + obj_state->os_zombie_count = ZOMBIE_COUNT_START; + _LOGD("obj-state: now%s zombie: %s", + dynamics ? " dynamic" : "", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + continue; + } + + _LOGD("obj-state:%s untrack: %s", + dynamics ? " dynamic" : "", + _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); + nm_prioq_remove(&self->priv.p->failedobj_prioq, + obj_state, + &obj_state->os_failedobj_prioq_idx); + g_hash_table_iter_remove(&h_iter); + } +} + static void _obj_states_update_all(NML3Cfg *self) { @@ -1064,7 +1111,7 @@ _obj_states_update_all(NML3Cfg *self) } obj_state = g_hash_table_lookup(self->priv.p->obj_state_hash, &obj); if (!obj_state) { - _obj_states_track_new(self, obj); + _obj_states_track_new(self, obj, FALSE); continue; } @@ -1078,32 +1125,7 @@ _obj_states_update_all(NML3Cfg *self) } if (any_dirty) { - GHashTableIter h_iter; - - g_hash_table_iter_init(&h_iter, self->priv.p->obj_state_hash); - while (g_hash_table_iter_next(&h_iter, (gpointer *) &obj_state, NULL)) { - if (!c_list_is_empty(&obj_state->os_zombie_lst)) - continue; - if (!obj_state->os_dirty) - continue; - - if (obj_state->os_plobj && obj_state->os_nm_configured) { - nm_assert(obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); - c_list_link_tail(&self->priv.p->obj_state_zombie_lst_head, - &obj_state->os_zombie_lst); - obj_state->os_zombie_count = ZOMBIE_COUNT_START; - _LOGD("obj-state: now zombie: %s", - _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); - continue; - } - - _LOGD("obj-state: untrack: %s", - _obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf))); - nm_prioq_remove(&self->priv.p->failedobj_prioq, - obj_state, - &obj_state->os_failedobj_prioq_idx); - g_hash_table_iter_remove(&h_iter); - } + _obj_states_remove_track(self, FALSE); } } @@ -1194,6 +1216,18 @@ _commit_collect_addresses(NML3Cfg *self, int addr_family, NML3CfgCommitType comm (gpointer) &sync_filter_data); } +static void +_obj_states_mark_dynamic_as_dirty(NML3Cfg *self) +{ + ObjStateData *obj_state; + + c_list_for_each_entry (obj_state, &self->priv.p->obj_state_lst_head, os_lst) { + if (!obj_state->os_dynamic) + continue; + obj_state->os_dynamic_dirty = TRUE; + } +} + static void _commit_collect_routes(NML3Cfg *self, int addr_family, @@ -1273,6 +1307,7 @@ loop_done: &singlehop_routes, NM_IN_SET(commit_type, NM_L3_CFG_COMMIT_TYPE_REAPPLY)); + _obj_states_mark_dynamic_as_dirty(self); if (singlehop_routes) { for (i = 0; i < singlehop_routes->len; i++) { const NMPObject *obj = singlehop_routes->pdata[i]; @@ -1280,7 +1315,9 @@ loop_done: obj_state = g_hash_table_lookup(self->priv.p->obj_state_hash, &obj); if (!obj_state) - _obj_states_track_new(self, obj); + _obj_states_track_new(self, obj, TRUE); + else + obj_state->os_dynamic_dirty = FALSE; if (!_obj_states_sync_filter(self, obj, commit_type)) continue; @@ -4893,6 +4930,9 @@ _l3_commit_one(NML3Cfg *self, if (route_table_sync == NM_IP_ROUTE_TABLE_SYNC_MODE_NONE) route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN; + if (IS_IPv4) + _obj_states_remove_track(self, TRUE); + if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) { gs_unref_array GArray *ipv6_temp_addrs_keep = NULL;