Users should be able to configure two logical routers that simultaneously interact with the same host routing table (vrf).
This is accomplished by maintaining an hmap while route_exchange_run() is running that keeps track of all the advertised routes that we have seen per table_id. Each loop over r_ctx_in->announce_routes is a different datapath if we try to add advertised routes to an entry that already has advertised routes then we know two different datapaths are trying to write advertised routes to the vrf table. Everytime we re_nl_sync_routes() routes that are not passed to it are deleted as stale, so passing any advertised previously advertised route on that table was a way to ensure that two routers can interact with a 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/controller/route-exchange.c b/controller/route-exchange.c index 82727f4e4..b36e08c53 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -243,11 +243,18 @@ static int route_exchange_nl_status; } \ } while (0) +struct advertised_routes_for_table_id_entry{ + struct hmap_node node; + + uint32_t table_id; + struct hmap routes; +}; + 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,15 +266,52 @@ 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; } + 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 vni table %"PRIu32"", + table_id); + } + break; + + } + } + /* In order to properly call re_nl_sync_routes() we need to + * store any previously processed advirtised routes in order + * to prevent those routes from being erroneously deleted as + * stale routes. + */ + if (entry == NULL) { + entry = xmalloc(sizeof *entry); + *entry = (struct advertised_routes_for_table_id_entry) { + .table_id = table_id, + .routes = HMAP_INITIALIZER(&entry->routes), + }; + hmap_insert(&advertised_routes, &entry->node, hash); + } + struct advertise_route_entry *are; + HMAP_FOR_EACH (are, node, &ad->routes) { + 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)); + } if (ad->maintain_vrf) { if (!sset_contains(&old_maintained_vrfs, ad->vrf_name)) { @@ -295,7 +339,7 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, struct vector received_routes = VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); - error = re_nl_sync_routes(table_id, &ad->routes, + error = re_nl_sync_routes(table_id, &entry->routes, &received_routes, ad->db); SET_ROUTE_EXCHANGE_NL_STATUS(error); @@ -339,7 +383,16 @@ 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); + struct advertised_routes_for_table_id_entry *arte; + HMAP_FOR_EACH_POP (arte, node, &advertised_routes) { + struct advertise_route_entry *ade; + HMAP_FOR_EACH_POP (ade, node, &arte->routes) { + free(ade); + } + hmap_destroy(&arte->routes); + free(arte); + } + hmap_destroy(&advertised_routes); } void diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 49aada46d..cce55a058 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -20486,6 +20486,11 @@ 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 +ovn-sbctl find Datapath_Binding + +ovn-sbctl find Learned_Route +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 +20588,38 @@ 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 +# to redestribute 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
