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)
(cherry picked from commit 95064b8025)
(cherry picked from commit 3157504062)
(cherry picked from commit c6828a6e46)

src/core/nm-l3cfg.c

@@ -4624,7 +4624,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 (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) {
         gs_unref_array GArray *ipv6_temp_addrs_keep = NULL;
@@ -4650,6 +4650,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,
@@ -4657,10 +4659,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 {

src/core/nm-manager.c

@@ -425,21 +425,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
+        /* The following elements of the array are a bitfield of capabilities.
-     * used to numerically compare the version (see also NM_ENCODE_VERSION,
+         * These capabilities should only depend on compile-time abilities
-     * nm_utils_version(), nm_encode_version() and nm_decode_version().
+         * (unlike NM_MANAGER_CAPABILITIES, NMCapability). The supported values
-     *
+         * are from NMVersionInfoCapability enum. This way to expose capabilities
-     * The following elements of the array are a bitfield of capabilities.
+         * is more cumbersome but more efficient compared to NM_MANAGER_CAPABILITIES.
-     * These capabilities should only depend on compile-time abilities
+         * As such, it is cheap to add capabilities for something, where you would
-     * (unlike NM_MANAGER_CAPABILITIES, NMCapability). The supported values
+         * avoid it as NM_MANAGER_CAPABILITIES due to the overhead.
-     * are from NMVersionInfoCapability enum. This way to expose capabilities
+         *
-     * is more cumbersome but more efficient compared to NM_MANAGER_CAPABILITIES.
+         * Each of the array's elements has 32 bits. This means that capabilities
-     * As such, it is cheap to add capabilities for something, where you would
+         * with index 0-31 goes to element #1, with index 32-63 to element #2,
-     * avoid it as NM_MANAGER_CAPABILITIES due to the overhead.
+         * 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));
 }

src/libnm-core-public/nm-dbus-interface.h

@@ -92,16 +92,19 @@
 
 /**
  * NMVersionInfoCapability:
- * %_NM_VERSION_INFO_CAPABILITY_UNUSED: a dummy capability. It has no meaning,
+ * @NM_VERSION_INFO_CAPABILITY_SYNC_ROUTE_WITH_TABLE: Contains the fix to a bug that
- *   don't use it.
+ *   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
+ * The numeric values represent the bit index of the capability. These capabilities
- * on D-Bus in the "VersionInfo" bit field.
+ * 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;
 
 /**

src/libnm-platform/nm-platform.c

@@ -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,
@@ -4733,11 +4735,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;
@@ -4752,9 +4767,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);
@@ -4776,6 +4802,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;
@@ -5229,7 +5260,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)) {
@@ -5238,7 +5270,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;
@@ -8530,6 +8563,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,

src/libnm-platform/nm-platform.h

@@ -2294,7 +2294,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,
@@ -2398,6 +2399,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);

src/libnm-platform/nmp-base.h

@@ -191,6 +191,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
@@ -202,6 +205,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,