1. Patchew
  2. MPTCP
  3. View series

[PATCH mptcp-net] selftests: mptcp: more stable diag tests

Paolo Abeni posted 1 patch 3 years, 3 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/f318c8ca562695b07272ea3753a52f950555c18a.1655362313.git.pabeni@redhat.com
Maintainers: "David S. Miller" <davem@davemloft.net>, Shuah Khan <shuah@kernel.org>, Jakub Kicinski <kuba@kernel.org>, Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <mathew.j.martineau@linux.intel.com>, Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>
tools/testing/selftests/net/mptcp/diag.sh | 40 ++++++++++++++++++++---
1 file changed, 36 insertions(+), 4 deletions(-)
[PATCH mptcp-net] selftests: mptcp: more stable diag tests
Posted by Paolo Abeni 3 years, 3 months ago 
The mentioned test-case still use an hard-coded-len sleep to
wait for a relative large number of connection to be established.

On very slow VM and with debug build such timeout could be exceeded,
causing failures in our CI.

Address the issue polling for the expected condition several times,
up to an unreasonable high amount of time. On reasonably fast system
the self-tests will be faster then before, on very slow one we will
still catch the correct condition.

Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/mptcp/diag.sh | 40 ++++++++++++++++++++---
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 9dd43d7d957b..ee2f238255ec 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -61,6 +61,39 @@ chk_msk_nr()
 	__chk_nr "grep -c token:" $*
 }
 
+wait_msk_nr()
+{
+	local condition="grep -c token:"
+	local expected=$1
+	local timeout=20
+	local msg nr
+	local max=0
+	local i=0
+
+	shift 1
+	msg=$*
+
+	while [ $i -lt $timeout ]; do
+		nr=$(ss -inmHMN $ns | $condition)
+		[ $nr == $expected ] && break;
+		[ $nr -gt $max ] && max=$nr
+		i=$((i + 1))
+		sleep 1
+	done
+
+	printf "%-50s" "$msg"
+	if [ $i -ge $timeout ]; then
+		echo "[ fail ] timeout while expecting $expected max $max last $nr"
+		ret=$test_cnt
+	elif [ $nr != $expected ]; then
+		echo "[ fail ] expected $expected found $nr"
+		ret=$test_cnt
+	else
+		echo "[  ok  ]"
+	fi
+	test_cnt=$((test_cnt+1))
+}
+
 chk_msk_fallback_nr()
 {
 		__chk_nr "grep -c fallback" $*
@@ -184,7 +217,7 @@ for I in `seq 1 $NR_CLIENTS`; do
 	echo "a" | \
 		timeout ${timeout_test} \
 			ip netns exec $ns \
-				./mptcp_connect -p $((I+10001)) -l -w 10 \
+				./mptcp_connect -p $((I+10001)) -l -w 20 \
 					-t ${timeout_poll} 0.0.0.0 >/dev/null &
 done
 wait_local_port_listen $ns $((NR_CLIENTS + 10001))
@@ -193,12 +226,11 @@ for I in `seq 1 $NR_CLIENTS`; do
 	echo "b" | \
 		timeout ${timeout_test} \
 			ip netns exec $ns \
-				./mptcp_connect -p $((I+10001)) -w 10 \
+				./mptcp_connect -p $((I+10001)) -w 20 \
 					-t ${timeout_poll} 127.0.0.1 >/dev/null &
 done
-sleep 1.5
 
-chk_msk_nr $((NR_CLIENTS*2)) "many msk socket present"
+wait_msk_nr $((NR_CLIENTS*2)) "many msk socket present"
 flush_pids
 
 exit $ret
-- 
2.35.3
Re: [PATCH mptcp-net] selftests: mptcp: more stable diag tests
Posted by Matthieu Baerts 3 years, 2 months ago 
Hi Paolo, Mat,

On 16/06/2022 08:51, Paolo Abeni wrote:
> The mentioned test-case still use an hard-coded-len sleep to
> wait for a relative large number of connection to be established.
> 
> On very slow VM and with debug build such timeout could be exceeded,
> causing failures in our CI.
> 
> Address the issue polling for the expected condition several times,
> up to an unreasonable high amount of time. On reasonably fast system
> the self-tests will be faster then before, on very slow one we will
> still catch the correct condition.
> 
> Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the fix and the review!

Great to see the CI reporting less issues :-)

