Hi Alexandra,
Thanks for the review.


Em ter., 24 de fev. de 2026 às 09:16, Rukomoinikova Aleksandra
<[email protected]> escreveu:

> Hi Lucas!
>
> > Avoid learned route loop if two differents AZs
> > configure the same address and adv ther same prefix.
> > Do not remove the route from routes learned structure,
> > mark the route as not stale.
>
> nit: Maybe, it's worth clarifying that the patch solves the problem when
> such learning happen through a single transit switch?
>
> In case where there are multiple transit switches, routes won't be
> endlessly learned, but there will be a different problem with the
> endless updating of external_ids, which will be updated with different
> transit switch's TS. This problem is not related to the problem of
> endless route learning in general, but it seems worth mentioning for
> easier tracking later.
>
>
I agree. I'll adjust the commit message.

Regards,
Lucas




> >
> > Signed-off-by: Lucas Vargas Dias <lucas.vdias at luizalabs.com <
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev>>
> > ---
> >   ic/ovn-ic.c     | 17 ++++++++------
> >   tests/ovn-ic.at | 60 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 70 insertions(+), 7 deletions(-)
> >
> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > index 95d73cb4b..3f3c0d99d 100644
> > --- a/ic/ovn-ic.c
> > +++ b/ic/ovn-ic.c
> > @@ -1300,6 +1300,7 @@ struct ic_route_info {
> >       const struct nbrec_logical_router_static_route *nb_route;
> >       const struct nbrec_logical_router_port *nb_lrp;
> >       const struct nbrec_load_balancer *nb_lb;
> > +    bool stale; /* True if the route is stale and should be removed. */
> >   };
> >
> >   static uint32_t
> > @@ -1393,6 +1394,7 @@ add_to_routes_learned(struct hmap *routes_learned,
> >       ic_route->origin = origin;
> >       ic_route->route_table = nb_route->route_table;
> >       ic_route->nb_lr = nb_lr;
> > +    ic_route->stale = true;
> >       hmap_insert(routes_learned, &ic_route->node,
> >                   ic_route_hash(&prefix, plen, &nexthop, origin,
> >                                 nb_route->route_table));
> > @@ -2168,8 +2170,7 @@ sync_learned_routes(struct ic_context *ctx,
> >                           route_learned->nb_route, "ic-learned-route",
> uuid_s);
> >                       free(uuid_s);
> >                   }
> > -                hmap_remove(&ic_lr->routes_learned,
> &route_learned->node);
> > -                free(route_learned);
> > +                route_learned->stale = false;
> >               } else {
> >                   /* Create the missing route in NB. */
> >                   const struct nbrec_logical_router_static_route
> *nb_route =
> > @@ -2197,11 +2198,13 @@ sync_learned_routes(struct ic_context *ctx,
> >       /* Delete extra learned routes. */
> >       struct ic_route_info *route_learned;
> >       HMAP_FOR_EACH_SAFE (route_learned, node, &ic_lr->routes_learned) {
> > -        VLOG_DBG("Delete route %s -> %s that is not in IC-SB from NB.",
> > -                 route_learned->nb_route->ip_prefix,
> > -                 route_learned->nb_route->nexthop);
> > -        nbrec_logical_router_update_static_routes_delvalue(
> > -            ic_lr->lr, route_learned->nb_route);
> > +        if (route_learned->stale) {
> > +            VLOG_DBG("Delete route %s -> %s that is not in IC-SB from
> NB.",
> > +                     route_learned->nb_route->ip_prefix,
> > +                     route_learned->nb_route->nexthop);
> > +            nbrec_logical_router_update_static_routes_delvalue(
> > +                ic_lr->lr, route_learned->nb_route);
> > +        }
> >           hmap_remove(&ic_lr->routes_learned, &route_learned->node);
> >           free(route_learned);
> >       }
> > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> > index 1a826aa1c..ead16aa91 100644
> > --- a/tests/ovn-ic.at
> > +++ b/tests/ovn-ic.at
> > @@ -3394,6 +3394,66 @@ OVN_CLEANUP_IC([az1], [az2])
> >   AT_CLEANUP
> >   ])
> >
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ovn-ic -- check loop with LSP of duplicated address])
> > +
> > +ovn_init_ic_db
> > +
> > +for i in 1 2 3; do
> > +    ovn_start az$i
> > +    ovn_as az$i
> > +
> > +    # Enable route learning at AZ level
> > +    check ovn-nbctl set nb_global . options:ic-route-learn=true
> > +    # Enable route advertising at AZ level
> > +    check ovn-nbctl set nb_global . options:ic-route-adv=true
> > +done
> > +
> > +# Create new transit switches and LRs. Test topology is next:
> > +#                            logical router (lr12)
> > +#                                   |
> > +# logical router (lr11) -  / transit switch (ts11) \ - logical router
> (lr13)
> > +#
> > +
> > +# Create lr11, lr13, ts11 and connect them
> > +for i in 1 3; do
> > +    ovn_as az$i
> > +
> > +    lr=lr1$i
> > +    check ovn-nbctl lr-add $lr
> > +
> > +    check ovn-ic-nbctl --wait=sb --may-exist ts-add ts11
> > +
> > +    lrp=lrp-$lr-ts11
> > +    lsp=lsp-ts11-$lr
> > +    # Create LRP and connect to TS
> > +    check ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:a1:0$i
> 169.254.101.1/24
> > +    check ovn-nbctl lsp-add-router-port ts11 $lsp $lrp
> > +done
> > +
> > +# Create lr12 and connect it to ts11
> > +ovn_as az2
> > +check ovn-nbctl lr-add lr12
> > +
> > +# Create LRP and connect to TS
> > +check ovn-nbctl lrp-add lr12 lrp-lr12-ts11 aa:aa:aa:aa:a1:03
> 169.254.101.2/24
> > +check ovn-nbctl lsp-add-router-port ts11 lsp-lr12-ts11 lrp-lr12-ts11
> > +
> > +
> > +# Create directly-connected route in lr11
> > +check ovn_as az1 ovn-nbctl lrp-add lr11 lrp-lr11 aa:aa:aa:aa:bb:01 "
> 192.168.0.1/24"
> > +check ovn_as az3 ovn-nbctl lrp-add lr13 lrp-lr13 aa:aa:aa:aa:bb:03 "
> 192.168.0.1/24"
> > +sleep 2
>
> nit: It seems like a sleep is not needed here, bc line below calls WAIT
> macro
>
> > +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep
> 192.168 |
> > +             grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> > +192.168.0.0/24 169.254.101.1
> > +])
> > +
> > +OVN_CLEANUP_IC([az1], [az2], [az3])
> > +
> > +AT_CLEANUP
> > +])
> > +
> >   OVN_FOR_EACH_NORTHD([
> >   AT_SETUP([ovn-ic -- prefix filter -- filtering routes])
> >   ovn_init_ic_db
> > --
> > 2.43.0
>
> |Reviewed-by:| Alexandra Rukomoinikova <[email protected]>
>
> --
> regards,
> Alexandra.
>
>

-- 




_‘Esta mensagem é direcionada apenas para os endereços constantes no 
cabeçalho inicial. Se você não está listado nos endereços constantes no 
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa 
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão 
imediatamente anuladas e proibidas’._


* **‘Apesar do Magazine Luiza tomar 
todas as precauções razoáveis para assegurar que nenhum vírus esteja 
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por 
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.*



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to