I realised that this no longer applies on master. I'll send a rebased version.
On 13 December 2013 15:00, Alex Wang <al...@nicira.com> wrote: > Hey Ben, > > Could you help review and push it? > > Thanks, > Alex Wang, > > > On Fri, Dec 13, 2013 at 11:29 AM, Alex Wang <al...@nicira.com> wrote: >> >> Looks good to me, ran the cfm flap_count test 300 (clang and sparse) times >> on my vm >> and didn't see the failure any more, >> >> >> On Fri, Dec 13, 2013 at 10:52 AM, Joe Stringer <joestrin...@nicira.com> >> wrote: >>> >>> Currently, every time a monitoring port is added or reconfigured, the >>> main thread notifies the monitoring thread to wake up immediately using >>> monitor_seq. When adding a large number of ports at once, this causes >>> contention as the threads fight over access to the monitor heap---one >>> thread adding new ports, the other popping and processing the new ports. >>> >>> This patch removes this mechanism in favour of a simple periodic wakeup >>> in the monitor thread. When configuring batches of 500 tunnels at once, >>> up to a total of 5000 tunnels, this reduces configuration time by up to >>> 35%. New port monitor port configuration may not take effect for as long >>> as ~100ms, although it may be less if the timer has not recently timed >>> out, or if a monitoring port is due to send a packet. >>> >>> Signed-off-by: Joe Stringer <joestrin...@nicira.com> >>> --- >>> v4: Make cfm flap_count test more reliable by dropping irrelevant check >>> --- >>> ofproto/ofproto-dpif-monitor.c | 18 ++++++++++-------- >>> tests/bfd.at | 16 +++++++--------- >>> tests/cfm.at | 5 ++--- >>> 3 files changed, 19 insertions(+), 20 deletions(-) >>> >>> diff --git a/ofproto/ofproto-dpif-monitor.c >>> b/ofproto/ofproto-dpif-monitor.c >>> index d06b2e1..2a833bb 100644 >>> --- a/ofproto/ofproto-dpif-monitor.c >>> +++ b/ofproto/ofproto-dpif-monitor.c >>> @@ -63,7 +63,6 @@ static pthread_t monitor_tid; >>> /* True if the monitor thread is running. */ >>> static bool monitor_running; >>> >>> -static struct seq *monitor_seq; >>> static struct latch monitor_exit_latch; >>> static struct ovs_rwlock monitor_rwlock = OVS_RWLOCK_INITIALIZER; >>> >>> @@ -149,10 +148,9 @@ mport_update(struct mport *mport, struct bfd *bfd, >>> struct cfm *cfm, >>> memcpy(mport->hw_addr, hw_addr, ETH_ADDR_LEN); >>> } >>> /* If bfd/cfm is added or reconfigured, move the mport on top of the >>> heap >>> - * and wakes up the monitor thread. */ >>> + * so that the monitor thread can run the mport next time it wakes >>> up. */ >>> if (mport->bfd || mport->cfm) { >>> heap_change(&monitor_heap, &mport->heap_node, LLONG_MAX); >>> - seq_change(monitor_seq); >>> } >>> } >>> >>> @@ -165,7 +163,6 @@ monitor_init(void) >>> >>> if (ovsthread_once_start(&once)) { >>> hmap_init(&monitor_hmap); >>> - monitor_seq = seq_create(); >>> ovsthread_once_done(&once); >>> } >>> } >>> @@ -177,17 +174,18 @@ monitor_main(void * args OVS_UNUSED) >>> set_subprogram_name("monitor"); >>> VLOG_INFO("monitor thread created"); >>> while (!latch_is_set(&monitor_exit_latch)) { >>> - uint64_t seq = seq_read(monitor_seq); >>> - >>> monitor_run(); >>> latch_wait(&monitor_exit_latch); >>> - seq_wait(monitor_seq, seq); >>> poll_block(); >>> } >>> VLOG_INFO("monitor thread terminated"); >>> return NULL; >>> } >>> >>> +/* The monitor thread should wake up this often to ensure that newly >>> added or >>> + * reconfigured monitoring ports are run in a timely manner. */ >>> +#define MONITOR_INTERVAL_MSEC 100 >>> + >>> /* Checks the sending of control packets on mports that have timed out. >>> * Sends the control packets if needed. Executes bfd and cfm periodic >>> * functions (run, wait) on those mports. */ >>> @@ -234,7 +232,11 @@ monitor_run(void) >>> >>> /* Waits on the earliest next wakeup time. */ >>> if (!heap_is_empty(&monitor_heap)) { >>> - >>> poll_timer_wait_until(PRIO_TO_MSEC(heap_max(&monitor_heap)->priority)); >>> + long long int next_timeout, next_mport_wakeup; >>> + >>> + next_timeout = time_msec() + MONITOR_INTERVAL_MSEC; >>> + next_mport_wakeup = >>> PRIO_TO_MSEC(heap_max(&monitor_heap)->priority); >>> + poll_timer_wait_until(MIN(next_timeout, next_mport_wakeup)); >>> } >>> ovs_rwlock_unlock(&monitor_rwlock); >>> ofpbuf_uninit(&packet); >>> diff --git a/tests/bfd.at b/tests/bfd.at >>> index ccb62b5..6583dd5 100644 >>> --- a/tests/bfd.at >>> +++ b/tests/bfd.at >>> @@ -334,6 +334,7 @@ BFD_CHECK([p1], [true], [false], [none], [up], [No >>> Diagnostic], [none], [up], [N >>> # for decay_min_rx < 2000ms, the decay detection time is set to 2000ms. >>> # this should reset the min_rx. >>> AT_CHECK([ovs-vsctl set Interface p0 bfd:decay_min_rx=1000]) >>> +ovs-appctl time/warp 100 >>> BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> BFD_CHECK_RX([p0], [500ms], [300ms], [500ms]) >>> >>> @@ -360,8 +361,7 @@ for i in `seq 0 9`; do ovs-appctl time/warp 500; done >>> # Test-4 BFD decay: set min_rx to 800ms. >>> # this should firstly reset the min_rx and then re-decay to 1000ms. >>> AT_CHECK([ovs-vsctl set Interface p0 bfd:min_rx=800]) >>> -BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> -BFD_CHECK_RX([p0], [800ms], [800ms], [500ms]) >>> +ovs-appctl time/warp 100 >>> >>> # for the following 1600ms, there should be no decay, >>> # since the decay detection time is set to 2000ms. >>> @@ -385,8 +385,7 @@ for i in `seq 0 9`; do ovs-appctl time/warp 500; done >>> >>> # Test-5 BFD decay: set min_rx to 300ms and decay_min_rx to 5000ms >>> together. >>> AT_CHECK([ovs-vsctl set Interface p0 bfd:min_rx=300 >>> bfd:decay_min_rx=5000]) >>> -BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> -BFD_CHECK_RX([p0], [500ms], [300ms], [500ms]) >>> +ovs-appctl time/warp 100 >>> >>> # for decay_min_rx > 2000ms, the decay detection time is set to >>> # decay_min_rx (5000ms). >>> @@ -412,9 +411,8 @@ for i in `seq 0 9`; do ovs-appctl time/warp 500; done >>> >>> # Test-6 BFD decay: set decay_min_rx to 0 to disable bfd decay. >>> AT_CHECK([ovs-vsctl set Interface p0 bfd:decay_min_rx=0]) >>> -# min_rx is reset. >>> -BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> -BFD_CHECK_RX([p0], [500ms], [300ms], [500ms]) >>> +ovs-appctl time/warp 100 >>> + >>> for i in `seq 0 20` >>> do >>> ovs-appctl time/warp 500 >>> @@ -444,6 +442,8 @@ do >>> done >>> # reset the p1's min_tx to 500ms. >>> AT_CHECK([ovs-vsctl set Interface p1 bfd:min_tx=500]) >>> +ovs-appctl time/warp 100 >>> + >>> # since p0 has been in decay, now the RX will show 3000ms. >>> BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> BFD_CHECK_RX([p0], [3000ms], [3000ms], [500ms]) >>> @@ -629,8 +629,6 @@ done >>> >>> # reconfigure the decay_min_rx to 1000ms. >>> AT_CHECK([ovs-vsctl set interface p0 bfd:decay_min_rx=1000]) >>> -BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) >>> -BFD_CHECK_RX([p0], [500ms], [300ms], [500ms]) >>> >>> # wait for 5000ms to decay. >>> for i in `seq 0 9`; do ovs-appctl time/warp 500; done >>> diff --git a/tests/cfm.at b/tests/cfm.at >>> index 9e351d0..9f9591d 100644 >>> --- a/tests/cfm.at >>> +++ b/tests/cfm.at >>> @@ -83,17 +83,16 @@ for i in `seq 0 100`; do ovs-appctl time/warp 100; >>> done >>> CFM_CHECK_EXTENDED([p0], [1], [100], [up], [up], [100ms], [2], [up]) >>> CFM_CHECK_EXTENDED([p1], [2], [100], [up], [up], [100ms], [1], [up]) >>> >>> -# turn cfm on p1 off, should increment the cfm_flap_count on p1. >>> +# turn cfm on p1 off, should increment the cfm_flap_count on p0. >>> AT_CHECK([ovs-vsctl remove interface p1 cfm_mpid 2]) >>> for i in `seq 0 10`; do ovs-appctl time/warp 100; done >>> CFM_VSCTL_LIST_IFACE([p0], [cfm_flap_count], [cfm_flap_count : 1]) >>> CFM_VSCTL_LIST_IFACE([p1], [cfm_flap_count], [cfm_flap_count : >>> [[]]]) >>> >>> -# turn cfm on p1 on again, should increment the cfm_flap_count on p1. >>> +# turn cfm on p1 on again, should increment the cfm_flap_count on p0. >>> AT_CHECK([ovs-vsctl set interface p1 cfm_mpid=2]) >>> for i in `seq 0 10`; do ovs-appctl time/warp 100; done >>> CFM_VSCTL_LIST_IFACE([p0], [cfm_flap_count], [cfm_flap_count : 2]) >>> -CFM_VSCTL_LIST_IFACE([p1], [cfm_flap_count], [cfm_flap_count : 0]) >>> >>> OVS_VSWITCHD_STOP >>> AT_CLEANUP >>> \ No newline at end of file >>> -- >>> 1.7.9.5 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> http://openvswitch.org/mailman/listinfo/dev >> >> > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev