[ovs-dev] [PATCH v2 ovn] controller: introduce coverage_counters for ovn-controller incremental processing

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Wed Feb 17 19:33:28 UTC 2021


In order to help understanding system behaviour for debugging purpose,
introduce coverage counters for run handlers of ovn-controller
I-P engine nodes. Moreover add a global counter for engine abort.

https://bugzilla.redhat.com/show_bug.cgi?id=1890902
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
Changes since v1:
- drop handler counters and add global abort counter
- improve documentation and naming scheme
- introduce engine_set_node_updated utility macro
---
 controller/ovn-controller.c | 39 +++++++++++++++++++++++++++----------
 lib/inc-proc-eng.c          |  5 +++++
 lib/inc-proc-eng.h          | 12 +++++++++++-
 3 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4343650fc..42eed9ebd 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -69,6 +69,7 @@
 #include "stopwatch.h"
 #include "lib/inc-proc-eng.h"
 #include "hmapx.h"
+#include "coverage.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -85,6 +86,18 @@ static unixctl_cb_func lflow_cache_flush_cmd;
 static unixctl_cb_func lflow_cache_show_stats_cmd;
 static unixctl_cb_func debug_delay_nb_cfg_report;
 
+/* Coverage counters for run handlers of OVN controller
+ * incremental processing nodes
+ */
+ENGINE_RUN_COVERAGE_DEFINE(flow_output);
+ENGINE_RUN_COVERAGE_DEFINE(runtime_data);
+ENGINE_RUN_COVERAGE_DEFINE(addr_sets);
+ENGINE_RUN_COVERAGE_DEFINE(port_groups);
+ENGINE_RUN_COVERAGE_DEFINE(ct_zones);
+ENGINE_RUN_COVERAGE_DEFINE(mff_ovn_geneve);
+ENGINE_RUN_COVERAGE_DEFINE(ofctrl_is_connected);
+ENGINE_RUN_COVERAGE_DEFINE(physical_flow_changes);
+
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
@@ -955,6 +968,10 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     SB_NODE(dns, "dns") \
     SB_NODE(load_balancer, "load_balancer")
 
+#define SB_NODE(NAME, NAME_STR) ENGINE_RUN_COVERAGE_DEFINE(sb_##NAME);
+    SB_NODES
+#undef SB_NODE
+
 enum sb_engine_node {
 #define SB_NODE(NAME, NAME_STR) SB_##NAME,
     SB_NODES
@@ -972,6 +989,10 @@ enum sb_engine_node {
     OVS_NODE(interface, "interface") \
     OVS_NODE(qos, "qos")
 
+#define OVS_NODE(NAME, NAME_STR) ENGINE_RUN_COVERAGE_DEFINE(ovs_##NAME);
+    OVS_NODES
+#undef OVS_NODE
+
 enum ovs_engine_node {
 #define OVS_NODE(NAME, NAME_STR) OVS_##NAME,
     OVS_NODES
@@ -1011,7 +1032,7 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data)
             ofctrl_seqno_flush();
             binding_seqno_flush();
         }
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_node_updated(node, ofctrl_is_connected);
         return;
     }
     engine_set_node_state(node, EN_UNCHANGED);
@@ -1067,7 +1088,7 @@ en_addr_sets_run(struct engine_node *node, void *data)
     addr_sets_init(as_table, &as->addr_sets);
 
     as->change_tracked = false;
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, addr_sets);
 }
 
 static bool
@@ -1147,7 +1168,7 @@ en_port_groups_run(struct engine_node *node, void *data)
     port_groups_init(pg_table, &pg->port_groups);
 
     pg->change_tracked = false;
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, port_groups);
 }
 
 static bool
@@ -1482,7 +1503,7 @@ en_runtime_data_run(struct engine_node *node, void *data)
 
     binding_run(&b_ctx_in, &b_ctx_out);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, runtime_data);
 }
 
 static bool
@@ -1604,8 +1625,7 @@ en_ct_zones_run(struct engine_node *node, void *data)
                     &ct_zones_data->current, ct_zones_data->bitmap,
                     &ct_zones_data->pending, &rt_data->ct_updated_datapaths);
 
