controller: Allow multiple LRs to advertise routes to the same VRF.

When several Logical_Routers were configured with the same
dynamic-routing-vrf-id, only one of them (or none) actually had its
routes synced to the shared host routing table (VRF).

Route exchange reconciles each VRF routing table as a single
authoritative set: re_nl_sync_routes() dumps the table and removes every
OVN owned route that is not part of the supplied set. Only one
datapath's routes were kept per table, so when a second Logical_Router
used the same dynamic-routing-vrf-id the collision was detected, logged
and synchronisation of that table was skipped entirely.

Pass the routes of all datapaths sharing a table to re_nl_sync_routes()
and reconcile their union. Equivalent routes advertised by several
datapaths are installed exactly once: while building the union only the
occurrence from the first table is kept, which also lets
handle_route_msg() stop at the first match.

Add a system test with two Logical_Routers exporting their connected
routes and a shared static prefix to the same VRF, including removal of
both routers.

CC: Jacob Tanenbaum <jtanenba@redhat.com>
Assisted-by: Claude Opus 4.8, Claude Code
Signed-off-by: Jakob Mueller <me@jakobm.de>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>

Author: ibot3

NEWS

@@ -51,7 +51,8 @@ OVN v26.09.0 - xxx xx xxxx
      "lb-add", "meter-add", "lr-policy-add", and "lr-policy-del", and
      fixed the "nfg-list" signature.
    - Dynamic Routing:
-     * Allow multiple routers to read the same VRF table.
+     * Allow multiple routers to read from and advertise their routes to
+       the same VRF table.
      * Add support for hub-and-spoke propagation via the "hub-spoke" option
        in dynamic-routing-redistribute settings.
      * Add ECMP/multi-homing support for EVPN FDB entries. FDB entries

controller/route-exchange-netlink.c

@@ -203,7 +203,9 @@ struct route_msg_handle_data {
     struct hmapx *routes_to_advertise;
     struct vector *learned_routes;
     struct vector *stale_routes;
-    const struct hmap *routes;
+    /* Vector of "const struct hmap *", each holding advertise_route_entry
+     * nodes for a datapath sharing this routing table. */
+    const struct vector *route_tables;
 };
 
 static void
