[ovs-dev] [PATCHv4] monitor: Replace monitor_seq with periodic wakeup

Alex Wang alexw at nicira.com
Fri Dec 13 19:29:20 UTC 2013


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 <joestringer at 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 <joestringer at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20131213/0822aee5/attachment-0003.html>


More information about the dev mailing list