On Wed, Nov 12, 2025 at 8:42 AM Dumitru Ceara <[email protected]> wrote:
> On 11/12/25 7:41 AM, Ales Musil wrote:
> > When ovn-controller tried to learn two routes with the same
> > destination and next-hop, but different metric, it would end up
> > in a transaction error and loop. Make sure we are also checking
> > routes that are learned in the current I-P run to prevent that
> > issue.
> >
>
> Hi Ales,
>
> Thanks for the fix!
>
Hi Dumitru,
Thank you for the review.
>
> > Fixes: 866a5014ae45 ("controller: Support learning routes.")
>
> This should probably also include:
>
> Reported-at: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2127934
> Reported-by
> <https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/2127934Reported-by>:
> Martin Kalcok <[email protected]>
>
Ah I have missed that, added in v2.
>
> > Reported-at: https://issues.redhat.com/browse/FDP-2081
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
> > controller/route-exchange.c | 19 +++++++++++++------
>
> Unless I'm missing something, it should be possible to add a system test
> to cover this scenario, right?
>
I've modified two of the existing tests which will fail if I remove the fix.
>
> The fix itself looks good to me.
>
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/controller/route-exchange.c b/controller/route-exchange.c
> > index d8deae1da..e60c0754e 100644
> > --- a/controller/route-exchange.c
> > +++ b/controller/route-exchange.c
> > @@ -51,6 +51,7 @@ struct route_entry {
> > struct hmap_node hmap_node;
> >
> > const struct sbrec_learned_route *sb_route;
> > + bool stale;
> > };
> >
> > static uint32_t
> > @@ -87,10 +88,13 @@ maintained_route_table_add(uint32_t table_id)
> >
> > static void
> > route_add_entry(struct hmap *routes,
> > - const struct sbrec_learned_route *sb_route)
> > + const struct sbrec_learned_route *sb_route, bool stale)
>
> Nit: I'd add the 'stale' arg on the next line.
>
Sure changed in v2.
>
> > {
> > struct route_entry *route_e = xmalloc(sizeof *route_e);
> > - route_e->sb_route = sb_route;
> > + *route_e = (struct route_entry) {
> > + .sb_route = sb_route,
> > + .stale = stale,
> > + };
> >
> > uint32_t hash = uuid_hash(&sb_route->datapath->header_.uuid);
> > hash = hash_string(sb_route->logical_port->logical_port, hash);
> > @@ -157,7 +161,7 @@ sb_sync_learned_routes(const struct vector
> *learned_routes,
> > sb_route->logical_port->logical_port)) {
> > continue;
> > }
> > - route_add_entry(&sync_routes, sb_route);
> > + route_add_entry(&sync_routes, sb_route, true);
> > }
> > sbrec_learned_route_index_destroy_row(filter);
> >
> > @@ -185,8 +189,7 @@ sb_sync_learned_routes(const struct vector
> *learned_routes,
> > route_e = route_lookup(&sync_routes, datapath,
> > logical_port, ip_prefix, nexthop);
> > if (route_e) {
> > - hmap_remove(&sync_routes, &route_e->hmap_node);
> > - free(route_e);
> > + route_e->stale = false;
> > } else {
> > if (!ovnsb_idl_txn) {
> > *sb_changes_pending = true;
> > @@ -197,6 +200,8 @@ sb_sync_learned_routes(const struct vector
> *learned_routes,
> > sbrec_learned_route_set_logical_port(sb_route,
> logical_port);
> > sbrec_learned_route_set_ip_prefix(sb_route, ip_prefix);
> > sbrec_learned_route_set_nexthop(sb_route, nexthop);
> > +
> > + route_add_entry(&sync_routes, sb_route, false);
> > }
> > }
> > free(ip_prefix);
> > @@ -204,7 +209,9 @@ sb_sync_learned_routes(const struct vector
> *learned_routes,
> > }
> >
> > HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) {
> > - sbrec_learned_route_delete(route_e->sb_route);
> > + if (route_e->stale) {
> > + sbrec_learned_route_delete(route_e->sb_route);
> > + }
> > free(route_e);
> > }
> > hmap_destroy(&sync_routes);
>
> Regards,
> Dumitru
>
>
>
Regards,
Ales
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
