On Fri, Sep 9, 2022 at 9:00 PM Han Zhou <[email protected]> wrote:

> This patch adjusts the order of adding tracked and untracked columns
> to monitoring, to workaround a problem in OVS IDL that could end up
> overwriting the track flag. A XXX comment is added for future
> improvement.
>
> Signed-off-by: Han Zhou <[email protected]>
> ---
>  controller/ovn-controller.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 89a495a04..e9f5149a9 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -927,23 +927,20 @@ static void
>  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>  {
>      /* We do not monitor all tables by default, so modules must register
> -     * their interest explicitly. */
> +     * their interest explicitly.
> +     * XXX: when the same column is monitored in different modes by
> different
> +     * modules, there is a chance that "track" flag added by
> +     * ovsdb_idl_track_add_column by one module being overwritten by a
> +     * following ovsdb_idl_add_column by another module. Before this is
> fixed
> +     * in OVSDB IDL, we need to be careful about the order so that the
> "track"
> +     * calls are after the "non-track" calls. */
>      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_open_vswitch);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_other_config);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_bridges);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_datapaths);
>      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_interface);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd_status);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_type);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_ofport);
>      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_port);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_name);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_interfaces);
> -    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_external_ids);
>      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_bridge);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_bridge_col_ports);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_bridge_col_name);
> @@ -963,6 +960,15 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      bfd_register_ovs_idl(ovs_idl);
>      physical_register_ovs_idl(ovs_idl);
>      vif_plug_register_ovs_idl(ovs_idl);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd_status);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_type);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_options);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_ofport);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_name);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_interfaces);
> +    ovsdb_idl_track_add_column(ovs_idl, &ovsrec_port_col_external_ids);
>  }
>
>  #define SB_NODES \
> --
> 2.30.2
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to