[ovs-dev] [PATCH v5 ovn] controller: introduce stats counters for ovn-controller incremental processing
Mark Gray
mark.d.gray at redhat.com
Tue Feb 23 18:55:25 UTC 2021
On 23/02/2021 14:52, Lorenzo Bianconi wrote:
> 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
> SB_address_set
> recompute 1 abort 0 compute 0
> addr_sets
> recompute 2 abort 1 compute 0
> SB_port_group
> recompute 0 abort 0 compute 0
> port_groups
> recompute 2 abort 0 compute 0
> ofctrl_is_connected
> recompute 1 abort 0 compute 0
> OVS_open_vswitch
> recompute 0 abort 0 compute 0
> OVS_bridge
> recompute 0 abort 0 compute 0
> OVS_qos
> recompute 0 abort 0 compute 0
> SB_chassis
> recompute 1 abort 0 compute 0
> ....
>
> flow_output
> recompute 2 abort 0 compute 34
>
> 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 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
> ---
> lib/inc-proc-eng.c | 41 +++++++++++++++++++++++++++++++++++++++++
> lib/inc-proc-eng.h | 10 ++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c
> index 916dbbe39..6afe692bb 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,37 @@ 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, "%s\n", node->name);
> + ds_put_format(&dump, "\trecompute\t%lu", node->stats.recompute);
> + ds_put_format(&dump, "\tcompute\t%lu", node->stats.compute);
> + ds_put_format(&dump, "\tabort\t%lu\n", 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 +147,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
> @@ -283,11 +320,13 @@ 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;
node->run() itself could cause an abort. You should move this counter
into engine_run() and check the state of each node after
engine_node_run() has been invoked. In this way, you ensure you catch
all cases.
> }
>
> /* Run the node handler which might change state. */
> node->run(node, node->data);
> + node->stats.recompute++;
There are two other places that we node->run() in this file. One is a
the state of engine_run_node(). This is for root nodes. I think we
should increment there as well. The other is engine_need_run() - I am
not sure about this but to me, it looks like it is doing a recompute.
We will need to increment this there as well.
> }
>
> /* Return true if the node could be computed, false otherwise. */
> @@ -312,6 +351,8 @@ engine_compute(struct engine_node *node, bool recompute_allowed)
> }
> }
> }
> + node->stats.compute++;
> +
> return true;
> }
>
> diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
> index 857234677..31456e435 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 {
> + unsigned long recompute;
> + unsigned long compute;
> + unsigned long 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
> @@ -352,4 +361,5 @@ 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);
>
> +
> #endif /* lib/inc-proc-eng.h */
>
More information about the dev
mailing list