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 <[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


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to