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)); + 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
