diff mbox series

[ovs-dev,v5,6/6] northd: Do not calculate database sequence numbers incrementally

Message ID 20211109193607.31630-7-mrmarkgray@gmail.com
State Accepted
Delegated to: Han Zhou
Headers show
Series northd: Introduce incremental processing framework | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Mark Gray Nov. 9, 2021, 7:36 p.m. UTC 
From: Mark Gray <mark.d.gray@redhat.com>

In order to remove the IDL loop variable from the engine context,
we do not calculate the database sequence numbers incrementally.

Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
---
 lib/inc-proc-eng.h       |  2 -
 northd/en-northd.c       |  3 +-
 northd/inc-proc-northd.c |  2 -
 northd/inc-proc-northd.h |  1 -
 northd/northd.c          | 80 +++----------------------------------
 northd/northd.h          |  3 +-
 northd/ovn-northd.c      | 85 ++++++++++++++++++++++++++++++++++++++--
 7 files changed, 89 insertions(+), 87 deletions(-)
diff mbox series

Patch

diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 1823750c814c..6f3918ae7789 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -73,8 +73,6 @@  struct engine_context {
     struct ovsdb_idl_txn *ovnsb_idl_txn;
     struct ovsdb_idl_txn *ovnnb_idl_txn;
 
-    struct ovsdb_idl_loop *ovnsb_idl_loop;
-
     void *client_ctx;
 };
 
diff --git a/northd/en-northd.c b/northd/en-northd.c
index 2fda3a9d219d..0523560f8da9 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -110,8 +110,7 @@  void en_northd_run(struct engine_node *node, void *data)
 
     northd_run(&input_data, data,
                eng_ctx->ovnnb_idl_txn,
-               eng_ctx->ovnsb_idl_txn,
-               eng_ctx->ovnsb_idl_loop);
+               eng_ctx->ovnsb_idl_txn);
     engine_set_node_state(node, EN_UPDATED);
 
 }
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index d4c913883d9a..c7f49d59934d 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -248,7 +248,6 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
 
 void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
                          struct ovsdb_idl_txn *ovnsb_txn,
-                         struct ovsdb_idl_loop *ovnsb_idl_loop,
                          bool recompute) {
     engine_set_force_recompute(recompute);
     engine_init_run();
@@ -256,7 +255,6 @@  void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
     struct engine_context eng_ctx = {
         .ovnnb_idl_txn = ovnnb_txn,
         .ovnsb_idl_txn = ovnsb_txn,
-        .ovnsb_idl_loop = ovnsb_idl_loop,
     };
 
     engine_set_context(&eng_ctx);
diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h
index 4aeb387b7b0f..1300253791b6 100644
--- a/northd/inc-proc-northd.h
+++ b/northd/inc-proc-northd.h
@@ -10,7 +10,6 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
                           struct ovsdb_idl_loop *sb);
 void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
                          struct ovsdb_idl_txn *ovnsb_txn,
-                         struct ovsdb_idl_loop *ovnsb_idl_loop,
                          bool recompute);
 void inc_proc_northd_cleanup(void);
 
diff --git a/northd/northd.c b/northd/northd.c
index dd543378463f..88be1775527f 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -14553,9 +14553,7 @@  ovnnb_db_run(struct northd_input *input_data,
              struct ovsdb_idl_txn *ovnnb_txn,
              struct ovsdb_idl_txn *ovnsb_txn,
              struct ovsdb_idl_index *sbrec_chassis_by_name,
-             struct ovsdb_idl_index *sbrec_chassis_by_hostname,
-             struct ovsdb_idl_loop *sb_loop,
-             int64_t loop_start_time)
+             struct ovsdb_idl_index *sbrec_chassis_by_hostname)
 {
     if (!ovnsb_txn || !ovnnb_txn) {
         return;
@@ -14578,12 +14576,7 @@  ovnnb_db_run(struct northd_input *input_data,
     if (nb->ipsec != sb->ipsec) {
         sbrec_sb_global_set_ipsec(sb, nb->ipsec);
     }
-    if (nb->nb_cfg != sb->nb_cfg) {
-        sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg);
-        nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time);
-    }
     sbrec_sb_global_set_options(sb, &nb->options);
-    sb_loop->next_cfg = nb->nb_cfg;
 
     const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options,
                                                           "mac_prefix"));
@@ -14884,68 +14877,12 @@  handle_port_binding_changes(struct northd_input *input_data,
     }
 }
 
