On Thu, Jul 3, 2025 at 4:14 PM Dumitru Ceara <dce...@redhat.com> wrote:
> The en_bfd_sync ovn-northd incremental processing node was always
> returning "EN_CHANGED" triggering the en_lflow node to compute (and fail
> to do so incrementally) even when no BFD state changes happened. Even
> worse this happened even when no BFD configuration existed in the
> database.
>
> Instead of that, compare the old with the new BFD state and set the
> en_bfd_sync I-P node state accordingly.
>
> Fixes: 15c9c9f42ad8 ("northd: Add bfd, static_routes, route_policies and
> bfd_sync nodes to I-P engine.")
> Suggested-by: Ales Musil <amu...@redhat.com>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
> northd/en-northd.c | 14 ++++++++++----
> northd/northd.c | 18 ++++++++++++------
> northd/northd.h | 1 +
> 3 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index 8402ab943d..db94d6023b 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -422,14 +422,20 @@ en_bfd_sync_run(struct engine_node *node, void *data)
> EN_OVSDB_GET(engine_get_input("NB_bfd", node));
> struct bfd_sync_data *bfd_sync_data = data;
>
> - bfd_sync_destroy(data);
> - bfd_sync_init(data);
> + struct sset new_bfd_ports = SSET_INITIALIZER(&new_bfd_ports);
> bfd_table_sync(eng_ctx->ovnsb_idl_txn, nbrec_bfd_table,
> &northd_data->lr_ports, &bfd_data->bfd_connections,
> &route_policies_data->bfd_active_connections,
> &routes_data->bfd_active_connections,
> - &bfd_sync_data->bfd_ports);
> - return EN_UPDATED;
> + &new_bfd_ports);
> +
> + enum engine_node_state new_state =
> + sset_equals(&new_bfd_ports, &bfd_sync_data->bfd_ports)
> + ? EN_UNCHANGED : EN_UPDATED;
> +
> + bfd_sync_swap(bfd_sync_data, &new_bfd_ports);
> + sset_destroy(&new_bfd_ports);
> + return new_state;
> }
>
> void
> diff --git a/northd/northd.c b/northd/northd.c
> index 9b21786fad..052e15c2ab 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -19349,6 +19349,18 @@ bfd_sync_init(struct bfd_sync_data *data)
> sset_init(&data->bfd_ports);
> }
>
> +void
> +bfd_sync_swap(struct bfd_sync_data *data, struct sset *bfd_ports)
> +{
> + sset_swap(&data->bfd_ports, bfd_ports);
> +}
> +
> +void
> +bfd_sync_destroy(struct bfd_sync_data *data)
> +{
> + sset_destroy(&data->bfd_ports);
> +}
> +
> void
> northd_destroy(struct northd_data *data)
> {
> @@ -19404,12 +19416,6 @@ bfd_destroy(struct bfd_data *data)
> __bfd_destroy(&data->bfd_connections);
> }
>
> -void
> -bfd_sync_destroy(struct bfd_sync_data *data)
> -{
> - sset_destroy(&data->bfd_ports);
> -}
> -
> void
> route_policies_destroy(struct route_policies_data *data)
> {
> diff --git a/northd/northd.h b/northd/northd.h
> index 0846343281..eeacc7b991 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -857,6 +857,7 @@ void bfd_init(struct bfd_data *);
> void bfd_destroy(struct bfd_data *);
>
> void bfd_sync_init(struct bfd_sync_data *);
> +void bfd_sync_swap(struct bfd_sync_data *, struct sset *bfd_ports);
> void bfd_sync_destroy(struct bfd_sync_data *);
>
> struct lflow_table;
> --
> 2.49.0
>
>
Thank you Dumitru,
I went ahead and merged this into main. I didn't backport this as it's
performance improvement.
Regards,
Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
