Re: [ovs-dev] [PATCH v5 04/15] test: Add delay on revalidator flush for offload test cases.

2022-11-23 Thread Roi Dayan via dev



On 23/11/2022 13:14, Eelco Chaudron wrote:
> The revalidator/purge commands in the system test cases sometimes
> get called immediately after a partial test is completed. This
> could cause the revalidator thread to log an error that it can
> not find/delete a flow due to the slower flow installation nature
> of TC.
> 
> This patch uses a macro to call the revalidator/purge command,
> which can be overwritten when the system tests are run on a tc
> enabled datapath.
> 
> Signed-off-by: Eelco Chaudron 
> ---
>  tests/system-common-macros.at |4 
>  tests/system-offloads.at  |8 +++-
>  tests/system-traffic.at   |   38 +++---
>  3 files changed, 30 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index 8b9f5c752..d95d79791 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -343,3 +343,7 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
>  # OVS_CHECK_CT_CLEAR()
>  m4_define([OVS_CHECK_CT_CLEAR],
>  [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
> ovs-vswitchd.log])])
> +
> +# OVS_REVALIDATOR_PURGE()
> +m4_define([OVS_REVALIDATOR_PURGE],
> +[AT_CHECK([ovs-appctl revalidator/purge], [0])])
> diff --git a/tests/system-offloads.at b/tests/system-offloads.at
> index fbe1dc99a..7b6deccf0 100644
> --- a/tests/system-offloads.at
> +++ b/tests/system-offloads.at
> @@ -30,11 +30,17 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
> AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
> uuidfilt])], [0], [$2])
>  ])
>  
> +
> +# We override the OVS_REVALIDATOR_PURGE macro, allowing a bit more time for 
> the
> +# tc-datapath entries to be installed.
> +m4_define([OVS_REVALIDATOR_PURGE],
> +[AT_CHECK([sleep 2; ovs-appctl revalidator/purge], [0])])
> +
> +
>  # The list below are tests that will not pass for a "test environment" 
> specific
>  # issue.
>  m4_define([OVS_TEST_SKIP_LIST],
>  [ovs_test_skip_list="
> -datapath - basic truncate action
>  datapath - truncate and output to gre tunnel by simulated packets
>  datapath - truncate and output to gre tunnel
>  conntrack - force commit
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index cd3ad0f68..1d0d0dfd5 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1517,12 +1517,12 @@ on_exit 'rm -f payload200.bin'
>  NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
>  
>  dnl packet with truncated size
> -AT_CHECK([ovs-appctl revalidator/purge], [0])
> +OVS_REVALIDATOR_PURGE()
>  AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" |  sed -n 
> 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
>  n_bytes=100
>  ])
>  dnl packet with original size
> -AT_CHECK([ovs-appctl revalidator/purge], [0])
> +OVS_REVALIDATOR_PURGE()
>  AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=5" | sed -n 
> 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
>  n_bytes=242
>  ])
> @@ -1539,7 +1539,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>  NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
>  
>  dnl 100 + 100 + 242 + min(65535,242) = 684
> -AT_CHECK([ovs-appctl revalidator/purge], [0])
> +OVS_REVALIDATOR_PURGE()
>  AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" | sed -n 
> 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
>  n_bytes=684
>  ])
> @@ -1569,7 +1569,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
>  NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
>  
>  dnl 100 + 100 + 242 + min(65535,242) = 684
> -AT_CHECK([ovs-appctl revalidator/purge], [0])
> +OVS_REVALIDATOR_PURGE()
>  AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" | sed -n 
> 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
>  n_bytes=684
>  ])
> @@ -1653,7 +1653,7 @@ AT_CHECK([ovs-ofctl add-flows br-underlay 
> flows-underlay.txt])
>  
>  dnl check tunnel push path, from at_ns1 to at_ns0
>  NS_CHECK_EXEC([at_ns1], [nc $NC_EOF_OPT -u 10.1.1.1 1234 < payload200.bin])
> -AT_CHECK([ovs-appctl revalidator/purge], [0])
> +OVS_REVALIDATOR_PURGE()
>  
>  dnl Before truncation = ETH(14) + IP(20) + UDP(8) + 200 = 242B
>  AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=2" | sed -n 
> 's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
> @@ -1669,7 +1669,7 @@ dnl This 200-byte packet is simulated on behalf of 
> ns_gre0
>  ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
> 

[ovs-dev] [PATCH v5 04/15] test: Add delay on revalidator flush for offload test cases.

