[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