[ovs-dev] [PATCH v3 ovn] controller: introduce coverage counters for ovn-controller incremental processing

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Fri Feb 19 14:26:21 UTC 2021


Introduce inc-engine/stats ovs-applctl command in order to dump
ovn-controller incremental processing engine statistics. So far for each
node a counter for run, abort and engine_handler have been added.
Counters are incremented when the node move to "updated" state.
In order to dump I-P stats we can can use the following commands:

$ovs-appctl -t ovn-controller inc-engine/stats
SB_address_set
        run     1       abort   0       handler 0
addr_sets
        run     2       abort   1       handler 0
SB_port_group
        run     0       abort   0       handler 0
port_groups
        run     2       abort   0       handler 0
ofctrl_is_connected
        run     1       abort   0       handler 0
OVS_open_vswitch
        run     0       abort   0       handler 0
OVS_bridge
        run     0       abort   0       handler 0
OVS_qos
        run     0       abort   0       handler 0
SB_chassis
        run     1       abort   0       handler 0
....

flow_output
        run     2       abort   0       handler 34

Morover introduce the inc-engine/stats-clear command to reset engine
statistics

$ovs-appctl -t ovn-controller inc-engine/stats-clear

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
Changes since v2:
- introduce inc-engine/stats and inc-engine/stats-clear commands and drop
  COVERAGE_* dependency

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 | 53 ++++++++++++++++++++++---------------
 lib/inc-proc-eng.c          | 48 +++++++++++++++++++++++++++++++++
 lib/inc-proc-eng.h          | 26 ++++++++++++++++++
 3 files changed, 106 insertions(+), 21 deletions(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4343650fc..a937803c8 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1011,7 +1011,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_note_update_from_run(node);
         return;
     }
     engine_set_node_state(node, EN_UNCHANGED);
@@ -1067,7 +1067,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_note_update_from_run(node);
 }
 
 static bool
@@ -1088,7 +1088,7 @@ addr_sets_sb_address_set_handler(struct engine_node *node, void *data)
 
     if (!sset_is_empty(&as->new) || !sset_is_empty(&as->deleted) ||
             !sset_is_empty(&as->updated)) {
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_note_update_from_handler(node);
     } else {
         engine_set_node_state(node, EN_UNCHANGED);
     }
@@ -1147,7 +1147,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_note_update_from_run(node);
 }
 
 static bool
@@ -1168,7 +1168,7 @@ port_groups_sb_port_group_handler(struct engine_node *node, void *data)
 
     if (!sset_is_empty(&pg->new) || !sset_is_empty(&pg->deleted) ||
             !sset_is_empty(&pg->updated)) {
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_note_update_from_handler(node);
     } else {
         engine_set_node_state(node, EN_UNCHANGED);
     }
@@ -1482,7 +1482,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_note_update_from_run(node);
 }
 
 static bool
@@ -1500,7 +1500,7 @@ runtime_data_ovs_interface_handler(struct engine_node *node, void *data)
     }
 
     if (b_ctx_out.local_lports_changed) {
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_note_update_from_handler(node);
         rt_data->local_lports_changed = b_ctx_out.local_lports_changed;
     }
 
@@ -1528,7 +1528,7 @@ runtime_data_sb_port_binding_handler(struct engine_node *node, void *data)
     if (b_ctx_out.local_lport_ids_changed ||
             b_ctx_out.non_vif_ports_changed ||
             !hmap_is_empty(b_ctx_out.tracked_dp_bindings)) {
-        engine_set_node_state(node, EN_UPDATED);
+        engine_set_note_update_from_handler(node);
     }
 
     return true;
@@ -1605,7 +1605,7 @@ en_ct_zones_run(struct engine_node *node, void *data)
                     &ct_zones_data->pending, &rt_data->ct_updated_datapaths);
 
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_run(node);
 }
 
 /* The data in the ct_zones node is always valid (i.e., no stale pointers). */
@@ -1639,7 +1639,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_note_update_from_run(node);
         return;
     }
     engine_set_node_state(node, EN_UNCHANGED);
