upcall: Remove datapath flows when setting n-threads.

Previously, we would delete all ukeys when changing the number of
threads, but leave all flows in the datapath. This would cause
double-counting of stats for any flows that remain in the datapath. This
patch fixes the issue by ensuring that all flows are deleted from the
datapath before changing the number of threads.

Signed-off-by: Joe Stringer <joestringer@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 638f1d2..641e3ea 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -230,6 +230,7 @@ static void *udpif_revalidator(void *);
 static uint64_t udpif_get_n_flows(struct udpif *);
 static void revalidate_udumps(struct revalidator *, struct list *udumps);
 static void revalidator_sweep(struct revalidator *);
+static void revalidator_purge(struct revalidator *);
 static void upcall_unixctl_show(struct unixctl_conn *conn, int argc,
                                 const char *argv[], void *aux);
 static void upcall_unixctl_disable_megaflows(struct unixctl_conn *, int argc,
@@ -325,7 +326,6 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers,
         for (i = 0; i < udpif->n_revalidators; i++) {
             struct revalidator *revalidator = &udpif->revalidators[i];
             struct udpif_flow_dump *udump, *next_udump;
-            struct udpif_key *ukey, *next_ukey;
 
             LIST_FOR_EACH_SAFE (udump, next_udump, list_node,
                                 &revalidator->udumps) {
@@ -333,10 +333,9 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers,
                 free(udump);
             }
 
-            HMAP_FOR_EACH_SAFE (ukey, next_ukey, hmap_node,
-                                &revalidator->ukeys) {
-                ukey_delete(revalidator, ukey);
-            }
+            /* Delete ukeys, and delete all flows from the datapath to prevent
+             * double-counting stats. */
+            revalidator_purge(revalidator);
             hmap_destroy(&revalidator->ukeys);
             ovs_mutex_destroy(&revalidator->mutex);
 
@@ -1546,7 +1545,7 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
 }
 
 static void
-revalidator_sweep(struct revalidator *revalidator)
+revalidator_sweep__(struct revalidator *revalidator, bool purge)
 {
     struct dump_op ops[REVALIDATE_MAX_BATCH];
     struct udpif_key *ukey, *next;
@@ -1555,7 +1554,7 @@ revalidator_sweep(struct revalidator *revalidator)
     n_ops = 0;
 
     HMAP_FOR_EACH_SAFE (ukey, next, hmap_node, &revalidator->ukeys) {
-        if (ukey->mark) {
+        if (!purge && ukey->mark) {
             ukey->mark = false;
         } else {
             struct dump_op *op = &ops[n_ops++];
@@ -1575,6 +1574,18 @@ revalidator_sweep(struct revalidator *revalidator)
         push_dump_ops(revalidator, ops, n_ops);
     }
 }
+
+static void
+revalidator_sweep(struct revalidator *revalidator)
+{
+    revalidator_sweep__(revalidator, false);
+}
+
+static void
+revalidator_purge(struct revalidator *revalidator)
+{
+    revalidator_sweep__(revalidator, true);
+}
 \f
 static void
 upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index ff9ead1..1a58da6 100644 (file)
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2212,6 +2212,29 @@ AT_CHECK([STRIP_XIDS stdout | sed -n 's/duration=[[0-9]]*\.[[0-9]]*s/duration=0.
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([ofproto-dpif - flow stats, set-n-threads])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl add-flow br0 "ip,actions=NORMAL"])
+AT_CHECK([ovs-ofctl add-flow br0 "icmp,actions=NORMAL"])
+
+ovs-appctl time/stop
+
+for i in `seq 1 10`; do
+    ovs-appctl netdev-dummy/receive br0 'in_port(0),eth(src=50:54:00:00:00:07,dst=50:54:00:00:00:05),eth_type(0x0800),ipv4(src=192.168.0.2,dst=192.168.0.1,proto=6,tos=0,ttl=64,frag=no)'
+done
+
+ovs-appctl time/warp 100
+AT_CHECK([ovs-vsctl set Open_vSwitch . other-config:n-revalidator-threads=2])
+ovs-appctl time/warp 1000
+
+AT_CHECK([ovs-ofctl dump-flows br0], [0], [stdout])
+AT_CHECK([STRIP_XIDS stdout | sed -n 's/duration=[[0-9]]*\.[[0-9]]*s/duration=0.0s/p' | sort], [0], [dnl
+ cookie=0x0, duration=0.0s, table=0, n_packets=0, n_bytes=0, idle_age=1, icmp actions=NORMAL
+ cookie=0x0, duration=0.0s, table=0, n_packets=10, n_bytes=600, idle_age=1, ip actions=NORMAL
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([idle_age and hard_age increase over time])
 OVS_VSWITCHD_START