diff mbox series

[ovs-dev,v2] tests: Fix flakiness of policy based routing on slower systems

Message ID 20230510114128.219858-1-amusil@redhat.com
State Accepted
Headers show
Series [ovs-dev,v2] tests: Fix flakiness of policy based routing on slower systems | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed

Commit Message

Ales Musil May 10, 2023, 11:41 a.m. UTC
The test expected that the packet statistics will be
immediately reflected after packet inject, however that
might not be true on slower systems. Use OVS_WAIT_UNTIL
instead to ensure that the packet really went through.
Also align the IPv4 and IPv6 variant and fix some checks
that were suggesting flow statistics check, but checked
logical flows instead.

Signed-off-by: Ales Musil <amusil@redhat.com>
---
v2: Address comments from Xavier:
    Remove the additional sleep.
    Make sure that all statistics checks are done through
    OVS_WAIT_UNTIL.
---
 tests/ovn.at | 74 ++++++++++++++--------------------------------------
 1 file changed, 19 insertions(+), 55 deletions(-)

Comments

Dumitru Ceara May 17, 2023, 3:08 p.m. UTC | #1
On 5/10/23 13:41, Ales Musil wrote:
> The test expected that the packet statistics will be
> immediately reflected after packet inject, however that
> might not be true on slower systems. Use OVS_WAIT_UNTIL
> instead to ensure that the packet really went through.
> Also align the IPv4 and IPv6 variant and fix some checks
> that were suggesting flow statistics check, but checked
> logical flows instead.
> 
> Signed-off-by: Ales Musil <amusil@redhat.com>
> ---
> v2: Address comments from Xavier:
>     Remove the additional sleep.
>     Make sure that all statistics checks are done through
>     OVS_WAIT_UNTIL.
> ---

Thanks, Ales and Xavier!

I applied this patch to the main branch and backported it to all stable
branches down to 22.03 LTS.

I folded in the incremental change posted below.  It replaces "wc -l"
with "-c" as argument to "grep".

Regards,
Dumitru

diff --git a/tests/ovn.at b/tests/ovn.at
index efd37658f2..2ef370c26d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -7823,7 +7823,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the drop policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24 actions=drop" | \
-    grep "priority=10" | grep "n_packets=1" | wc -l)"])
+    grep "priority=10" | grep "n_packets=1" -c)"])
 
 # Expected to drop the packet.
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" pbr-hv/vif2-tx.pcap > vif2.packets
@@ -7834,7 +7834,7 @@ AT_FAIL_IF([test "$rcvd_packet" != ""])
 check ovn-nbctl --wait=hv lr-policy-add R1 20 "ip4.src==192.168.1.0/24 && ip4.dst==172.16.1.0/24" allow
 
 # Check logical flow
-AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "192.168.1.0" | wc -l], [0], [dnl
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "192.168.1.0" -c], [0], [dnl
 2
 ])
 
@@ -7847,7 +7847,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the allow policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24" | \
-    grep "priority=20" | grep "n_packets=1" | wc -l)"])
+    grep "priority=20" | grep "n_packets=1" -c)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls2_ro_mac && eth.dst==$ls2_p1_mac &&
@@ -7863,7 +7863,7 @@ check ovn-nbctl --wait=hv lr-policy-add R1 30 "ip4.src==192.168.1.0/24 && ip4.ds
 # Check logical flow
 AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | \
     grep "192.168.1.0" | \
-    grep "priority=30" | wc -l], [0], [dnl
+    grep "priority=30" -c], [0], [dnl
 1
 ])
 
@@ -7876,7 +7876,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the allow policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24" | \
-    grep "priority=30" | grep "n_packets=1" | wc -l)"])
+    grep "priority=30" | grep "n_packets=1" -c)"])
 echo "packet hit reroute policy"
 
 # Expected packet has TTL decreased by 1
@@ -7979,7 +7979,7 @@ ls3_p1_mac=00:00:00:01:02:05
 check ovn-nbctl --wait=sb lr-policy-add R1 10 "ip6.src==2001::/64 && ip6.dst==2002::/64" drop
 
 # Check logical flow
-AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" | wc -l], [0], [dnl
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" -c], [0], [dnl
 1
 ])
 
@@ -7993,7 +7993,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the drop policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "ipv6_src=2001::/64,ipv6_dst=2002::/64 actions=drop" | \
-    grep "priority=10" | grep "n_packets=1" | wc -l)"])
+    grep "priority=10" | grep "n_packets=1" -c)"])
 
 # Expected to drop the packet.
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" pbr-hv/vif2-tx.pcap > vif2.packets
@@ -8003,7 +8003,7 @@ AT_FAIL_IF([test -s vif2.packets])
 check ovn-nbctl --wait=sb lr-policy-add R1 20 "ip6.src==2001::/64 && ip6.dst==2002::/64" allow
 
 # Check logical flow
-AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" | wc -l], [0], [dnl
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" -c], [0], [dnl
 2
 ])
 
@@ -8016,7 +8016,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the allow policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "ipv6_src=2001::/64,ipv6_dst=2002::/64"  | \
-    grep "priority=20" | grep "n_packets=1" | wc -l)"])
+    grep "priority=20" | grep "n_packets=1" -c)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls2_ro_mac && eth.dst==$ls2_p1_mac &&
@@ -8032,7 +8032,7 @@ check ovn-nbctl --wait=sb lr-policy-add R1 30 "ip6.src==2001::/64 && ip6.dst==20
 # Check logical flow
 AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | \
     grep "2001" | \
-    grep "priority=30" | wc -l], [0], [dnl
+    grep "priority=30" -c], [0], [dnl
 1
 ])
 
@@ -8045,7 +8045,7 @@ OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 # Check if packet hit the allow policy
 OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "ipv6_src=2001::/64,ipv6_dst=2002::/64"  | \
-    grep "priority=30" | grep "n_packets=1" | wc -l)"])
+    grep "priority=30" | grep "n_packets=1" -c)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls3_ro_mac && eth.dst==$ls3_p1_mac &&
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 31f8d34d8..e9f6e2206 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -7809,7 +7809,6 @@  ls3_p1_mac=00:00:00:01:02:05
 check ovn-nbctl --wait=hv lr-policy-add R1 10 "ip4.src==192.168.1.0/24 && ip4.dst==172.16.1.0/24" drop
 
 # Check logical flow
-ovn-sbctl dump-flows > sbflows
 AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "192.168.1.0" | wc -l], [0], [dnl
 1
 ])
@@ -7822,12 +7821,9 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 
 # Check if packet hit the drop policy
-AT_CHECK([ovs-ofctl dump-flows br-int | \
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24 actions=drop" | \
-    grep "priority=10" | \
-    grep "n_packets=1" | wc -l], [0], [dnl
-1
-])
+    grep "priority=10" | grep "n_packets=1" | wc -l)"])
 
 # Expected to drop the packet.
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" pbr-hv/vif2-tx.pcap > vif2.packets
@@ -7849,12 +7845,9 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 
 # Check if packet hit the allow policy
-sleep 1
-AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | \
-    grep "192.168.1.0" | \
-    grep "priority=20" | wc -l], [0], [dnl
-1
-])
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+    grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24" | \
+    grep "priority=20" | grep "n_packets=1" | wc -l)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls2_ro_mac && eth.dst==$ls2_p1_mac &&
@@ -7879,20 +7872,11 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
        ip4 && ip.ttl==64 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
        udp && udp.src==53 && udp.dst==4369"
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
-sleep 1
 
-echo "southbound flows"
-ovn-sbctl --ovs dump-flows > sbflows
-AT_CAPTURE_FILE([sbflows])
-echo "ovs flows"
-ovs-ofctl dump-flows br-int > brflows
-AT_CAPTURE_FILE([brflows])
 # Check if packet hit the allow policy
-AT_CHECK([grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24" brflows | \
-    grep "priority=30" | \
-    grep "n_packets=1" | wc -l], [0], [dnl
-1
-])
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+    grep "nw_src=192.168.1.0/24,nw_dst=172.16.1.0/24" | \
+    grep "priority=30" | grep "n_packets=1" | wc -l)"])
 echo "packet hit reroute policy"
 
 # Expected packet has TTL decreased by 1
