Thanks for the detail, I'll look into it.
On 12 December 2013 21:50, Alex Wang <[email protected]> wrote: > Yes, I could reproduce it on my VM setup, > > It is the timing issue again, if the p1 starts sending ccm first, it will > have flap count = 0. > > If p1 receives ccm from p0 first, it will go to [rdi] and then []. so it > will have flap count = 2. > > I think the safest way is to drop the "CFM_VSCTL_LIST_IFACE([p1], > [cfm_flap_count], [cfm_flap_count : 0])" and don't check > p1 flap count. > > > > > On Thu, Dec 12, 2013 at 8:20 PM, Ethan Jackson <[email protected]> wrote: >> >> This patch occasionally fails the cfm flap count test for me. Can >> either of you reproduce it? >> >> Ethan >> >> On Thu, Dec 12, 2013 at 5:52 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]> >> > --- >> > v3: 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