2022-11-23 Thread Eelco Chaudron
The revalidator/purge commands in the system test cases sometimes
get called immediately after a partial test is completed. This
could cause the revalidator thread to log an error that it can
not find/delete a flow due to the slower flow installation nature
of TC.

This patch uses a macro to call the revalidator/purge command,
which can be overwritten when the system tests are run on a tc
enabled datapath.

Signed-off-by: Eelco Chaudron 
---
 tests/system-common-macros.at |4 
 tests/system-offloads.at  |8 +++-
 tests/system-traffic.at   |   38 +++---
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 8b9f5c752..d95d79791 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -343,3 +343,7 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
 # OVS_CHECK_CT_CLEAR()
 m4_define([OVS_CHECK_CT_CLEAR],
 [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
ovs-vswitchd.log])])
+
+# OVS_REVALIDATOR_PURGE()
+m4_define([OVS_REVALIDATOR_PURGE],
+[AT_CHECK([ovs-appctl revalidator/purge], [0])])
diff --git a/tests/system-offloads.at b/tests/system-offloads.at
index fbe1dc99a..7b6deccf0 100644
--- a/tests/system-offloads.at
+++ b/tests/system-offloads.at
@@ -30,11 +30,17 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| 
uuidfilt])], [0], [$2])
 ])
 
+
+# We override the OVS_REVALIDATOR_PURGE macro, allowing a bit more time for the
+# tc-datapath entries to be installed.
+m4_define([OVS_REVALIDATOR_PURGE],
+[AT_CHECK([sleep 2; ovs-appctl revalidator/purge], [0])])
+
+
 # The list below are tests that will not pass for a "test environment" specific
 # issue.
 m4_define([OVS_TEST_SKIP_LIST],
 [ovs_test_skip_list="
-datapath - basic truncate action
 datapath - truncate and output to gre tunnel by simulated packets
 datapath - truncate and output to gre tunnel
 conntrack - force commit
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cd3ad0f68..1d0d0dfd5 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1517,12 +1517,12 @@ on_exit 'rm -f payload200.bin'
 NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
 
 dnl packet with truncated size
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" |  sed -n 
's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
 n_bytes=100
 ])
 dnl packet with original size
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=5" | sed -n 
's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
 n_bytes=242
 ])
@@ -1539,7 +1539,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
 
 dnl 100 + 100 + 242 + min(65535,242) = 684
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" | sed -n 
's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
 n_bytes=684
 ])
@@ -1569,7 +1569,7 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 NS_CHECK_EXEC([at_ns0], [nc $NC_EOF_OPT -u 10.1.1.2 1234 < payload200.bin])
 
 dnl 100 + 100 + 242 + min(65535,242) = 684
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 AT_CHECK([ovs-ofctl dump-flows br0 table=0 | grep "in_port=3" | sed -n 
's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
 n_bytes=684
 ])
@@ -1653,7 +1653,7 @@ AT_CHECK([ovs-ofctl add-flows br-underlay 
flows-underlay.txt])
 
 dnl check tunnel push path, from at_ns1 to at_ns0
 NS_CHECK_EXEC([at_ns1], [nc $NC_EOF_OPT -u 10.1.1.1 1234 < payload200.bin])
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 
 dnl Before truncation = ETH(14) + IP(20) + UDP(8) + 200 = 242B
 AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=2" | sed -n 
's/.*\(n\_bytes=[[0-9]]*\).*/\1/p'], [0], [dnl
@@ -1669,7 +1669,7 @@ dnl This 200-byte packet is simulated on behalf of ns_gre0
 ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=02908ca8a149faadfa25056008004500010a9e9d4000402f4084ac1f0101ac1f01646558e666c122e666c111080045e46f8e40004011b4760a0101010a010102e026162e00d016e6a366ebf904c74132c6fed42a9e9e46240b4d9fd13c9b47d9704a388e70a5e77db16934a6188dc01d86aa20007ace2cf9cdb111f208474b88ffc851c871f0e3fb4fff138c1d288d437efff487e2b86a9c99fbf4229a6485e133bcf3e16f6e345207fda0932d9eeb602740456fd077b4847d25481337bd716155cc245be129ccc11bf82b834767b3760b52fe913c0e24f31c0e1b27f88acf7bba6b985fb64ee2cd6fc6bba1a9c1f021e253e1728b046fd4d023307e3296361a37ea2617ebcb2537e0284a81050dd0ee
 actions=LOCAL"
 
 dnl After truncation = 100 byte at loopback device p2(4)
-AT_CHECK([ovs-appctl revalidator/purge], [0])
+OVS_REVALIDATOR_PURGE()
 AT_CHECK([ovs-ofctl dump-flows br0 | grep "in_port=4" |