Message ID | ZO7frUoo9gZk9zm5@SIT-SLAP8639.int.lidl.net |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,v2,1/2] ovn-ic fix multiple routers in an az | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | success | apply and check: success |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Wed, Aug 30, 2023 at 8:21 AM Felix Huettner via dev < ovs-dev@openvswitch.org> 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 <mmichels@redhat.com> > Co-Authored-By: Maxim Korezkij <maxim.korezkij@mail.schwarz> > Signed-off-by: Maxim Korezkij <maxim.korezkij@mail.schwarz> > Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > Looks good to me, thanks. Acked-by: Ales Musil <amusil@redhat.com>
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])