Re: [PATCH v2 mptcp-next] selftests: mptcp: fix diag instability

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Fri, 4 Feb 2022, Matthieu Baerts wrote:

> Hi Paolo, Mat,
>
> On 04/02/2022 19:32, Paolo Abeni wrote:
>> On Fri, 2022-02-04 at 16:53 +0100, Matthieu Baerts wrote:
>>> Hi Paolo,
>>>
>>> On 04/02/2022 13:17, Paolo Abeni wrote:
>>>> Instead of waiting for an arbitrary amount of time for the MPTCP
>>>> MP_CAPABLE handshake to complete, explicitly wait for the relevant
>>>> socket to enter into the established status.
>>>>
>>>> Additionally let the data transfer application use the slowest
>>>> transfer mode available (-r), to cope with very slow host, or
>>>> high jitter caused by hosting VMs.
>>>>
>>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>>> ---
>>>> v1 -> v2:
>>>>  - use wait_for_ instead larger sleep
>>>>  - hopefully better commit message
>>>
>>> Thank you for the new version!
>>>
>>> It is being tested in the background. More than 175 attempts in a slow
>>> environment with a debug kernel and no failures so far!
>>>
>>>> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
>>>> index 2674ba20d524..ff821025d309 100755
>>>> --- a/tools/testing/selftests/net/mptcp/diag.sh
>>>> +++ b/tools/testing/selftests/net/mptcp/diag.sh-
>>>> @@ -119,7 +149,7 @@ for I in `seq 1 $NR_CLIENTS`; do
>>>>  				./mptcp_connect -p $((I+10001)) -l -w 10 \
>>>>  					-t ${timeout_poll} 0.0.0.0 >/dev/null &
>>>>  done
>>>> -sleep 0.1
>>>> +wait_local_port_listen $ns $((NR_CLIENTS + 10001))
>>>>
>>>>  for I in `seq 1 $NR_CLIENTS`; do
>>>>  	echo "b" | \
>>>
>>> Do we need the change the last sleep (sleep 1.5) as well? We could wait
>>> for a shorter time but well that's only 1.5 second once. And I guess we
>>> might need to look for each connection because looking at the last one
>>> might not be enough if there was an issue with a previous one.
>>>
>>> So best to keep the "sleep 1.5"?
>>
>> We need a more complex test to replace sleep 1.5 reliably. I'll look at
>> that later, surely in a different patch.
>
> If we don't see other issues with the public CI, it might be OK like that!
>
>>> If yes, do you prefer if the test run for a longer time or can I already
>>> apply it?
>>
>> Please, go ahead, thanks!
>
> Thanks!
>
> Should I add a Fixes tag and put it in -net?
>
> Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
>
> @Mat: Also OK for you?
>

Yes, good for me. And:

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

--
Mat Martineau
Intel