Message ID | ZMiyzFhPY9ILWPaq@SIT-SDELAP10083.int.lidl.net |
---|---|
State | Changes Requested |
Headers | show |
Series | [ovs-dev,1/2] ovn-ic fix multiple routers in an az | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
Looks good to me. Acked-by: Mark Michelson <mmichels@redhat.com> Having said that, I'd like for some OVN devs that are more familiar with the ins and outs of ovn-ic to make the determination if this is behavior that we want or not. For non-IC OVN, routers learn routes to their neighbor routers automatically, unless the "dynamic_neigh_routers" option is enabled. For ovn-ic, do we have a way to ensure that routers do not learn neighbor routes if they do not want to? On 8/1/23 03:22, maximkorezkij via dev 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. > > Signed-off-by: Maxim Korezkij <maxim.korezkij@mail.schwarz> > Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz> > --- > ic/ovn-ic.c | 52 ++++++++++++++++++++++++++++++++++++------------- > tests/ovn-ic.at | 2 ++ > 2 files changed, 40 insertions(+), 14 deletions(-) > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > index 49850954f..cea14d008 100644 > --- a/ic/ovn-ic.c > +++ b/ic/ovn-ic.c > @@ -861,6 +861,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" > @@ -937,7 +939,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; > @@ -959,6 +962,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)); > @@ -1099,7 +1103,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 = ""; > @@ -1117,6 +1122,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); > @@ -1130,6 +1136,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) > { > @@ -1172,14 +1179,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; > @@ -1218,7 +1226,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 > @@ -1322,7 +1330,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); > @@ -1345,7 +1352,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; > } > > @@ -1435,16 +1450,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. */ > @@ -1549,7 +1572,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, > @@ -1559,7 +1582,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); > } > } > @@ -1571,7 +1594,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. */ > @@ -1582,7 +1606,7 @@ build_ts_routes_to_adv(struct ic_context *ctx, > } > > static void > -advertise_lr_routes(struct ic_context *ctx, > +collect_lr_routes(struct ic_context *ctx, > struct ic_router_info *ic_lr, > struct shash *routes_ad_by_ts) > { > @@ -1727,8 +1751,8 @@ route_run(struct ic_context *ctx, > struct ic_router_info *ic_lr; > struct shash routes_ad_by_ts = SHASH_INITIALIZER(&routes_ad_by_ts); > HMAP_FOR_EACH_SAFE (ic_lr, node, &ic_lrs) { > - advertise_lr_routes(ctx, ic_lr, &routes_ad_by_ts); > - sync_learned_routes(ctx, az, ic_lr); > + collect_lr_routes(ctx, ic_lr, &routes_ad_by_ts); > + 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 4b470d5c8..a49d47041 100644 > --- a/tests/ovn-ic.at > +++ b/tests/ovn-ic.at > @@ -1233,12 +1233,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.34.1 > > 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
Thank you for the feedback, the route learning feature needs to be explicitly enabled [1]. This is currently a global setting, so if we want to limit this to individual routers i would rather make this a general ovn-ic feature than to include this here. Thanks Felix [1] https://docs.ovn.org/en/latest/tutorials/ovn-interconnection.html#route-advertisement On Wed, Aug 02, 2023 at 03:13:01PM -0400, Mark Michelson wrote: > Looks good to me. > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Having said that, I'd like for some OVN devs that are more familiar with the > ins and outs of ovn-ic to make the determination if this is behavior that we > want or not. For non-IC OVN, routers learn routes to their neighbor routers > automatically, unless the "dynamic_neigh_routers" option is enabled. For > ovn-ic, do we have a way to ensure that routers do not learn neighbor routes > if they do not want to? > > On 8/1/23 03:22, maximkorezkij via dev 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. > > > > Signed-off-by: Maxim Korezkij <maxim.korezkij@mail.schwarz> > > Signed-off-by: Felix Huettner <felix.huettner@mail.schwarz> > > --- > > ic/ovn-ic.c | 52 ++++++++++++++++++++++++++++++++++++------------- > > tests/ovn-ic.at | 2 ++ > > 2 files changed, 40 insertions(+), 14 deletions(-) > > > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c > > index 49850954f..cea14d008 100644 > > --- a/ic/ovn-ic.c > > +++ b/ic/ovn-ic.c > > @@ -861,6 +861,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" > > @@ -937,7 +939,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; > > @@ -959,6 +962,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)); > > @@ -1099,7 +1103,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 = ""; > > @@ -1117,6 +1122,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); > > @@ -1130,6 +1136,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) > > { > > @@ -1172,14 +1179,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; > > @@ -1218,7 +1226,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 > > @@ -1322,7 +1330,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); > > @@ -1345,7 +1352,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; > > } > > > > @@ -1435,16 +1450,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. */ > > @@ -1549,7 +1572,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, > > @@ -1559,7 +1582,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); > > } > > } > > @@ -1571,7 +1594,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. */ > > @@ -1582,7 +1606,7 @@ build_ts_routes_to_adv(struct ic_context *ctx, > > } > > > > static void > > -advertise_lr_routes(struct ic_context *ctx, > > +collect_lr_routes(struct ic_context *ctx, > > struct ic_router_info *ic_lr, > > struct shash *routes_ad_by_ts) > > { > > @@ -1727,8 +1751,8 @@ route_run(struct ic_context *ctx, > > struct ic_router_info *ic_lr; > > struct shash routes_ad_by_ts = SHASH_INITIALIZER(&routes_ad_by_ts); > > HMAP_FOR_EACH_SAFE (ic_lr, node, &ic_lrs) { > > - advertise_lr_routes(ctx, ic_lr, &routes_ad_by_ts); > > - sync_learned_routes(ctx, az, ic_lr); > > + collect_lr_routes(ctx, ic_lr, &routes_ad_by_ts); > > + 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 4b470d5c8..a49d47041 100644 > > --- a/tests/ovn-ic.at > > +++ b/tests/ovn-ic.at > > @@ -1233,12 +1233,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.34.1 > > > > 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 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev 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/>.
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c index 49850954f..cea14d008 100644 --- a/ic/ovn-ic.c +++ b/ic/ovn-ic.c @@ -861,6 +861,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" @@ -937,7 +939,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; @@ -959,6 +962,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)); @@ -1099,7 +1103,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 = ""; @@ -1117,6 +1122,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); @@ -1130,6 +1136,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) { @@ -1172,14 +1179,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; @@ -1218,7 +1226,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 @@ -1322,7 +1330,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); @@ -1345,7 +1352,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; } @@ -1435,16 +1450,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. */ @@ -1549,7 +1572,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, @@ -1559,7 +1582,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); } } @@ -1571,7 +1594,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. */ @@ -1582,7 +1606,7 @@ build_ts_routes_to_adv(struct ic_context *ctx, } static void -advertise_lr_routes(struct ic_context *ctx, +collect_lr_routes(struct ic_context *ctx, struct ic_router_info *ic_lr, struct shash *routes_ad_by_ts) { @@ -1727,8 +1751,8 @@ route_run(struct ic_context *ctx, struct ic_router_info *ic_lr; struct shash routes_ad_by_ts = SHASH_INITIALIZER(&routes_ad_by_ts); HMAP_FOR_EACH_SAFE (ic_lr, node, &ic_lrs) { - advertise_lr_routes(ctx, ic_lr, &routes_ad_by_ts); - sync_learned_routes(ctx, az, ic_lr); + collect_lr_routes(ctx, ic_lr, &routes_ad_by_ts); + 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 4b470d5c8..a49d47041 100644 --- a/tests/ovn-ic.at +++ b/tests/ovn-ic.at @@ -1233,12 +1233,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])