On Tue, Jul 5, 2022 at 3:41 PM Han Zhou <[email protected]> wrote:
>
> The column was not tracked before while it should. The column includes
> many ovn-controller global configurations that could impact the way
> flows are computed. It worked before because lots of the configurations
> are also populated to OVN-SB DB's chassis table, and as a result the SB
> DB change would notify back to the ovn-controller itself, thus
> triggering a recompute to make the configure change effective. However,
> this is not the way it is supposed to work. We should track the
> open_vswitch:external_ids column directly, and the I-P engine would
> immediately recompute and apply the change.
>
> This patch also adjust 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 | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 69615308e..dacef0204 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -922,23 +922,19 @@ 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 there are overlapping columns monitored 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);
> @@ -958,6 +954,16 @@ 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_open_vswitch_col_external_ids);
> +    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
>

A kind reminder for this patch, which can be reviewed and applied
independent of the rest of the series.

Thanks,
Han
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to