diff mbox series

[ovs-dev,3/3] Measure performance of ovn-controller loop.

Message ID 20171218225139.13789-4-mmichels@redhat.com
State Changes Requested
Headers show
Series Add performance measurement library | expand

Commit Message

Mark Michelson Dec. 18, 2017, 10:51 p.m. UTC
This modifies ovn-controller to measure the amount of time it takes to
detect a change in the southbound database, generate the resulting flow
table, and write the flow table down to vswitchd.

The statistics can be queried using:

ovs-appctl -t ovn-controller performance/show ovn-controller-loop

Signed-off-by: Mark Michelson <mmichels@redhat.com>
---
 ovn/controller/ovn-controller.c | 53 +++++++++++++++++++++++++++++++++++++++++
 vswitchd/vswitch.xml            | 12 ++++++++++
 2 files changed, 65 insertions(+)

Comments

Ben Pfaff Jan. 5, 2018, 12:44 a.m. UTC | #1
On Mon, Dec 18, 2017 at 04:51:39PM -0600, Mark Michelson wrote:
> This modifies ovn-controller to measure the amount of time it takes to
> detect a change in the southbound database, generate the resulting flow
> table, and write the flow table down to vswitchd.
> 
> The statistics can be queried using:
> 
> ovs-appctl -t ovn-controller performance/show ovn-controller-loop
> 
> Signed-off-by: Mark Michelson <mmichels@redhat.com>

In ctrl_register_ovs_idl(), I think that the new code here should be in
a performance_*() function, since it's the performance code that knows
what columns are needed.

Since these columns are "write-only" from ovn-controller's perspective,
ovsdb_idl_omit_alert() should be called on each of them.  Maybe there
should be a helper function to call both ovsdb_idl_add_column() and
ovsdb_idl_omit_alert() on a column.

This patch adds an unneeded blank line in main().

I am a bit uncomfortable adding this amount of stuff to the ovs-vswitchd
database exclusively for ovn-controller.  I wonder whether
ovn-controller should have its own hypervisor-local database, or whether
this data should instead be turned over to some external time series
database.  Whatever we cook up is going to be inferior to any dedicated
time series database.
diff mbox series

Patch

diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
index c286ccbca..4f1769cfc 100644
--- a/ovn/controller/ovn-controller.c
+++ b/ovn/controller/ovn-controller.c
@@ -56,6 +56,9 @@ 
 #include "stream.h"
 #include "unixctl.h"
 #include "util.h"
+#include "timeval.h"
+#include "timer.h"
+#include "performance.h"
 
 VLOG_DEFINE_THIS_MODULE(main);
 
@@ -66,6 +69,9 @@  static unixctl_cb_func inject_pkt;
 #define DEFAULT_BRIDGE_NAME "br-int"
 #define DEFAULT_PROBE_INTERVAL_MSEC 5000
 
+#define CONTROLLER_LOOP_PERFORMANCE_NAME "ovn-controller-loop"
+#define DEFAULT_CONTROLLER_LOOP_PERFORMANCE_RATE 10000
+
 static void update_probe_interval(struct controller_ctx *,
                                   const char *ovnsb_remote);
 static void parse_options(int argc, char *argv[]);
@@ -524,6 +530,26 @@  ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
     ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_ca_cert);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_certificate);
     ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_private_key);
+    ovsdb_idl_add_table(ovs_idl, &ovsrec_table_performance);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_name);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_one_minute_samples);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_one_minute_min);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_one_minute_max);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_one_minute_average);
+    ovsdb_idl_add_column(ovs_idl,
+        &ovsrec_performance_col_one_minute_ninety_five_percentile);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_five_minute_samples);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_five_minute_min);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_five_minute_max);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_five_minute_average);
+    ovsdb_idl_add_column(ovs_idl,
+        &ovsrec_performance_col_five_minute_ninety_five_percentile);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_ten_minute_samples);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_ten_minute_min);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_ten_minute_max);
+    ovsdb_idl_add_column(ovs_idl, &ovsrec_performance_col_ten_minute_average);
+    ovsdb_idl_add_column(ovs_idl,
+        &ovsrec_performance_col_ten_minute_ninety_five_percentile);
     chassis_register_ovs_idl(ovs_idl);
     encaps_register_ovs_idl(ovs_idl);
     binding_register_ovs_idl(ovs_idl);
