core: merge branch 'th/external-routes-no-sync'

https://bugzilla.redhat.com/show_bug.cgi?id=1979192

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/935

6 files changed, 121 insertions, 62 deletions

diff --git a/src/core/nm-ip4-config.c b/src/core/nm-ip4-config.c
index 47f0ee3a80..e7e90c4330 100644
--- a/src/core/nm-ip4-config.c
+++ b/src/core/nm-ip4-config.c
@@ -163,6 +163,11 @@ _nm_ip_config_add_obj(NMDedupMultiIndex *          multi_idx,
                     obj_new_stackinit.ip_route.rt_source = obj_old->ip_route.rt_source;
                     modified                             = TRUE;
                 }
+                if (!obj_new->ip_route.is_external && obj_old->ip_route.is_external) {
+                    obj_new = nmp_object_stackinit_obj(&obj_new_stackinit, obj_new);
+                    obj_new_stackinit.ip_route.is_external = FALSE;
+                    modified                               = TRUE;
+                }
                 break;
             default:
                 nm_assert_not_reached();
diff --git a/src/core/platform/nm-fake-platform.c b/src/core/platform/nm-fake-platform.c
index 0108b581bf..b773df9a43 100644
--- a/src/core/platform/nm-fake-platform.c
+++ b/src/core/platform/nm-fake-platform.c
@@ -1117,6 +1117,9 @@ ip_route_add(NMPlatform *             platform,
                                                 : NMP_OBJECT_TYPE_IP6_ROUTE,
                          (const NMPlatformObject *) route);
     r = NMP_OBJECT_CAST_IP_ROUTE(obj);
