On Thu, Jun 04, 2026 at 03:33:17PM -0400, Jacob Tanenbaum wrote: > Users should be able to configure two logical routers that > simultaneously interact with the same host routing table (vrf). > > This is accomplished by first processing all the datapaths before > syncing the vrf tables. Now re_nl_sync_routes() is only called once per > vrf table. > > Reported-at: https://redhat.atlassian.net/browse/FDP-3472 > Reported-by: Dumitru Ceara <[email protected]> > Signed-off-by: Jacob Tanenbaum <[email protected]> > > diff --git a/NEWS b/NEWS > index 9839d19b9..0816cf748 100644 > --- a/NEWS > +++ b/NEWS > @@ -7,6 +7,7 @@ Post v26.03.0 > "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. > * 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 > diff --git a/controller/route-exchange.c b/controller/route-exchange.c > index 08e44e4f6..6bc08c1a6 100644 > --- a/controller/route-exchange.c > +++ b/controller/route-exchange.c > @@ -243,11 +243,26 @@ static int route_exchange_nl_status; > } \ > } while (0) > > +struct advertised_routes_for_table_id_entry { > + struct hmap_node node; > + > + struct hmap routes; > + uint32_t table_id; > + struct hmap datapaths; > + bool can_sync; > +}; > + > +struct datapath_entry { > + struct hmap_node node; > + > + const struct sbrec_datapath_binding *db; > +}; > + > void > route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, > struct route_exchange_ctx_out *r_ctx_out) > { > - struct hmap table_ids = HMAP_INITIALIZER(&table_ids); > + struct hmap advertised_routes = HMAP_INITIALIZER(&advertised_routes); > struct sset old_maintained_vrfs = SSET_INITIALIZER(&old_maintained_vrfs); > sset_swap(&_maintained_vrfs, &old_maintained_vrfs); > struct hmap old_maintained_route_table = > @@ -259,13 +274,10 @@ route_exchange_run(const struct route_exchange_ctx_in > *r_ctx_in, > const struct advertise_datapath_entry *ad; > HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) { > uint32_t table_id = route_get_table_id(ad->db); > - > - bool valid = TABLE_ID_VALID(table_id); > - if (!valid || !ovn_add_tnlid(&table_ids, table_id)) { > + if (!TABLE_ID_VALID(table_id)) { > VLOG_WARN_RL(&rl, "Unable to sync routes for datapath > "UUID_FMT": " > - "%s table id: %"PRIu32, > - UUID_ARGS(&ad->db->header_.uuid), > - !valid ? "invalid" : "duplicate", table_id); > + "invalid table id: %"PRIu32, > + UUID_ARGS(&ad->db->header_.uuid), table_id); > continue; > } > > @@ -290,24 +302,107 @@ route_exchange_run(const struct route_exchange_ctx_in > *r_ctx_in, > sset_find_and_delete(&old_maintained_vrfs, ad->vrf_name); > } > > - maintained_route_table_add(table_id); > - > - struct vector received_routes = > - VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); > - > - error = re_nl_sync_routes(table_id, &ad->routes, > - &received_routes); > - SET_ROUTE_EXCHANGE_NL_STATUS(error); > - > - sb_sync_learned_routes(&received_routes, ad->db, > - &ad->bound_ports, r_ctx_in->ovnsb_idl_txn, > - r_ctx_in->sbrec_port_binding_by_name, > - r_ctx_in->sbrec_learned_route_by_datapath, > - &r_ctx_out->sb_changes_pending); > - > - vector_push(r_ctx_out->route_table_watches, &table_id); > + struct advertised_routes_for_table_id_entry *entry; > + 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) && > + !hmap_is_empty(&entry->routes)) { > + VLOG_WARN_RL(&rl, "Multiple datapaths are distributing " > + "routes on routing table %"PRIu32"", > + table_id); > + entry->can_sync = false; > + } > + break; > + } > + } > + if (entry == NULL) { > + entry = xmalloc(sizeof *entry); > + *entry = (struct advertised_routes_for_table_id_entry) { > + .table_id = table_id, > + .routes = HMAP_INITIALIZER(&entry->routes), > + .datapaths = HMAP_INITIALIZER(&entry->datapaths), > + .can_sync = true, > + }; > + hmap_insert(&advertised_routes, &entry->node, hash); > + } > + if (!entry->can_sync) { > + continue; > + } > + struct datapath_entry *dp_entry = xmalloc(sizeof *dp_entry); > + *dp_entry = (struct datapath_entry) { > + .db = ad->db, > + }; > + > + hmap_insert(&entry->datapaths, > + &dp_entry->node, > + uuid_hash(&dp_entry->db->header_.uuid)); > + struct advertise_route_entry *are; > + HMAP_FOR_EACH (are, node, &ad->routes) { > + /* Copy the advertised routes into the > + * advertised_routes_for_table_id to keep track of > + * advertised routes that we see */ > + struct advertise_route_entry *copy = xmalloc(sizeof *copy); > + *copy = (struct advertise_route_entry) { > + .addr = are->addr, > + .plen = are->plen, > + .priority = are->priority, > + .nexthop = are->nexthop, > + }; > + hmap_insert(&entry->routes, > + ©->node, > + hmap_node_hash(&are->node)); > + } > + } > > - vector_destroy(&received_routes); > + struct advertised_routes_for_table_id_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 datapath_entry *dp; > + HMAP_FOR_EACH_POP (dp, node, &arte->datapaths) { > + struct advertise_datapath_entry *adpe = > + advertise_datapath_find(r_ctx_in->announce_routes, > + dp->db); > + if (!adpe) { > + VLOG_WARN_RL(&rl, "Cannot sync datapath binding > "UUID_FMT", bound " > + "ports not found", > + UUID_ARGS(&dp->db->header_.uuid)); Nit: I would move "Cannot .." to new line and fix alighment of "ports.. and UUID. > + free(dp); > + continue; > + } > + sb_sync_learned_routes(&received_routes, dp->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); > + > + free(dp); > + } > + vector_push(r_ctx_out->route_table_watches, &arte->table_id); > + vector_destroy(&received_routes); > + } else { > + struct datapath_entry *dp; > + HMAP_FOR_EACH_POP (dp, node, &arte->datapaths) { > + free(dp); > + } > + } > + hmap_destroy(&arte->datapaths); > + struct advertise_route_entry *ade; > + HMAP_FOR_EACH_POP (ade, node, &arte->routes) { > + free(ade); > + } > + hmap_destroy(&arte->routes); > + free(arte); > } > > /* Remove routes in tables previously maintained by us. */ > @@ -339,7 +434,7 @@ route_exchange_run(const struct route_exchange_ctx_in > *r_ctx_in, > sset_delete(&old_maintained_vrfs, SSET_NODE_FROM_NAME(vrf_name)); > } > sset_destroy(&old_maintained_vrfs); > - ovn_destroy_tnlids(&table_ids); > + hmap_destroy(&advertised_routes); > } > > void > diff --git a/controller/route.c b/controller/route.c > index 49344231f..13e6d3010 100644 > --- a/controller/route.c > +++ b/controller/route.c > @@ -115,6 +115,19 @@ route_exchange_find_port(struct ovsdb_idl_index > *sbrec_port_binding_by_name, > return NULL; > } > > +struct advertise_datapath_entry * > +advertise_datapath_find(const struct hmap *datapaths, > + const struct sbrec_datapath_binding *db) > +{ > + struct advertise_datapath_entry *ade; > + HMAP_FOR_EACH_WITH_HASH (ade, node, db->tunnel_key, datapaths) { > + if (ade->db == db) { > + return ade; > + } > + } > + return NULL; > +} > + > static void > build_port_mapping(struct smap *mapping, const char *port_mapping) > { > @@ -172,19 +185,6 @@ advertise_datapath_cleanup(struct > advertise_datapath_entry *ad) > free(ad); > } > > -static struct advertise_datapath_entry* > -advertise_datapath_find(const struct hmap *datapaths, > - const struct sbrec_datapath_binding *db) > -{ > - struct advertise_datapath_entry *ade; > - HMAP_FOR_EACH_WITH_HASH (ade, node, db->tunnel_key, datapaths) { > - if (ade->db == db) { > - return ade; > - } > - } > - return NULL; > -} > - > static struct advertise_datapath_entry * > advertised_datapath_alloc(const struct sbrec_datapath_binding *datapath) > { > diff --git a/controller/route.h b/controller/route.h > index 108e34200..43d11730b 100644 > --- a/controller/route.h > +++ b/controller/route.h > @@ -105,5 +105,8 @@ struct advertise_route_entry * > advertise_route_find(unsigned int priority, const struct in6_addr *prefix, > unsigned int plen, const struct in6_addr *nexthop, > const struct hmap *advertised_routes); > +struct advertise_datapath_entry * > +advertise_datapath_find(const struct hmap *datapaths, > + const struct sbrec_datapath_binding *db); > > #endif /* ROUTE_H */ > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 49aada46d..9594d594d 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -20486,6 +20486,8 @@ check ovn-nbctl --wait=hv set Logical_Router lr-frr > options:dynamic-routing-no-l > # Verify routes do not appear in SB database. > wait_row_count Learned_Route 0 > > +check ovn-nbctl --wait=hv remove Logical_Router lr-tenant options > dynamic-routing dynamic-routing-vrf-id > + > check ovn-nbctl --wait=hv set Logical_Router lr-frr > options:dynamic-routing-no-learning=false > > # Verify learned route appears in SB database > @@ -20583,6 +20585,37 @@ ip_prefix : "10.10.3.1" > ip_prefix : "10.10.4.1" > ]) > > +# Verify that we can have one router read and another write from the same > vrf table > +# > +# The router that advertises needs to have the gateway chassis set so remove > the > +# redistribute routes from lr-frr > + > +check ovn-nbctl remove Logical_Router lr-frr options > dynamic-routing-redistribute > +check ovn-nbctl --wait=hv set Logical_Router lr-frr \ > + options:dynamic-routing-vrf-id=$vni > + > +check ovn-nbctl --wait=hv set Logical_Router lr-tenant \ > + options:dynamic-routing-vrf-id=$vni \ > + options:dynamic-routing=true \ > + options:dynamic-routing-redistribute=static \ > + options:dynamic-routing-no-learning=true > + > + > +# Verify that lr-frr has Learned_Routes but lr-tenant does not > +wait_row_count Learned_Route 2 > +wait_row_count Advertised_Route 1 > +OVN_ROUTE_EQUAL([vrf-$vni], [dnl > +blackhole 10.10.1.1 proto ovn metric 1000 > +10.10.3.1 via 20.0.0.25 dev local-bgp-port proto zebra > +10.10.4.1 via 20.0.0.25 dev local-bgp-port proto zebra > +20.0.0.0/8 dev local-bgp-port proto kernel scope link src 20.0.0.4]) > +OVS_WAIT_FOR_OUTPUT([ip route show table $vni type blackhole | wc -l], [0], > [dnl > +1 > +]) > + > +check ovn-nbctl remove Logical_Router lr-tenant options > dynamic-routing-vrf-id dynamic-routing > + > + > # Remove lrp-local-bgp-port port. > AS_BOX([$(date +%H:%M:%S.%03N) Remove lrp]) > check ovn-nbctl --wait=hv lrp-del lrp-local-bgp-port > -- > 2.54.0 > LGTM One small nit that can be fixed on merge.
Acked-by: Mairtin O'Loingsigh [email protected] _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