I guess I can close issue 264:

  https://github.com/multipath-tcp/mptcp_net-next/issues/264

New patches for t/upstream-net:
- 718b9e36a088: selftests: mptcp: more stable diag tests
- Results: bd3a280300a4..2500388fcff9 (export-net)

New patches for t/upstream:
- 718b9e36a088: selftests: mptcp: more stable diag tests
- Results: f267d3bf87de..1445c5fa4907 (export)

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20220620T160817
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export-net
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220620T160817
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: [PATCH mptcp-net] selftests: mptcp: more stable diag tests
Posted by Mat Martineau 3 years, 3 months ago 
On Thu, 16 Jun 2022, Paolo Abeni wrote:

> The mentioned test-case still use an hard-coded-len sleep to
> wait for a relative large number of connection to be established.
>
> On very slow VM and with debug build such timeout could be exceeded,
> causing failures in our CI.
>
> Address the issue polling for the expected condition several times,
> up to an unreasonable high amount of time. On reasonably fast system
> the self-tests will be faster then before, on very slow one we will
> still catch the correct condition.
>
> Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Looks good to me, thanks Paolo.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> ---
> tools/testing/selftests/net/mptcp/diag.sh | 40 ++++++++++++++++++++---
> 1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index 9dd43d7d957b..ee2f238255ec 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -61,6 +61,39 @@ chk_msk_nr()
> 	__chk_nr "grep -c token:" $*
> }
>
> +wait_msk_nr()
> +{
> +	local condition="grep -c token:"
> +	local expected=$1
> +	local timeout=20
> +	local msg nr
> +	local max=0
> +	local i=0
> +
> +	shift 1
> +	msg=$*
> +
> +	while [ $i -lt $timeout ]; do
> +		nr=$(ss -inmHMN $ns | $condition)
> +		[ $nr == $expected ] && break;
> +		[ $nr -gt $max ] && max=$nr
> +		i=$((i + 1))
> +		sleep 1
> +	done
> +
> +	printf "%-50s" "$msg"
> +	if [ $i -ge $timeout ]; then
> +		echo "[ fail ] timeout while expecting $expected max $max last $nr"
> +		ret=$test_cnt
> +	elif [ $nr != $expected ]; then
> +		echo "[ fail ] expected $expected found $nr"
> +		ret=$test_cnt
> +	else
> +		echo "[  ok  ]"
> +	fi
> +	test_cnt=$((test_cnt+1))
> +}
> +
> chk_msk_fallback_nr()
> {
> 		__chk_nr "grep -c fallback" $*
> @@ -184,7 +217,7 @@ for I in `seq 1 $NR_CLIENTS`; do
> 	echo "a" | \
> 		timeout ${timeout_test} \
> 			ip netns exec $ns \
> -				./mptcp_connect -p $((I+10001)) -l -w 10 \
> +				./mptcp_connect -p $((I+10001)) -l -w 20 \
> 					-t ${timeout_poll} 0.0.0.0 >/dev/null &
> done
> wait_local_port_listen $ns $((NR_CLIENTS + 10001))
> @@ -193,12 +226,11 @@ for I in `seq 1 $NR_CLIENTS`; do
> 	echo "b" | \
> 		timeout ${timeout_test} \
> 			ip netns exec $ns \
> -				./mptcp_connect -p $((I+10001)) -w 10 \
> +				./mptcp_connect -p $((I+10001)) -w 20 \
> 					-t ${timeout_poll} 127.0.0.1 >/dev/null &
> done
> -sleep 1.5
>
> -chk_msk_nr $((NR_CLIENTS*2)) "many msk socket present"
> +wait_msk_nr $((NR_CLIENTS*2)) "many msk socket present"
> flush_pids
>
> exit $ret
> -- 
> 2.35.3
>
>
>

--
Mat Martineau
Intel

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.