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

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

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 3088 bytes --]

Hi Paolo,

On 07/08/2020 17:59, Paolo Abeni wrote:
> During diag self-tests we introduce long wait in the mptcp test
> program to give the script enough time to access the sockets
> dump.
> 
> Such wait is introduced after shutting down one sockets end. Since
> commit 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
> if boths sides shutdown the socket is correctly tranistioned

(small typo: s/boths/both/ and s/tranistioned/transitioned/

> into CLOSED status.
> 
> As a side effect some sockets are not dumped via the diag interface,
> because the socket state (CLOSED) does not match the default filter, and
> this cause self-tests instability.
> 
> Address the issue moving the above mentioned wait before shutting
> down the socket.

I just did a quick test and it seems it fixes the issue on my VM, 
thanks! I guess it should fix the issue on my CI as well.

===== 8< =====
# selftests: net/mptcp: diag.sh 

# no msk on netns creation                          [  ok  ] 

# after MPC handshake                               [  ok  ] 

# ....chk remote_key                                [  ok  ] 

# ....chk no fallback                               [  ok  ] 

# check fallback                                    [  ok  ] 
 
 

# many msk socket present                           [  ok  ] 

ok 4 selftests: net/mptcp: diag.sh
===== 8< =====

> Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")

Your SoB is missing ;-)

Also do not hesitate to add:

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

> ---
>   tools/testing/selftests/net/mptcp/mptcp_connect.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index cad6f73a5fd0..090620c3e10c 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -406,10 +406,11 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd)
>   
>   				/* ... but we still receive.
>   				 * Close our write side, ev. give some time
> -				 * for address notification
> +				 * for address notification and/or checking
> +				 * the current status
>   				 */
> -				if (cfg_join)
> -					usleep(400000);
> +				if (cfg_wait)
> +					usleep(cfg_wait);

Would it be OK for the stability to keep a wait? I guess it would be 
hard to wait for an event (check counters, etc.) to happen.

(if it is possible, it can also be done in other patch, good to have a 
fix here :-) )

>   				shutdown(peerfd, SHUT_WR);
>   			} else {
>   				if (errno == EINTR)
> @@ -427,7 +428,7 @@ static int copyfd_io_poll(int infd, int peerfd, int outfd)
>   	}
>   
>   	/* leave some time for late join/announce */
> -	if (cfg_wait)
> +	if (cfg_join)
>   		usleep(cfg_wait);

(same here)

Cheers,
Matt

>   
>   	close(peerfd);
> 

-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

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

[David Miller]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

From: Paolo Abeni <pabeni(a)redhat.com>
Date: Fri,  7 Aug 2020 18:31:00 +0200

> During diag self-tests we introduce long wait in the mptcp test
> program to give the script enough time to access the sockets
> dump.
> 
> Such wait is introduced after shutting down one sockets end. Since
> commit 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state
> machine") if both sides shutdown the socket is correctly transitioned
> into CLOSED status.
> 
> As a side effect some sockets are not dumped via the diag interface,
> because the socket state (CLOSED) does not match the default filter, and
> this cause self-tests instability.
> 
> Address the issue moving the above mentioned wait before shutting
> down the socket.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/68
> Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
> Tested-and-acked-by: Matthieu Baerts <matthieu.baerts(a)tessares.net>
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>

Applied, thanks.