ofproto-dpif: More responsive average subfacet statistics.

Before this patch, statistics about subfacet average life span,
and count in the datapath, were calculated over the entire lifetime
of the ofproto.  Furthermore, the subfacet lifespan was only
updated when a subfacet was removed from the datapath causing long
lived subfacets to be ignored.  It's far more useful to know these
numbers averaged over all subfacets in the recent past.  This patch
changes the code to implement an exponentially weighted moving
average updated every time statistics are pulled from the datapath.

Signed-off-by: Ethan Jackson <ethan@nicira.com>

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ec2d95d..f8c0abc 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -732,6 +732,8 @@ struct ofproto_dpif {
      * performance in new situations.  */
     long long int created;         /* Time when it is created. */
     unsigned int max_n_subfacet;   /* Maximum number of flows */
+    unsigned int avg_n_subfacet;   /* Average number of flows. */
+    long long int avg_subfacet_life_span;
 
     /* The average number of subfacets... */
     struct avg_subfacet_rates hourly; /* ...over the last hour. */
@@ -745,23 +747,8 @@ struct ofproto_dpif {
     /* Number of subfacets added or deleted from 'created' to 'last_minute.' */
     unsigned long long int total_subfacet_add_count;
     unsigned long long int total_subfacet_del_count;
-
-    /* Sum of the number of milliseconds that each subfacet existed,
-     * over the subfacets that have been added and then later deleted. */
-    unsigned long long int total_subfacet_life_span;
-
-    /* Incremented by the number of currently existing subfacets, each
-     * time we pull statistics from the kernel. */
-    unsigned long long int total_subfacet_count;
-
-    /* Number of times we pull statistics from the kernel. */
-    unsigned long long int n_update_stats;
 };
-static unsigned long long int avg_subfacet_life_span(
-                                        const struct ofproto_dpif *);
-static double avg_subfacet_count(const struct ofproto_dpif *ofproto);
 static void update_moving_averages(struct ofproto_dpif *ofproto);
-static void update_max_subfacet_count(struct ofproto_dpif *ofproto);
 
 /* Defer flow mod completion until "ovs-appctl ofproto/unclog"?  (Useful only
  * for debugging the asynchronous flow_mod implementation.) */
@@ -1437,9 +1424,8 @@ construct(struct ofproto *ofproto_)
     ofproto->subfacet_del_count = 0;
     ofproto->total_subfacet_add_count = 0;
     ofproto->total_subfacet_del_count = 0;
-    ofproto->total_subfacet_life_span = 0;
-    ofproto->total_subfacet_count = 0;
-    ofproto->n_update_stats = 0;
+    ofproto->avg_subfacet_life_span = 0;
+    ofproto->avg_n_subfacet = 0;
 
     return error;
 }
@@ -4315,15 +4301,31 @@ expire(struct dpif_backer *backer)
     update_stats(backer);
 
     HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
+        long long int avg_subfacet_life_span;
         struct rule *rule, *next_rule;
+        struct subfacet *subfacet;
         int dp_max_idle;
 
         if (ofproto->backer != backer) {
             continue;
         }
 
-        /* Keep track of the max number of flows per ofproto_dpif. */
-        update_max_subfacet_count(ofproto);
+        avg_subfacet_life_span = 0;
+        if (!hmap_is_empty(&ofproto->subfacets)) {
+            long long int now = time_msec();
+            HMAP_FOR_EACH (subfacet, hmap_node, &ofproto->subfacets) {
+                avg_subfacet_life_span += now - subfacet->created;
+            }
+            avg_subfacet_life_span /= hmap_count(&ofproto->subfacets);
+        }
+        ofproto->avg_subfacet_life_span += avg_subfacet_life_span;
+        ofproto->avg_subfacet_life_span /= 2;
+
+        ofproto->avg_n_subfacet += hmap_count(&ofproto->subfacets);
+        ofproto->avg_n_subfacet /= 2;
+
+        ofproto->max_n_subfacet = MAX(ofproto->max_n_subfacet,
+                                      hmap_count(&ofproto->subfacets));
 
         /* Expire subfacets that have been idle too long. */
         dp_max_idle = subfacet_max_idle(ofproto);