@@ -1714,7 +1714,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_note_update_from_run(node);
 }
 
 /* ct_zone changes are not handled incrementally but a handler is required
@@ -1734,7 +1734,7 @@ physical_flow_changes_ovs_iface_handler(struct engine_node *node, void *data)
 {
     struct ed_type_pfc_data *pfc_tdata = data;
     pfc_tdata->ovs_ifaces_changed = true;
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return true;
 }
 
@@ -2035,7 +2035,7 @@ en_flow_output_run(struct engine_node *node, void *data)
 
     physical_run(&p_ctx, &fo->flow_table);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_run(node);
 }
 
 static bool
@@ -2059,7 +2059,7 @@ flow_output_sb_logical_flow_handler(struct engine_node *node, void *data)
 
     bool handled = lflow_handle_changed_flows(&l_ctx_in, &l_ctx_out);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return handled;
 }
 
@@ -2085,7 +2085,7 @@ flow_output_sb_mac_binding_handler(struct engine_node *node, void *data)
     lflow_handle_changed_neighbors(sbrec_port_binding_by_name,
             mac_binding_table, local_datapaths, flow_table);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return true;
 }
 
@@ -2108,7 +2108,7 @@ flow_output_sb_port_binding_handler(struct engine_node *node,
      */
     physical_handle_port_binding_changes(&p_ctx, flow_table);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return true;
 }
 
@@ -2126,7 +2126,7 @@ flow_output_sb_multicast_group_handler(struct engine_node *node, void *data)
 
     physical_handle_mc_group_changes(&p_ctx, flow_table);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return true;
 
 }
@@ -2135,6 +2135,7 @@ static bool
 _flow_output_resource_ref_handler(struct engine_node *node, void *data,
                                   enum ref_type ref_type)
 {
+    bool update_counter = false;
     struct ed_type_runtime_data *rt_data =
         engine_get_input_data("runtime_data", node);
 
@@ -2208,6 +2209,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_counter = true;
         }
     }
     SSET_FOR_EACH (ref_name, updated) {
@@ -2217,6 +2219,7 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_counter = true;
         }
     }
     SSET_FOR_EACH (ref_name, new) {
@@ -2226,8 +2229,12 @@ _flow_output_resource_ref_handler(struct engine_node *node, void *data,
         }
         if (changed) {
             engine_set_node_state(node, EN_UPDATED);
+            update_counter = true;
         }
     }
+    if (update_counter) {
+        node->stats.handler++;
+    }
 
     return true;
 }
@@ -2254,7 +2261,7 @@ flow_output_physical_flow_changes_handler(struct engine_node *node, void *data)
     struct physical_ctx p_ctx;
     init_physical_ctx(node, rt_data, &p_ctx);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     struct ed_type_pfc_data *pfc_data =
         engine_get_input_data("physical_flow_changes", node);
 
@@ -2293,7 +2300,7 @@ flow_output_runtime_data_handler(struct engine_node *node,
     struct hmap *tracked_dp_bindings = &rt_data->tracked_dp_bindings;
     if (hmap_is_empty(tracked_dp_bindings)) {
         if (rt_data->local_lports_changed) {
-            engine_set_node_state(node, EN_UPDATED);
+            engine_set_note_update_from_handler(node);
         }
         return true;
     }
@@ -2325,7 +2332,7 @@ flow_output_runtime_data_handler(struct engine_node *node,
         }
     }
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return true;
 }
 
@@ -2342,7 +2349,7 @@ flow_output_sb_load_balancer_handler(struct engine_node *node, void *data)
 
     bool handled = lflow_handle_changed_lbs(&l_ctx_in, &l_ctx_out);
 
-    engine_set_node_state(node, EN_UPDATED);
+    engine_set_note_update_from_handler(node);
     return handled;
 }
 
@@ -2681,6 +2688,10 @@ main(int argc, char *argv[])
 
     unixctl_command_register("recompute", "", 0, 0, engine_recompute_cmd,
                              NULL);
+    unixctl_command_register("inc-engine/stats", "", 0, 0, engine_dump_stats,
+                             NULL);
+    unixctl_command_register("inc-engine/stats-clear", "", 0, 0,
+                             engine_clear_stats, NULL);
     unixctl_command_register("lflow-cache/flush", "", 0, 0,
                              lflow_cache_flush_cmd,
                              &flow_output_data->pd);
diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
index 916dbbe39..ce3fed562 100644
--- a/lib/inc-proc-eng.c
+++ b/lib/inc-proc-eng.c
@@ -32,6 +32,7 @@ VLOG_DEFINE_THIS_MODULE(inc_proc_eng);
 
 static bool engine_force_recompute = false;
 static bool engine_run_aborted = false;
