From 0a8763fcb31bfca0d8d854c235c531005088fcb9 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Mon, 27 Jan 2014 16:40:27 -0800 Subject: [PATCH] ofproto-dpif-upcall: Hardcode max_idle to 1500ms. Before this patch, OVS tried to guess an optimal max idle time for datapath flows based on the number of datapath flows relative to the limit. This caused instability because the limit was based on the dump duration which was affected by the max idle time. This patch chooses instead to hardcode the max idle time to 1.5s except in extreme case where the datapath flow limit is exceeded. 1.5s was chosen to ensure pings occurring at once per second stay cached in the datapath. Signed-off-by: Ethan Jackson Acked-by: Joe Stringer --- ofproto/ofproto-dpif-upcall.c | 25 ++++--------------------- tests/ofproto-dpif.at | 10 ++-------- 2 files changed, 6 insertions(+), 29 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index ff979ba16..bfe85fe91 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -41,6 +41,7 @@ #define MAX_QUEUE_LENGTH 512 #define FLOW_MISS_MAX_BATCH 50 #define REVALIDATE_MAX_BATCH 50 +#define MAX_IDLE 1500 VLOG_DEFINE_THIS_MODULE(ofproto_dpif_upcall); @@ -125,7 +126,6 @@ struct udpif { unsigned int avg_n_flows; /* Following fields are accessed and modified by different threads. */ - atomic_llong max_idle; /* Maximum datapath flow idle time. */ atomic_uint flow_limit; /* Datapath flow hard limit. */ /* n_flows_mutex prevents multiple threads updating these concurrently. */ @@ -259,7 +259,6 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif) udpif->dpif = dpif; udpif->backer = backer; - atomic_init(&udpif->max_idle, 5000); atomic_init(&udpif->flow_limit, MIN(ofproto_flow_limit, 10000)); udpif->secret = random_uint32(); udpif->reval_seq = seq_create(); @@ -283,7 +282,6 @@ udpif_destroy(struct udpif *udpif) latch_destroy(&udpif->exit_latch); seq_destroy(udpif->reval_seq); seq_destroy(udpif->dump_seq); - atomic_destroy(&udpif->max_idle); atomic_destroy(&udpif->flow_limit); atomic_destroy(&udpif->n_flows); atomic_destroy(&udpif->n_flows_timestamp); @@ -533,7 +531,6 @@ udpif_flow_dumper(void *arg) struct dpif_flow_dump dump; size_t key_len, mask_len; unsigned int flow_limit; - long long int max_idle; bool need_revalidate; uint64_t reval_seq; size_t n_flows, i; @@ -546,18 +543,6 @@ udpif_flow_dumper(void *arg) udpif->max_n_flows = MAX(n_flows, udpif->max_n_flows); udpif->avg_n_flows = (udpif->avg_n_flows + n_flows) / 2; - atomic_read(&udpif->flow_limit, &flow_limit); - if (n_flows < flow_limit / 8) { - max_idle = 5000; - } else if (n_flows < flow_limit / 4) { - max_idle = 2000; - } else if (n_flows < flow_limit / 2) { - max_idle = 1000; - } else { - max_idle = 500; - } - atomic_store(&udpif->max_idle, max_idle); - start_time = time_msec(); dpif_flow_dump_start(&dump, udpif->dpif); while (dpif_flow_dump_next(&dump, &key, &key_len, &mask, &mask_len, @@ -617,6 +602,7 @@ udpif_flow_dumper(void *arg) duration = MAX(time_msec() - start_time, 1); udpif->dump_duration = duration; + atomic_read(&udpif->flow_limit, &flow_limit); if (duration > 2000) { flow_limit /= duration / 1000; } else if (duration > 1300) { @@ -633,7 +619,7 @@ udpif_flow_dumper(void *arg) duration); } - poll_timer_wait_until(start_time + MIN(max_idle, 500)); + poll_timer_wait_until(start_time + MIN(MAX_IDLE, 500)); seq_wait(udpif->reval_seq, udpif->last_reval_seq); latch_wait(&udpif->exit_latch); poll_block(); @@ -1386,12 +1372,12 @@ revalidate_udumps(struct revalidator *revalidator, struct list *udumps) long long int max_idle; bool must_del; - atomic_read(&udpif->max_idle, &max_idle); atomic_read(&udpif->flow_limit, &flow_limit); n_flows = udpif_get_n_flows(udpif); must_del = false; + max_idle = MAX_IDLE; if (n_flows > flow_limit) { must_del = n_flows > 2 * flow_limit; max_idle = 100; @@ -1526,17 +1512,14 @@ upcall_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED, LIST_FOR_EACH (udpif, list_node, &all_udpifs) { unsigned int flow_limit; - long long int max_idle; size_t i; atomic_read(&udpif->flow_limit, &flow_limit); - atomic_read(&udpif->max_idle, &max_idle); ds_put_format(&ds, "%s:\n", dpif_name(udpif->dpif)); ds_put_format(&ds, "\tflows : (current %"PRIu64")" " (avg %u) (max %u) (limit %u)\n", udpif_get_n_flows(udpif), udpif->avg_n_flows, udpif->max_n_flows, flow_limit); - ds_put_format(&ds, "\tmax idle : %lldms\n", max_idle); ds_put_format(&ds, "\tdump duration : %lldms\n", udpif->dump_duration); ds_put_char(&ds, '\n'); diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 23a1f1480..134879040 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -2442,20 +2442,14 @@ AT_CHECK([ovs-ofctl add-flow br1 actions=LOCAL,output:1,output:3]) for i in $(seq 1 10); do ovs-appctl netdev-dummy/receive br0 'in_port(100),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' - if [[ $i -eq 1 ]]; then - sleep 1 - fi done for i in $(seq 1 5); do ovs-appctl netdev-dummy/receive br1 'in_port(101),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=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)' - if [[ $i -eq 1 ]]; then - sleep 1 - fi done -AT_CHECK([ovs-appctl time/warp 1000 && ovs-appctl time/warp 1000], [0], [warped -warped +AT_CHECK([ovs-appctl time/warp 500], [0], +[warped ]) AT_CHECK([ovs-appctl dpif/show], [0], [dnl -- 2.20.1