diff mbox series

[ovs-dev,1/3] ovn.at: Fix virtual port tests.

Message ID 20230223063526.2363478-2-hzhou@ovn.org
State Accepted
Headers show
Series virtual port faster failover. | 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

Han Zhou Feb. 23, 2023, 6:35 a.m. UTC 
The check_virtual_offlows_not_present() function in the case "virtual
ports" has the wrong table id 45, which should be 44. However,
correcting the table id makes the case failing, because the two ACLs
added by the case were in fact overlapping:

check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir") && ip' allow
check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir6") && ip' allow

Because ip4 v.s. ip6 is not specified, both ACLs would generate OVS
flows for both ip4 and ip6 when the virtual ports are resisdent on the
chassis, and the OVS flows would remain on the chassis if one of the
ports are released but the other is remaining. This is why the
check_virtual_offlows_not_present() would always fail.

This patch corrects the table id and fixes the ACLs with proper IP
protocol, and updates the check_virtual_offlows_xxx() functions so that
only ipv4 flows are dumpped and checked which is what those functions
are used for.

Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 tests/ovn.at | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Simon Horman March 1, 2023, 9:22 a.m. UTC | #1 
On Wed, Feb 22, 2023 at 10:35:24PM -0800, Han Zhou wrote:
> The check_virtual_offlows_not_present() function in the case "virtual
> ports" has the wrong table id 45, which should be 44. However,
> correcting the table id makes the case failing, because the two ACLs
> added by the case were in fact overlapping:
> 
> check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir") && ip' allow
> check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir6") && ip' allow
> 
> Because ip4 v.s. ip6 is not specified, both ACLs would generate OVS
> flows for both ip4 and ip6 when the virtual ports are resisdent on the
> chassis, and the OVS flows would remain on the chassis if one of the
> ports are released but the other is remaining. This is why the
> check_virtual_offlows_not_present() would always fail.
> 
> This patch corrects the table id and fixes the ACLs with proper IP
> protocol, and updates the check_virtual_offlows_xxx() functions so that
> only ipv4 flows are dumpped and checked which is what those functions
> are used for.
> 
> Signed-off-by: Han Zhou <hzhou@ovn.org>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Simon Horman <simon.horman@corigine.com>
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index dc5c5df3f747..e7542db42503 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -20921,8 +20921,8 @@  check ovn-nbctl lsp-set-addresses sw1-lr0 00:00:00:00:ff:02
 check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
 
 # Add an ACL that matches on sw0-vir/sw0-vir6 being bound locally.
-check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir") && ip' allow
-check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir6") && ip' allow
+check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir") && ip4' allow
+check ovn-nbctl acl-add sw0 to-lport 1000 'is_chassis_resident("sw0-vir6") && ip6' allow
 
 check ovn-nbctl ls-add public
 check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 2001:db8::1/64
@@ -21007,9 +21007,8 @@  check_virtual_offlows_present() {
     lr0_dp_key=$(printf "%x" $(fetch_column Datapath_Binding tunnel_key external_ids:name=lr0))
     lr0_public_dp_key=$(printf "%x" $(fetch_column Port_Binding tunnel_key logical_port=lr0-public))
 
-    AT_CHECK_UNQUOTED([as $hv ovs-ofctl dump-flows br-int table=44 | ofctl_strip_all | grep "priority=2000"], [0], [dnl
+    AT_CHECK_UNQUOTED([as $hv ovs-ofctl dump-flows br-int table=44,ip | ofctl_strip_all | grep "priority=2000"], [0], [dnl
  table=44, priority=2000,ip,metadata=0x$sw0_dp_key actions=resubmit(,45)
- table=44, priority=2000,ipv6,metadata=0x$sw0_dp_key actions=resubmit(,45)
 ])
 
     AT_CHECK_UNQUOTED([as $hv ovs-ofctl dump-flows br-int table=11 | ofctl_strip_all | \
@@ -21020,7 +21019,7 @@  check_virtual_offlows_present() {
 
 check_virtual_offlows_not_present() {
     hv=$1
-    AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=45 | ofctl_strip_all | grep "priority=2000"], [1], [dnl
+    AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=44,ip | ofctl_strip_all | grep "priority=2000"], [1], [dnl
 ])
 
     AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=11 | ofctl_strip_all | \