[ovs-dev] [PATCH v6 ovn] controller: introduce stats counters for ovn-controller incremental processing
Mark Michelson
mmichels at redhat.com
Tue Mar 16 20:33:44 UTC 2021
I pushed this to master.
On 2/24/21 5:21 PM, Mark Gray wrote:
> 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
>>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
More information about the dev
mailing list