[ovs-dev] [PATCHv2] monitor: Replace monitor_seq with periodic wakeup
Joe Stringer
joestringer at nicira.com
Fri Dec 13 00:45:51 UTC 2013
Sure.
On 12 December 2013 16:44, Ethan Jackson <ethan at nicira.com> wrote:
> Joe, could you rebase it and resend it please?
>
> Ethan
>
> On Thu, Dec 12, 2013 at 5:17 PM, Alex Wang <alexw at nicira.com> 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 <alexw at nicira.com> wrote:
>>>
>>> This patch looks good to me, thx for doing this.
>>>
>>>
>>> On Thu, Dec 12, 2013 at 1:03 PM, 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>
>>>> ---
>>>> 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
>>>> dev at openvswitch.org
>>>> http://openvswitch.org/mailman/listinfo/dev
>>>
>>>
>>
More information about the dev
mailing list