On 2/26/25 1:24 PM, martin.kal...@canonical.com wrote:
> On Tue, 2025-02-25 at 16:54 +0100, Dumitru Ceara wrote:
>> Change the en_dynamic_routes I-P node to compute a map of 'advertised
>> routes' (struct ar_entry) instead of parsed routes. Like that we can
>> avoid having to re-parse NAT/LB IPs when advertising them.
>>
>> Fixes: cd4ad2f56179 ("northd: Redistribution of NAT/LB routes.")
>> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
>
> Hi Dumitru,
Hi Martin, Felix,
Thanks for the reviews!
> thanks for the update. Overall you can add acked-by me. I noticed few
> points in code where function was renamed (to not include
> "parsed_route" in name), but the related comment above function still
> mentions "parsed route". Those comments should probably be updated too.
Ah, good catch. I had indeed done a search and replace of
"parsed_route". :)
> I left comments below where I noticed this discrepancy.
Awesome, that helps!
> IMO, it doesn't require v3, since they are just comments, they can be
> fixed on merge.
Cool, I fixed them up then added your ack and applied the patch to main
and 25.03.
Regards,
Dumitru
>
> Thanks,
> Martin.
>
>> ---
>> V2:
>> - Addressed Felix's review comments:
>> - added OVS_NOT_REACHED() for cases that shouldn't happen.
>> - split the functionality of should_advertise_route() to make it
>> more
>> clear
>> - Addressed Martin's review comments:
>> - removed stale TODO item
>> - renamed build_lb_nat_parsed_route_*() functions because they don't
>> build parsed_route structs anymore.
>> ---
>> TODO.rst | 7 -
>> northd/en-advertised-route-sync.c | 492 +++++++++++++++++-----------
>> --
>> northd/northd.h | 3 +-
>> 3 files changed, 282 insertions(+), 220 deletions(-)
>>
>> diff --git a/TODO.rst b/TODO.rst
>> index 593b9a580f..6c2b0aa931 100644
>> --- a/TODO.rst
>> +++ b/TODO.rst
>> @@ -159,10 +159,3 @@ OVN To-do List
>> monitoring conditions to update before we actually try to learn
>> routes.
>> Otherwise we could try to add duplicated Learned_Routes and the
>> ovnsb
>> commit would fail.
>> -
>> - * Consider splitting parsed_route structure. When creating parsed
>> routes
>> - with tracked_port explicitly set, other members of this
>> structure are
>> - usually unused/default. A new structure dedicated to routes with
>> - explicitly defined tracked_port would be more efficient.
>> - More details in
>> -
>> https://mail.openvswitch.org/pipermail/ovs-dev/2025-February/420985.html
>> diff --git a/northd/en-advertised-route-sync.c b/northd/en-
>> advertised-route-sync.c
>> index 486bcc2fd2..913f58bfa6 100644
>> --- a/northd/en-advertised-route-sync.c
>> +++ b/northd/en-advertised-route-sync.c
>> @@ -27,6 +27,102 @@
>> #include "openvswitch/hmap.h"
>> #include "ovn-util.h"
>>
>> +struct ar_entry {
>> + struct hmap_node hmap_node;
>> +
>> + const struct ovn_datapath *od; /* Datapath the route is
>> + * advertised on. */
>> + const struct ovn_port *op; /* Port the route is
>> advertised
>> + * on. */
>> + char *ip_prefix;
>> + const struct ovn_port *tracked_port; /* If set, the port whose
>> chassis
>> + * advertises this route
>> with a
>> + * higher priority. */
>> + enum route_source source;
>> +};
>> +
>> +/* Add a new entries to the to-be-advertised routes.
>> + * Takes ownership of ip_prefix. */
>> +static struct ar_entry *
>> +ar_entry_add_nocopy(struct hmap *routes, const struct ovn_datapath
>> *od,
>> + const struct ovn_port *op, char *ip_prefix,
>> + const struct ovn_port *tracked_port,
>> + enum route_source source)
>> +{
>> + struct ar_entry *route_e = xzalloc(sizeof *route_e);
>> +
>> + route_e->od = od;
>> + route_e->op = op;
>> + route_e->ip_prefix = ip_prefix;
>> + route_e->tracked_port = tracked_port;
>> + route_e->source = source;
>> + uint32_t hash = uuid_hash(&od->sb->header_.uuid);
>> + hash = hash_string(op->sb->logical_port, hash);
>> + hash = hash_string(ip_prefix, hash);
>> + hmap_insert(routes, &route_e->hmap_node, hash);
>> +
>> + return route_e;
>> +}
>> +
>> +/* Add a new entries to the to-be-advertised routes.
>> + * Makes a copy of ip_prefix. */
>> +static struct ar_entry *
>> +ar_entry_add(struct hmap *routes, const struct ovn_datapath *od,
>> + const struct ovn_port *op, const char *ip_prefix,
>> + const struct ovn_port *tracked_port,
>> + enum route_source source)
>> +{
>> + return ar_entry_add_nocopy(routes, od, op, xstrdup(ip_prefix),
>> + tracked_port, source);
>> +}
>> +
>> +static struct ar_entry *
>> +ar_entry_find(struct hmap *route_map,
>> + const struct sbrec_datapath_binding *sb_db,
>> + const struct sbrec_port_binding *logical_port,
>> + const char *ip_prefix,
>> + const struct sbrec_port_binding *tracked_port)
>> +{
>> + struct ar_entry *route_e;
>> + uint32_t hash;
>> +
>> + hash = uuid_hash(&sb_db->header_.uuid);
>> + hash = hash_string(logical_port->logical_port, hash);
>> + hash = hash_string(ip_prefix, hash);
>> +
>> + HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) {
>> + if (!uuid_equals(&sb_db->header_.uuid,
>> + &route_e->od->sb->header_.uuid)) {
>> + continue;
>> + }
>> + if (!uuid_equals(&logical_port->header_.uuid,
>> + &route_e->op->sb->header_.uuid)) {
>> + continue;
>> + }
>> + if (strcmp(ip_prefix, route_e->ip_prefix)) {
>> + continue;
>> + }
>> +
>> + if (tracked_port) {
>> + if (!route_e->tracked_port ||
>> + tracked_port != route_e->tracked_port->sb) {
>> + continue;
>> + }
>> + }
>> +
>> + return route_e;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void
>> +ar_entry_free(struct ar_entry *route_e)
>> +{
>> + free(route_e->ip_prefix);
>> + free(route_e);
>> +}
>> +
>> static void
>> advertised_route_table_sync(
>> struct ovsdb_idl_txn *ovnsb_txn,
>> @@ -158,7 +254,7 @@ en_advertised_route_sync_run(struct engine_node
>> *node, void *data OVS_UNUSED)
>> sbrec_advertised_route_table,
>> &lr_stateful_data->table,
>> &routes_data->parsed_routes,
>> - &dynamic_routes_data->parsed_routes,
>> + &dynamic_routes_data->routes,
>> routes_sync_data);
>>
>> stopwatch_stop(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME,
>> time_msec());
>> @@ -171,10 +267,10 @@ en_advertised_route_sync_run(struct engine_node
>> *node, void *data OVS_UNUSED)
>> * DGP (for example if it's set on gateway router), no tracked port
>> will
>> * be set.*/
>
> It's not seen in this diff, but the comment above this function still
> mentions "parsed route".
>
>> static void
>> -build_nat_parsed_route_for_port(const struct ovn_port
>> *advertising_op,
>> - const struct lr_nat_record *lr_nat,
>> - const struct hmap *ls_ports,
>> - struct hmap *routes)
>> +build_nat_route_for_port(const struct ovn_port *advertising_op,
>> + const struct lr_nat_record *lr_nat,
>> + const struct hmap *ls_ports,
>> + struct hmap *routes)
>> {
>> const struct ovn_datapath *advertising_od = advertising_op->od;
>>
>> @@ -184,28 +280,28 @@ build_nat_parsed_route_for_port(const struct
>> ovn_port *advertising_op,
>> continue;
>> }
>>
>> - int plen = nat_entry_is_v6(nat) ? 128 : 32;
>> - struct in6_addr prefix;
>> - ip46_parse(nat->nb->external_ip, &prefix);
>> -
>> const struct ovn_port *tracked_port =
>> nat->is_distributed
>> ? ovn_port_find(ls_ports, nat->nb->logical_port)
>> : nat->l3dgw_port;
>> - parsed_route_add(advertising_od, NULL, &prefix, plen, false,
>> - nat->nb->external_ip, advertising_op, 0,
>> false,
>> - false, NULL, ROUTE_SOURCE_NAT, &nat->nb-
>>> header_,
>> - tracked_port, routes);
>> +
>> + if (!ar_entry_find(routes, advertising_od->sb,
>> advertising_op->sb,
>> + nat->nb->external_ip,
>> + tracked_port ? tracked_port->sb : NULL))
>> {
>> + ar_entry_add(routes, advertising_od, advertising_op,
>> + nat->nb->external_ip, tracked_port,
>> + ROUTE_SOURCE_NAT);
>> + }
>> }
>> }
>>
>> /* Generate parsed routes for NAT external IPs in lr_nat, for each
>> ovn port
>
> Comment still references "parsed routes".
>
>> * in "od" that has enabled redistribution of NAT adresses.*/
>> static void
>> -build_nat_parsed_routes(const struct ovn_datapath *od,
>> - const struct lr_nat_record *lr_nat,
>> - const struct hmap *ls_ports,
>> - struct hmap *routes)
>> +build_nat_routes(const struct ovn_datapath *od,
>> + const struct lr_nat_record *lr_nat,
>> + const struct hmap *ls_ports,
>> + struct hmap *routes)
>> {
>> const struct ovn_port *op;
>> HMAP_FOR_EACH (op, dp_node, &od->ports) {
>> @@ -213,17 +309,17 @@ build_nat_parsed_routes(const struct
>> ovn_datapath *od,
>> continue;
>> }
>>
>> - build_nat_parsed_route_for_port(op, lr_nat, ls_ports,
>> routes);
>> + build_nat_route_for_port(op, lr_nat, ls_ports, routes);
>> }
>> }
>>
>> -/* Similar to build_nat_parsed_routes, this function generates
>> parsed routes
>> +/* Similar to build_nat_routes, this function generates parsed
>> routes
>
> Comment still references "parsed routes".
>
>> * for nat records in neighboring routers. For each ovn port in "od"
>> that has
>> * enabled redistribution of NAT adresses, look up their neighbors
>> (either
>> * directly routers, or routers connected through common LS) and
>> advertise
>> * thier external NAT IPs too.*/
>> static void
>> -build_nat_connected_parsed_routes(
>> +build_nat_connected_routes(
>> const struct ovn_datapath *od,
>> const struct lr_stateful_table *lr_stateful_table,
>> const struct hmap *ls_ports,
>> @@ -255,8 +351,8 @@ build_nat_connected_parsed_routes(
>> }
>>
>> /* Advertise peer's NAT routes via the local port too.
>> */
>> - build_nat_parsed_route_for_port(op, peer_lr_stateful-
>>> lrnat_rec,
>> - ls_ports, routes);
>> + build_nat_route_for_port(op, peer_lr_stateful-
>>> lrnat_rec,
>> + ls_ports, routes);
>> return;
>> }
>>
>> @@ -277,50 +373,41 @@ build_nat_connected_parsed_routes(
>> }
>>
>> /* Advertise peer's NAT routes via the local port too.
>> */
>> - build_nat_parsed_route_for_port(op, peer_lr_stateful-
>>> lrnat_rec,
>> - ls_ports, routes);
>> + build_nat_route_for_port(op, peer_lr_stateful-
>>> lrnat_rec,
>> + ls_ports, routes);
>> }
>> }
>> }
>>
>> /* This function adds new parsed route for each IP in lb_ips to
>> "routes".*/
>
> Comment still references "parsed routes".
>
>> static void
>> -build_lb_parsed_route_for_port(const struct ovn_port
>> *advertising_op,
>> - const struct ovn_port *tracked_port,
>> - const struct ovn_lb_ip_set *lb_ips,
>> - struct hmap *routes)
>> +build_lb_route_for_port(const struct ovn_port *advertising_op,
>> + const struct ovn_port *tracked_port,
>> + const struct ovn_lb_ip_set *lb_ips,
>> + struct hmap *routes)
>> {
>> const struct ovn_datapath *advertising_od = advertising_op->od;
>>
>> const char *ip_address;
>> SSET_FOR_EACH (ip_address, &lb_ips->ips_v4) {
>> - struct in6_addr prefix;
>> - ip46_parse(ip_address, &prefix);
>> - parsed_route_add(advertising_od, NULL, &prefix, 32, false,
>> - ip_address, advertising_op, 0, false,
>> false,
>> - NULL, ROUTE_SOURCE_LB, &advertising_op-
>>> nbrp->header_,
>> - tracked_port, routes);
>> + ar_entry_add(routes, advertising_od, advertising_op,
>> + ip_address, tracked_port, ROUTE_SOURCE_LB);
>> }
>> SSET_FOR_EACH (ip_address, &lb_ips->ips_v6) {
>> - struct in6_addr prefix;
>> - ip46_parse(ip_address, &prefix);
>> - parsed_route_add(advertising_od, NULL, &prefix, 128, false,
>> - ip_address, advertising_op, 0, false,
>> false,
>> - NULL, ROUTE_SOURCE_LB, &advertising_op-
>>> nbrp->header_,
>> - tracked_port, routes);
>> + ar_entry_add(routes, advertising_od, advertising_op,
>> + ip_address, tracked_port, ROUTE_SOURCE_LB);
>> }
>> }
>>
>> -/* Similar to build_lb_parsed_routes, this function generates parsed
>> routes
>
> Comment still references "parsed routes".
>
>> +/* Similar to build_lb_routes, this function generates parsed routes
>> * for LB VIPs of neighboring routers. For each ovn port in "od"
>> that has
>> * enabled redistribution of LB VIPs, look up their neighbors
>> (either
>> * directly routers, or routers connected through common LS) and
>> advertise
>> * thier LB VIPs too.*/
>> static void
>> -build_lb_connected_parsed_routes(
>> - const struct ovn_datapath *od,
>> - const struct lr_stateful_table *lr_stateful_table,
>> - struct hmap *routes)
>> +build_lb_connected_routes(const struct ovn_datapath *od,
>> + const struct lr_stateful_table
>> *lr_stateful_table,
>> + struct hmap *routes)
>> {
>> const struct ovn_port *op;
>> HMAP_FOR_EACH (op, dp_node, &od->ports) {
>> @@ -342,8 +429,8 @@ build_lb_connected_parsed_routes(
>> lr_stateful_rec = lr_stateful_table_find_by_index(
>> lr_stateful_table, peer_od->index);
>> peer_port = op->peer;
>> - build_lb_parsed_route_for_port(op, peer_port,
>> - lr_stateful_rec->lb_ips,
>> routes);
>> + build_lb_route_for_port(op, peer_port, lr_stateful_rec-
>>> lb_ips,
>> + routes);
>> return;
>> }
>>
>> @@ -359,16 +446,16 @@ build_lb_connected_parsed_routes(
>> lr_stateful_rec = lr_stateful_table_find_by_index(
>> lr_stateful_table, peer_port->od->index);
>>
>> - build_lb_parsed_route_for_port(op, peer_port,
>> - lr_stateful_rec->lb_ips,
>> routes);
>> + build_lb_route_for_port(op, peer_port, lr_stateful_rec-
>>> lb_ips,
>> + routes);
>> }
>> }
>> }
>>
>> static void
>> -build_lb_parsed_routes(const struct ovn_datapath *od,
>> - const struct ovn_lb_ip_set *lb_ips,
>> - struct hmap *routes)
>> +build_lb_routes(const struct ovn_datapath *od,
>> + const struct ovn_lb_ip_set *lb_ips,
>> + struct hmap *routes)
>> {
>> const struct ovn_port *op;
>> HMAP_FOR_EACH (op, dp_node, &od->ports) {
>> @@ -390,8 +477,7 @@ build_lb_parsed_routes(const struct ovn_datapath
>> *od,
>> : &op_;
>>
>> for (size_t i = 0; i < n_tracked_ports; i++) {
>> - build_lb_parsed_route_for_port(op, tracked_ports[i],
>> lb_ips,
>> - routes);
>> + build_lb_route_for_port(op, tracked_ports[i], lb_ips,
>> routes);
>> }
>> }
>> }
>> @@ -402,7 +488,7 @@ en_dynamic_routes_init(struct engine_node *node
>> OVS_UNUSED,
>> {
>> struct dynamic_routes_data *data = xmalloc(sizeof *data);
>> *data = (struct dynamic_routes_data) {
>> - .parsed_routes = HMAP_INITIALIZER(&data->parsed_routes),
>> + .routes = HMAP_INITIALIZER(&data->routes),
>> };
>>
>> return data;
>> @@ -411,9 +497,9 @@ en_dynamic_routes_init(struct engine_node *node
>> OVS_UNUSED,
>> static void
>> en_dynamic_routes_clear(struct dynamic_routes_data *data)
>> {
>> - struct parsed_route *r;
>> - HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) {
>> - parsed_route_free(r);
>> + struct ar_entry *ar;
>> + HMAP_FOR_EACH_POP (ar, hmap_node, &data->routes) {
>> + ar_entry_free(ar);
>> }
>> }
>> void
>> @@ -422,7 +508,7 @@ en_dynamic_routes_cleanup(void *data_)
>> struct dynamic_routes_data *data = data_;
>>
>> en_dynamic_routes_clear(data);
>> - hmap_destroy(&data->parsed_routes);
>> + hmap_destroy(&data->routes);
>> }
>>
>> void
>> @@ -445,102 +531,22 @@ en_dynamic_routes_run(struct engine_node
>> *node, void *data)
>> if (!od->dynamic_routing) {
>> continue;
>> }
>> - build_nat_parsed_routes(od, lr_stateful_rec->lrnat_rec,
>> - &northd_data->ls_ports,
>> - &dynamic_routes_data-
>>> parsed_routes);
>> - build_nat_connected_parsed_routes(od, &lr_stateful_data-
>>> table,
>> - &northd_data->ls_ports,
>> - &dynamic_routes_data-
>>> parsed_routes);
>> -
>> - build_lb_parsed_routes(od, lr_stateful_rec->lb_ips,
>> - &dynamic_routes_data->parsed_routes);
>> - build_lb_connected_parsed_routes(od, &lr_stateful_data-
>>> table,
>> - &dynamic_routes_data-
>>> parsed_routes);
>> + build_nat_routes(od, lr_stateful_rec->lrnat_rec,
>> + &northd_data->ls_ports,
>> + &dynamic_routes_data->routes);
>> + build_nat_connected_routes(od, &lr_stateful_data->table,
>> + &northd_data->ls_ports,
>> + &dynamic_routes_data->routes);
>> +
>> + build_lb_routes(od, lr_stateful_rec->lb_ips,
>> + &dynamic_routes_data->routes);
>> + build_lb_connected_routes(od, &lr_stateful_data->table,
>> + &dynamic_routes_data->routes);
>> }
>> stopwatch_stop(DYNAMIC_ROUTES_RUN_STOPWATCH_NAME, time_msec());
>> engine_set_node_state(node, EN_UPDATED);
>> }
>>
>> -struct ar_entry {
>> - struct hmap_node hmap_node;
>> -
>> - const struct ovn_datapath *od; /* Datapath the route is
>> - * advertised on. */
>> - const struct ovn_port *op; /* Port the route is
>> advertised
>> - * on. */
>> - char *ip_prefix;
>> - const struct ovn_port *tracked_port; /* If set, the port whose
>> chassis
>> - * advertises this route
>> with a
>> - * higher priority. */
>> -};
>> -
>> -/* Add a new entries to the to-be-advertised routes.
>> - * Takes ownership of ip_prefix. */
>> -static struct ar_entry *
>> -ar_entry_add(struct hmap *routes, const struct ovn_datapath *od,
>> - const struct ovn_port *op, char *ip_prefix,
>> - const struct ovn_port *tracked_port)
>> -{
>> - struct ar_entry *route_e = xzalloc(sizeof *route_e);
>> -
>> - route_e->od = od;
>> - route_e->op = op;
>> - route_e->ip_prefix = ip_prefix;
>> - route_e->tracked_port = tracked_port;
>> - uint32_t hash = uuid_hash(&od->sb->header_.uuid);
>> - hash = hash_string(op->sb->logical_port, hash);
>> - hash = hash_string(ip_prefix, hash);
>> - hmap_insert(routes, &route_e->hmap_node, hash);
>> -
>> - return route_e;
>> -}
>> -
>> -static struct ar_entry *
>> -ar_entry_find(struct hmap *route_map,
>> - const struct sbrec_datapath_binding *sb_db,
>> - const struct sbrec_port_binding *logical_port,
>> - const char *ip_prefix,
>> - const struct sbrec_port_binding *tracked_port)
>> -{
>> - struct ar_entry *route_e;
>> - uint32_t hash;
>> -
>> - hash = uuid_hash(&sb_db->header_.uuid);
>> - hash = hash_string(logical_port->logical_port, hash);
>> - hash = hash_string(ip_prefix, hash);
>> -
>> - HMAP_FOR_EACH_WITH_HASH (route_e, hmap_node, hash, route_map) {
>> - if (!uuid_equals(&sb_db->header_.uuid,
>> - &route_e->od->sb->header_.uuid)) {
>> - continue;
>> - }
>> - if (!uuid_equals(&logical_port->header_.uuid,
>> - &route_e->op->sb->header_.uuid)) {
>> - continue;
>> - }
>> - if (strcmp(ip_prefix, route_e->ip_prefix)) {
>> - continue;
>> - }
>> -
>> - if (tracked_port) {
>> - if (!route_e->tracked_port ||
>> - tracked_port != route_e->tracked_port->sb) {
>> - continue;
>> - }
>> - }
>> - return route_e;
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> -static void
>> -ar_entry_free(struct ar_entry *route_e)
>> -{
>> - free(route_e->ip_prefix);
>> - free(route_e);
>> -}
>> -
>> static void
>> publish_lport_addresses(struct hmap *sync_routes,
>> const struct ovn_datapath *od,
>> @@ -550,16 +556,16 @@ publish_lport_addresses(struct hmap
>> *sync_routes,
>> {
>> for (size_t i = 0; i < addresses->n_ipv4_addrs; i++) {
>> const struct ipv4_netaddr *addr = &addresses->ipv4_addrs[i];
>> - ar_entry_add(sync_routes, od, logical_port, xstrdup(addr-
>>> addr_s),
>> - tracking_port);
>> + ar_entry_add(sync_routes, od, logical_port, addr->addr_s,
>> + tracking_port, ROUTE_SOURCE_CONNECTED);
>> }
>> for (size_t i = 0; i < addresses->n_ipv6_addrs; i++) {
>> if (in6_is_lla(&addresses->ipv6_addrs[i].network)) {
>> continue;
>> }
>> const struct ipv6_netaddr *addr = &addresses->ipv6_addrs[i];
>> - ar_entry_add(sync_routes, od, logical_port, xstrdup(addr-
>>> addr_s),
>> - tracking_port);
>> + ar_entry_add(sync_routes, od, logical_port, addr->addr_s,
>> + tracking_port, ROUTE_SOURCE_CONNECTED);
>> }
>> }
>>
>> @@ -640,78 +646,107 @@ publish_host_routes(struct hmap *sync_routes,
>> }
>> }
>>
>> -static void
>> -advertised_route_table_sync_route_add(
>> - const struct lr_stateful_table *lr_stateful_table,
>> - struct advertised_route_sync_data *data,
>> - struct uuidset *host_route_lrps,
>> - struct hmap *sync_routes,
>> - const struct parsed_route *route)
>> +static bool
>> +should_advertise_route(const struct uuidset *host_route_lrps,
>> + const struct ovn_datapath *advertising_od,
>> + const struct ovn_port *advertising_op,
>> + enum route_source source)
>> {
>> - if (route->is_discard_route) {
>> - return;
>> - }
>> - if (prefix_is_link_local(&route->prefix, route->plen)) {
>> - return;
>> - }
>> - if (!route->od->dynamic_routing) {
>> - return;
>> + if (!advertising_od->dynamic_routing) {
>> + return false;
>> }
>>
>> enum dynamic_routing_redistribute_mode drr =
>> - route->out_port->dynamic_routing_redistribute;
>> - if (route->source == ROUTE_SOURCE_CONNECTED) {
>> + advertising_op->dynamic_routing_redistribute;
>> +
>> + switch (source) {
>> + case ROUTE_SOURCE_CONNECTED:
>> if (!drr_mode_CONNECTED_is_set(drr)) {
>> - return;
>> + return false;
>> }
>> +
>> /* If we advertise host routes, we only need to do so once
>> per
>> * LRP. */
>> - const struct uuid *lrp_uuid = &route->out_port->nbrp-
>>> header_.uuid;
>> + const struct uuid *lrp_uuid = &advertising_op->nbrp-
>>> header_.uuid;
>> if (drr_mode_CONNECTED_AS_HOST_is_set(drr) &&
>> - !uuidset_contains(host_route_lrps, lrp_uuid)) {
>> + uuidset_contains(host_route_lrps, lrp_uuid)) {
>> + return false;
>> + }
>> + return true;
>> + case ROUTE_SOURCE_STATIC:
>> + return drr_mode_STATIC_is_set(drr);
>> + case ROUTE_SOURCE_NAT:
>> + return drr_mode_NAT_is_set(drr);
>> + case ROUTE_SOURCE_LB:
>> + return drr_mode_LB_is_set(drr);
>> + case ROUTE_SOURCE_LEARNED:
>> + OVS_NOT_REACHED();
>> + default:
>> + OVS_NOT_REACHED();
>> + }
>> +}
>> +
>> +/* Returns true if the route has already been fully processed for
>> + * advertising (e.g., for connected routes when connected-as-host is
>> set).
>> + * Returns false otherwise, in which case the a new 'ar_entry' needs
>> to
>> + * be created for this route.
>> + */
>> +static bool
>> +process_prereqs_advertise_route(
>> + const struct lr_stateful_table *lr_stateful_table,
>> + struct advertised_route_sync_data *data,
>> + struct uuidset *host_route_lrps,
>> + struct hmap *sync_routes,
>> + const struct ovn_datapath *advertising_od,
>> + const struct ovn_port *advertising_op,
>> + const struct ovn_port *tracked_op,
>> + enum route_source source)
>> +{
>> + enum dynamic_routing_redistribute_mode drr =
>> + advertising_op->dynamic_routing_redistribute;
>> +
>> + switch (source) {
>> + case ROUTE_SOURCE_CONNECTED:
>> + if (drr_mode_CONNECTED_AS_HOST_is_set(drr)) {
>> + const struct uuid *lrp_uuid = &advertising_op->nbrp-
>>> header_.uuid;
>> uuidset_insert(host_route_lrps, lrp_uuid);
>> publish_host_routes(sync_routes, lr_stateful_table,
>> - route->od, route->out_port,
>> + advertising_od, advertising_op,
>> �� data);
>> - return;
>> - }
>> - }
>> - if (route->source == ROUTE_SOURCE_STATIC &&
>> !drr_mode_STATIC_is_set(drr)) {
>> - return;
>> - }
>> - if (route->source == ROUTE_SOURCE_NAT) {
>> - if (!drr_mode_NAT_is_set(drr)) {
>> - return;
>> + return true;
>> }
>> + break;
>> + case ROUTE_SOURCE_STATIC:
>> + break;
>> + case ROUTE_SOURCE_NAT:
>> /* If NAT route tracks port on a different DP than the one
>> that
>> * advertises the route, we need to watch for changes on
>> that DP as
>> * well. */
>> - if (route->tracked_port && route->tracked_port->od != route-
>>> od) {
>> - if (route->tracked_port->od->nbr) {
>> + if (tracked_op && tracked_op->od != advertising_od) {
>> + if (tracked_op->od->nbr) {
>> uuidset_insert(&data->nb_lr,
>> - &route->tracked_port->od->nbr-
>>> header_.uuid);
>> - } else if (route->tracked_port->od->nbs) {
>> + &tracked_op->od->nbr->header_.uuid);
>> + } else if (tracked_op->od->nbs) {
>> uuidset_insert(&data->nb_ls,
>> - &route->tracked_port->od->nbs-
>>> header_.uuid);
>> + &tracked_op->od->nbs->header_.uuid);
>> }
>> }
>> - }
>> - if (route->source == ROUTE_SOURCE_LB) {
>> - if (!drr_mode_LB_is_set(drr)) {
>> - return;
>> - }
>> + break;
>> + case ROUTE_SOURCE_LB:
>> /* If LB route tracks port on a different DP than the one
>> that
>> * advertises the route, we need to watch for changes on
>> that DP as
>> * well. */
>> - if (route->tracked_port && route->tracked_port->od != route-
>>> od) {
>> + if (tracked_op && tracked_op->od != advertising_od) {
>> uuidset_insert(&data->nb_lr,
>> - &route->tracked_port->od->nbr-
>>> header_.uuid);
>> + &tracked_op->od->nbr->header_.uuid);
>> }
>> + break;
>> + case ROUTE_SOURCE_LEARNED:
>> + OVS_NOT_REACHED();
>> + default:
>> + OVS_NOT_REACHED();
>> }
>> -
>> - char *ip_prefix = normalize_v46_prefix(&route->prefix, route-
>>> plen);
>> - ar_entry_add(sync_routes, route->od, route->out_port, ip_prefix,
>> - route->tracked_port);
>> + return false;
>> }
>>
>> static void
>> @@ -725,23 +760,56 @@ advertised_route_table_sync(
>> {
>> struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
>> struct uuidset host_route_lrps =
>> UUIDSET_INITIALIZER(&host_route_lrps);
>> - const struct parsed_route *route;
>> -
>> - struct ar_entry *route_e;
>>
>> /* First build the set of non-dynamic routes that need sync-ing.
>> */
>> + const struct parsed_route *route;
>> HMAP_FOR_EACH (route, key_node, routes) {
>> - advertised_route_table_sync_route_add(lr_stateful_table,
>> - data,
>> &host_route_lrps,
>> - &sync_routes,
>> - route);
>> + if (route->is_discard_route) {
>> + continue;
>> + }
>> +
>> + if (prefix_is_link_local(&route->prefix, route->plen)) {
>> + continue;
>> + }
>> +
>> + if (!should_advertise_route(&host_route_lrps, route->od,
>> + route->out_port, route->source))
>> {
>> + continue;
>> + }
>> +
>> + if (process_prereqs_advertise_route(lr_stateful_table, data,
>> + &host_route_lrps,
>> &sync_routes,
>> + route->od, route-
>>> out_port,
>> + route->tracked_port,
>> + route->source)) {
>> + continue;
>> + }
>> +
>> + ar_entry_add_nocopy(&sync_routes, route->od, route-
>>> out_port,
>> + normalize_v46_prefix(&route->prefix,
>> route->plen),
>> + route->tracked_port,
>> + route->source);
>> }
>> +
>> /* Then add the set of dynamic routes that need sync-ing. */
>> - HMAP_FOR_EACH (route, key_node, dynamic_routes) {
>> - advertised_route_table_sync_route_add(lr_stateful_table,
>> - data,
>> &host_route_lrps,
>> - &sync_routes,
>> - route);
>> + struct ar_entry *route_e;
>> + HMAP_FOR_EACH (route_e, hmap_node, dynamic_routes) {
>> + if (!should_advertise_route(&host_route_lrps, route_e->od,
>> route_e->op,
>> + route_e->source)) {
>> + continue;
>> + }
>> +
>> + if (process_prereqs_advertise_route(lr_stateful_table, data,
>> + &host_route_lrps,
>> &sync_routes,
>> + route_e->od, route_e-
>>> op,
>> + route_e->tracked_port,
>> + route_e->source)) {
>> + continue;
>> + }
>> +
>> + ar_entry_add(&sync_routes, route_e->od, route_e->op,
>> + route_e->ip_prefix, route_e->tracked_port,
>> + route_e->source);
>> }
>> uuidset_destroy(&host_route_lrps);
>>
>> diff --git a/northd/northd.h b/northd/northd.h
>> index 12fccd7751..c221365e62 100644
>> --- a/northd/northd.h
>> +++ b/northd/northd.h
>> @@ -192,7 +192,8 @@ struct routes_data {
>> };
>>
>> struct dynamic_routes_data {
>> - struct hmap parsed_routes; /* Stores struct parsed_route. */
>> + struct hmap routes; /* Stores struct ar_entry, one for each
>> + * dynamic route. */
>> };
>>
>> struct route_policies_data {
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
