[PATCH mptcp-next] selftests: mptcp: improve 'fair usage on close' stability

[PATCH mptcp-next] selftests: mptcp: improve 'fair usage on close' stability

[Paolo Abeni]

The mentioned tests has to wait for a subflow creation failure.
The current code looks for TCP sockets in TW state and sometimes
misses the relevant event. Switch to a more stable check, looking
for the associated mib counter.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/257
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 67be3e1215d7..556da9481730 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1137,14 +1137,14 @@ chk_link_usage()
 	fi
 }
 
-wait_for_tw()
+wait_attempt_fail()
 {
 	local timeout_ms=$((timeout_poll * 1000))
 	local time=0
 	local ns=$1
 
 	while [ $time -lt $timeout_ms ]; do
-		local cnt=$(ip netns exec $ns ss -t state time-wait |wc -l)
+		local cnt=$(ip netns exec $ns nstat -as TcpAttemptFails | grep TcpAttemptFails | awk '{print $2}')
 
 		[ "$cnt" = 1 ] && return 1
 		time=$((time + 100))
@@ -1256,7 +1256,7 @@ subflows_error_tests()
 	TEST_COUNT=$((TEST_COUNT+1))
 
 	# mpj subflow will be in TW after the reset
-	wait_for_tw $ns2
+	wait_attempt_fail $ns2
 	pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow
 	wait
 
-- 
2.34.1

Re: [PATCH mptcp-next] selftests: mptcp: improve 'fair usage on close' stability

[Mat Martineau]

On Fri, 4 Feb 2022, Paolo Abeni wrote:

> The mentioned tests has to wait for a subflow creation failure.
> The current code looks for TCP sockets in TW state and sometimes
> misses the relevant event. Switch to a more stable check, looking
> for the associated mib counter.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/257
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>

Looks good for the export branch, test passes consistenly on my vm with 
this change (although I was not seeing the failure frequently).

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

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 67be3e1215d7..556da9481730 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1137,14 +1137,14 @@ chk_link_usage()
> 	fi
> }
>
> -wait_for_tw()
> +wait_attempt_fail()
> {
> 	local timeout_ms=$((timeout_poll * 1000))
> 	local time=0
> 	local ns=$1
>
> 	while [ $time -lt $timeout_ms ]; do
> -		local cnt=$(ip netns exec $ns ss -t state time-wait |wc -l)
> +		local cnt=$(ip netns exec $ns nstat -as TcpAttemptFails | grep TcpAttemptFails | awk '{print $2}')
>
> 		[ "$cnt" = 1 ] && return 1
> 		time=$((time + 100))
> @@ -1256,7 +1256,7 @@ subflows_error_tests()
> 	TEST_COUNT=$((TEST_COUNT+1))
>
> 	# mpj subflow will be in TW after the reset
> -	wait_for_tw $ns2
> +	wait_attempt_fail $ns2
> 	pm_nl_add_endpoint $ns2 10.0.2.2 flags subflow
> 	wait
>
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next] selftests: mptcp: improve 'fair usage on close' stability

[Matthieu Baerts]

Hi Paolo, Mat,

On 04/02/2022 19:33, Paolo Abeni wrote:
> The mentioned tests has to wait for a subflow creation failure. The
> current code looks for TCP sockets in TW state and sometimes misses
> the relevant event. Switch to a more stable check, looking for the
> associated mib counter.

Thank you for the patch and the review!

It has been running all night on my side, more than 2000 attempts,
without issues!

Now in our tree (fixes for -net) with Fixes, my R&Tb and Mat's
RvB tags:

- c43e2e4ed88d: selftests: mptcp: improve 'fair usage on close' stability
- Results: 3150c57070f7..1baae8d646d8

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220205T103656
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-next] selftests: mptcp: improve 'fair usage on close' stability

[Mat Martineau]

On Sat, 5 Feb 2022, Matthieu Baerts wrote:

> Hi Paolo, Mat,
>
> On 04/02/2022 19:33, Paolo Abeni wrote:
>> The mentioned tests has to wait for a subflow creation failure. The
>> current code looks for TCP sockets in TW state and sometimes misses
>> the relevant event. Switch to a more stable check, looking for the
>> associated mib counter.
>
> Thank you for the patch and the review!
>
> It has been running all night on my side, more than 2000 attempts,
> without issues!
>
> Now in our tree (fixes for -net) with Fixes, my R&Tb and Mat's

Hi Matthieu -

I noticed both this patch and "selftests: mptcp: fix diag instability" 
were listed as mptcp-next, but queued in the export branch in the "fixes 
net" section. Is this accidental or did I miss some discussion?

Thanks,

Mat

> RvB tags:
>
> - c43e2e4ed88d: selftests: mptcp: improve 'fair usage on close' stability
> - Results: 3150c57070f7..1baae8d646d8
>
> Builds and tests are now in progress:
>
> https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220205T103656
> 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
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next] selftests: mptcp: improve 'fair usage on close' stability

[Matthieu Baerts]

Hi Mat,

On 10/02/2022 01:55, Mat Martineau wrote:
> On Sat, 5 Feb 2022, Matthieu Baerts wrote:
> 
>> Hi Paolo, Mat,
>>
>> On 04/02/2022 19:33, Paolo Abeni wrote:
>>> The mentioned tests has to wait for a subflow creation failure. The
>>> current code looks for TCP sockets in TW state and sometimes misses
>>> the relevant event. Switch to a more stable check, looking for the
>>> associated mib counter.
>>
>> Thank you for the patch and the review!
>>
>> It has been running all night on my side, more than 2000 attempts,
>> without issues!
>>
>> Now in our tree (fixes for -net) with Fixes, my R&Tb and Mat's
> 
> Hi Matthieu -
> 
> I noticed both this patch and "selftests: mptcp: fix diag instability"
> were listed as mptcp-next, but queued in the export branch in the "fixes
> net" section. Is this accidental or did I miss some discussion?

It was intentional:

https://lore.kernel.org/mptcp/510a18db-2949-e95d-050f-942e15e9ea96@tessares.net/

But it is true that it was not really clear I did the same for this one
as well, I'm sorry for that.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net