Sure.
On 12 December 2013 16:44, Ethan Jackson <[email protected]> wrote: > Joe, could you rebase it and resend it please? > > Ethan > > On Thu, Dec 12, 2013 at 5:17 PM, Alex Wang <[email protected]> wrote: >> Hey Ben, Ethan, >> >> Could any of you apply this patch? >> >> Thanks, >> Alex Wang, >> >> >> On Thu, Dec 12, 2013 at 4:15 PM, Alex Wang <[email protected]> wrote: >>> >>> This patch looks good to me, thx for doing this. >>> >>> >>> On Thu, Dec 12, 2013 at 1:03 PM, Joe Stringer <[email protected]> >>> 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 <[email protected]> >>>> --- >>>> v2: Rebase >>>> --- >>>> ofproto/ofproto-dpif-monitor.c | 18 ++++++++++-------- >>>> tests/bfd.at | 16 +++++++--------- >>>> tests/cfm.at | 8 +++++--- >>>> 3 files changed, 22 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..620e3e0 100644 >>>> --- a/tests/cfm.at >>>> +++ b/tests/cfm.at >>>> @@ -83,17 +83,19 @@ 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. >>>> +# After p1 is configured, but before it is first run, it will receive >>>> +# a ccm with the rdi bit set, and detect a flap as well. >>>> 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]) >>>> +CFM_VSCTL_LIST_IFACE([p1], [cfm_flap_count], [cfm_flap_count : 2]) >>>> >>>> OVS_VSWITCHD_STOP >>>> AT_CLEANUP >>>> \ No newline at end of file >>>> -- >>>> 1.7.9.5 >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> [email protected] >>>> http://openvswitch.org/mailman/listinfo/dev >>> >>> >> _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