+
+    r->is_external = TRUE;
+
     nm_platform_ip_route_normalize(addr_family, r);
 
     switch (addr_family) {
diff --git a/src/core/platform/tests/test-route.c b/src/core/platform/tests/test-route.c
index 2b9b2f8f67..4b1db7fad7 100644
--- a/src/core/platform/tests/test-route.c
+++ b/src/core/platform/tests/test-route.c
@@ -332,30 +332,33 @@ test_ip4_route(void)
     /* Test route listing */
     routes = nmtstp_ip4_route_get_all(NM_PLATFORM_GET, ifindex);
     memset(rts, 0, sizeof(rts));
-    rts[0].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[0].network   = gateway;
-    rts[0].plen      = 32;
-    rts[0].ifindex   = ifindex;
-    rts[0].gateway   = INADDR_ANY;
-    rts[0].metric    = metric;
-    rts[0].mss       = mss;
-    rts[0].scope_inv = nm_platform_route_scope_inv(RT_SCOPE_LINK);
-    rts[1].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[1].network   = network;
-    rts[1].plen      = plen;
-    rts[1].ifindex   = ifindex;
-    rts[1].gateway   = gateway;
-    rts[1].metric    = metric;
-    rts[1].mss       = mss;
-    rts[1].scope_inv = nm_platform_route_scope_inv(RT_SCOPE_UNIVERSE);
-    rts[2].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[2].network   = 0;
-    rts[2].plen      = 0;
-    rts[2].ifindex   = ifindex;
-    rts[2].gateway   = gateway;
-    rts[2].metric    = metric;
-    rts[2].mss       = mss;
-    rts[2].scope_inv = nm_platform_route_scope_inv(RT_SCOPE_UNIVERSE);
+    rts[0].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[0].network     = gateway;
+    rts[0].plen        = 32;
+    rts[0].ifindex     = ifindex;
+    rts[0].gateway     = INADDR_ANY;
+    rts[0].metric      = metric;
+    rts[0].mss         = mss;
+    rts[0].is_external = TRUE;
+    rts[0].scope_inv   = nm_platform_route_scope_inv(RT_SCOPE_LINK);
+    rts[1].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[1].network     = network;
+    rts[1].plen        = plen;
+    rts[1].ifindex     = ifindex;
+    rts[1].gateway     = gateway;
+    rts[1].metric      = metric;
+    rts[1].mss         = mss;
+    rts[1].scope_inv   = nm_platform_route_scope_inv(RT_SCOPE_UNIVERSE);
+    rts[1].is_external = TRUE;
+    rts[2].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[2].network     = 0;
+    rts[2].plen        = 0;
+    rts[2].ifindex     = ifindex;
+    rts[2].gateway     = gateway;
+    rts[2].metric      = metric;
+    rts[2].mss         = mss;
+    rts[2].scope_inv   = nm_platform_route_scope_inv(RT_SCOPE_UNIVERSE);
+    rts[2].is_external = TRUE;
     g_assert_cmpint(routes->len, ==, 3);
     nmtst_platform_ip4_routes_equal_aptr((const NMPObject *const *) routes->pdata,
                                          rts,
@@ -489,30 +492,33 @@ test_ip6_route(void)
     /* Test route listing */
     routes = nmtstp_ip6_route_get_all(NM_PLATFORM_GET, ifindex);
     memset(rts, 0, sizeof(rts));
-    rts[0].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[0].network   = gateway;
-    rts[0].plen      = 128;
-    rts[0].ifindex   = ifindex;
-    rts[0].gateway   = in6addr_any;
-    rts[0].pref_src  = in6addr_any;
-    rts[0].metric    = metric;
-    rts[0].mss       = mss;
-    rts[1].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[1].network   = network;
-    rts[1].plen      = plen;
-    rts[1].ifindex   = ifindex;
-    rts[1].gateway   = gateway;
-    rts[1].pref_src  = pref_src;
-    rts[1].metric    = metric;
-    rts[1].mss       = mss;
-    rts[2].rt_source = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
-    rts[2].network   = in6addr_any;
-    rts[2].plen      = 0;
-    rts[2].ifindex   = ifindex;
-    rts[2].gateway   = gateway;
-    rts[2].pref_src  = in6addr_any;
-    rts[2].metric    = metric;
-    rts[2].mss       = mss;
+    rts[0].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[0].network     = gateway;
+    rts[0].plen        = 128;
+    rts[0].ifindex     = ifindex;
+    rts[0].gateway     = in6addr_any;
+    rts[0].pref_src    = in6addr_any;
+    rts[0].metric      = metric;
+    rts[0].mss         = mss;
+    rts[0].is_external = TRUE;
+    rts[1].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[1].network     = network;
+    rts[1].plen        = plen;
+    rts[1].ifindex     = ifindex;
+    rts[1].gateway     = gateway;
+    rts[1].pref_src    = pref_src;
+    rts[1].metric      = metric;
+    rts[1].mss         = mss;
+    rts[1].is_external = TRUE;
+    rts[2].rt_source   = nmp_utils_ip_config_source_round_trip_rtprot(NM_IP_CONFIG_SOURCE_USER);
+    rts[2].network     = in6addr_any;
+    rts[2].plen        = 0;
+    rts[2].ifindex     = ifindex;
+    rts[2].gateway     = gateway;
+    rts[2].pref_src    = in6addr_any;
+    rts[2].metric      = metric;
+    rts[2].mss         = mss;
+    rts[2].is_external = TRUE;
     g_assert_cmpint(routes->len, ==, 3);
     nmtst_platform_ip6_routes_equal_aptr((const NMPObject *const *) routes->pdata,
                                          rts,
@@ -709,6 +715,7 @@ test_ip4_route_options(gconstpointer test_data)
     for (i = 0; i < rts_n; i++) {
         rts_cmp[i] = rts_add[i];
         nm_platform_ip_route_normalize(AF_INET, NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i]));
+        rts_cmp[i].is_external = TRUE;
     }
 
     routes = nmtstp_ip4_route_get_all(NM_PLATFORM_GET, IFINDEX);
@@ -880,6 +887,7 @@ test_ip6_route_options(gconstpointer test_data)
     for (i = 0; i < rts_n; i++) {
         rts_cmp[i] = rts_add[i];
         nm_platform_ip_route_normalize(AF_INET6, NM_PLATFORM_IP_ROUTE_CAST(&rts_cmp[i]));
+        rts_cmp[i].is_external = TRUE;
     }
 
     routes = nmtstp_ip6_route_get_all(NM_PLATFORM_GET, IFINDEX);
diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c
index 0801fae29d..a2a82278ce 100644
--- a/src/libnm-platform/nm-linux-platform.c
+++ b/src/libnm-platform/nm-linux-platform.c
@@ -3519,6 +3519,7 @@ rta_multipath_done:;
 
     obj = nmp_object_new(is_v4 ? NMP_OBJECT_TYPE_IP4_ROUTE : NMP_OBJECT_TYPE_IP6_ROUTE, NULL);
 
+    obj->ip_route.is_external   = TRUE;
     obj->ip_route.type_coerced  = nm_platform_route_type_coerce(rtm->rtm_type);
     obj->ip_route.table_coerced = nm_platform_route_table_coerce(
         tb[RTA_TABLE] ? nla_get_u32(tb[RTA_TABLE]) : (guint32) rtm->rtm_table);
diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c
index 089d0d42c1..6c0d0015d6 100644
--- a/src/libnm-platform/nm-platform.c
+++ b/src/libnm-platform/nm-platform.c
@@ -4489,6 +4489,20 @@ nm_platform_ip_route_sync(NMPlatform *self,
 
             conf_o = routes->pdata[i];
 
+            if (NMP_OBJECT_CAST_IP_ROUTE(conf_o)->is_external) {
+                /* This route is added externally. We don't have our own agenda to
+                 * add it, so skip. */
+                continue;
+            }
+
+            /* User space cannot add IPv6 routes with metric 0. However, kernel can, and we might track such
+             * routes in @route as they are present external. As we already skipped external routes above,
+             * we don't expect a user's choice to add such a route (it won't work anyway). */
+            nm_assert(
+                IS_IPv4
+                || nm_platform_ip6_route_get_effective_metric(NMP_OBJECT_CAST_IP6_ROUTE(conf_o))
+                       != 0);
+
 #define VTABLE_IS_DEVICE_ROUTE(vt, o)                          \
     (vt->is_ip4 ? (NMP_OBJECT_CAST_IP4_ROUTE(o)->gateway == 0) \
                 : IN6_IS_ADDR_UNSPECIFIED(&NMP_OBJECT_CAST_IP6_ROUTE(o)->gateway))
@@ -4505,7 +4519,7 @@ nm_platform_ip_route_sync(NMPlatform *self,
                 routes_idx = g_hash_table_new((GHashFunc) nmp_object_id_hash,
                                               (GEqualFunc) nmp_object_id_equal);
             }
-            if (!g_hash_table_insert(routes_idx, (gpointer) conf_o, (gpointer) conf_o)) {
+            if (!g_hash_table_add(routes_idx, (gpointer) conf_o)) {
                 _LOG3D("route-sync: skip adding duplicate route %s",
                        nmp_object_to_string(conf_o,
                                             NMP_OBJECT_TO_STRING_PUBLIC,
@@ -4514,14 +4528,6 @@ nm_platform_ip_route_sync(NMPlatform *self,
                 continue;
             }
 
-            if (!IS_IPv4
-                && nm_platform_ip6_route_get_effective_metric(NMP_OBJECT_CAST_IP6_ROUTE(conf_o))
-                       == 0) {
-                /* User space cannot add routes with metric 0. However, kernel can, and we might track such
-                 * routes in @route as they are present external. Skip them silently. */
-                continue;
-            }
-
             plat_entry = nm_platform_lookup_entry(self, NMP_CACHE_ID_TYPE_OBJECT_TYPE, conf_o);
             if (plat_entry) {
                 const NMPObject *plat_o;
@@ -4684,6 +4690,24 @@ sync_route_add:
     }
 
     if (routes_prune) {
+        if (routes) {
+            for (i = 0; i < routes->len; i++) {
+                conf_o = routes->pdata[i];
+
+                if (NMP_OBJECT_CAST_IP_ROUTE(conf_o)->is_external) {
+                    /* this is only to catch the case where an external route is
+                     * both in @routes and @routes_prune list. In that case,
+                     * @routes should win and we should not remove the address. */
+                    if (!routes_idx) {
+                        routes_idx = g_hash_table_new((GHashFunc) nmp_object_id_hash,
+                                                      (GEqualFunc) nmp_object_id_equal);
+                    }
+                    g_hash_table_add(routes_idx, (gpointer) conf_o);
+                    continue;
+                }
+            }
+        }
+
         for (i = 0; i < routes_prune->len; i++) {
             const NMPObject *prune_o;
 
@@ -4694,7 +4718,7 @@ sync_route_add:
                       || (!NM_IS_IPv4(addr_family)
                           && NMP_OBJECT_GET_TYPE(prune_o) == NMP_OBJECT_TYPE_IP6_ROUTE));
 
-            if (routes_idx && g_hash_table_lookup(routes_idx, prune_o))
+            if (nm_g_hash_table_lookup(routes_idx, prune_o))
                 continue;
 
             if (!nm_platform_lookup_entry(self, NMP_CACHE_ID_TYPE_OBJECT_TYPE, prune_o))
@@ -6524,6 +6548,7 @@ nm_platform_ip4_route_to_string(const NMPlatformIP4Route *route, char *buf, gsiz
         "%s"                                   /* initcwnd */
         "%s"                                   /* initrwnd */
         "%s"                                   /* mtu */
+        "%s"                                   /* is_external */
         "",
         nm_net_aux_rtnl_rtntype_n2a_maybe_buf(nm_platform_route_type_uncoerce(route->type_coerced),
                                               str_type),
@@ -6579,7 +6604,8 @@ nm_platform_ip4_route_to_string(const NMPlatformIP4Route *route, char *buf, gsiz
                                                        " mtu %s%" G_GUINT32_FORMAT,
                                                        route->lock_mtu ? "lock " : "",
                                                        route->mtu)
-                                      : "");
+                                      : "",
+        route->is_external ? " (E)" : "");
     return buf;
 }
 
@@ -6649,6 +6675,7 @@ nm_platform_ip6_route_to_string(const NMPlatformIP6Route *route, char *buf, gsiz
         "%s"                                   /* initrwnd */
         "%s"                                   /* mtu */
         "%s"                                   /* pref */
+        "%s"                                   /* is_external */
         "",
         nm_net_aux_rtnl_rtntype_n2a_maybe_buf(nm_platform_route_type_uncoerce(route->type_coerced),
                                               str_type),
@@ -6708,7 +6735,8 @@ nm_platform_ip6_route_to_string(const NMPlatformIP6Route *route, char *buf, gsiz
             str_pref,
             " pref %s",
             nm_icmpv6_router_pref_to_string(route->rt_pref, str_pref2, sizeof(str_pref2)))
-                       : "");
+                       : "",
+        route->is_external ? " (E)" : "");
 
     return buf;
 }
@@ -8005,7 +8033,8 @@ nm_platform_ip4_route_hash_update(const NMPlatformIP4Route *obj,
                                                   obj->lock_cwnd,
                                                   obj->lock_initcwnd,
                                                   obj->lock_initrwnd,
-                                                  obj->lock_mtu));
+                                                  obj->lock_mtu,
+                                                  obj->is_external));
         break;
     }
 }
