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

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

[Paolo Abeni]

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: selftests: mptcp: more stable diag tests: Tests Results

[MPTCP CI]

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): packetdrill_add_addr 🔴:
  - Task: https://cirrus-ci.com/task/6186585342345216
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6186585342345216/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 1 failed test(s): selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/5623635388923904
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5623635388923904/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/0ae9fc4e3450


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

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

[Mat Martineau]

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

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

[Matthieu Baerts]

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