colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

l3cfg: handle dynamic added routes tracking and deletion

Browse Source 
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.

Without this, the state of those ECMP single hop routes is not properly
tracked, and they are for example not removed by NML3Cfg when they
should.

Usually, routes to be configured originate from the combined
NML3ConfigData and are resolved early during a commit. For example,
_obj_states_update_all() creates for each such route an obj_state_hash
entry. Let's call those static, or "non-dynamic".

Later, we can receive additional routes. We get them back from NMNetns
during nm_netns_ip_route_ecmp_commit() (_commit_collect_routes()).
Let's call them "dynamic".

For those routes, we also must track an obj-state.

Now we have two reasons why an obj-state is tracked. The "non-dynamic"
and the "dynamic" one. Add two flags "os_dynamic" and "os_non_dynamic"
to the ObjStateData and consider the flags at the necessary places.

Co-authored-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
This commit is contained in:
Thomas Haller
2023-11-24 14:46:27 +01:00
committed by Fernando Fernandez Mancera
parent ffe22c498f
commit 5d26452da5
1 changed files with 235 additions and 128 deletions
Download Patch File Download Diff File Expand all files Collapse all files

363
src/core/nm-l3cfg.c
View File

@@ -165,7 +165,18 @@ typedef struct {
/* This flag is only used temporarily to do a bulk update and /* This flag is only used temporarily to do a bulk update and
* clear all the ones that are no longer in used. */ * clear all the ones that are no longer in used. */
bool os_dirty : 1; bool os_non_dynamic_dirty : 1;
/* Indicates that we have a object in combined_l3cd_commited that keeps the
* object state alive. */
bool os_non_dynamic : 1;
/* Indicates that there is a dynamic route from _commit_collect_routes(), that keeps the
* object state alive. */
bool os_dynamic : 1;
/* Indicates that this dynamic obj-state is marked as dirty. */
bool os_dynamic_dirty : 1;
} ObjStateData; } ObjStateData;
G_STATIC_ASSERT(G_STRUCT_OFFSET(ObjStateData, obj) == 0); G_STATIC_ASSERT(G_STRUCT_OFFSET(ObjStateData, obj) == 0);
@@ -772,59 +783,70 @@ _nm_n_acd_data_probe_new(NML3Cfg *self, in_addr_t addr, guint32 timeout_msec, gp
/*****************************************************************************/ /*****************************************************************************/
#define nm_assert_obj_state(self, obj_state) \ #define nm_assert_obj_state(self, obj_state) \
G_STMT_START \ G_STMT_START \
{ \ { \
if (NM_MORE_ASSERTS > 0) { \ if (NM_MORE_ASSERTS > 0) { \
const NML3Cfg *_self = (self); \ const NML3Cfg *_self = (self); \
const ObjStateData *_obj_state = (obj_state); \ const ObjStateData *_obj_state = (obj_state); \
\ \
nm_assert(_obj_state); \ nm_assert(_obj_state); \
nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \ nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \
NMP_OBJECT_TYPE_IP4_ADDRESS, \ NMP_OBJECT_TYPE_IP4_ADDRESS, \
NMP_OBJECT_TYPE_IP6_ADDRESS, \ NMP_OBJECT_TYPE_IP6_ADDRESS, \
NMP_OBJECT_TYPE_IP4_ROUTE, \ NMP_OBJECT_TYPE_IP4_ROUTE, \
NMP_OBJECT_TYPE_IP6_ROUTE)); \ NMP_OBJECT_TYPE_IP6_ROUTE)); \
nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \ nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \
nm_assert(_obj_state->os_failedobj_expiry_msec != 0 \ nm_assert(_obj_state->os_failedobj_expiry_msec != 0 \
|| _obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); \ || _obj_state->os_failedobj_prioq_idx == NM_PRIOQ_IDX_NULL); \
nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || !_obj_state->os_plobj); \ nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || !_obj_state->os_plobj); \
nm_assert(_obj_state->os_failedobj_expiry_msec == 0 \ nm_assert(_obj_state->os_failedobj_expiry_msec == 0 \
|| c_list_is_empty(&_obj_state->os_zombie_lst)); \ || c_list_is_empty(&_obj_state->os_zombie_lst)); \
nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || _obj_state->obj); \ nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || _obj_state->obj); \
if (_self) { \ nm_assert(!_obj_state->os_plobj \
if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \ || NMP_OBJECT_GET_TYPE(_obj_state->obj) \
nm_assert(_self->priv.p->combined_l3cd_commited); \ == NMP_OBJECT_GET_TYPE(_obj_state->os_plobj)); \
\ if (_self) { \
if (NM_MORE_ASSERTS > 5) { \ if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \
nm_assert(_self->priv.p->combined_l3cd_commited); \
\
if (NM_MORE_ASSERTS > 5) { \
/* metric-any must be resolved before adding the object. Otherwise, /* metric-any must be resolved before adding the object. Otherwise,
* their real metric is not known, and they cannot be compared to objects * their real metric is not known, and they cannot be compared to objects
* from NMPlatform cache. */ \ * from NMPlatform cache. */ \
nm_assert(!NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \ nm_assert(!NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \
NMP_OBJECT_TYPE_IP4_ROUTE, \ NMP_OBJECT_TYPE_IP4_ROUTE, \
NMP_OBJECT_TYPE_IP6_ROUTE) \ NMP_OBJECT_TYPE_IP6_ROUTE) \
|| !NMP_OBJECT_CAST_IP_ROUTE(_obj_state->obj)->metric_any); \ || !NMP_OBJECT_CAST_IP_ROUTE(_obj_state->obj)->metric_any); \
\ \
nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \ nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \
&_obj_state->os_lst)); \ &_obj_state->os_lst)); \
nm_assert(_obj_state->os_plobj \ nm_assert(_obj_state->os_plobj \
== nm_platform_lookup_obj(_self->priv.platform, \ == nm_platform_lookup_obj(_self->priv.platform, \
NMP_CACHE_ID_TYPE_OBJECT_TYPE, \ NMP_CACHE_ID_TYPE_OBJECT_TYPE, \
_obj_state->obj)); \ _obj_state->obj)); \
nm_assert( \ if (!c_list_is_empty(&obj_state->os_zombie_lst)) { \
c_list_is_empty(&obj_state->os_zombie_lst) \ nm_assert(!obj_state->os_non_dynamic); \
? (_obj_state->obj \ nm_assert(!obj_state->os_non_dynamic_dirty); \
== nm_dedup_multi_entry_get_obj(nm_l3_config_data_lookup_obj( \ nm_assert(!obj_state->os_dynamic); \
_self->priv.p->combined_l3cd_commited, \ nm_assert(!obj_state->os_dynamic_dirty); \
_obj_state->obj))) \ } \
: (!nm_l3_config_data_lookup_obj( \ if (obj_state->os_non_dynamic) { \
_self->priv.p->combined_l3cd_commited, \ nm_assert( \
_obj_state->obj))); \ _obj_state->obj \
} \ == nm_dedup_multi_entry_get_obj(nm_l3_config_data_lookup_obj( \
} \ _self->priv.p->combined_l3cd_commited, \
} \ _obj_state->obj))); \
} \ } else { \
} \ nm_assert(!nm_l3_config_data_lookup_obj( \
_self->priv.p->combined_l3cd_commited, \
_obj_state->obj)); \
} \
} \
} \
} \
} \
} \
G_STMT_END G_STMT_END
static ObjStateData * static ObjStateData *
@@ -838,7 +860,10 @@ _obj_state_data_new(const NMPObject *obj, const NMPObject *plobj)
.os_plobj = nmp_object_ref(plobj), .os_plobj = nmp_object_ref(plobj),
.os_was_in_platform = !!plobj, .os_was_in_platform = !!plobj,
.os_nm_configured = FALSE, .os_nm_configured = FALSE,
.os_dirty = FALSE, .os_non_dynamic_dirty = FALSE,
.os_non_dynamic = FALSE,
.os_dynamic = FALSE,
.os_dynamic_dirty = FALSE,
.os_failedobj_expiry_msec = 0, .os_failedobj_expiry_msec = 0,
.os_failedobj_prioq_idx = NM_PRIOQ_IDX_NULL, .os_failedobj_prioq_idx = NM_PRIOQ_IDX_NULL,
.os_zombie_lst = C_LIST_INIT(obj_state->os_zombie_lst), .os_zombie_lst = C_LIST_INIT(obj_state->os_zombie_lst),
@@ -907,34 +932,6 @@ _obj_state_data_to_string(const ObjStateData *obj_state, char *buf, gsize buf_si
return buf0; return buf0;
} }
static gboolean
_obj_state_data_update(ObjStateData *obj_state, const NMPObject *obj)
{
gboolean changed = FALSE;
nm_assert_obj_state(NULL, obj_state);
nm_assert(obj);
nm_assert(nmp_object_id_equal(obj_state->obj, obj));
obj_state->os_dirty = FALSE;
if (obj_state->obj != obj) {
nm_auto_nmpobj const NMPObject *obj_old = NULL;
if (!nmp_object_equal(obj_state->obj, obj))
changed = TRUE;
obj_old = g_steal_pointer(&obj_state->obj);
obj_state->obj = nmp_object_ref(obj);
}
if (!c_list_is_empty(&obj_state->os_zombie_lst)) {
c_list_unlink(&obj_state->os_zombie_lst);
changed = TRUE;
}
return changed;
}
/*****************************************************************************/ /*****************************************************************************/
static void static void
@@ -1009,7 +1006,7 @@ out:
} }
static void static void
_obj_states_track_new(NML3Cfg *self, const NMPObject *obj) _obj_states_track_new(NML3Cfg *self, const NMPObject *obj, gboolean dynamic)
{ {
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
ObjStateData *obj_state; ObjStateData *obj_state;
@@ -1017,12 +1014,142 @@ _obj_states_track_new(NML3Cfg *self, const NMPObject *obj)
obj_state = _obj_state_data_new( obj_state = _obj_state_data_new(
obj, obj,
nm_platform_lookup_obj(self->priv.platform, NMP_CACHE_ID_TYPE_OBJECT_TYPE, obj)); nm_platform_lookup_obj(self->priv.platform, NMP_CACHE_ID_TYPE_OBJECT_TYPE, obj));
obj_state->os_dynamic = dynamic;
obj_state->os_non_dynamic = !dynamic;
c_list_link_tail(&self->priv.p->obj_state_lst_head, &obj_state->os_lst); 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); 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))); _LOGD("obj-state: track%s: %s",
dynamic ? " (dynamic)" : "",
_obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf)));
nm_assert_obj_state(self, obj_state); nm_assert_obj_state(self, obj_state);
} }
static gboolean
_obj_states_track_update(NML3Cfg *self,
ObjStateData *obj_state,
const NMPObject *obj,
gboolean dynamic)
{
gboolean changed = FALSE;
nm_assert_obj_state(NULL, obj_state);
nm_assert(obj);
nm_assert(nmp_object_id_equal(obj_state->obj, obj));
if (dynamic) {
if (!obj_state->os_dynamic)
changed = TRUE;
obj_state->os_dynamic_dirty = FALSE;
obj_state->os_dynamic = TRUE;
} else {
if (!obj_state->os_non_dynamic)
changed = TRUE;
obj_state->os_non_dynamic_dirty = FALSE;
obj_state->os_non_dynamic = TRUE;
}
if (obj_state->obj != obj && (!dynamic || !obj_state->os_non_dynamic)) {
nm_auto_nmpobj const NMPObject *obj_old = NULL;
if (!nmp_object_equal(obj_state->obj, obj))
changed = TRUE;
obj_old = g_steal_pointer(&obj_state->obj);
obj_state->obj = nmp_object_ref(obj);
}
if (!c_list_is_empty(&obj_state->os_zombie_lst)) {
c_list_unlink(&obj_state->os_zombie_lst);
changed = TRUE;
}
if (changed) {
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
_LOGD("obj-state: update: %s (static: %d, dynamic: %d)",
_obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf)),
!!obj_state->os_non_dynamic,
!!obj_state->os_dynamic);
}
nm_assert_obj_state(self, obj_state);
return changed;
}
static gboolean
_obj_states_track_mark_dirty(NML3Cfg *self, gboolean dynamic)
{
ObjStateData *obj_state;
gboolean any_dirty = FALSE;
c_list_for_each_entry (obj_state, &self->priv.p->obj_state_lst_head, os_lst) {
if (!c_list_is_empty(&obj_state->os_zombie_lst)) {
/* we can ignore zombies. */
continue;
}
if (dynamic) {
if (!obj_state->os_dynamic)
continue;
obj_state->os_dynamic_dirty = TRUE;
} else {
if (!obj_state->os_non_dynamic)
continue;
obj_state->os_non_dynamic_dirty = TRUE;
}
any_dirty = TRUE;
}
return any_dirty;
}
static void
_obj_states_track_prune_dirty(NML3Cfg *self, gboolean also_dynamic)
{
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)) {
/* The object is half-dead already and only kept for cleanup. But
* it does not need to be untracked. */
continue;
}
/* Resolve the "dirty" flags. */
if (obj_state->os_non_dynamic_dirty) {
obj_state->os_non_dynamic = FALSE;
obj_state->os_non_dynamic_dirty = FALSE;
}
if (also_dynamic) {
if (obj_state->os_dynamic_dirty) {
obj_state->os_dynamic = FALSE;
obj_state->os_dynamic_dirty = FALSE;
}
}
if (obj_state->os_non_dynamic || obj_state->os_dynamic) {
/* This obj-state is still alive. Keep it. */
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);
}
}
static void static void
_obj_states_update_all(NML3Cfg *self) _obj_states_update_all(NML3Cfg *self)
{ {
@@ -1032,21 +1159,13 @@ _obj_states_update_all(NML3Cfg *self)
NMP_OBJECT_TYPE_IP4_ROUTE, NMP_OBJECT_TYPE_IP4_ROUTE,
NMP_OBJECT_TYPE_IP6_ROUTE, NMP_OBJECT_TYPE_IP6_ROUTE,
}; };
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
ObjStateData *obj_state; ObjStateData *obj_state;
int i; int i;
gboolean any_dirty = FALSE; gboolean any_dirty = FALSE;
nm_assert(NM_IS_L3CFG(self)); nm_assert(NM_IS_L3CFG(self));
c_list_for_each_entry (obj_state, &self->priv.p->obj_state_lst_head, os_lst) { any_dirty = _obj_states_track_mark_dirty(self, FALSE);
if (!c_list_is_empty(&obj_state->os_zombie_lst)) {
/* we can ignore zombies. */
continue;
}
any_dirty = TRUE;
obj_state->os_dirty = TRUE;
}
for (i = 0; i < (int) G_N_ELEMENTS(obj_types); i++) { for (i = 0; i < (int) G_N_ELEMENTS(obj_types); i++) {
const NMPObjectType obj_type = obj_types[i]; const NMPObjectType obj_type = obj_types[i];
@@ -1064,50 +1183,24 @@ _obj_states_update_all(NML3Cfg *self)
/* this is a nodev route. We don't track an obj-state for this. */ /* this is a nodev route. We don't track an obj-state for this. */
continue; continue;
} }
if (obj_type == NMP_OBJECT_TYPE_IP4_ROUTE
&& NMP_OBJECT_CAST_IP4_ROUTE(obj)->weight > 0) {
/* this route weight is bigger than 0, that means we don't know
* which kind of route this will be. It can only be determined during commit. */
continue;
}
obj_state = g_hash_table_lookup(self->priv.p->obj_state_hash, &obj); obj_state = g_hash_table_lookup(self->priv.p->obj_state_hash, &obj);
if (!obj_state) { if (!obj_state) {
_obj_states_track_new(self, obj); _obj_states_track_new(self, obj, FALSE);
continue; continue;
} }
if (_obj_state_data_update(obj_state, obj)) { _obj_states_track_update(self, obj_state, obj, FALSE);
_LOGD("obj-state: update: %s",
_obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf)));
}
nm_assert_obj_state(self, obj_state);
} }
} }
if (any_dirty) { if (any_dirty)
GHashTableIter h_iter; _obj_states_track_prune_dirty(self, FALSE);
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);
}
}
} }
typedef struct { typedef struct {
@@ -1279,6 +1372,13 @@ loop_done:
if (singlehop_routes) { if (singlehop_routes) {
for (i = 0; i < singlehop_routes->len; i++) { for (i = 0; i < singlehop_routes->len; i++) {
const NMPObject *obj = singlehop_routes->pdata[i]; const NMPObject *obj = singlehop_routes->pdata[i];
ObjStateData *obj_state;
obj_state = g_hash_table_lookup(self->priv.p->obj_state_hash, &obj);
if (!obj_state)
_obj_states_track_new(self, obj, TRUE);
else
_obj_states_track_update(self, obj_state, obj, TRUE);
if (!_obj_states_sync_filter(self, obj, commit_type)) if (!_obj_states_sync_filter(self, obj, commit_type))
continue; continue;
@@ -4855,6 +4955,7 @@ _l3_commit_one(NML3Cfg *self,
NMIPRouteTableSyncMode route_table_sync; NMIPRouteTableSyncMode route_table_sync;
char sbuf_commit_type[50]; char sbuf_commit_type[50];
guint i; guint i;
gboolean any_dirty = FALSE;
nm_assert(NM_IS_L3CFG(self)); nm_assert(NM_IS_L3CFG(self));
nm_assert(NM_IN_SET(commit_type, nm_assert(NM_IN_SET(commit_type,
@@ -4867,6 +4968,9 @@ _l3_commit_one(NML3Cfg *self,
nm_utils_addr_family_to_char(addr_family), nm_utils_addr_family_to_char(addr_family),
_l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type))); _l3_cfg_commit_type_to_string(commit_type, sbuf_commit_type, sizeof(sbuf_commit_type)));
if (IS_IPv4)
any_dirty = _obj_states_track_mark_dirty(self, TRUE);
addresses = _commit_collect_addresses(self, addr_family, commit_type); addresses = _commit_collect_addresses(self, addr_family, commit_type);
_commit_collect_routes(self, _commit_collect_routes(self,
@@ -4891,6 +4995,9 @@ _l3_commit_one(NML3Cfg *self,
if (route_table_sync == NM_IP_ROUTE_TABLE_SYNC_MODE_NONE) if (route_table_sync == NM_IP_ROUTE_TABLE_SYNC_MODE_NONE)
route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN; route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN;
if (any_dirty)
_obj_states_track_prune_dirty(self, TRUE);
if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) { if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) {
gs_unref_array GArray *ipv6_temp_addrs_keep = NULL; gs_unref_array GArray *ipv6_temp_addrs_keep = NULL;