On Thu, Jul 8, 2021 at 12:52 AM Mark Gray <[email protected]> wrote: > > On 08/07/2021 02:10, Han Zhou wrote: > > On Wed, Jul 7, 2021 at 1:28 AM Mark Gray <[email protected]> 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' will not be updated to reflect the new, correct > >> name. This will cause 'external_ids' to be incorrectly displayed by some > >> tools and commands such as `ovn-sbctl dump-flows`. > >> > >> This commit, reconciles these changes as part of build_lflows() when > >> 'ovn_internal_version' is updated. > >> > >> Suggested-by: Ilya Maximets <[email protected]> > >> Signed-off-by: Mark Gray <[email protected]> > >> --- > >> > >> Notes: > >> v2: Update all 'external_ids' rather than just 'stage-name' > >> v4: Fix line length errors from 0-day > >> > >> lib/ovn-util.c | 4 ++-- > >> northd/ovn-northd.c | 43 ++++++++++++++++++++++++++++++++++++++++--- > >> 2 files changed, 42 insertions(+), 5 deletions(-) > >> > >> diff --git a/lib/ovn-util.c b/lib/ovn-util.c > >> index c5af8d1ab340..acf4b1cd6059 100644 > >> --- a/lib/ovn-util.c > >> +++ b/lib/ovn-util.c > >> @@ -758,8 +758,8 @@ ip_address_and_port_from_lb_key(const char *key, char > > **ip_address, > >> return true; > >> } > >> > >> -/* Increment this for any logical flow changes or if existing OVN action > > is > >> - * modified. */ > >> +/* Increment this for any logical flow changes, if an existing OVN > > action is > >> + * modified or a stage is added to a logical pipeline. */ > >> #define OVN_INTERNAL_MINOR_VER 0 > >> > >> /* Returns the OVN version. The caller must free the returned value. */ > >> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > >> index 570c6a3efd77..eb25e31b1f7d 100644 > >> --- a/northd/ovn-northd.c > >> +++ b/northd/ovn-northd.c > >> @@ -12447,7 +12447,8 @@ build_lflows(struct northd_context *ctx, struct > > hmap *datapaths, > >> struct hmap *ports, struct hmap *port_groups, > >> struct hmap *mcgroups, struct hmap *igmp_groups, > >> struct shash *meter_groups, > >> - struct hmap *lbs, struct hmap *bfd_connections) > >> + struct hmap *lbs, struct hmap *bfd_connections, > >> + bool ovn_internal_version_changed) > >> { > >> struct hmap lflows; > >> > >> @@ -12559,6 +12560,32 @@ 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) { > >> + if (ovn_internal_version_changed) { > >> + const char *stage_name = > > smap_get_def(&sbflow->external_ids, > >> + "stage-name", ""); > >> + const char *stage_hint = > > smap_get_def(&sbflow->external_ids, > >> + "stage-hint", ""); > >> + const char *source = smap_get_def(&sbflow->external_ids, > >> + "source", ""); > >> + > >> + 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)); > >> + } > >> + if (lflow->stage_hint) { > >> + if (strcmp(stage_hint, lflow->stage_hint)) { > >> + > > sbrec_logical_flow_update_external_ids_setkey(sbflow, > >> + "stage-hint", lflow->stage_hint); > >> + } > >> + } > >> + if (lflow->where) { > >> + if (strcmp(source, lflow->where)) { > >> + > > sbrec_logical_flow_update_external_ids_setkey(sbflow, > >> + "source", lflow->where); > >> + } > >> + } > >> + } > >> + > >> /* This is a valid lflow. Checking if the datapath group > > needs > >> * updates. */ > >> bool update_dp_group = false; > >> @@ -13390,6 +13417,7 @@ ovnnb_db_run(struct northd_context *ctx, > >> struct shash meter_groups = SHASH_INITIALIZER(&meter_groups); > >> struct hmap lbs; > >> struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections); > >> + bool ovn_internal_version_changed = true; > >> > >> /* Sync ipsec configuration. > >> * Copy nb_cfg from northbound to southbound database. > >> @@ -13441,7 +13469,13 @@ ovnnb_db_run(struct northd_context *ctx, > >> smap_replace(&options, "max_tunid", max_tunid); > >> free(max_tunid); > >> > >> - smap_replace(&options, "northd_internal_version", > > ovn_internal_version); > >> + if (!strcmp(ovn_internal_version, > >> + smap_get_def(&options, "northd_internal_version", ""))) { > >> + ovn_internal_version_changed = false; > >> + } else { > >> + smap_replace(&options, "northd_internal_version", > >> + ovn_internal_version); > >> + } > >> > >> nbrec_nb_global_verify_options(nb); > >> nbrec_nb_global_set_options(nb, &options); > >> @@ -13481,7 +13515,8 @@ ovnnb_db_run(struct northd_context *ctx, > >> build_meter_groups(ctx, &meter_groups); > >> build_bfd_table(ctx, &bfd_connections, ports); > >> build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups, > >> - &igmp_groups, &meter_groups, &lbs, &bfd_connections); > >> + &igmp_groups, &meter_groups, &lbs, &bfd_connections, > >> + ovn_internal_version_changed); > >> ovn_update_ipv6_prefix(ports); > >> > >> sync_address_sets(ctx); > >> @@ -14351,6 +14386,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); > >> > > > > Sorry that I didn't see a reason to monitor external_ids. Did I miss > > anything? > > We need to read and write to this column so I set it. If I do not > monitor it, I get the following warning: > > "2021-07-08T07:49:48.982Z|01628|ovsdb_idl|WARN|cannot partially update > non-monitored column"
My bad, sorry for the noise. I just applied it to master. Thanks, Han > > > > Other than this: > > Acked-by: Han Zhou <[email protected]> > > > >> ovsdb_idl_add_table(ovnsb_idl_loop.idl, > >> &sbrec_table_logical_dp_group); > >> -- > >> 2.27.0 > >> > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
