[ovs-dev] [PATCH ovn v3 1/4] northd: update stage-name if changed

Mark Gray mark.d.gray at redhat.com
Fri Jul 2 08:57:20 UTC 2021


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 <i.maximets at ovn.org>
Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
---

Notes:
    v2:  Update all 'external_ids' rather than just 'stage-name'

 lib/ovn-util.c      |  4 ++--
 northd/ovn-northd.c | 42 +++++++++++++++++++++++++++++++++++++++---
 2 files changed, 41 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 83746f4ab444..5e5e0ce6d39f 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -12325,7 +12325,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;
 
@@ -12437,6 +12438,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;
@@ -13268,6 +13295,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.
@@ -13319,7 +13347,12 @@ 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);
@@ -13358,7 +13391,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);
@@ -14228,6 +14262,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



More information about the dev mailing list