@@ -4448,9 +4450,6 @@ update_stats(struct dpif_backer *backer)
             continue;
         }
 
-        ofproto->total_subfacet_count += hmap_count(&ofproto->subfacets);
-        ofproto->n_update_stats++;
-
         key_hash = odp_flow_key_hash(key, key_len);
         subfacet = subfacet_find(ofproto, key, key_len, key_hash);
         switch (subfacet ? subfacet->path : SF_NOT_INSTALLED) {
@@ -4474,7 +4473,6 @@ update_stats(struct dpif_backer *backer)
     HMAP_FOR_EACH (ofproto, all_ofproto_dpifs_node, &all_ofproto_dpifs) {
         update_moving_averages(ofproto);
     }
-
 }
 
 /* Calculates and returns the number of milliseconds of idle time after which
@@ -5269,7 +5267,6 @@ subfacet_destroy__(struct subfacet *subfacet)
 
     /* Update ofproto stats before uninstall the subfacet. */
     ofproto->subfacet_del_count++;
-    ofproto->total_subfacet_life_span += (time_msec() - subfacet->created);
 
     subfacet_uninstall(subfacet);
     hmap_remove(&ofproto->subfacets, &subfacet->hmap_node);
@@ -8302,12 +8299,12 @@ show_dp_format(const struct ofproto_dpif *ofproto, struct ds *ds)
     ds_put_format(ds,
                   "\tlookups: hit:%"PRIu64" missed:%"PRIu64"\n",
                   ofproto->n_hit, ofproto->n_missed);
-    ds_put_format(ds, "\tflows: cur: %zu, avg: %5.3f, max: %d,"
-                  " life span: %llu(ms)\n",
+    ds_put_format(ds, "\tflows: cur: %zu, avg: %u, max: %d,"
+                  " life span: %lld(ms)\n",
                   hmap_count(&ofproto->subfacets),
-                  avg_subfacet_count(ofproto),
+                  ofproto->avg_n_subfacet,
                   ofproto->max_n_subfacet,
-                  avg_subfacet_life_span(ofproto));
+                  ofproto->avg_subfacet_life_span);
     if (minutes >= 60) {
         show_dp_rates(ds, "\t\thourly avg:", &ofproto->hourly);
     }
@@ -8732,30 +8729,6 @@ odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, uint32_t odp_port)
         return OFPP_NONE;
     }
 }
-static unsigned long long int
-avg_subfacet_life_span(const struct ofproto_dpif *ofproto)
-{
-    unsigned long long int dc;
-    unsigned long long int avg;
-
-    dc = ofproto->total_subfacet_del_count + ofproto->subfacet_del_count;
-    avg = dc ? ofproto->total_subfacet_life_span / dc : 0;
-
-    return avg;
-}
-
-static double
-avg_subfacet_count(const struct ofproto_dpif *ofproto)
-{
-    double avg_c = 0.0;
-
-    if (ofproto->n_update_stats) {
-        avg_c = (double)ofproto->total_subfacet_count
-                / ofproto->n_update_stats;
-    }
-
-    return avg_c;
-}
 
 static void
 show_dp_rates(struct ds *ds, const char *heading,
@@ -8765,13 +8738,6 @@ show_dp_rates(struct ds *ds, const char *heading,
                   heading, rates->add_rate, rates->del_rate);
 }
 
