fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-03-03 20:40:36 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

fixup! l3cfg: handle dynamic added routes tracking and deletion

Browse source 
A "os_non_dynamic" flag is required.

A ObjStateData is alive as long as it either has a static route (from
the combined l3cd) or a dynamic (from nm_netns_ip_route_ecmp_commit()).
Note that it can be both!

So, _obj_states_track_prune_dirty() (formerly
_obj_states_remove_track()) needs to be able to look at an obj_state and
determine whether it's still kept alive (by either).

You might think, "but it cannot be both". The routes from the
non-dynamic combined-l3cd are IPv4 single-hop routes with a weight of
zero. On the other hands, the routes from nm_netns_ip_route_ecmp_commit()
are the ones we passed to NMNetns that didn't find an ECMP merge
partner. They should have a positive "weight", and compare different.

Wrong. The route that comes back from nm_netns_ip_route_ecmp_commit()
must have a weight of zero too. The following commit will normalize
the weight of those routes to zero. Then indeed the non-dynamic and
dynamic routes both have weight zero, and they compare equal.

Just try it by configuring two routes that are identical except one has
weight zero and the other weight 42. (note that to see it, you will need
to follow-up patch to normalize the weight).

But why normalize the weight? Because when we get a route back from
nm_netns_ip_route_ecmp_commit(), we want to be able to look into the
platform cache (whether the route exists). NML3Cfg also tracks routes
from platform, and associates them with the obj-state. The Routes from
platform all have "weight" zero. Normalizing the route from
ecmp_commit() is necessary to be the route we want to add and as it
appears in platform (that is, the route with a zero weight).
This commit is contained in:
Thomas Haller 2023-11-28 15:40:39 +01:00
parent 3dbe8234df
commit d3e1de5000
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
1 changed files with 182 additions and 125 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

307
src/core/nm-l3cfg.c
View file

@ -165,9 +165,14 @@ 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;
bool os_non_dynamic_dirty : 1;
/* Indicates whether this is configured dynamically or statically on l3cd. */
/* 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. */
@ -778,51 +783,62 @@ _nm_n_acd_data_probe_new(NML3Cfg *self, in_addr_t addr, guint32 timeout_msec, gp
/*****************************************************************************/
#define nm_assert_obj_state(self, obj_state) \
G_STMT_START \
{ \
if (NM_MORE_ASSERTS > 0) { \
const NML3Cfg *_self = (self); \
const ObjStateData *_obj_state = (obj_state); \
\
nm_assert(_obj_state); \
nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \
NMP_OBJECT_TYPE_IP4_ADDRESS, \
NMP_OBJECT_TYPE_IP6_ADDRESS, \
NMP_OBJECT_TYPE_IP4_ROUTE, \
NMP_OBJECT_TYPE_IP6_ROUTE)); \
nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \
nm_assert(_obj_state->os_failedobj_expiry_msec != 0 \
|| _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 \
|| c_list_is_empty(&_obj_state->os_zombie_lst)); \
nm_assert(_obj_state->os_failedobj_expiry_msec == 0 || _obj_state->obj); \
if (_self) { \
if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \
nm_assert(_self->priv.p->combined_l3cd_commited); \
\
if (NM_MORE_ASSERTS > 5) { \
nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \
&_obj_state->os_lst)); \
nm_assert(_obj_state->os_plobj \
== nm_platform_lookup_obj(_self->priv.platform, \
NMP_CACHE_ID_TYPE_OBJECT_TYPE, \
_obj_state->obj)); \
nm_assert( \
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, \
_obj_state->obj))) \
: (!nm_l3_config_data_lookup_obj( \
_self->priv.p->combined_l3cd_commited, \
_obj_state->obj))); \
} \
} \
} \
} \
} \
#define nm_assert_obj_state(self, obj_state) \
G_STMT_START \
{ \
if (NM_MORE_ASSERTS > 0) { \
const NML3Cfg *_self = (self); \
const ObjStateData *_obj_state = (obj_state); \
\
nm_assert(_obj_state); \
nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(_obj_state->obj), \
NMP_OBJECT_TYPE_IP4_ADDRESS, \
NMP_OBJECT_TYPE_IP6_ADDRESS, \
NMP_OBJECT_TYPE_IP4_ROUTE, \
NMP_OBJECT_TYPE_IP6_ROUTE)); \
nm_assert(!_obj_state->os_plobj || _obj_state->os_was_in_platform); \
nm_assert(_obj_state->os_failedobj_expiry_msec != 0 \
|| _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 \
|| 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_plobj \
|| NMP_OBJECT_GET_TYPE(_obj_state->obj) \
== NMP_OBJECT_GET_TYPE(_obj_state->os_plobj)); \
if (_self) { \
if (c_list_is_empty(&_obj_state->os_zombie_lst)) { \
nm_assert(_self->priv.p->combined_l3cd_commited); \
\
if (NM_MORE_ASSERTS > 5) { \
nm_assert(c_list_contains(&_self->priv.p->obj_state_lst_head, \
&_obj_state->os_lst)); \
nm_assert(_obj_state->os_plobj \
== nm_platform_lookup_obj(_self->priv.platform, \
NMP_CACHE_ID_TYPE_OBJECT_TYPE, \
_obj_state->obj)); \
if (!c_list_is_empty(&obj_state->os_zombie_lst)) { \
nm_assert(!obj_state->os_non_dynamic); \
nm_assert(!obj_state->os_non_dynamic_dirty); \
nm_assert(!obj_state->os_dynamic); \
nm_assert(!obj_state->os_dynamic_dirty); \
} \
if (obj_state->os_non_dynamic) { \
nm_assert( \
_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
static ObjStateData *
@ -836,7 +852,8 @@ _obj_state_data_new(const NMPObject *obj, const NMPObject *plobj)
.os_plobj = nmp_object_ref(plobj),
.os_was_in_platform = !!plobj,
.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,
@ -907,34 +924,6 @@ _obj_state_data_to_string(const ObjStateData *obj_state, char *buf, gsize buf_si
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
@ -1009,7 +998,7 @@ out:
}
static void
_obj_states_track_new(NML3Cfg *self, const NMPObject *obj, bool is_dynamic)
_obj_states_track_new(NML3Cfg *self, const NMPObject *obj, gboolean dynamic)
{
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
ObjStateData *obj_state;
@ -1017,15 +1006,95 @@ _obj_states_track_new(NML3Cfg *self, const NMPObject *obj, bool is_dynamic)
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;
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);
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);
}
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_remove_track(NML3Cfg *self, bool dynamics)
_obj_states_track_prune_dirty(NML3Cfg *self, gboolean also_dynamic)
{
GHashTableIter h_iter;
ObjStateData *obj_state;
@ -1033,28 +1102,39 @@ _obj_states_remove_track(NML3Cfg *self, bool dynamics)
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))
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;
if (!dynamics && !obj_state->os_dirty)
continue;
if (dynamics && !obj_state->os_dynamic)
continue;
if (dynamics && !obj_state->os_dynamic_dirty)
}
/* 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%s zombie: %s",
dynamics ? " dynamic" : "",
_LOGD("obj-state: now zombie: %s",
_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)));
_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);
@ -1071,21 +1151,13 @@ _obj_states_update_all(NML3Cfg *self)
NMP_OBJECT_TYPE_IP4_ROUTE,
NMP_OBJECT_TYPE_IP6_ROUTE,
};
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
ObjStateData *obj_state;
int i;
gboolean any_dirty = FALSE;
nm_assert(NM_IS_L3CFG(self));
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;
}
any_dirty = TRUE;
obj_state->os_dirty = TRUE;
}
any_dirty = _obj_states_track_mark_dirty(self, FALSE);
for (i = 0; i < (int) G_N_ELEMENTS(obj_types); i++) {
const NMPObjectType obj_type = obj_types[i];
@ -1115,18 +1187,12 @@ _obj_states_update_all(NML3Cfg *self)
continue;
}
if (_obj_state_data_update(obj_state, obj)) {
_LOGD("obj-state: update: %s",
_obj_state_data_to_string(obj_state, sbuf, sizeof(sbuf)));
}
nm_assert_obj_state(self, obj_state);
_obj_states_track_update(self, obj_state, obj, FALSE);
}
}
if (any_dirty) {
_obj_states_remove_track(self, FALSE);
}
if (any_dirty)
_obj_states_track_prune_dirty(self, FALSE);
}
typedef struct {
@ -1216,18 +1282,6 @@ _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,
@ -1307,7 +1361,6 @@ 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];
@ -1317,7 +1370,7 @@ loop_done:
if (!obj_state)
_obj_states_track_new(self, obj, TRUE);
else
obj_state->os_dynamic_dirty = FALSE;
_obj_states_track_update(self, obj_state, obj, TRUE);
if (!_obj_states_sync_filter(self, obj, commit_type))
continue;
@ -4894,6 +4947,7 @@ _l3_commit_one(NML3Cfg *self,
NMIPRouteTableSyncMode route_table_sync;
char sbuf_commit_type[50];
guint i;
gboolean any_dirty = FALSE;
nm_assert(NM_IS_L3CFG(self));
nm_assert(NM_IN_SET(commit_type,
@ -4906,6 +4960,9 @@ _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 (IS_IPv4)
any_dirty = _obj_states_track_mark_dirty(self, TRUE);
addresses = _commit_collect_addresses(self, addr_family, commit_type);
_commit_collect_routes(self,
@ -4930,8 +4987,8 @@ _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 (any_dirty)
_obj_states_track_prune_dirty(self, TRUE);
if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) {
gs_unref_array GArray *ipv6_temp_addrs_keep = NULL;