[ovs-dev] [PATCH 4/5] ofproto.at: Fix races in rule eviciton tests

Kmindg G kmindg at gmail.com
Tue Apr 1 06:21:47 UTC 2014


On Tue, Apr 1, 2014 at 1:12 PM, YAMAMOTO Takashi <yamamoto at valinux.co.jp> wrote:
>>>  # Sleep and modify the one that expires soonest
>>> -sleep 2
>>> +ovs-appctl time/warp 20000
>>
>> I think we should warp as small as possible to verify precision.
>> What do you think about warp 12000?
>
> i agree smaller is better.
> how about the following?
>
> YAMAMOTO Takashi
>
>
> commit b4a624761fe90aa0d57264013287f4236077ac9c
> Author: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
> Date:   Mon Mar 31 14:04:35 2014 +0900
>
>     ofproto.at: Fix races in rule eviciton tests
>
>     Bump timeout differences, because timeouts different by 1s might end up
>     to have the same position in the heap as rule_eviction_priority() uses
>     1024ms as a unit.
>
>     Also, use time/stop to avoid relying on how long an add-flow would take.
>
>     These tests were introduced by commit 6d56c1f1.
>     ("ofproto: Update rule's priority in eviction group.")
>
>     Signed-off-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
>     Cc: Kmindg G <kmindg at gmail.com>
>     Acked-by: Ben Pfaff <blp at nicira.com>
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index e98b526..a5116db 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1375,27 +1375,28 @@ AT_CHECK(
>     | ${PERL} $srcdir/uuidfilt.pl],
>    [0], [<0>
>  ])
> +ovs-appctl time/stop
>  # Add 4 flows.
>  for in_port in 4 3 2 1; do
> -    ovs-ofctl add-flow br0 hard_timeout=1${in_port},in_port=$in_port,actions=drop
> +    ovs-ofctl add-flow br0 hard_timeout=$((10 + in_port * 2)),in_port=$in_port,actions=drop
>  done
>  AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=12, in_port=2 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=12, in_port=1 actions=drop
> + hard_timeout=14, in_port=2 actions=drop
> + hard_timeout=16, in_port=3 actions=drop
> + hard_timeout=18, in_port=4 actions=drop
>  NXST_FLOW reply:
>  ])
>  # Sleep and modify the one that expires soonest
> -sleep 2
> +ovs-appctl time/warp 3000
>  AT_CHECK([ovs-ofctl mod-flows br0 in_port=1,actions=drop])

at this point, flow table would be:
hard_timeout=12, in_port=1 actions=drop
hard_timeout=11, in_port=2 actions=drop
hard_timeout=13, in_port=3 actions=drop
hard_timeout=15, in_port=4 actions=drop
And according to your comment, 1s interval may not be suitable.

I would like to change (10 + in_port * 2) to (10 + in_port * 3), and warp 4000.
What do you think?
Thanks.

> -sleep 2
> +ovs-appctl time/warp 2000
>  # Adding another flow will cause the one that expires soonest to be evicted.
>  AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
>  AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - hard_timeout=11, in_port=1 actions=drop
> - hard_timeout=13, in_port=3 actions=drop
> - hard_timeout=14, in_port=4 actions=drop
> + hard_timeout=12, in_port=1 actions=drop
> + hard_timeout=16, in_port=3 actions=drop
> + hard_timeout=18, in_port=4 actions=drop
>   in_port=5 actions=drop
>  NXST_FLOW reply:
>  ])
> @@ -1414,26 +1415,27 @@ AT_CHECK(
>  ])
>  # Add 4 flows.
>  for in_port in 4 3 2 1; do
> -    ovs-ofctl add-flow br0 idle_timeout=1${in_port},in_port=$in_port,actions=drop
> +    ovs-ofctl add-flow br0 idle_timeout=$((10 + in_port * 2)),in_port=$in_port,actions=drop
>  done
> +ovs-appctl time/stop
>  AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=11, in_port=1 actions=drop
> - idle_timeout=12, in_port=2 actions=drop
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=12, in_port=1 actions=drop
> + idle_timeout=14, in_port=2 actions=drop
> + idle_timeout=16, in_port=3 actions=drop
> + idle_timeout=18, in_port=4 actions=drop
>  NXST_FLOW reply:
>  ])
>  # Sleep and receive on the flow that expires soonest
> -sleep 2
> +ovs-appctl time/warp 3000
>  AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1)'])
> -sleep 2
> +ovs-appctl time/warp 2000
>  # Adding another flow will cause the one that expires soonest to be evicted.
>  AT_CHECK([ovs-ofctl add-flow br0 in_port=5,actions=drop])
>  AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl
> - idle_timeout=13, in_port=3 actions=drop
> - idle_timeout=14, in_port=4 actions=drop
> + idle_timeout=16, in_port=3 actions=drop
> + idle_timeout=18, in_port=4 actions=drop
>   in_port=5 actions=drop
> - n_packets=1, n_bytes=60, idle_timeout=11, in_port=1 actions=drop
> + n_packets=1, n_bytes=60, idle_timeout=12, in_port=1 actions=drop
>  NXST_FLOW reply:
>  ])
>  OVS_VSWITCHD_STOP



More information about the dev mailing list