On 24/02/2021 15:38, Lorenzo Bianconi wrote: Thanks for all the work on this Lorenzo, I think it is a really useful feature.
I have some reservations about the output format as I think it may be hard to grep for a node but that is only an aesthetic NIT so I suggest it is good to go. I ran some tests and it worked. Acked-by: Mark Gray <[email protected]> Thanks! > Introduce inc-engine/show-stats ovs-appctl 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/show-stats > Node: SB_address_set > - recompute: 38 > - compute: 0 > - abort: 0 > Node: addr_sets > - recompute: 2 > - compute: 0 > - abort: 1 > Node: SB_port_group > - recompute: 37 > - compute: 0 > - abort: 0 > .... > Node: flow_output > - recompute: 2 > - compute: 20 > - abort: 0 > > Moreover introduce the inc-engine/clear-stats command to reset engine > statistics > > $ovs-appctl -t ovn-controller inc-engine/clear-stats > > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Changes since v5: > - cosmetics > - add missing recompute increments > - move about counter in engine_run routine > - add missing ovs-appctl cmd documentation > Changes since v4: > - rename handlers in inc-engine/show-stats and inc-engine/clear-stats > - move all the code in lib/inc-proc-eng.{c,h} > - instad of run and change_handler, track node recompute and node compute > Changes since v3: > - drop engine_set_note_update_from_run/engine_set_note_update_from_handler > macros and move stats code in lib/inc-proc-eng.c > - fix commit log > 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 > --- > NEWS | 1 + > controller/ovn-controller.8.xml | 22 ++++++++++++++++ > lib/inc-proc-eng.c | 46 +++++++++++++++++++++++++++++++++ > lib/inc-proc-eng.h | 9 +++++++ > 4 files changed, 78 insertions(+) > > diff --git a/NEWS b/NEWS > index fca96f658..6cbb88add 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > Post-v21.03.0 > ------------------------- > + - Introduce ovn-controller incremetal processing engine statistics > > OVN v21.03.0 - 5 Mar 2020 > ------------------------- > diff --git a/controller/ovn-controller.8.xml b/controller/ovn-controller.8.xml > index 51c0c372c..8886df568 100644 > --- a/controller/ovn-controller.8.xml > +++ b/controller/ovn-controller.8.xml > @@ -578,6 +578,28 @@ > Displays logical flow cache statistics: enabled/disabled, per cache > type entry counts. > </dd> > + > + <dt><code>inc-engine/show-stats</code></dt> > + <dd> > + Display <code>ovn-controller</code> engine counters. For each engine > + node the following counters have been added: > + <ul> > + <li> > + <code>recompute</code> > + </li> > + <li> > + <code>compute</code> > + </li> > + <li> > + <code>abort</code> > + </li> > + </ul> > + </dd> > + > + <dt><code>inc-engine/clear-stats</code></dt> > + <dd> > + Reset <code>ovn-controller</code> engine counters. > + </dd> > </dl> > </p> > > diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c > index 916dbbe39..a6337a1d9 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 "unixctl.h" > > VLOG_DEFINE_THIS_MODULE(inc_proc_eng); > > @@ -102,6 +103,40 @@ engine_get_nodes(struct engine_node *node, size_t > *n_count) > return engine_topo_sort(node, NULL, n_count, &n_size); > } > > +static void > +engine_clear_stats(struct unixctl_conn *conn, int argc OVS_UNUSED, > + const char *argv[] OVS_UNUSED, void *arg OVS_UNUSED) > +{ > + for (size_t i = 0; i < engine_n_nodes; i++) { > + struct engine_node *node = engine_nodes[i]; > + > + memset(&node->stats, 0, sizeof node->stats); > + } > + unixctl_command_reply(conn, NULL); > +} > + > +static 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; > + > + for (size_t i = 0; i < engine_n_nodes; i++) { > + struct engine_node *node = engine_nodes[i]; > + > + ds_put_format(&dump, > + "Node: %s\n" > + "- recompute: %12"PRIu64"\n" > + "- compute: %12"PRIu64"\n" > + "- abort: %12"PRIu64"\n", > + node->name, node->stats.recompute, > + node->stats.compute, node->stats.abort); > + } > + unixctl_command_reply(conn, ds_cstr(&dump)); > + > + ds_destroy(&dump); > +} > + > void > engine_init(struct engine_node *node, struct engine_arg *arg) > { > @@ -115,6 +150,11 @@ engine_init(struct engine_node *node, struct engine_arg > *arg) > engine_nodes[i]->data = NULL; > } > } > + > + unixctl_command_register("inc-engine/show-stats", "", 0, 0, > + engine_dump_stats, NULL); > + unixctl_command_register("inc-engine/clear-stats", "", 0, 0, > + engine_clear_stats, NULL); > } > > void > @@ -288,6 +328,7 @@ engine_recompute(struct engine_node *node, bool forced, > bool allowed) > > /* Run the node handler which might change state. */ > node->run(node, node->data); > + node->stats.recompute++; > } > > /* Return true if the node could be computed, false otherwise. */ > @@ -312,6 +353,8 @@ engine_compute(struct engine_node *node, bool > recompute_allowed) > } > } > } > + node->stats.compute++; > + > return true; > } > > @@ -321,6 +364,7 @@ engine_run_node(struct engine_node *node, bool > recompute_allowed) > if (!node->n_inputs) { > /* Run the node handler which might change state. */ > node->run(node, node->data); > + node->stats.recompute++; > return; > } > > @@ -377,6 +421,7 @@ engine_run(bool recompute_allowed) > engine_run_node(engine_nodes[i], recompute_allowed); > > if (engine_nodes[i]->state == EN_ABORTED) { > + engine_nodes[i]->stats.abort++; > engine_run_aborted = true; > return; > } > @@ -393,6 +438,7 @@ engine_need_run(void) > } > > engine_nodes[i]->run(engine_nodes[i], engine_nodes[i]->data); > + engine_nodes[i]->stats.recompute++; > VLOG_DBG("input node: %s, state: %s", engine_nodes[i]->name, > engine_node_state_name[engine_nodes[i]->state]); > if (engine_nodes[i]->state == EN_UPDATED) { > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > index 857234677..7e9f5bb70 100644 > --- a/lib/inc-proc-eng.h > +++ b/lib/inc-proc-eng.h > @@ -107,6 +107,12 @@ enum engine_node_state { > EN_STATE_MAX, > }; > > +struct engine_stats { > + uint64_t recompute; > + uint64_t compute; > + uint64_t abort; > +}; > + > struct engine_node { > /* A unique name for each node. */ > char *name; > @@ -154,6 +160,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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
