l3cfg: remove routes added by NM on reapply

By default, on reapply we were only syncing the main routes table. This
causes that routes added by NM to other tables are not removed on
reapply. This was done to preserve routes added externally, but routes
added by NM itself should be removed.

Add a new route table syncing mode "main + NM routes". This mode
maintains the normal behaviour of syncing completely the main table,
and for other tables removes only routes that were added by us, leaving
the rest untouched. Use this mode by default, as this is what a user
would expect on reapply.

Note: this might not work if NM is restarted between the profile being
modified and the reapply, because NM forgets what routes were added by
itself because of the restart. This is a rare corner case, though.

Use the D-Bus property "VersionInfo" to expose a capability flag
indicating that this bug is fixed. It is the first capability that we
expose in this way. However, it is convenient to do it this way as it's
something that clients like nmstate needs to know, so they can decide
whether a conn down is needed or not. It is not enough to decide that by
version number because it might be fixed via a downstream patch in distros
like RHEL.

https://issues.redhat.com/browse/RHEL-67324
https://issues.redhat.com/browse/RHEL-66262

Fixes: e9c17fcc9b ('l3cfg: default to 'main' route table sync mode')
(cherry picked from commit e330eb9c4a)
(cherry picked from commit 7f6e84b26e)
This commit is contained in:
Íñigo Huguet 2024-11-27 13:53:02 +01:00
parent c537e9b750
commit 95064b8025
6 changed files with 127 additions and 24 deletions

View file

@ -4999,7 +4999,7 @@ _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;
route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES;
if (any_dirty)
_obj_states_track_prune_dirty(self, TRUE);
@ -5028,6 +5028,8 @@ _l3_commit_one(NML3Cfg *self,
}
if (c_list_is_empty(&self->priv.p->blocked_lst_head_x[IS_IPv4])) {
gs_unref_ptrarray GPtrArray *routes_old = NULL;
addresses_prune =
nm_platform_ip_address_get_prune_list(self->priv.platform,
addr_family,
@ -5035,10 +5037,26 @@ _l3_commit_one(NML3Cfg *self,
nm_g_array_data(ipv6_temp_addrs_keep),
nm_g_array_len(ipv6_temp_addrs_keep));
if (route_table_sync == NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES) {
NMDedupMultiIter iter;
const NMPObject *rt_obj;
routes_old = g_ptr_array_new();
nm_l3_config_data_iter_obj_for_each (&iter,
l3cd_old,
&rt_obj,
NMP_OBJECT_TYPE_IP_ROUTE(IS_IPv4))
g_ptr_array_add(routes_old, (gpointer) rt_obj);
nm_platform_route_objs_sort(routes_old, NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY);
}
routes_prune = nm_platform_ip_route_get_prune_list(self->priv.platform,
addr_family,
self->priv.ifindex,
route_table_sync);
route_table_sync,
routes_old);
_obj_state_zombie_lst_prune_all(self, addr_family);
}
} else {

View file

@ -462,21 +462,24 @@ static GVariant *
_version_info_get(void)
{
const guint32 arr[] = {
/* The array contains as first element NM_VERSION, which can be
* used to numerically compare the version (see also NM_ENCODE_VERSION,
* nm_utils_version(), nm_encode_version() and nm_decode_version(). */
NM_VERSION,
};
/* The array contains as first element NM_VERSION, which can be
* used to numerically compare the version (see also NM_ENCODE_VERSION,
* nm_utils_version(), nm_encode_version() and nm_decode_version().
*
* The following elements of the array are a bitfield of capabilities.
* These capabilities should only depend on compile-time abilities
* (unlike NM_MANAGER_CAPABILITIES, NMCapability). The supported values
* are from NMVersionInfoCapability enum. This way to expose capabilities
* is more cumbersome but more efficient compared to NM_MANAGER_CAPABILITIES.
* As such, it is cheap to add capabilities for something, where you would
* avoid it as NM_MANAGER_CAPABILITIES due to the overhead.
*/
/* The following elements of the array are a bitfield of capabilities.
* These capabilities should only depend on compile-time abilities
* (unlike NM_MANAGER_CAPABILITIES, NMCapability). The supported values
* are from NMVersionInfoCapability enum. This way to expose capabilities
* is more cumbersome but more efficient compared to NM_MANAGER_CAPABILITIES.
* As such, it is cheap to add capabilities for something, where you would
* avoid it as NM_MANAGER_CAPABILITIES due to the overhead.
*
* Each of the array's elements has 32 bits. This means that capabilities
* with index 0-31 goes to element #1, with index 32-63 to element #2,
* with index 64-95 to element #3 and so on. */
1 << NM_VERSION_INFO_CAPABILITY_SYNC_ROUTE_WITH_TABLE,
};
return nm_g_variant_new_au(arr, G_N_ELEMENTS(arr));
}

