On Wed, Sep 13, 2023 at 8:24 AM Felix Huettner via dev <
ovs-dev@openvswitch.org> wrote:
> previously if multiple routers in the same az are connected to the same
> transit switch then ovn-ic would only propagate the routes of one of
> these routers to the ic-sb.
> This commit fixes this behaviour and allows multiple routers in a single
> az to use route advertisements.
>
> 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 | 27 +++++++++++-----
> tests/ovn-ic.at | 86 +++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 105 insertions(+), 8 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index 11b533981..2df1235dc 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -1592,9 +1592,9 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> }
>
> static void
> -advertise_lr_routes(struct ic_context *ctx,
> - const struct icsbrec_availability_zone *az,
> - struct ic_router_info *ic_lr)
> +collect_lr_routes(struct ic_context *ctx,
> + struct ic_router_info *ic_lr,
> + struct shash *routes_ad_by_ts)
> {
> const struct nbrec_nb_global *nb_global =
> nbrec_nb_global_first(ctx->ovnnb_idl);
> @@ -1605,7 +1605,7 @@ advertise_lr_routes(struct ic_context *ctx,
> struct lport_addresses ts_port_addrs;
> const struct icnbrec_transit_switch *key;
>
> - struct hmap routes_ad = HMAP_INITIALIZER(&routes_ad);
> + struct hmap *routes_ad;
> for (int i = 0; i < ic_lr->n_isb_pbs; i++) {
> isb_pb = ic_lr->isb_pbs[i];
> key = icnbrec_transit_switch_index_init_row(
> @@ -1614,6 +1614,12 @@ advertise_lr_routes(struct ic_context *ctx,
> ts_name = icnbrec_transit_switch_index_find(
> ctx->icnbrec_transit_switch_by_name, key)->name;
> icnbrec_transit_switch_index_destroy_row(key);
> + routes_ad = shash_find_data(routes_ad_by_ts, ts_name);
> + if (!routes_ad) {
> + routes_ad = xzalloc(sizeof *routes_ad);
> + hmap_init(routes_ad);
> + shash_add(routes_ad_by_ts, ts_name, routes_ad);
> + }
>
> if (!extract_lsp_addresses(isb_pb->address, &ts_port_addrs)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> @@ -1625,12 +1631,10 @@ advertise_lr_routes(struct ic_context *ctx,
> }
> lrp_name = get_lrp_name_by_ts_port_name(ctx,
> isb_pb->logical_port);
> route_table = get_route_table_by_lrp_name(ctx, lrp_name);
> - build_ts_routes_to_adv(ctx, ic_lr, &routes_ad, &ts_port_addrs,
> + build_ts_routes_to_adv(ctx, ic_lr, routes_ad, &ts_port_addrs,
> nb_global, route_table);
> - advertise_routes(ctx, az, ts_name, &routes_ad);
> destroy_lport_addresses(&ts_port_addrs);
> }
> - hmap_destroy(&routes_ad);
> }
>
> static void
> @@ -1731,14 +1735,21 @@ route_run(struct ic_context *ctx,
> icsbrec_port_binding_index_destroy_row(isb_pb_key);
>
> 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, az, ic_lr);
> + collect_lr_routes(ctx, ic_lr, &routes_ad_by_ts);
> sync_learned_routes(ctx, az, ic_lr);
> free(ic_lr->isb_pbs);
> hmap_destroy(&ic_lr->routes_learned);
> hmap_remove(&ic_lrs, &ic_lr->node);
> free(ic_lr);
> }
> + struct shash_node *node;
> + SHASH_FOR_EACH (node, &routes_ad_by_ts) {
> + advertise_routes(ctx, az, node->name, node->data);
> + hmap_destroy(node->data);
> + }
> + shash_destroy_free_data(&routes_ad_by_ts);
> hmap_destroy(&ic_lrs);
> }
>
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 9a5f3e312..4b1c33c99 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -1166,3 +1166,89 @@ AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr12 |
> grep dst-ip | sort], [0], [d
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-ic -- route sync -- multiple logical routers])
> +
> +ovn_init_ic_db
> +ovn-ic-nbctl ts-add ts1
> +
> +for i in 1 2; do
> + ovn_start az$i
> + ovn_as az$i
> +
> + # Enable route learning at AZ level
> + ovn-nbctl set nb_global . options:ic-route-learn=true
> + # Enable route advertising at AZ level
> + ovn-nbctl set nb_global . options:ic-route-adv=true
> +done
> +
> +# Create new transit switches and LRs. Test topology is next:
> +#
> +# logical router (lr11) - transit switch (ts1) - logical router (lr21)
> +# \- logical router (lr22)
> +#
> +# each LR has one connected subnet except TS port
> +
> +
> +# create lr11, lr21, lr22, ts1 and connect them
> +ovn-ic-nbctl ts-add ts1
> +
> +ovn_as az1
> +
> +lr=lr11
> +ovn-nbctl lr-add $lr
> +
> +lrp=lrp-$lr-ts1
> +lsp=lsp-ts1-$lr
> +# Create LRP and connect to TS
> +ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:a1:01 169.254.10.11/24
> +ovn-nbctl lsp-add ts1 $lsp \
> + -- lsp-set-addresses $lsp router \
> + -- lsp-set-type $lsp router \
> + -- lsp-set-options $lsp router-port=$lrp
> +
> +ovn_as az2
> +for i in 1 2; do
> + lr=lr2$i
> + ovn-nbctl lr-add $lr
> +
> + lrp=lrp-$lr-ts1
> + lsp=lsp-ts1-$lr
> + # Create LRP and connect to TS
> + ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:a2:0$i 169.254.10.2$i/24
> + ovn-nbctl lsp-add ts1 $lsp \
> + -- lsp-set-addresses $lsp router \
> + -- lsp-set-type $lsp router \
> + -- lsp-set-options $lsp router-port=$lrp
> +done
> +
> +
> +# Create directly-connected routes
> +ovn_as az1 ovn-nbctl lrp-add lr11 lrp-lr11 aa:aa:aa:aa:bb:01 "
> 192.168.0.1/24"
> +ovn_as az2 ovn-nbctl lrp-add lr21 lrp-lr21 aa:aa:aa:aa:bc:01 "
> 192.168.1.1/24"
> +ovn_as az2 ovn-nbctl lrp-add lr22 lrp-lr22 aa:aa:aa:aa:bc:02 "
> 192.168.2.1/24"
> +
> +# Test direct routes from lr21 and lr22 were learned to lr11
> +AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep 192.168 |
> + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> +192.168.1.0/24 169.254.10.21
> +192.168.2.0/24 169.254.10.22
> +])
> +
> +# Test direct routes from lr11 and lr22 were learned to lr21
> +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
> +])
> +
> +# 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
> +])
> +
> +OVN_CLEANUP_IC([az1], [az2])
> +
> +AT_CLEANUP
> +])
> --
> 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
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Looks good to me, thanks.
Acked-by: Ales Musil <amu...@redhat.com>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
amu...@redhat.com IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
