Hi Lucas.
I had a look at this, and I have a feeling that this could be a
problem. Consider a case where two logical routers, LR1 and LR2 both
refer to the same northbound Logical_Router_Static_Route row. In the
current code, because of the way that parsed_route_add() and
parsed_route_lookup() work, this results in two parsed routes being
created. The first has its od field set to LR1 and the second has its
od field set to LR2. With this change, this scenario now results in
only one parsed route being created. Its od field will be set to
whichever logical router we process first.
I'm not sure what impact this ends up having in the long-run, but I'm
guessing it's not a good thing that a logical router might be using a
parsed route that appears to belong to a different logical router.
I think the fix for this could be simple, though. In
parsed_route_lookup(), just add a new check, like:
if (pr->od != new_pr->od) {
continue;
}
This will ensure that separate parsed routes get created when more
than one datapath refers to the same database row.
On Tue, Apr 21, 2026 at 5:12 PM Lucas Vargas Dias via dev
<[email protected]> wrote:
>
> Search by parsed route can be direct using uuid hash
> from source (static route, learned route, connect
> route), this avoids the search by ovn_datapath,
> and after, the search for parsed route.
>
> Signed-off-by: Lucas Vargas Dias <[email protected]>
> ---
> northd/en-learned-route-sync.c | 11 ++---------
> northd/northd.c | 11 +++++------
> northd/northd.h | 4 ++--
> 3 files changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
> index 4f7a12a28..38bd09739 100644
> --- a/northd/en-learned-route-sync.c
> +++ b/northd/en-learned-route-sync.c
> @@ -230,15 +230,9 @@ routes_table_sync(
>
> static struct parsed_route *
> find_learned_route(const struct sbrec_learned_route *learned_route,
> - const struct ovn_datapaths *lr_datapaths,
> const struct hmap *routes)
> {
> - const struct ovn_datapath *od = ovn_datapath_from_sbrec_(
> - &lr_datapaths->datapaths, learned_route->datapath);
> - if (!od) {
> - return NULL;
> - }
> - return parsed_route_lookup_by_source(od, ROUTE_SOURCE_LEARNED,
> + return parsed_route_lookup_by_source(ROUTE_SOURCE_LEARNED,
> &learned_route->header_, routes);
> }
>
> @@ -273,8 +267,7 @@ learned_route_sync_sb_learned_route_change_handler(struct
> engine_node *node,
>
> if (sbrec_learned_route_is_deleted(changed_route)) {
> struct parsed_route *route = find_learned_route(
> - changed_route, &northd_data->lr_datapaths,
> - &data->parsed_routes);
> + changed_route, &data->parsed_routes);
> if (!route) {
> goto fail;
> }
> diff --git a/northd/northd.c b/northd/northd.c
> index 0b52db6cf..4fd4b9de9 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12225,15 +12225,14 @@ parsed_route_clone(const struct parsed_route *pr)
> return new_pr;
> }
>
> -/* Searches for a parsed_route in a hmap based on datapath, source and
> +/* Searches for a parsed_route in a hmap based on source and
> * source_hint. */
> struct parsed_route *
> -parsed_route_lookup_by_source(const struct ovn_datapath *od,
> - enum route_source source,
> +parsed_route_lookup_by_source(enum route_source source,
> const struct ovsdb_idl_row *source_hint,
> const struct hmap *routes)
> {
> - size_t hash = uuid_hash(&od->key);
> + size_t hash = uuid_hash(&source_hint->uuid);
> struct parsed_route *route;
> HMAP_FOR_EACH_WITH_HASH (route, key_node, hash, routes) {
> if (route->source == source &&
> @@ -12247,10 +12246,10 @@ parsed_route_lookup_by_source(const struct
> ovn_datapath *od,
>
> /* This hash needs to be equal to the one used in
> * build_route_flows_for_lrouter to iterate over all routes of a datapath.
> - * This is distinct from route_hash which is stored in parsed_route->hash. */
> + * This is equal to route_hash which is stored in parsed_route->hash. */
> size_t
> parsed_route_hash(const struct parsed_route *pr) {
> - return uuid_hash(&pr->od->key);
> + return uuid_hash(&pr->source_hint->uuid);
> }
>
> void
> diff --git a/northd/northd.h b/northd/northd.h
> index e86d39f9a..a9070d6f6 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -859,8 +859,8 @@ struct parsed_route {
>
> struct parsed_route *parsed_route_clone(const struct parsed_route *);
> struct parsed_route *parsed_route_lookup_by_source(
> - const struct ovn_datapath *od, enum route_source source,
> - const struct ovsdb_idl_row *source_hint, const struct hmap *routes);
> + enum route_source source, const struct ovsdb_idl_row *source_hint,
> + const struct hmap *routes);
> size_t parsed_route_hash(const struct parsed_route *);
> void parsed_route_free(struct parsed_route *);
>
> --
> 2.43.0
>
>
> --
>
>
>
>
> _'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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