@@ -8095,6 +8124,8 @@ nm_platform_ip4_route_cmp(const NMPlatformIP4Route *a,
         NM_CMP_FIELD(a, b, initcwnd);
         NM_CMP_FIELD(a, b, initrwnd);
         NM_CMP_FIELD(a, b, mtu);
+        if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL)
+            NM_CMP_FIELD_UNSAFE(a, b, is_external);
         break;
     }
     return 0;
@@ -8186,7 +8217,8 @@ nm_platform_ip6_route_hash_update(const NMPlatformIP6Route *obj,
                                                   obj->lock_cwnd,
                                                   obj->lock_initcwnd,
                                                   obj->lock_initrwnd,
-                                                  obj->lock_mtu),
+                                                  obj->lock_mtu,
+                                                  obj->is_external),
                             obj->window,
                             obj->cwnd,
                             obj->initcwnd,
@@ -8269,6 +8301,8 @@ nm_platform_ip6_route_cmp(const NMPlatformIP6Route *a,
             NM_CMP_DIRECT(_route_pref_normalize(a->rt_pref), _route_pref_normalize(b->rt_pref));
         else
             NM_CMP_FIELD(a, b, rt_pref);
+        if (cmp_type == NM_PLATFORM_IP_ROUTE_CMP_TYPE_FULL)
+            NM_CMP_FIELD_UNSAFE(a, b, is_external);
         break;
     }
     return 0;
diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h
index 6fe723cba4..5b7a73e887 100644
--- a/src/libnm-platform/nm-platform.h
+++ b/src/libnm-platform/nm-platform.h
@@ -462,6 +462,14 @@ typedef union {
      * the "table_coerced" field is ignored (unlike for the metric). */            \
     bool table_any : 1;                                                                   \
                                                                                           \
+    /* This route is tracked as external route, that is not a route that NetworkManager
+     * actively wants to add, but a route that was added externally. In some cases, such
+     * a route should be ignored.
+     *
+     * Note that unlike most other fields here, this flag only exists inside NetworkManager
+     * and is not reflected on netlink. */   \
+    bool is_external : 1;                                                                 \
+                                                                                          \
     /* rtnh_flags
      *
      * Routes with rtm_flags RTM_F_CLONED are hidden by platform and