[ovs-dev] [PATCH ovn 2/3] northd: update stage-name if changed

[Han Zhou]

On Sat, Jun 19, 2021 at 2:51 AM Mark Gray <mark.d.gray at redhat.com> wrote:
>
> If a new table is added to a logical flow pipeline, the mapping between
> 'external_ids:stage-name' from the 'Logical_Flow' table in the
> 'OVN_Southbound' database and the 'stage' number may change for some
tables.
>
> If 'ovn-northd' is started against a populated Southbound database,
> 'external_ids:stage-name' will not be updated to reflect the new, correct
> name. This will cause the stage name to be incorrectly displayed by some
> tools and commands such as `ovn-sbctl dump-flows`.
>
> This commit, reconciles changes to the stage name as part of
build_lflows().
>

This is interesting. It means a flow F existed in stage S (named "foo") of
ovn-northd version V1, and in version V2, S becomes S + 1 ("foo"), but in
the V2's stage S ("bar") there happens to be an exactly same flow like F
existing, but the stage name shown in ovn-sbctl dump-flows will be "foo"
instead of "bar". Is this the scenario the patch is trying to fix?

If so, I think it is better not only fixing the "stage-name" but also other
external_ids, including "source" and "stage-hint", for the same reason.

Thanks,
Han

> Suggested-by: Ilya Maximets <i.maximets at ovn.org>
> Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
> ---
>  northd/ovn-northd.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index aae7314c8977..d97ab4a5b39c 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -12412,6 +12412,13 @@ build_lflows(struct northd_context *ctx, struct
hmap *datapaths,
>              ovn_stage_build(dp_type, pipeline, sbflow->table_id),
>              sbflow->priority, sbflow->match, sbflow->actions,
sbflow->hash);
>          if (lflow) {
> +            const char *stage_name = smap_get_def(&sbflow->external_ids,
> +                                                  "stage-name", "");
> +            if (strcmp(stage_name, ovn_stage_to_str(lflow->stage))) {
> +                sbrec_logical_flow_update_external_ids_setkey(sbflow,
> +                     "stage-name", ovn_stage_to_str(lflow->stage));
> +            }
> +
>              /* This is a valid lflow.  Checking if the datapath group
needs
>               * updates. */
>              bool update_dp_group = false;
> @@ -14197,6 +14204,8 @@ main(int argc, char *argv[])
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_logical_flow_col_priority);
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_logical_flow_col_match);
>      add_column_noalert(ovnsb_idl_loop.idl,
&sbrec_logical_flow_col_actions);
> +    ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> +                         &sbrec_logical_flow_col_external_ids);
>
>      ovsdb_idl_add_table(ovnsb_idl_loop.idl,
>                          &sbrec_table_logical_dp_group);
> --
> 2.27.0
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev