On 3/6/25 09:41, Alexandra Rukomoinikova wrote:
> When changing the order of stages in the LogicalFlow
> table in SB, some rules will be added with an incorrect
> stage-name in the external-ids of existing datapath groups.
> When adding a stage, new lflows in a new table with the same
> match and action that already existed with the same table-id
> will remain in the database with unupdated stage names.
> 
> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
> ---
>  lib/ovn-util.c     | 5 ++++-
>  lib/ovn-util.h     | 2 +-
>  northd/lflow-mgr.c | 4 +++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
> index c65b36bb5..d1572d5ec 100644
> --- a/lib/ovn-util.c
> +++ b/lib/ovn-util.c
> @@ -646,16 +646,19 @@ sbrec_logical_flow_hash(const struct sbrec_logical_flow 
> *lf)
>  {
>      return ovn_logical_flow_hash(lf->table_id,
>                                   ovn_pipeline_from_name(lf->pipeline),
> +                                 smap_get_def(&lf->external_ids,
> +                                              "stage-name", ""),
>                                   lf->priority, lf->match,
>                                   lf->actions);
>  }

Hi, Alexandra.  Thanks for the patch!

Though, IIRC, it was an intentional decision to not include the stage
name in the hash/comparison to avoid a significant performance impact
that this hashmap lookup and the string hashing bring.

The issue only affects debugging capabilities, but the performance
impact of hashing the name on each database update and comparing extra
strings on lflow generation as well as re-writing most of the flows
on stage updates is significant and affects every operation in northd.

Did you run some scale tests with this change?  What's the impact in
current OVN?

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to