From 147156847344c32840e489aad67873d6b927624c Mon Sep 17 00:00:00 2001 From: Joe Stringer Date: Tue, 11 Feb 2014 13:55:36 -0800 Subject: [PATCH] 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 Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-upcall.c | 25 ++++++++++++++++++------- tests/ofproto-dpif.at | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 638f1d24e..641e3eac4 100644 --- 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); +} 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 ff9ead147..1a58da671 100644 --- 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 -- 2.20.1