[ovs-dev] [PATCH 3/3] ovn-controller.c: Support option "ovn-monitor-all".

Han Zhou hzhou at ovn.org
Mon Jan 13 22:52:25 UTC 2020


Set this option to true will avoid using conditional monitoring in
ovn-controller.  Setting it to true helps in environments where
all (or most) workloads need to be reachable from each other, thus
the effectiveness of conditional monitoring is low, but the overhead
of conditional monitoring is high.

Requested-by: Girish Moodalbail <gmoodalbail at nvidia.com>
Signed-off-by: Han Zhou <hzhou at ovn.org>
---
 controller/ovn-controller.8.xml | 21 ++++++++++++++++++++
 controller/ovn-controller.c     | 44 +++++++++++++++++++++++++++++++++++------
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml
index a226802..a163ff5 100644
--- a/controller/ovn-controller.8.xml
+++ b/controller/ovn-controller.8.xml
@@ -98,6 +98,27 @@
         </p>
       </dd>
 
+      <dt><code>external_ids:ovn-monitor-all</code></dt>
+      <dd>
+        <p>
+          A boolean value that tells if <code>ovn-controller</code> should
+          monitor all records of tables in <var>ovs-database</var>.  If
+          set to <code>false</code>, it will conditionally monitor the records
+          that is needed in the current chassis.
+        </p>
+        <p>
+          It is more optimal to set it to <code>true</code> in use cases when
+          the chassis would anyway need to monitor most of the records in
+          <var>ovs-database</var>, which would save the overhead of conditions
+          processing, especially for server side.  Typically, set it to
+          <code>true</code> for environments that all workloads need to be
+          reachable from each other.
+        </p>
+        <p>
+          Default value is <var>false</var>.
+        </p>
+      </dd>
+
       <dt><code>external_ids:ovn-remote-probe-interval</code></dt>
       <dd>
         <p>
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index abb1b4c..ccd63b3 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -129,7 +129,8 @@ static void
 update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
                    const struct sbrec_chassis *chassis,
                    const struct sset *local_ifaces,
-                   struct hmap *local_datapaths)
+                   struct hmap *local_datapaths,
+                   bool monitor_all)
 {
     /* Monitor Port_Bindings rows for local interfaces and local datapaths.
      *
@@ -154,6 +155,19 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
     struct ovsdb_idl_condition ce =  OVSDB_IDL_CONDITION_INIT(&ce);
     struct ovsdb_idl_condition ip_mcast = OVSDB_IDL_CONDITION_INIT(&ip_mcast);
     struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp);
+
+    if (monitor_all) {
+        ovsdb_idl_condition_add_clause_true(&pb);
+        ovsdb_idl_condition_add_clause_true(&lf);
+        ovsdb_idl_condition_add_clause_true(&mb);
+        ovsdb_idl_condition_add_clause_true(&mg);
+        ovsdb_idl_condition_add_clause_true(&dns);
+        ovsdb_idl_condition_add_clause_true(&ce);
+        ovsdb_idl_condition_add_clause_true(&ip_mcast);
+        ovsdb_idl_condition_add_clause_true(&igmp);
+        goto out;
+    }
+
     sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "patch");
     /* XXX: We can optimize this, if we find a way to only monitor
      * ports that have a Gateway_Chassis that point's to our own
@@ -205,6 +219,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
                                                    uuid);
         }
     }
+
+out:
     sbrec_port_binding_set_condition(ovnsb_idl, &pb);
     sbrec_logical_flow_set_condition(ovnsb_idl, &lf);
     sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
@@ -428,7 +444,8 @@ get_ofctrl_probe_interval(struct ovsdb_idl *ovs_idl)
 /* Retrieves the pointer to the OVN Southbound database from 'ovs_idl' and
  * updates 'sbdb_idl' with that pointer. */
 static void
-update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl)
+update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl,
+             bool *monitor_all_p)
 {
     const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
     if (!cfg) {
@@ -445,6 +462,19 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl)
     int interval = smap_get_int(&cfg->external_ids,
                                 "ovn-remote-probe-interval", default_interval);
     ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
+
+    bool monitor_all = smap_get_bool(&cfg->external_ids, "ovn-monitor-all",
+                                     false);
+    if (monitor_all) {
+        /* Always call update_sb_monitors when monitor_all is true.
+         * Otherwise, don't call it here, because there would be unnecessary
+         * extra cost. Instead, it is called after the engine execution only
+         * when it is necessary. */
+        update_sb_monitors(ovnsb_idl, NULL, NULL, NULL, true);
+    }
+    if (monitor_all_p) {
+        *monitor_all_p = monitor_all;
+    }
 }
 
 static void
@@ -1887,7 +1917,7 @@ main(int argc, char *argv[])
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status);
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target);
 
-    update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
+    update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, false);
 
     stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
 
@@ -2009,6 +2039,7 @@ main(int argc, char *argv[])
     /* Main loop. */
     exiting = false;
     restart = false;
+    bool sb_monitor_all = false;
     while (!exiting) {
         engine_init_run();
 
@@ -2023,7 +2054,7 @@ main(int argc, char *argv[])
             ovs_cond_seqno = new_ovs_cond_seqno;
         }
 
-        update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl);
+        update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, &sb_monitor_all);
         update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
         ofctrl_set_probe_interval(get_ofctrl_probe_interval(ovs_idl_loop.idl));
 
@@ -2163,7 +2194,8 @@ main(int argc, char *argv[])
                         if (engine_node_changed(&en_runtime_data)) {
                             update_sb_monitors(ovnsb_idl_loop.idl, chassis,
                                                &runtime_data->local_lports,
-                                               &runtime_data->local_datapaths);
+                                               &runtime_data->local_datapaths,
+                                               sb_monitor_all);
                         }
                     }
                 }
@@ -2272,7 +2304,7 @@ main(int argc, char *argv[])
     if (!restart) {
         bool done = !ovsdb_idl_has_ever_connected(ovnsb_idl_loop.idl);
         while (!done) {
-            update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl);
+            update_sb_db(ovs_idl_loop.idl, ovnsb_idl_loop.idl, NULL);
             update_ssl_config(ovsrec_ssl_table_get(ovs_idl_loop.idl));
 
             struct ovsdb_idl_txn *ovs_idl_txn
-- 
2.1.0



More information about the dev mailing list