@@ -260,11 +262,14 @@ handle_route_msg(const struct route_table_msg *msg,
     const struct advertise_route_entry re =
             advertise_route_from_route_data(rd);
     if (handle_data->routes_to_advertise) {
-        ar = advertise_route_find(re.priority, &re.addr, re.plen,
-                                  &re.nexthop, handle_data->routes);
-        if (ar) {
-            hmapx_find_and_delete(handle_data->routes_to_advertise, ar);
-            return;
+        const struct hmap *routes;
+        VECTOR_FOR_EACH (handle_data->route_tables, routes) {
+            ar = advertise_route_find(re.priority, &re.addr, re.plen,
+                                      &re.nexthop, routes);
+            if (ar) {
+                hmapx_find_and_delete(handle_data->routes_to_advertise, ar);
+                return;
+            }
         }
     }
 
@@ -319,23 +324,44 @@ re_nl_encode_nexthop(struct ofpbuf *request, bool dst_is_ipv4,
 }
 
 int
-re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
+re_nl_sync_routes(uint32_t table_id, const struct vector *route_tables,
                   struct vector *learned_routes)
 {
     struct hmapx routes_to_advertise = HMAPX_INITIALIZER(&routes_to_advertise);
     struct vector stale_routes =
         VECTOR_EMPTY_INITIALIZER(struct advertise_route_entry);
     struct advertise_route_entry *ar;
 
-    HMAP_FOR_EACH (ar, node, routes) {
-        hmapx_add(&routes_to_advertise, ar);
+    /* Equivalent routes may be advertised by multiple datapaths sharing this
+     * routing table.  Only keep the occurrence from the first table that has
+     * it, so each route is installed in the kernel exactly once and
+     * handle_route_msg() can stop at the first match as well. */
+    size_t n_prev = 0;
+    const struct hmap *routes;
+    VECTOR_FOR_EACH (route_tables, routes) {
+        HMAP_FOR_EACH (ar, node, routes) {
+            bool duplicate = false;
+            for (size_t i = 0; i < n_prev; i++) {
+                const struct hmap *prev = vector_get(route_tables, i,
+                                                     const struct hmap *);
+                if (advertise_route_find(ar->priority, &ar->addr, ar->plen,
+                                         &ar->nexthop, prev)) {
+                    duplicate = true;
+                    break;
+                }
+            }
+            if (!duplicate) {
+                hmapx_add(&routes_to_advertise, ar);
+            }
+        }
+        n_prev++;
     }
 
-    /* Remove routes from the system that are not in the routes hmap and
-     * remove entries from routes hmap that match routes already installed
-     * in the system. */
+    /* Remove routes from the system that are not in any of the route tables
+     * and remove entries from routes_to_advertise that match routes already
+     * installed in the system. */
     struct route_msg_handle_data data = {
-        .routes = routes,
+        .route_tables = route_tables,
         .routes_to_advertise = &routes_to_advertise,
         .learned_routes = learned_routes,
         .stale_routes = &stale_routes,

controller/route-exchange-netlink.h

@@ -57,7 +57,7 @@ void re_route_format(struct ds *, uint32_t table_id,
                      const struct in6_addr *dst, unsigned int plen,
                      const struct in6_addr *nexthop, int err);
 
-int re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
+int re_nl_sync_routes(uint32_t table_id, const struct vector *route_tables,
                       struct vector *learned_routes);
 
 int re_nl_cleanup_routes(uint32_t table_id);

controller/route-exchange.c

@@ -302,10 +302,11 @@ static int route_exchange_nl_status;
 struct advertised_routes_entry {
     struct hmap_node node;
 
+    /* Contains "struct advertise_datapath_entry *" for all datapaths that
+     * advertise routes on this routing table. Multiple datapaths may share a
+     * single table when they use the same dynamic-routing-vrf-id. */
     struct hmapx datapaths;
-    const struct hmap *routes;
     uint32_t table_id;
-    bool can_sync;
 };
 
 void
@@ -356,17 +357,6 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
         uint32_t hash = maintained_route_table_hash(table_id);
         HMAP_FOR_EACH_WITH_HASH (entry, node, hash, &advertised_routes) {
             if (entry->table_id == table_id) {
-                if (!hmap_is_empty(&ad->routes)) {
-                    if (entry->routes && !hmap_is_empty(entry->routes)) {
-                        VLOG_WARN_RL(&rl,
-                                     "Multiple datapaths are distributing "
-                                     "routes on routing table %"PRIu32,
-                                     table_id);
-                        entry->can_sync = false;
-                    } else {
-                        entry->routes = &ad->routes;
-                    }
-                }
                 break;
             }
         }
@@ -375,55 +365,51 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in,
             entry = xmalloc(sizeof *entry);
             *entry = (struct advertised_routes_entry) {
                 .datapaths = HMAPX_INITIALIZER(&entry->datapaths),
-                .routes = &ad->routes,
                 .table_id = table_id,
-                .can_sync = true,
             };
             hmap_insert(&advertised_routes, &entry->node, hash);
         }
 
-        if (!entry->can_sync) {
-            continue;
-        }
-
-        hmapx_add(&entry->datapaths, CONST_CAST(void *, ad->db));
+        hmapx_add(&entry->datapaths, CONST_CAST(void *, ad));
     }
 
     struct advertised_routes_entry *arte;
     HMAP_FOR_EACH_POP (arte, node, &advertised_routes) {
         maintained_route_table_add(arte->table_id);
-        if (arte->can_sync) {
-            struct vector received_routes =
-                VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node);
-            error = re_nl_sync_routes(arte->table_id, arte->routes,
-                                      &received_routes);
-            SET_ROUTE_EXCHANGE_NL_STATUS(error);
-
-            struct ovsdb_idl_index *sbrec_learned_route_by_datapath =
-                r_ctx_in->sbrec_learned_route_by_datapath;
-            struct hmapx_node *dp_node;
-            HMAPX_FOR_EACH (dp_node, &arte->datapaths) {
-                const struct sbrec_datapath_binding *db = dp_node->data;
-                struct advertise_datapath_entry *adpe =
-                    advertise_datapath_find(r_ctx_in->announce_routes,
-                                            db);
-                if (!adpe) {
-                    VLOG_WARN_RL(&rl, "Cannot sync datapath binding "
-                                 UUID_FMT", bound ports not found",
-                                 UUID_ARGS(&db->header_.uuid));
-                    continue;
-                }
-                sb_sync_learned_routes(&received_routes, db,
-                                       &adpe->bound_ports,
-                                       r_ctx_in->ovnsb_idl_txn,
-                                       r_ctx_in->sbrec_port_binding_by_name,
-                                       sbrec_learned_route_by_datapath,
-                                       &r_ctx_out->sb_changes_pending,
-                                       r_ctx_in->chassis);
-            }
-            vector_push(r_ctx_out->route_table_watches, &arte->table_id);
-            vector_destroy(&received_routes);
+
+        struct hmapx_node *dp_node;
+
+        /* Collect the route tables of all datapaths sharing this routing
+         * table so they are synced together as a single authoritative set. */
+        struct vector route_tables =
+            VECTOR_EMPTY_INITIALIZER(const struct hmap *);
+        HMAPX_FOR_EACH (dp_node, &arte->datapaths) {
+            const struct advertise_datapath_entry *adpe = dp_node->data;
+            const struct hmap *routes = &adpe->routes;
+            vector_push(&route_tables, &routes);
+        }
+
+        struct vector received_routes =
+            VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node);
+        error = re_nl_sync_routes(arte->table_id, &route_tables,
+                                  &received_routes);
+        SET_ROUTE_EXCHANGE_NL_STATUS(error);
+        vector_destroy(&route_tables);
+
+        struct ovsdb_idl_index *sbrec_learned_route_by_datapath =
+            r_ctx_in->sbrec_learned_route_by_datapath;
+        HMAPX_FOR_EACH (dp_node, &arte->datapaths) {
+            const struct advertise_datapath_entry *adpe = dp_node->data;
+            sb_sync_learned_routes(&received_routes, adpe->db,
+                                   &adpe->bound_ports,
+                                   r_ctx_in->ovnsb_idl_txn,
+                                   r_ctx_in->sbrec_port_binding_by_name,
+                                   sbrec_learned_route_by_datapath,
+                                   &r_ctx_out->sb_changes_pending,
+                                   r_ctx_in->chassis);
         }
+        vector_push(r_ctx_out->route_table_watches, &arte->table_id);
+        vector_destroy(&received_routes);
 
         hmapx_destroy(&arte->datapaths);
         free(arte);

tests/system-ovn.at

@@ -17340,6 +17340,105 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([dynamic-routing - multiple LRs sharing a VRF])
+AT_KEYWORDS([dynamic-routing])
+
+VRF_RESERVE([1337])
+
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_BR([br-int])
+check ovs-vsctl                                                                     \
+    -- set Open_vSwitch . external-ids:system-id=hv1                                \
+    -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+    -- set Open_vSwitch . external-ids:ovn-encap-type=geneve                        \
+    -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1                       \
+    -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+start_daemon ovn-controller
+
+dnl Two independent logical routers configured with the same
+dnl dynamic-routing-vrf-id. Each redistributes a distinct connected subnet
+dnl plus a static route for the same shared prefix. Only lr1 maintains the
+dnl VRF device, but both must export their routes to it.
+check ovn-nbctl                                                               \
+    -- lr-add lr1                                                             \
+      -- set logical_router lr1 options:dynamic-routing=true                  \
+                                options:dynamic-routing-vrf-id=1337           \
+      -- lrp-add lr1 lr1-gw 00:00:00:01:00:10 42.10.10.12/24                  \
+        -- lrp-set-gateway-chassis lr1-gw hv1 10                              \
+        -- lrp-set-options lr1-gw dynamic-routing-maintain-vrf=true           \
+      -- lrp-add lr1 lr1-int 00:00:00:00:01:02 30.0.1.1/24                    \
+        -- lrp-set-options lr1-int                                            \
+             dynamic-routing-redistribute=connected,static                    \
+    -- lr-add lr2                                                             \
+      -- set logical_router lr2 options:dynamic-routing=true                  \
+                                options:dynamic-routing-vrf-id=1337           \
+      -- lrp-add lr2 lr2-gw 00:00:00:02:00:10 42.20.10.22/24                  \
+        -- lrp-set-gateway-chassis lr2-gw hv1 10                              \
+      -- lrp-add lr2 lr2-int 00:00:00:00:02:02 30.0.2.1/24                    \
+        -- lrp-set-options lr2-int                                            \
+             dynamic-routing-redistribute=connected,static                    \
+    -- ls-add ls1                                                             \
+      -- lsp-add-router-port ls1 ls1-lr1-gw lr1-gw                            \
+    -- ls-add ls2                                                             \
+      -- lsp-add-router-port ls2 ls2-lr2-gw lr2-gw                            \
+    -- ls-add ls-int1                                                         \
+      -- lsp-add-router-port ls-int1 ls-int1-lr lr1-int                       \
+      -- lsp-add ls-int1 w1                                                   \
+        -- lsp-set-addresses w1 "00:00:00:00:00:01 30.0.1.11"                 \
+    -- ls-add ls-int2                                                         \
+      -- lsp-add-router-port ls-int2 ls-int2-lr lr2-int                       \
+      -- lsp-add ls-int2 w2                                                   \
+        -- lsp-set-addresses w2 "00:00:00:00:00:02 30.0.2.11"
+
+dnl Both routers advertise a static route for the same prefix.
+check ovn-nbctl lr-route-add lr1 203.0.113.0/24 30.0.1.11
+check ovn-nbctl lr-route-add lr2 203.0.113.0/24 30.0.2.11
+
+check ovs-vsctl add-port br-int w1 \
+    -- set interface w1 type=internal external_ids:iface-id=w1
+check ovs-vsctl add-port br-int w2 \
+    -- set interface w2 type=internal external_ids:iface-id=w2
+check ovn-nbctl --wait=hv sync
+wait_for_ports_up w1 w2
+
+AT_CHECK([ip vrf show ovnvrf1337], [0], [dnl
+ovnvrf1337 1337
+])
+
+dnl Both routers must have their connected routes installed in the shared VRF.
+dnl The shared static prefix is advertised by both routers but must be
+dnl installed only once.
+OVN_ROUTE_EQUAL([ovnvrf1337], [dnl
+blackhole 30.0.1.0/24 proto ovn metric 1000
+blackhole 30.0.2.0/24 proto ovn metric 1000
+blackhole 203.0.113.0/24 proto ovn metric 1000])
+
+dnl Removing one router must not disturb the remaining router's routes,
+dnl including the shared prefix it still advertises.
+check ovn-nbctl --wait=hv lr-del lr2
+OVN_ROUTE_EQUAL([ovnvrf1337], [dnl
+blackhole 30.0.1.0/24 proto ovn metric 1000
+blackhole 203.0.113.0/24 proto ovn metric 1000])
+
+dnl Removing the last router must clean up the routes and the VRF device.
+check ovn-nbctl --wait=hv lr-del lr1
+OVS_WAIT_WHILE([ip link show dev ovnvrf1337])
+
+OVN_CLEANUP_CONTROLLER([hv1])
+
+OVN_CLEANUP_NORTHD
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([dynamic-routing - multiple DGP with same priority])
 

tests/test-ovn-netlink.c

@@ -231,8 +231,14 @@ test_route_sync(struct ovs_cmdl_context *ctx)
                     advertise_route_hash(&ar->addr, &ar->nexthop, ar->plen));
     }
 
-    ovs_assert(re_nl_sync_routes(table_id, &routes_to_advertise,
+    struct vector route_tables =
+        VECTOR_EMPTY_INITIALIZER(const struct hmap *);
+    const struct hmap *routes = &routes_to_advertise;
+    vector_push(&route_tables, &routes);
+
+    ovs_assert(re_nl_sync_routes(table_id, &route_tables,
                                  &received_routes) == 0);
+    vector_destroy(&route_tables);
 
     struct ds msg = DS_EMPTY_INITIALIZER;