+static bool engine_stats_reset = false;
 static const struct engine_context *engine_context;
 
 static struct engine_node **engine_nodes;
@@ -283,6 +284,7 @@ engine_recompute(struct engine_node *node, bool forced, bool allowed)
     if (!allowed) {
         VLOG_DBG("node: %s, recompute aborted", node->name);
         engine_set_node_state(node, EN_ABORTED);
+        node->stats.abort++;
         return;
     }
 
@@ -383,9 +385,26 @@ engine_run(bool recompute_allowed)
     }
 }
 
+static void
+engine_reset_stats(void)
+{
+    if (!engine_stats_reset) {
+        return;
+    }
+
+    engine_stats_reset = false;
+    for (size_t i = 0; i < engine_n_nodes; i++) {
+        struct engine_node *node = engine_nodes[i];
+
+        memset(&node->stats, 0, sizeof(node->stats));
+    }
+}
+
 bool
 engine_need_run(void)
 {
+    engine_reset_stats();
+
     for (size_t i = 0; i < engine_n_nodes; i++) {
         /* Check only leaf nodes for updates. */
         if (engine_nodes[i]->n_inputs) {
@@ -401,3 +420,32 @@ engine_need_run(void)
     }
     return false;
 }
+
+void
+engine_clear_stats(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                   const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
+{
+    engine_stats_reset = true;
+    unixctl_command_reply(conn, NULL);
+}
+
+void
+engine_dump_stats(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                  const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED)
+{
+    struct ds dump = DS_EMPTY_INITIALIZER;
+
+    engine_reset_stats();
+
+    for (size_t i = 0; i < engine_n_nodes; i++) {
+        struct engine_node *node = engine_nodes[i];
+
+        ds_put_format(&dump, "%s\n", node->name);
+        ds_put_format(&dump, "\trun\t%lu", node->stats.run);
+        ds_put_format(&dump, "\tabort\t%lu", node->stats.abort);
+        ds_put_format(&dump, "\thandler\t%lu\n", node->stats.handler);
+    }
+    unixctl_command_reply(conn, ds_cstr(&dump));
+
+    ds_destroy(&dump);
+}
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 857234677..0eeca7ced 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -60,6 +60,8 @@
  * against all its inputs.
  */
 
+#include "unixctl.h"
+
 #define ENGINE_MAX_INPUT 256
 #define ENGINE_MAX_OVSDB_INDEX 256
 
@@ -107,6 +109,12 @@ enum engine_node_state {
     EN_STATE_MAX,
 };
 
+struct engine_stats {
+    unsigned long run;
+    unsigned long abort;
+    unsigned long handler;
+};
+
 struct engine_node {
     /* A unique name for each node. */
     char *name;
@@ -154,6 +162,9 @@ struct engine_node {
     /* Method to clear up tracked data maintained by the engine node in the
      * engine 'data'. It may be NULL. */
     void (*clear_tracked_data)(void *tracked_data);
+
+    /* Engine stats */
+    struct engine_stats stats;
 };
 
 /* Initialize the data for the engine nodes. It calls each node's
@@ -247,6 +258,14 @@ 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_note_update_from_run(node)   \
+    engine_set_node_state(node, EN_UPDATED);    \
+    node->stats.run++;
+
+#define engine_set_note_update_from_handler(node)   \
+    engine_set_node_state(node, EN_UPDATED);        \
+    node->stats.handler++;
+
 struct ed_ovsdb_index {
     const char *name;
     struct ovsdb_idl_index *index;
@@ -312,6 +331,7 @@ en_##DB_NAME##_##TBL_NAME##_run(struct engine_node *node, \
         EN_OVSDB_GET(node); \
     if (DB_NAME##rec_##TBL_NAME##_table_track_get_first(table)) { \
         engine_set_node_state(node, EN_UPDATED); \
+        node->stats.run++; \
         return; \
     } \
     engine_set_node_state(node, EN_UNCHANGED); \
@@ -352,4 +372,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);
 
+
+void engine_dump_stats(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                       const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED);
+void engine_clear_stats(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                        const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED);
+
 #endif /* lib/inc-proc-eng.h */
-- 
2.29.2



More information about the dev mailing list