-
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, ct_zones);
 }
 
 /* The data in the ct_zones node is always valid (i.e., no stale pointers). */
@@ -1639,7 +1659,7 @@ en_mff_ovn_geneve_run(struct engine_node *node, void *data)
     enum mf_field_id mff_ovn_geneve = ofctrl_get_mf_field_id();
     if (ed_mff_ovn_geneve->mff_ovn_geneve != mff_ovn_geneve) {
         ed_mff_ovn_geneve->mff_ovn_geneve = mff_ovn_geneve;
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_node_updated(node, mff_ovn_geneve);
         return;
     }
     engine_set_node_state(node, EN_UNCHANGED);
@@ -1714,7 +1734,7 @@ en_physical_flow_changes_run(struct engine_node *node, void *data)
 {
     struct ed_type_pfc_data *pfc_tdata = data;
     pfc_tdata->recompute_physical_flows = true;
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, physical_flow_changes);
 }
 
 /* ct_zone changes are not handled incrementally but a handler is required
@@ -2034,8 +2054,7 @@ en_flow_output_run(struct engine_node *node, void *data)
     init_physical_ctx(node, rt_data, &p_ctx);
 
     physical_run(&p_ctx, &fo->flow_table);
-
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_node_updated(node, flow_output);
 }
 
 static bool
diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index 916dbbe39..a19ad68f8 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -27,6 +27,7 @@
 #include "openvswitch/hmap.h"
 #include "openvswitch/vlog.h"
 #include "inc-proc-eng.h"
+#include "coverage.h"
 
 VLOG_DEFINE_THIS_MODULE(inc_proc_eng);
 
@@ -44,6 +45,9 @@ static const char *engine_node_state_name[EN_STATE_MAX] = {
     [EN_ABORTED]   = "Aborted",
 };
 
+/* global counter for engine abort */
+COVERAGE_DEFINE(engine_abort);
+
 void
 engine_set_force_recompute(bool val)
 {
@@ -282,6 +286,7 @@ engine_recompute(struct engine_node *node, bool forced, bool allowed)
 
     if (!allowed) {
         VLOG_DBG("node: %s, recompute aborted", node->name);
+        COVERAGE_INC(engine_abort);
         engine_set_node_state(node, EN_ABORTED);
         return;
     }
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 857234677..6067a88c1 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -247,6 +247,10 @@ void *engine_get_internal_data(struct engine_node *node);
 #define engine_set_node_state(node, state) \
     engine_set_node_state_at(node, state, OVS_SOURCE_LOCATOR)
 
+#define engine_set_node_updated(node, NAME) \
+    engine_set_node_state(node, EN_UPDATED); \
+    ENGINE_RUN_COVERAGE_INC(NAME);
+
 struct ed_ovsdb_index {
     const char *name;
     struct ovsdb_idl_index *index;
@@ -311,7 +315,7 @@ en_##DB_NAME##_##TBL_NAME##_run(struct engine_node *node, \
     const struct DB_NAME##rec_##TBL_NAME##_table *table = \
         EN_OVSDB_GET(node); \
     if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \
-        engine_set_node_state(node, EN_UPDATED); \
+        engine_set_node_updated(node, DB_NAME##_##TBL_NAME); \
         return; \
     } \
     engine_set_node_state(node, EN_UNCHANGED); \
@@ -352,4 +356,10 @@ static void en_##DB_NAME##_##TBL_NAME##_cleanup(void *data OVS_UNUSED) \
 #define ENGINE_NODE_OVS(TBL_NAME, TBL_NAME_STR) \
     ENGINE_NODE_OVSDB(ovs, "OVS", TBL_NAME, TBL_NAME_STR);
 
+#define ENGINE_RUN_COVERAGE_DEFINE(NAME)    \
+    COVERAGE_DEFINE(en_##NAME##_run);
+
+#define ENGINE_RUN_COVERAGE_INC(NAME)       \
+    COVERAGE_INC(en_##NAME##_run);
+
 #endif /* lib/inc-proc-eng.h */
-- 
2.29.2



More information about the dev mailing list