View file

@ -93,16 +93,19 @@
/**
* NMVersionInfoCapability:
* %_NM_VERSION_INFO_CAPABILITY_UNUSED: a dummy capability. It has no meaning,
* don't use it.
* @NM_VERSION_INFO_CAPABILITY_SYNC_ROUTE_WITH_TABLE: Contains the fix to a bug that
* caused that routes in table other than main were not removed on reapply nor
* on connection down.
* https://issues.redhat.com/browse/RHEL-66262
* https://issues.redhat.com/browse/RHEL-67324
*
* Currently no enum values are defined. These capabilities are exposed
* on D-Bus in the "VersionInfo" bit field.
* The numeric values represent the bit index of the capability. These capabilities
* can be queried in the "VersionInfo" D-Bus property.
*
* Since: 1.42
*/
typedef enum {
_NM_VERSION_INFO_CAPABILITY_UNUSED = 0x7FFFFFFFu,
NM_VERSION_INFO_CAPABILITY_SYNC_ROUTE_WITH_TABLE = 0,
} NMVersionInfoCapability;
/**

View file

@ -61,6 +61,8 @@ G_STATIC_ASSERT(sizeof(((NMPlatformLink *) NULL)->l_address.data) == _NM_UTILS_H
G_STATIC_ASSERT(sizeof(((NMPlatformLink *) NULL)->l_perm_address.data) == _NM_UTILS_HWADDR_LEN_MAX);
G_STATIC_ASSERT(sizeof(((NMPlatformLink *) NULL)->l_broadcast.data) == _NM_UTILS_HWADDR_LEN_MAX);
static int _route_objs_cmp_values(gconstpointer a, gconstpointer b, gpointer user_data);
static const char *
_nmp_link_port_data_to_string(NMPortKind port_kind,
const NMPlatformLinkPortData *port_data,
@ -4872,11 +4874,24 @@ nm_platform_ip_address_get_prune_list(NMPlatform *self,
return result;
}
static gboolean
_route_obj_find_bsearch(GPtrArray *sorted_routes_objs, const NMPObject *route_obj)
{
gssize pos =
nm_ptrarray_find_bsearch((gconstpointer *) sorted_routes_objs->pdata,
sorted_routes_objs->len,
route_obj,
_route_objs_cmp_values,
GINT_TO_POINTER((int) NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY));
return pos >= 0;
}
GPtrArray *
nm_platform_ip_route_get_prune_list(NMPlatform *self,
int addr_family,
int ifindex,
NMIPRouteTableSyncMode route_table_sync)
NMIPRouteTableSyncMode route_table_sync,
GPtrArray *sorted_old_routes_objs)
{
NMPLookup lookup;
GPtrArray *routes_prune = NULL;
@ -4891,9 +4906,20 @@ nm_platform_ip_route_get_prune_list(NMPlatform *self,
nm_assert(NM_IN_SET(route_table_sync,
NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_EXCEPT_LOCAL,
NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE));
if (route_table_sync == NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES) {
nm_assert(sorted_old_routes_objs);
nm_assert(nm_utils_ptrarray_is_sorted(
(gconstpointer *) sorted_old_routes_objs->pdata,
sorted_old_routes_objs->len,
FALSE,
_route_objs_cmp_values,
GINT_TO_POINTER((int) NM_PLATFORM_IP_ROUTE_CMP_TYPE_SEMANTICALLY)));
}
nmp_lookup_init_object_by_ifindex(&lookup,
NMP_OBJECT_TYPE_IP_ROUTE(NM_IS_IPv4(addr_family)),
ifindex);
@ -4915,6 +4941,11 @@ nm_platform_ip_route_get_prune_list(NMPlatform *self,
if (!nm_platform_route_table_is_main(nm_platform_ip_route_get_effective_table(&rt->rx)))
continue;
break;
case NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES:
if (!nm_platform_route_table_is_main(nm_platform_ip_route_get_effective_table(&rt->rx))
&& !_route_obj_find_bsearch(sorted_old_routes_objs, obj))
continue;
break;
case NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_EXCEPT_LOCAL:
if (nm_platform_ip_route_get_effective_table(&rt->rx) == RT_TABLE_LOCAL)
continue;
@ -5284,7 +5315,8 @@ nm_platform_ip_route_flush(NMPlatform *self, int addr_family, int ifindex)
routes_prune = nm_platform_ip_route_get_prune_list(self,
AF_INET,
ifindex,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE);
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE,
NULL);
success &= nm_platform_ip_route_sync(self, AF_INET, ifindex, NULL, routes_prune, NULL);
}
if (NM_IN_SET(addr_family, AF_UNSPEC, AF_INET6)) {
@ -5293,7 +5325,8 @@ nm_platform_ip_route_flush(NMPlatform *self, int addr_family, int ifindex)
routes_prune = nm_platform_ip_route_get_prune_list(self,
AF_INET6,
ifindex,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE);
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE,
NULL);
success &= nm_platform_ip_route_sync(self, AF_INET6, ifindex, NULL, routes_prune, NULL);
}
return success;
@ -8767,6 +8800,45 @@ nm_platform_lnk_wireguard_cmp(const NMPlatformLnkWireGuard *a, const NMPlatformL
return 0;
}
static int
_route_objs_cmp_values(gconstpointer a, gconstpointer b, gpointer user_data)
{
const NMPObject *a_obj = a;
const NMPObject *b_obj = b;
NMPlatformIPRouteCmpType cmp_type = GPOINTER_TO_INT(user_data);
nm_assert(a_obj && b_obj);
nm_assert(NMP_OBJECT_CAST_IP_ROUTE(a_obj) && NMP_OBJECT_CAST_IP_ROUTE(b_obj));
if (NMP_OBJECT_GET_ADDR_FAMILY(a_obj) != NMP_OBJECT_GET_ADDR_FAMILY(b_obj)) {
return NMP_OBJECT_GET_ADDR_FAMILY(a_obj) == AF_INET ? 1 : -1;
} else if (NMP_OBJECT_GET_ADDR_FAMILY(a_obj) == AF_INET) {
return nm_platform_ip4_route_cmp(NMP_OBJECT_CAST_IP4_ROUTE(a_obj),
NMP_OBJECT_CAST_IP4_ROUTE(b_obj),
cmp_type);
} else {
return nm_platform_ip6_route_cmp(NMP_OBJECT_CAST_IP6_ROUTE(a_obj),
NMP_OBJECT_CAST_IP6_ROUTE(b_obj),
cmp_type);
}
}
static int
_route_objs_cmp(gconstpointer a, gconstpointer b, gpointer user_data)
{
nm_assert(a && b);
return _route_objs_cmp_values(*((const NMPObject **) a), *((const NMPObject **) b), user_data);
}
void
nm_platform_route_objs_sort(GPtrArray *routes_objs, NMPlatformIPRouteCmpType cmp_type)
{
nm_assert(routes_objs);
g_ptr_array_sort_with_data(routes_objs, _route_objs_cmp, GINT_TO_POINTER((int) cmp_type));
}
void
nm_platform_ip4_rt_nexthop_hash_update(const NMPlatformIP4RtNextHop *obj,
gboolean for_id,

View file

@ -2389,7 +2389,8 @@ int nm_platform_ip6_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatf
GPtrArray *nm_platform_ip_route_get_prune_list(NMPlatform *self,
int addr_family,
int ifindex,
NMIPRouteTableSyncMode route_table_sync);
NMIPRouteTableSyncMode route_table_sync,
GPtrArray *old_routes_objs);
gboolean nm_platform_ip_route_sync(NMPlatform *self,
int addr_family,
@ -2495,6 +2496,8 @@ int nm_platform_lnk_wireguard_cmp(const NMPlatformLnkWireGuard *a, const NMPlatf
GHashTable *nm_platform_ip4_address_addr_to_hash(NMPlatform *self, int ifindex);
void nm_platform_route_objs_sort(GPtrArray *routes_objs, NMPlatformIPRouteCmpType cmp_type);
int nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a,
const NMPlatformIP4Route *b,
NMPlatformIPRouteCmpType cmp_type);

View file

@ -211,6 +211,9 @@ nmp_object_type_to_flags(NMPObjectType obj_type)
* @NM_IP_ROUTE_TABLE_SYNC_MODE_NONE: indicate an invalid setting.
* @NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN: only the main table is synced. For all
* other tables, NM won't delete any extra routes.
* @NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES: only the main table is synced,
* plus individual routes in other tables added by NM, leaving routes that
* were not added by NM untouched.
* @NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_EXCEPT_LOCAL: NM will sync all tables, except
* the local table (255).
* @NM_IP_ROUTE_TABLE_SYNC_MODE_ALL: NM will sync all tables, including the
@ -222,6 +225,7 @@ nmp_object_type_to_flags(NMPObjectType obj_type)
typedef enum {
NM_IP_ROUTE_TABLE_SYNC_MODE_NONE,
NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN,
NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN_AND_NM_ROUTES,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_EXCEPT_LOCAL,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL,
NM_IP_ROUTE_TABLE_SYNC_MODE_ALL_PRUNE,