[ovs-dev] [PATCH v6 ovn] controller: introduce stats counters for ovn-controller incremental processing
Mark Gray
mark.d.gray at redhat.com
Wed Feb 24 22:21:33 UTC 2021
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 <mark.d.gray at redhat.com>
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 <lorenzo.bianconi at redhat.com>
> ---
> 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
>
More information about the dev
mailing list