-static void
-update_max_subfacet_count(struct ofproto_dpif *ofproto)
-{
-    ofproto->max_n_subfacet = MAX(ofproto->max_n_subfacet,
-                                  hmap_count(&ofproto->subfacets));
-}
-
 /* Compute exponentially weighted moving average, adding 'new' as the newest,
  * most heavily weighted element.  'base' designates the rate of decay: after
  * 'base' further updates, 'new''s weight in the EWMA decays to about 1/e

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 94ba9e0..16d3e0c 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2031,14 +2031,14 @@ ADD_OF_PORTS([br1], [3])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (dummy)
        p2 2/2: (dummy)
 br1 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br1 65534/101: (dummy)
        p3 3/3: (dummy)
@@ -2047,7 +2047,7 @@ br1 (dummy@ovs-dummy):
 AT_CHECK([ovs-appctl dpif/show br0], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (dummy)
@@ -2117,6 +2117,8 @@ OVS_VSWITCHD_START([add-br br1 \
 ADD_OF_PORTS([br0], [2])
 ADD_OF_PORTS([br1], [3])
 
+AT_CHECK([ovs-appctl time/stop]) dnl Make life span averages consistent.
+
 AT_CHECK([ovs-ofctl add-flow br0 actions=LOCAL,output:1,output:2])
 AT_CHECK([ovs-ofctl add-flow br1 actions=LOCAL,output:1,output:3])
 
@@ -2135,14 +2137,14 @@ warped
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:9 missed:1
-       flows: cur: 1, avg: 1.000, max: 1, life span: 0(ms)
+       flows: cur: 1, avg: 0, max: 1, life span: 1250(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p2 2/2: (dummy)
        pbr0 1/none: (patch: peer=pbr1)
 br1 (dummy@ovs-dummy):
        lookups: hit:4 missed:1
-       flows: cur: 1, avg: 1.000, max: 1, life span: 0(ms)
+       flows: cur: 1, avg: 0, max: 1, life span: 1250(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br1 65534/101: (dummy)
        p3 3/3: (dummy)
@@ -2175,6 +2177,7 @@ AT_SETUP([ofproto-dpif - ovs-appctl dpif/show rates])
 OVS_VSWITCHD_START([set Bridge br0 fail-mode=secure])
 ADD_OF_PORTS([br0], 1, 2)
 
+AT_CHECK([ovs-appctl time/stop]) dnl Make life span averages consistent.
 AT_CHECK([ovs-ofctl add-flow br0 actions=LOCAL,output:1,output:2])
 
 for i in $(seq 1 61); do
@@ -2189,7 +2192,7 @@ AT_CHECK([ovs-appctl time/warp 10000], [0], [warped
 AT_CHECK([ovs-appctl dpif/show | sed 's/ 10[[0-9]]\{3\}(ms)$/ 10000(ms)/'], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:61
-       flows: cur: 0, avg: 1.000, max: 1, life span: 10000(ms)
+       flows: cur: 0, avg: 0, max: 1, life span: 1666(ms)
                hourly avg: add rate: 0.641/min, del rate: 0.641/min
                overall avg: add rate: 1.000/min, del rate: 1.000/min
        br0 65534/100: (dummy)

diff --git a/tests/tunnel.at b/tests/tunnel.at
index c69be44..16f5aef 100644 (file)
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -17,7 +17,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: remote_ip=1.1.1.1)
@@ -44,7 +44,7 @@ AT_CHECK([ovs-vsctl set Interface p2 type=gre options:local_ip=2.2.2.3 \
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: remote_ip=1.1.1.1)
@@ -83,7 +83,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: remote_ip=1.1.1.1)
@@ -131,7 +131,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: key=5, local_ip=2.2.2.2, remote_ip=1.1.1.1)
@@ -167,7 +167,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: remote_ip=1.1.1.1, tos=inherit, ttl=inherit)
@@ -213,7 +213,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: key=flow, remote_ip=1.1.1.1)
@@ -249,7 +249,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: key=1, remote_ip=1.1.1.1)
@@ -305,7 +305,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (gre: key=flow, remote_ip=1.1.1.1)
@@ -345,7 +345,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (vxlan: remote_ip=1.1.1.1)
@@ -361,7 +361,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=lisp \
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (lisp: remote_ip=1.1.1.1)
@@ -377,7 +377,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (vxlan: dst_port=4341, remote_ip=1.1.1.1)
@@ -390,7 +390,7 @@ AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=5000])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/2: (vxlan: dst_port=5000, remote_ip=1.1.1.1)
@@ -403,7 +403,7 @@ AT_CHECK([ovs-vsctl -- set Interface p1 options:dst_port=4789])
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
 br0 (dummy@ovs-dummy):
        lookups: hit:0 missed:0
-       flows: cur: 0, avg: 0.000, max: 0, life span: 0(ms)
+       flows: cur: 0, avg: 0, max: 0, life span: 0(ms)
                overall avg: add rate: 0.000/min, del rate: 0.000/min
        br0 65534/100: (dummy)
        p1 1/1: (vxlan: remote_ip=1.1.1.1)