-/* Updates the sb_cfg and hv_cfg columns in the northbound NB_Global table. */
-static void
-update_northbound_cfg(struct northd_input *input_data,
-                      struct ovsdb_idl_loop *sb_loop,
-                      int64_t loop_start_time)
-{
-    /* Update northbound sb_cfg if appropriate. */
-    const struct nbrec_nb_global *nbg = nbrec_nb_global_table_first(
-                               input_data->nbrec_nb_global_table);
-    int64_t sb_cfg = sb_loop->cur_cfg;
-    if (nbg && sb_cfg && nbg->sb_cfg != sb_cfg) {
-        nbrec_nb_global_set_sb_cfg(nbg, sb_cfg);
-        nbrec_nb_global_set_sb_cfg_timestamp(nbg, loop_start_time);
-    }
-
-    /* Update northbound hv_cfg if appropriate. */
-    if (nbg) {
-        /* Find minimum nb_cfg among all chassis. */
-        const struct sbrec_chassis_private *chassis_priv;
-        int64_t hv_cfg = nbg->nb_cfg;
-        int64_t hv_cfg_ts = 0;
-        SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_priv,
-                                    input_data->sbrec_chassis_private_table) {
-            const struct sbrec_chassis *chassis = chassis_priv->chassis;
-            if (chassis) {
-                if (smap_get_bool(&chassis->other_config,
-                                  "is-remote", false)) {
-                    /* Skip remote chassises. */
-                    continue;
-                }
-            } else {
-                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
-                VLOG_WARN_RL(&rl, "Chassis does not exist for "
-                             "Chassis_Private record, name: %s",
-                             chassis_priv->name);
-            }
-
-            if (chassis_priv->nb_cfg < hv_cfg) {
-                hv_cfg = chassis_priv->nb_cfg;
-                hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
-            } else if (chassis_priv->nb_cfg == hv_cfg &&
-                       chassis_priv->nb_cfg_timestamp > hv_cfg_ts) {
-                hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
-            }
-        }
-
-        /* Update hv_cfg. */
-        if (nbg->hv_cfg != hv_cfg) {
-            nbrec_nb_global_set_hv_cfg(nbg, hv_cfg);
-            nbrec_nb_global_set_hv_cfg_timestamp(nbg, hv_cfg_ts);
-        }
-    }
-}
-
 /* Handle a fairly small set of changes in the southbound database. */
 static void
 ovnsb_db_run(struct northd_input *input_data,
              struct ovsdb_idl_txn *ovnnb_txn,
              struct ovsdb_idl_txn *ovnsb_txn,
-             struct ovsdb_idl_loop *sb_loop,
-             struct hmap *ports,
-             int64_t loop_start_time)
+             struct hmap *ports)
 {
     if (!ovnnb_txn ||
         !ovsdb_idl_has_ever_connected(ovsdb_idl_txn_get_idl(ovnsb_txn))) {
@@ -14955,8 +14892,6 @@  ovnsb_db_run(struct northd_input *input_data,
     struct shash ha_ref_chassis_map = SHASH_INITIALIZER(&ha_ref_chassis_map);
     handle_port_binding_changes(input_data,
                                 ovnsb_txn, ports, &ha_ref_chassis_map);
-    update_northbound_cfg(input_data, sb_loop, loop_start_time);
-
     if (ovnsb_txn) {
         update_sb_ha_group_ref_chassis(input_data,
                                        &ha_ref_chassis_map);
@@ -14967,20 +14902,15 @@  ovnsb_db_run(struct northd_input *input_data,
 void northd_run(struct northd_input *input_data,
                 struct northd_data *data,
                 struct ovsdb_idl_txn *ovnnb_txn,
-                struct ovsdb_idl_txn *ovnsb_txn,
-                struct ovsdb_idl_loop *sb_loop)
+                struct ovsdb_idl_txn *ovnsb_txn)
 {
-    int64_t start_time = time_wall_msec();
-
     stopwatch_start(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
     ovnnb_db_run(input_data, data, ovnnb_txn, ovnsb_txn,
                  input_data->sbrec_chassis_by_name,
-                 input_data->sbrec_chassis_by_hostname,
-                 sb_loop, start_time);
+                 input_data->sbrec_chassis_by_hostname);
     stopwatch_stop(OVNNB_DB_RUN_STOPWATCH_NAME, time_msec());
     stopwatch_start(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
-    ovnsb_db_run(input_data, ovnnb_txn, ovnsb_txn, sb_loop,
-                 &data->ports, start_time);
+    ovnsb_db_run(input_data, ovnnb_txn, ovnsb_txn, &data->ports);
     stopwatch_stop(OVNSB_DB_RUN_STOPWATCH_NAME, time_msec());
 }
 
diff --git a/northd/northd.h b/northd/northd.h
index 2f04b6738e3d..07cf66f71df0 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -88,8 +88,7 @@  struct lflow_input {
 void northd_run(struct northd_input *input_data,
                 struct northd_data *data,
                 struct ovsdb_idl_txn *ovnnb_txn,
-                struct ovsdb_idl_txn *ovnsb_txn,
-                struct ovsdb_idl_loop *sb_loop);
+                struct ovsdb_idl_txn *ovnsb_txn);
 void northd_destroy(struct northd_data *data);
 void northd_init(struct northd_data *data);
 void northd_indices_create(struct northd_data *data,
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 24ff71003197..5bbfa03df84e 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -439,6 +439,78 @@  check_and_add_supported_dhcpv6_opts_to_sb_db(struct ovsdb_idl_txn *ovnsb_txn,
 
     hmap_destroy(&dhcpv6_opts_to_add);
 }
+
+/* Updates the nb_cfg, sb_cfg and hv_cfg columns in NB/SB databases. */
+static void
+update_sequence_numbers(int64_t loop_start_time,
+                        struct ovsdb_idl *ovnnb_idl,
+                        struct ovsdb_idl *ovnsb_idl,
+                        struct ovsdb_idl_txn *ovnnb_idl_txn,
+                        struct ovsdb_idl_txn *ovnsb_idl_txn,
+                        struct ovsdb_idl_loop *sb_loop)
+{
+    /* Create rows in global tables if neccessary */
+    const struct nbrec_nb_global *nb = nbrec_nb_global_first(ovnnb_idl);
+    if (!nb) {
+        nb = nbrec_nb_global_insert(ovnnb_idl_txn);
+    }
+    const struct sbrec_sb_global *sb = sbrec_sb_global_first(ovnsb_idl);
+    if (!sb) {
+        sb = sbrec_sb_global_insert(ovnsb_idl_txn);
+    }
+
+    /* Copy nb_cfg from northbound to southbound database.
+     * Also set up to update sb_cfg once our southbound transaction commits. */
+    if (nb->nb_cfg != sb->nb_cfg) {
+        sbrec_sb_global_set_nb_cfg(sb, nb->nb_cfg);
+        nbrec_nb_global_set_nb_cfg_timestamp(nb, loop_start_time);
+    }
+    sb_loop->next_cfg = nb->nb_cfg;
+
+    /* Update northbound sb_cfg if appropriate. */
+    int64_t sb_cfg = sb_loop->cur_cfg;
+    if (nb && sb_cfg && nb->sb_cfg != sb_cfg) {
+        nbrec_nb_global_set_sb_cfg(nb, sb_cfg);
+        nbrec_nb_global_set_sb_cfg_timestamp(nb, loop_start_time);
+    }
+
+    /* Update northbound hv_cfg if appropriate. */
+    if (nb) {
+        /* Find minimum nb_cfg among all chassis. */
+        const struct sbrec_chassis_private *chassis_priv;
+        int64_t hv_cfg = nb->nb_cfg;
+        int64_t hv_cfg_ts = 0;
+        SBREC_CHASSIS_PRIVATE_FOR_EACH (chassis_priv, ovnsb_idl) {
+            const struct sbrec_chassis *chassis = chassis_priv->chassis;
+            if (chassis) {
+                if (smap_get_bool(&chassis->other_config,
+                                  "is-remote", false)) {
+                    /* Skip remote chassises. */
+                    continue;
+                }
+            } else {
+                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+                VLOG_WARN_RL(&rl, "Chassis does not exist for "
+                             "Chassis_Private record, name: %s",
+                             chassis_priv->name);
+            }
+
+            if (chassis_priv->nb_cfg < hv_cfg) {
+                hv_cfg = chassis_priv->nb_cfg;
+                hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
+            } else if (chassis_priv->nb_cfg == hv_cfg &&
+                       chassis_priv->nb_cfg_timestamp > hv_cfg_ts) {
+                hv_cfg_ts = chassis_priv->nb_cfg_timestamp;
+            }
+        }
+
+        /* Update hv_cfg. */
+        if (nb->hv_cfg != hv_cfg) {
+            nbrec_nb_global_set_hv_cfg(nb, hv_cfg);
+            nbrec_nb_global_set_hv_cfg_timestamp(nb, hv_cfg_ts);
+        }
+    }
+}
 
 static void
 add_column_noalert(struct ovsdb_idl *idl,
@@ -997,9 +1069,8 @@  main(int argc, char *argv[])
             }
 
             if (ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
-                inc_proc_northd_run(ovnnb_txn, ovnsb_txn,
-                                    &ovnsb_idl_loop,
-                                    recompute);
+                int64_t loop_start_time = time_msec();
+                inc_proc_northd_run(ovnnb_txn, ovnsb_txn, recompute);
                 recompute = false;
                 if (ovnsb_txn) {
                     check_and_add_supported_dhcp_opts_to_sb_db(
@@ -1009,6 +1080,14 @@  main(int argc, char *argv[])
                     check_and_update_rbac(
                                  ovnsb_txn, ovnsb_idl_loop.idl);
                 }
+
+                if (ovnnb_txn && ovnsb_txn) {
+                    update_sequence_numbers(loop_start_time,
+                                            ovnnb_idl_loop.idl,
+                                            ovnsb_idl_loop.idl,
+                                            ovnnb_txn, ovnsb_txn,
+                                            &ovnsb_idl_loop);
+                }
             }
         } else {
             /* ovn-northd is paused