[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