@@ -637,8 +663,12 @@  main(int argc, char *argv[])
     unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
                              &pending_pkt);
 
+    performance_create(CONTROLLER_LOOP_PERFORMANCE_NAME,
+                       DEFAULT_CONTROLLER_LOOP_PERFORMANCE_RATE,
+                       ovs_idl_loop.idl);
     /* Main loop. */
     exiting = false;
+    unsigned int our_seqno = 0;
     while (!exiting) {
         /* Check OVN SB database. */
         char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
@@ -657,6 +687,11 @@  main(int argc, char *argv[])
             .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
         };
 
+        if (our_seqno != ovsdb_idl_get_seqno(ctx.ovnsb_idl) &&
+                start_sample(CONTROLLER_LOOP_PERFORMANCE_NAME)) {
+            our_seqno = ovsdb_idl_get_seqno(ctx.ovnsb_idl);
+        }
+
         update_probe_interval(&ctx, ovnsb_remote);
 
         update_ssl_config(ctx.ovs_idl);
@@ -726,6 +761,8 @@  main(int argc, char *argv[])
                     ofctrl_put(&flow_table, &pending_ct_zones,
                                get_nb_cfg(ctx.ovnsb_idl));
 
+                    end_sample(CONTROLLER_LOOP_PERFORMANCE_NAME);
+
                     hmap_destroy(&flow_table);
                 }
                 if (ctx.ovnsb_idl_txn) {
@@ -754,6 +791,7 @@  main(int argc, char *argv[])
 
             expr_addr_sets_destroy(&addr_sets);
             shash_destroy(&addr_sets);
+
         }
 
         /* If we haven't handled the pending packet insertion
@@ -790,6 +828,20 @@  main(int argc, char *argv[])
             ofctrl_wait();
             pinctrl_wait(&ctx);
         }
+
+        const struct ovsrec_open_vswitch *cfg;
+        cfg = ovsrec_open_vswitch_first(ctx.ovs_idl);
+        if (cfg) {
+            const char *sample_rate = smap_get(&cfg->external_ids,
+                "ovn-controller-loop-sample-rate");
+            int sample_rate_ms;
+            if (sample_rate && sample_rate[0]
+                && str_to_int(sample_rate, 10, &sample_rate_ms)) {
+                performance_set_sample_rate(CONTROLLER_LOOP_PERFORMANCE_NAME,
+                                            sample_rate_ms);
+            }
+        }
+        performance_run(CONTROLLER_LOOP_PERFORMANCE_NAME, ctx.ovs_idl_txn);
         ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
 
         if (ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop) == 1) {
@@ -837,6 +889,7 @@  main(int argc, char *argv[])
         poll_block();
     }
 
+    performance_destroy("ovn-controller-loop");
     unixctl_server_destroy(unixctl);
     lflow_destroy();
     ofctrl_destroy();
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 045d9b049..145ae4a99 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -5415,6 +5415,18 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
     <p>
       All times are measured in milliseconds.
     </p>
+
+    <p>
+      The following performance measurements are recorded in this database:
+    </p>
+    <ul>
+      <li>
+        <var>ovn-controller-loop</var>. This measures the amount of time that
+        it takes for <code>ovn-controller</code> to detect a change in the OVN
+        southbound database, generate the resulting flow table, and write the
+        table out for <code>ovs-vswitchd</code> to process.
+      </li>
+    </ul>
     <column name="name">
       Performance identifier. Must be unique amongst other performance
       measurements.