On Wed, Aug 30, 2023 at 8:21 AM Felix Huettner via dev < [email protected]> wrote:
> when connecting multiple logical routers to a transit switch per az then > previously the routers in the same az would not learn each others > routes while the routers in the others az would learn all of them. > > As this is confusing and would require each user to have additional > logical that configures static routing within each az. > > Acked-by: Mark Michelson <[email protected]> > Co-Authored-By: Maxim Korezkij <[email protected]> > Signed-off-by: Maxim Korezkij <[email protected]> > Signed-off-by: Felix Huettner <[email protected]> > --- > ic/ovn-ic.c | 48 ++++++++++++++++++++++++++++++++++++------------ > tests/ovn-ic.at | 2 ++ > 2 files changed, 38 insertions(+), 12 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index ec749e25f..0109afe40 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -866,6 +866,8 @@ struct ic_route_info { > const char *origin; > const char *route_table; > > + const struct nbrec_logical_router *nb_lr; > + > /* Either nb_route or nb_lrp is set and the other one must be NULL. > * - For a route that is learned from IC-SB, or a static route that is > * generated from a route that is configured in NB, the "nb_route" > @@ -942,7 +944,8 @@ parse_route(const char *s_prefix, const char > *s_nexthop, > /* Return false if can't be added due to bad format. */ > static bool > add_to_routes_learned(struct hmap *routes_learned, > - const struct nbrec_logical_router_static_route > *nb_route) > + const struct nbrec_logical_router_static_route > *nb_route, > + const struct nbrec_logical_router *nb_lr) > { > struct in6_addr prefix, nexthop; > unsigned int plen; > @@ -964,6 +967,7 @@ add_to_routes_learned(struct hmap *routes_learned, > ic_route->nb_route = nb_route; > ic_route->origin = origin; > ic_route->route_table = nb_route->route_table; > + ic_route->nb_lr = nb_lr; > hmap_insert(routes_learned, &ic_route->node, > ic_route_hash(&prefix, plen, &nexthop, origin, > nb_route->route_table)); > @@ -1104,7 +1108,8 @@ add_to_routes_ad(struct hmap *routes_ad, const > struct in6_addr prefix, > unsigned int plen, const struct in6_addr nexthop, > const char *origin, const char *route_table, > const struct nbrec_logical_router_port *nb_lrp, > - const struct nbrec_logical_router_static_route *nb_route) > + const struct nbrec_logical_router_static_route *nb_route, > + const struct nbrec_logical_router *nb_lr) > { > if (route_table == NULL) { > route_table = ""; > @@ -1122,6 +1127,7 @@ add_to_routes_ad(struct hmap *routes_ad, const > struct in6_addr prefix, > ic_route->origin = origin; > ic_route->route_table = route_table; > ic_route->nb_lrp = nb_lrp; > + ic_route->nb_lr = nb_lr; > hmap_insert(routes_ad, &ic_route->node, hash); > } else { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > @@ -1135,6 +1141,7 @@ static void > add_static_to_routes_ad( > struct hmap *routes_ad, > const struct nbrec_logical_router_static_route *nb_route, > + const struct nbrec_logical_router *nb_lr, > const struct lport_addresses *nexthop_addresses, > const struct smap *nb_options) > { > @@ -1177,14 +1184,15 @@ add_static_to_routes_ad( > } > > add_to_routes_ad(routes_ad, prefix, plen, nexthop, > ROUTE_ORIGIN_STATIC, > - nb_route->route_table, NULL, nb_route); > + nb_route->route_table, NULL, nb_route, nb_lr); > } > > static void > add_network_to_routes_ad(struct hmap *routes_ad, const char *network, > const struct nbrec_logical_router_port *nb_lrp, > const struct lport_addresses *nexthop_addresses, > - const struct smap *nb_options) > + const struct smap *nb_options, > + const struct nbrec_logical_router *nb_lr) > { > struct in6_addr prefix, nexthop; > unsigned int plen; > @@ -1223,7 +1231,7 @@ add_network_to_routes_ad(struct hmap *routes_ad, > const char *network, > > /* directly-connected routes go to <main> route table */ > add_to_routes_ad(routes_ad, prefix, plen, nexthop, > ROUTE_ORIGIN_CONNECTED, > - NULL, nb_lrp, NULL); > + NULL, nb_lrp, NULL, nb_lr); > } > > static bool > @@ -1327,7 +1335,6 @@ lrp_is_ts_port(struct ic_context *ctx, struct > ic_router_info *ic_lr, > > static void > sync_learned_routes(struct ic_context *ctx, > - const struct icsbrec_availability_zone *az, > struct ic_router_info *ic_lr) > { > ovs_assert(ctx->ovnnb_txn); > @@ -1350,7 +1357,15 @@ sync_learned_routes(struct ic_context *ctx, > > ICSBREC_ROUTE_FOR_EACH_EQUAL (isb_route, isb_route_key, > ctx->icsbrec_route_by_ts) { > - if (isb_route->availability_zone == az) { > + const char *lr_id = smap_get(&isb_route->external_ids, > "lr-id"); > + if (lr_id == NULL) { > + continue; > + } > + struct uuid lr_uuid; > + if (!uuid_from_string(&lr_uuid, lr_id)) { > + continue; > + } > + if (uuid_equals(&ic_lr->lr->header_.uuid, &lr_uuid)) { > continue; > } > > @@ -1440,16 +1455,24 @@ static void > ad_route_sync_external_ids(const struct ic_route_info *route_adv, > const struct icsbrec_route *isb_route) > { > - struct uuid isb_ext_id, nb_id; > + struct uuid isb_ext_id, nb_id, isb_ext_lr_id, lr_id; > smap_get_uuid(&isb_route->external_ids, "nb-id", &isb_ext_id); > + smap_get_uuid(&isb_route->external_ids, "lr-id", &isb_ext_lr_id); > nb_id = route_adv->nb_route ? route_adv->nb_route->header_.uuid > : route_adv->nb_lrp->header_.uuid; > + lr_id = route_adv->nb_lr->header_.uuid; > if (!uuid_equals(&isb_ext_id, &nb_id)) { > char *uuid_s = xasprintf(UUID_FMT, UUID_ARGS(&nb_id)); > icsbrec_route_update_external_ids_setkey(isb_route, "nb-id", > uuid_s); > free(uuid_s); > } > + if (!uuid_equals(&isb_ext_lr_id, &lr_id)) { > + char *uuid_s = xasprintf(UUID_FMT, UUID_ARGS(&lr_id)); > + icsbrec_route_update_external_ids_setkey(isb_route, "lr-id", > + uuid_s); > + free(uuid_s); > + } > } > > /* Sync routes from routes_ad to IC-SB. */ > @@ -1554,7 +1577,7 @@ build_ts_routes_to_adv(struct ic_context *ctx, > if (smap_get_uuid(&nb_route->external_ids, "ic-learned-route", > &isb_uuid)) { > /* It is a learned route */ > - if (!add_to_routes_learned(&ic_lr->routes_learned, nb_route)) > { > + if (!add_to_routes_learned(&ic_lr->routes_learned, nb_route, > lr)) { > static struct vlog_rate_limit rl = > VLOG_RATE_LIMIT_INIT(5, 1); > VLOG_WARN_RL(&rl, "Bad format of learned route in NB: " > "%s -> %s. Delete it.", nb_route->ip_prefix, > @@ -1564,7 +1587,7 @@ build_ts_routes_to_adv(struct ic_context *ctx, > } > } else if (!strcmp(ts_route_table, nb_route->route_table)) { > /* It may be a route to be advertised */ > - add_static_to_routes_ad(routes_ad, nb_route, ts_port_addrs, > + add_static_to_routes_ad(routes_ad, nb_route, lr, > ts_port_addrs, > &nb_global->options); > } > } > @@ -1576,7 +1599,8 @@ build_ts_routes_to_adv(struct ic_context *ctx, > for (int j = 0; j < lrp->n_networks; j++) { > add_network_to_routes_ad(routes_ad, lrp->networks[j], lrp, > ts_port_addrs, > - &nb_global->options); > + &nb_global->options, > + lr); > } > } else { > /* The router port of the TS port is ignored. */ > @@ -1733,7 +1757,7 @@ route_run(struct ic_context *ctx, > struct shash routes_ad_by_ts = SHASH_INITIALIZER(&routes_ad_by_ts); > HMAP_FOR_EACH_SAFE (ic_lr, node, &ic_lrs) { > collect_lr_routes(ctx, ic_lr, &routes_ad_by_ts); > - sync_learned_routes(ctx, az, ic_lr); > + sync_learned_routes(ctx, ic_lr); > free(ic_lr->isb_pbs); > hmap_destroy(&ic_lr->routes_learned); > hmap_remove(&ic_lrs, &ic_lr->node); > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at > index 8ef2362c4..84d14dcaa 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -1238,12 +1238,14 @@ AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 > | grep 192.168 | > AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr21 | grep 192.168 | > grep learned | awk '{print $1, $2}' | sort ], [0], [dnl > 192.168.0.0/24 169.254.10.11 > +192.168.2.0/24 169.254.10.22 > ]) > > # Test direct routes from lr11 and lr21 were learned to lr22 > AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr22 | grep 192.168 | > grep learned | awk '{print $1, $2}' | sort ], [0], [dnl > 192.168.0.0/24 169.254.10.11 > +192.168.1.0/24 169.254.10.21 > ]) > > OVN_CLEANUP_IC([az1], [az2]) > -- > 2.42.0 > > Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für > die Verwertung durch den vorgesehenen Empfänger bestimmt. > Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender > bitte unverzüglich in Kenntnis und löschen diese E Mail. > > Hinweise zum Datenschutz finden Sie hier<https://www.datenschutz.schwarz>. > > > This e-mail may contain confidential content and is intended only for the > specified recipient/s. > If you are not the intended recipient, please inform the sender > immediately and delete this e-mail. > > Information on data protection can be found here< > https://www.datenschutz.schwarz>. > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > Looks good to me, thanks. Acked-by: Ales Musil <[email protected]> -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
