[ovs-dev] tests/stp: Use long warps instead of multiple calls.

Message ID 1505292378-7745-1-git-send-email-i.maximets@samsung.com
State Accepted
Headers show
Series
  • [ovs-dev] tests/stp: Use long warps instead of multiple calls.
Related show

Commit Message

Ilya Maximets Sept. 13, 2017, 8:46 a.m.
This change fixes constant test failure on RHEL 7 system with
CFLAGS='-march=native':
>---------------------------------------------------------------<
 2222: STP - flush the fdb and mdb when topology changed  FAILED
 ...
 ./stp.at:609: ovs-appctl fdb/show br0
 --- -
 +++ ./tests/testsuite.dir/at-groups/2222/stdout
 @@ -1,2 +1,3 @@
   port  VLAN  MAC                Age
   +LOCAL     1  00:0c:29:a0:27:d1   33
>---------------------------------------------------------------<

Long warps takes threads a chance to perform some work on each
step unlike multiple appctl calls. Also, code looks cleaner and
works faster.

CC: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Fixes: 427e9751f300 ("tests: Add and improve stp tests.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 tests/stp.at | 54 +++++++-----------------------------------------------
 1 file changed, 7 insertions(+), 47 deletions(-)

Comments

Tonghao Zhang Sept. 14, 2017, 2:21 a.m. | #1
Thanks for your work.

Tested-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

On Wed, Sep 13, 2017 at 4:46 PM, Ilya Maximets <i.maximets@samsung.com> wrote:
> This change fixes constant test failure on RHEL 7 system with
> CFLAGS='-march=native':
>>---------------------------------------------------------------<
>  2222: STP - flush the fdb and mdb when topology changed  FAILED
>  ...
>  ./stp.at:609: ovs-appctl fdb/show br0
>  --- -
>  +++ ./tests/testsuite.dir/at-groups/2222/stdout
>  @@ -1,2 +1,3 @@
>    port  VLAN  MAC                Age
>    +LOCAL     1  00:0c:29:a0:27:d1   33
>>---------------------------------------------------------------<
>
> Long warps takes threads a chance to perform some work on each
> step unlike multiple appctl calls. Also, code looks cleaner and
> works faster.
>
> CC: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Fixes: 427e9751f300 ("tests: Add and improve stp tests.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  tests/stp.at | 54 +++++++-----------------------------------------------
>  1 file changed, 7 insertions(+), 47 deletions(-)
>
> diff --git a/tests/stp.at b/tests/stp.at
> index e27600e..9550f72 100644
> --- a/tests/stp.at
> +++ b/tests/stp.at
> @@ -429,9 +429,7 @@ AT_CHECK([ovs-ofctl add-flow br1 "in_port=8 icmp actions=2"])
>  AT_CHECK([ovs-ofctl add-flow br1 "in_port=2 icmp actions=8"])
>
>  # give time for STP to move initially
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 6000 3000
>
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -446,17 +444,7 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(8),eth(src=50:54:00:00:00:
>  ])
>
>  # give time for STP to synchronize
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 30000 3000
>
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -524,8 +512,7 @@ ovs-appctl netdev-dummy/set-admin-state up
>  ovs-appctl time/stop
>
>  # give time for STP to move initially
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 6000 3000
>
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -537,17 +524,7 @@ port <>: STP state changed from disabled to listening
>  ])
>
>  # give time for STP to synchronize
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 30000 3000
>
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_FORWARDING], [0], [dnl
>  port <>: STP state changed from learning to forwarding
> @@ -566,9 +543,7 @@ port <>: STP state changed from learning to forwarding
>  # of stp ports will stop after 1s. So the root bridge can send quickly
>  # topology change ack (other bridges may send TCN BPDU to root bridge) for
>  # avoiding root brdige to flush fdb and mdb frequently.
> -for i in $(seq 0 35); do
> -    ovs-appctl time/warp 1000
> -done
> +ovs-appctl time/warp 36000 1000
>
>  # root bridge sends query packet
>  # we don't want to lose that message, so send it twice
> @@ -591,20 +566,7 @@ OVS_WAIT_UNTIL([ovs-appctl mdb/show br2 | grep 'querier'])
>  AT_CHECK([ovs-vsctl del-port br0 p2])
>
>  # give time for STP to synchronize
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 36000 3000
>
>  # check fdb and mdb
>  AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl
> @@ -652,9 +614,7 @@ ovs-appctl netdev-dummy/set-admin-state up
>  ovs-appctl time/stop
>
>  # give time for STP to move initially
> -for i in $(seq 0 30); do
> -    ovs-appctl time/warp 1000
> -done
> +ovs-appctl time/warp 31000 1000
>
>  AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
>         p1         designated forwarding 19    128.1
> --
> 2.7.4
>
Ben Pfaff Oct. 25, 2017, 4:02 a.m. | #2
Thanks, I applied this to master and branch-2.8.

On Wed, Sep 13, 2017 at 11:46:18AM +0300, Ilya Maximets wrote:
> This change fixes constant test failure on RHEL 7 system with
> CFLAGS='-march=native':
> >---------------------------------------------------------------<
>  2222: STP - flush the fdb and mdb when topology changed  FAILED
>  ...
>  ./stp.at:609: ovs-appctl fdb/show br0
>  --- -
>  +++ ./tests/testsuite.dir/at-groups/2222/stdout
>  @@ -1,2 +1,3 @@
>    port  VLAN  MAC                Age
>    +LOCAL     1  00:0c:29:a0:27:d1   33
> >---------------------------------------------------------------<
> 
> Long warps takes threads a chance to perform some work on each
> step unlike multiple appctl calls. Also, code looks cleaner and
> works faster.
> 
> CC: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Fixes: 427e9751f300 ("tests: Add and improve stp tests.")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  tests/stp.at | 54 +++++++-----------------------------------------------
>  1 file changed, 7 insertions(+), 47 deletions(-)
> 
> diff --git a/tests/stp.at b/tests/stp.at
> index e27600e..9550f72 100644
> --- a/tests/stp.at
> +++ b/tests/stp.at
> @@ -429,9 +429,7 @@ AT_CHECK([ovs-ofctl add-flow br1 "in_port=8 icmp actions=2"])
>  AT_CHECK([ovs-ofctl add-flow br1 "in_port=2 icmp actions=8"])
>  
>  # give time for STP to move initially
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 6000 3000
>  
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -446,17 +444,7 @@ AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(8),eth(src=50:54:00:00:00:
>  ])
>  
>  # give time for STP to synchronize
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 30000 3000
>  
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -524,8 +512,7 @@ ovs-appctl netdev-dummy/set-admin-state up
>  ovs-appctl time/stop
>  
>  # give time for STP to move initially
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 6000 3000
>  
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
>  port <>: STP state changed from disabled to listening
> @@ -537,17 +524,7 @@ port <>: STP state changed from disabled to listening
>  ])
>  
>  # give time for STP to synchronize
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 30000 3000
>  
>  AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_FORWARDING], [0], [dnl
>  port <>: STP state changed from learning to forwarding
> @@ -566,9 +543,7 @@ port <>: STP state changed from learning to forwarding
>  # of stp ports will stop after 1s. So the root bridge can send quickly
>  # topology change ack (other bridges may send TCN BPDU to root bridge) for
>  # avoiding root brdige to flush fdb and mdb frequently.
> -for i in $(seq 0 35); do
> -    ovs-appctl time/warp 1000
> -done
> +ovs-appctl time/warp 36000 1000
>  
>  # root bridge sends query packet
>  # we don't want to lose that message, so send it twice
> @@ -591,20 +566,7 @@ OVS_WAIT_UNTIL([ovs-appctl mdb/show br2 | grep 'querier'])
>  AT_CHECK([ovs-vsctl del-port br0 p2])
>  
>  # give time for STP to synchronize
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> -
> -ovs-appctl time/warp 3000
> -ovs-appctl time/warp 3000
> +ovs-appctl time/warp 36000 3000
>  
>  # check fdb and mdb
>  AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl
> @@ -652,9 +614,7 @@ ovs-appctl netdev-dummy/set-admin-state up
>  ovs-appctl time/stop
>  
>  # give time for STP to move initially
> -for i in $(seq 0 30); do
> -    ovs-appctl time/warp 1000
> -done
> +ovs-appctl time/warp 31000 1000
>  
>  AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
>  	p1         designated forwarding 19    128.1
> -- 
> 2.7.4
>

Patch

diff --git a/tests/stp.at b/tests/stp.at
index e27600e..9550f72 100644
--- a/tests/stp.at
+++ b/tests/stp.at
@@ -429,9 +429,7 @@  AT_CHECK([ovs-ofctl add-flow br1 "in_port=8 icmp actions=2"])
 AT_CHECK([ovs-ofctl add-flow br1 "in_port=2 icmp actions=8"])
 
 # give time for STP to move initially
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+ovs-appctl time/warp 6000 3000
 
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
@@ -446,17 +444,7 @@  AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(8),eth(src=50:54:00:00:00:
 ])
 
 # give time for STP to synchronize
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+ovs-appctl time/warp 30000 3000
 
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY], [0], [dnl
 port <>: STP state changed from disabled to listening
@@ -524,8 +512,7 @@  ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 # give time for STP to move initially
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+ovs-appctl time/warp 6000 3000
 
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_LISTENING], [0], [dnl
 port <>: STP state changed from disabled to listening
@@ -537,17 +524,7 @@  port <>: STP state changed from disabled to listening
 ])
 
 # give time for STP to synchronize
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+ovs-appctl time/warp 30000 3000
 
 AT_CHECK([cat ovs-vswitchd.log | FILTER_STP_TOPOLOGY_FORWARDING], [0], [dnl
 port <>: STP state changed from learning to forwarding
@@ -566,9 +543,7 @@  port <>: STP state changed from learning to forwarding
 # of stp ports will stop after 1s. So the root bridge can send quickly
 # topology change ack (other bridges may send TCN BPDU to root bridge) for
 # avoiding root brdige to flush fdb and mdb frequently.
-for i in $(seq 0 35); do
-    ovs-appctl time/warp 1000
-done
+ovs-appctl time/warp 36000 1000
 
 # root bridge sends query packet
 # we don't want to lose that message, so send it twice
@@ -591,20 +566,7 @@  OVS_WAIT_UNTIL([ovs-appctl mdb/show br2 | grep 'querier'])
 AT_CHECK([ovs-vsctl del-port br0 p2])
 
 # give time for STP to synchronize
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
-
-ovs-appctl time/warp 3000
-ovs-appctl time/warp 3000
+ovs-appctl time/warp 36000 3000
 
 # check fdb and mdb
 AT_CHECK([ovs-appctl fdb/show br0], [0], [dnl
@@ -652,9 +614,7 @@  ovs-appctl netdev-dummy/set-admin-state up
 ovs-appctl time/stop
 
 # give time for STP to move initially
-for i in $(seq 0 30); do
-    ovs-appctl time/warp 1000
-done
+ovs-appctl time/warp 31000 1000
 
 AT_CHECK([ovs-appctl stp/show br0 | grep p1], [0], [dnl
 	p1         designated forwarding 19    128.1