@@ -7995,9 +7979,7 @@  ls3_p1_mac=00:00:00:01:02:05
 check ovn-nbctl --wait=sb lr-policy-add R1 10 "ip6.src==2001::/64 && ip6.dst==2002::/64" drop
 
 # Check logical flow
-ovn-sbctl dump-flows > sbflows
-AT_CAPTURE_FILE([sbflows])
-AT_CHECK([grep lr_in_policy sbflows | grep "2001" | wc -l], [0], [dnl
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" | wc -l], [0], [dnl
 1
 ])
 
@@ -8009,12 +7991,9 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 
 # Check if packet hit the drop policy
-AT_CHECK([ovs-ofctl dump-flows br-int | \
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
     grep "ipv6_src=2001::/64,ipv6_dst=2002::/64 actions=drop" | \
-    grep "priority=10" | \
-    grep "n_packets=1" | wc -l], [0], [dnl
-1
-])
+    grep "priority=10" | grep "n_packets=1" | wc -l)"])
 
 # Expected to drop the packet.
 $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" pbr-hv/vif2-tx.pcap > vif2.packets
@@ -8024,9 +8003,7 @@  AT_FAIL_IF([test -s vif2.packets])
 check ovn-nbctl --wait=sb lr-policy-add R1 20 "ip6.src==2001::/64 && ip6.dst==2002::/64" allow
 
 # Check logical flow
-ovn-sbctl dump-flows > sbflows2
-AT_CAPTURE_FILE([sbflows2])
-AT_CHECK([grep lr_in_policy sbflows2 | grep "2001" | wc -l], [0], [dnl
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "2001" | wc -l], [0], [dnl
 2
 ])
 
@@ -8037,13 +8014,9 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
 
 # Check if packet hit the allow policy
-ovn-sbctl dump-flows > sbflows3
-AT_CAPTURE_FILE([sbflows3])
-AT_CHECK([grep lr_in_policy sbflows3 | \
-    grep "2001" | \
-    grep "priority=20" | wc -l], [0], [dnl
-1
-])
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+    grep "ipv6_src=2001::/64,ipv6_dst=2002::/64"  | \
+    grep "priority=20" | grep "n_packets=1" | wc -l)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls2_ro_mac && eth.dst==$ls2_p1_mac &&
@@ -8057,9 +8030,7 @@  OVN_CHECK_PACKETS([pbr-hv/vif2-tx.pcap], [expected])
 check ovn-nbctl --wait=sb lr-policy-add R1 30 "ip6.src==2001::/64 && ip6.dst==2002::/64" reroute 2003::2
 
 # Check logical flow
-ovn-sbctl dump-flows > sbflows4
-AT_CAPTURE_FILE([sbflows4])
-AT_CHECK([grep lr_in_policy sbflows4 | \
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | \
     grep "2001" | \
     grep "priority=30" | wc -l], [0], [dnl
 1
@@ -8070,18 +8041,11 @@  packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
        ip6 && ip.ttl==64 && ip6.src==$ls1_p1_ip && ip6.dst==$ls2_p1_ip &&
        udp && udp.src==53 && udp.dst==4369"
 OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
-sleep 1
 
-ovn-sbctl dump-flows > sbflows5
-ovs-ofctl dump-flows br-int > offlows5
-AT_CAPTURE_FILE([sbflows5])
-AT_CAPTURE_FILE([offlows5])
 # Check if packet hit the allow policy
-AT_CHECK([grep "ipv6_src=2001::/64,ipv6_dst=2002::/64" offlows5 | \
-    grep "priority=30" | \
-    grep "n_packets=1" | wc -l], [0], [dnl
-1
-])
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+    grep "ipv6_src=2001::/64,ipv6_dst=2002::/64"  | \
+    grep "priority=30" | grep "n_packets=1" | wc -l)"])
 
 # Expected packet has TTL decreased by 1
 expected="eth.src==$ls3_ro_mac && eth.dst==$ls3_p1_mac &&