[PATCH mptcp-net v2 0/2] selftests: mptcp: stabilise 'endpoint' Join tests

[PATCH mptcp-net v2 0/2] selftests: mptcp: stabilise 'endpoint' Join tests

[Matthieu Baerts]

The first patch is the one of Paolo where I only added the
initialisation of "dump_stats" var as discussed on the mailing list.

The second patch is also somehow a fix as it stops the test earlier not
to wait quite a bit of time for nothing. It is good to have it now as
after the modification of patch 1/2, the waiting time is longer.

To: mptcp@lists.linux.dev

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
Matthieu Baerts (1):
      selftests: mptcp: stop tests earlier

Paolo Abeni (1):
      selftests: mptcp: allow more slack for slow test-case

 tools/testing/selftests/net/mptcp/mptcp_join.sh | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)
---
base-commit: 0825e0ffa15862dc5c95b2195656f250a47be8ae
change-id: 20230131-mptcp-issue-323-join-delete-re-add-48a8d8d06120

Best regards,
-- 
Matthieu Baerts <matthieu.baerts@tessares.net>

[PATCH mptcp-net v2 1/2] selftests: mptcp: allow more slack for slow test-case

[Matthieu Baerts]

From: Paolo Abeni <pabeni@redhat.com>

A test-case is frequently failing on some extremelly slow VMs.
The mptcp transfer completes before the script is able to do
all the required PM manipulation.

Address the issue in the simplest possible way, making the
transfer even more slow.

Additionally dump more info in case of failures, to help debugging
similar problems in the future.

Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/323
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 387abdcec011..52484a7c286a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1687,6 +1687,7 @@ chk_subflow_nr()
 	local subflow_nr=$3
 	local cnt1
 	local cnt2
+	local dump_stats
 
 	if [ -n "${need_title}" ]; then
 		printf "%03u %-36s %s" "${TEST_COUNT}" "${TEST_NAME}" "${msg}"
@@ -1704,7 +1705,12 @@ chk_subflow_nr()
 		echo "[ ok ]"
 	fi
 
-	[ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni | grep token; ip -n $ns1 mptcp endpoint )
+	if [ "${dump_stats}" = 1 ]; then
+		ss -N $ns1 -tOni
+		ss -N $ns1 -tOni | grep token
+		ip -n $ns1 mptcp endpoint
+		dump_stats
+	fi
 }
 
 chk_link_usage()
@@ -3103,7 +3109,7 @@ endpoint_tests()
 		pm_nl_set_limits $ns1 1 1
 		pm_nl_set_limits $ns2 1 1
 		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
-		run_tests $ns1 $ns2 10.0.1.1 4 0 0 slow &
+		run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 &
 
 		wait_mpj $ns2
 		pm_nl_del_endpoint $ns2 2 10.0.2.2

-- 
2.38.1

[PATCH mptcp-net v2 2/2] selftests: mptcp: stop tests earlier

[Matthieu Baerts]

These 'endpoint' tests from 'mptcp_join.sh' selftest start a transfer in
the background and check the status during this transfer.

Once the expected events have been recorded, there is no reason to wait
for the data transfer to finish. It can be stopped earlier to reduce the
execution time by more than half.

For these tests, the exchanged data were not verified. Errors, if any,
were ignored but that's fine, plenty of other tests are looking at that.
It is then OK to mute stderr now that we are sure errors will be printed
(and still ignored) because the transfer is stopped before the end.

Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 52484a7c286a..42e3bd1a05f5 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -498,6 +498,12 @@ kill_events_pids()
 	kill_wait $evts_ns2_pid
 }
 
+kill_tests_wait()
+{
+	kill -SIGUSR1 $(ip netns pids $ns2) $(ip netns pids $ns1)
+	wait
+}
+
 pm_nl_set_limits()
 {
 	local ns=$1
@@ -3089,7 +3095,7 @@ endpoint_tests()
 		pm_nl_set_limits $ns1 2 2
 		pm_nl_set_limits $ns2 2 2
 		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
-		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow &
+		run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow 2>/dev/null &
 
 		wait_mpj $ns1
 		pm_nl_check_endpoint 1 "creation" \
@@ -3102,14 +3108,14 @@ endpoint_tests()
 		pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
 		pm_nl_check_endpoint 0 "modif is allowed" \
 			$ns2 10.0.2.2 id 1 flags signal
-		wait
+		kill_tests_wait
 	fi
 
 	if reset "delete and re-add"; then
 		pm_nl_set_limits $ns1 1 1
 		pm_nl_set_limits $ns2 1 1
 		pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
-		run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 &
+		run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 2>/dev/null &
 
 		wait_mpj $ns2
 		pm_nl_del_endpoint $ns2 2 10.0.2.2
@@ -3119,7 +3125,7 @@ endpoint_tests()
 		pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
 		wait_mpj $ns2
 		chk_subflow_nr "" "after re-add" 2
-		wait
+		kill_tests_wait
 	fi
 }
 

-- 
2.38.1

Re: selftests: mptcp: stop tests earlier: Tests Results

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5272704723451904
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5272704723451904/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6398604630294528
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6398604630294528/summary/summary.txt

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

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6117129653583872
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6117129653583872/summary/summary.txt

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


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 v2 1/2] selftests: mptcp: allow more slack for slow test-case

[Geliang Tang]

Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
>
> From: Paolo Abeni <pabeni@redhat.com>
>
> A test-case is frequently failing on some extremelly slow VMs.
> The mptcp transfer completes before the script is able to do
> all the required PM manipulation.
>
> Address the issue in the simplest possible way, making the
> transfer even more slow.
>
> Additionally dump more info in case of failures, to help debugging
> similar problems in the future.
>
> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/323
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 387abdcec011..52484a7c286a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1687,6 +1687,7 @@ chk_subflow_nr()
>         local subflow_nr=$3
>         local cnt1
>         local cnt2
> +       local dump_stats
>
>         if [ -n "${need_title}" ]; then
>                 printf "%03u %-36s %s" "${TEST_COUNT}" "${TEST_NAME}" "${msg}"
> @@ -1704,7 +1705,12 @@ chk_subflow_nr()
>                 echo "[ ok ]"
>         fi
>
> -       [ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni | grep token; ip -n $ns1 mptcp endpoint )
> +       if [ "${dump_stats}" = 1 ]; then
> +               ss -N $ns1 -tOni
> +               ss -N $ns1 -tOni | grep token
> +               ip -n $ns1 mptcp endpoint
> +               dump_stats
> +       fi

Hi Matt,

Does this work:

[ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni |
grep token; ip -n $ns1 mptcp endpoint; dump_stats )

This is more like the original code.

Thanks,
-Geliang

>  }
>
>  chk_link_usage()
> @@ -3103,7 +3109,7 @@ endpoint_tests()
>                 pm_nl_set_limits $ns1 1 1
>                 pm_nl_set_limits $ns2 1 1
>                 pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
> -               run_tests $ns1 $ns2 10.0.1.1 4 0 0 slow &
> +               run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 &
>
>                 wait_mpj $ns2
>                 pm_nl_del_endpoint $ns2 2 10.0.2.2
>
> --
> 2.38.1
>
>

Re: [PATCH mptcp-net v2 2/2] selftests: mptcp: stop tests earlier

[Geliang Tang]

Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
>
> These 'endpoint' tests from 'mptcp_join.sh' selftest start a transfer in
> the background and check the status during this transfer.
>
> Once the expected events have been recorded, there is no reason to wait
> for the data transfer to finish. It can be stopped earlier to reduce the
> execution time by more than half.
>
> For these tests, the exchanged data were not verified. Errors, if any,
> were ignored but that's fine, plenty of other tests are looking at that.
> It is then OK to mute stderr now that we are sure errors will be printed
> (and still ignored) because the transfer is stopped before the end.
>
> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 52484a7c286a..42e3bd1a05f5 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -498,6 +498,12 @@ kill_events_pids()
>         kill_wait $evts_ns2_pid
>  }
>
> +kill_tests_wait()
> +{
> +       kill -SIGUSR1 $(ip netns pids $ns2) $(ip netns pids $ns1)
> +       wait
> +}

Can we use the existing function kill_wait() to do this?

Thanks,
-Geliang

> +
>  pm_nl_set_limits()
>  {
>         local ns=$1
> @@ -3089,7 +3095,7 @@ endpoint_tests()
>                 pm_nl_set_limits $ns1 2 2
>                 pm_nl_set_limits $ns2 2 2
>                 pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
> -               run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow &
> +               run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow 2>/dev/null &
>
>                 wait_mpj $ns1
>                 pm_nl_check_endpoint 1 "creation" \
> @@ -3102,14 +3108,14 @@ endpoint_tests()
>                 pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
>                 pm_nl_check_endpoint 0 "modif is allowed" \
>                         $ns2 10.0.2.2 id 1 flags signal
> -               wait
> +               kill_tests_wait
>         fi
>
>         if reset "delete and re-add"; then
>                 pm_nl_set_limits $ns1 1 1
>                 pm_nl_set_limits $ns2 1 1
>                 pm_nl_add_endpoint $ns2 10.0.2.2 id 2 dev ns2eth2 flags subflow
> -               run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 &
> +               run_tests $ns1 $ns2 10.0.1.1 4 0 0 speed_20 2>/dev/null &
>
>                 wait_mpj $ns2
>                 pm_nl_del_endpoint $ns2 2 10.0.2.2
> @@ -3119,7 +3125,7 @@ endpoint_tests()
>                 pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
>                 wait_mpj $ns2
>                 chk_subflow_nr "" "after re-add" 2
> -               wait
> +               kill_tests_wait
>         fi
>  }
>
>
> --
> 2.38.1
>
>

Re: [PATCH mptcp-net v2 1/2] selftests: mptcp: allow more slack for slow test-case

[Matthieu Baerts]

Hi Geliang,

On 01/02/2023 06:14, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
>>
>> From: Paolo Abeni <pabeni@redhat.com>
>>
>> A test-case is frequently failing on some extremelly slow VMs.
>> The mptcp transfer completes before the script is able to do
>> all the required PM manipulation.
>>
>> Address the issue in the simplest possible way, making the
>> transfer even more slow.
>>
>> Additionally dump more info in case of failures, to help debugging
>> similar problems in the future.
>>
>> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/323
>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> index 387abdcec011..52484a7c286a 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> @@ -1687,6 +1687,7 @@ chk_subflow_nr()
>>         local subflow_nr=$3
>>         local cnt1
>>         local cnt2
>> +       local dump_stats
>>
>>         if [ -n "${need_title}" ]; then
>>                 printf "%03u %-36s %s" "${TEST_COUNT}" "${TEST_NAME}" "${msg}"
>> @@ -1704,7 +1705,12 @@ chk_subflow_nr()
>>                 echo "[ ok ]"
>>         fi
>>
>> -       [ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni | grep token; ip -n $ns1 mptcp endpoint )
>> +       if [ "${dump_stats}" = 1 ]; then
>> +               ss -N $ns1 -tOni
>> +               ss -N $ns1 -tOni | grep token
>> +               ip -n $ns1 mptcp endpoint
>> +               dump_stats
>> +       fi
> 
> Hi Matt,

Thank you for the review!

> Does this work:
> 
> [ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni |
> grep token; ip -n $ns1 mptcp endpoint; dump_stats )
> 
> This is more like the original code.

This works indeed but I think it was a good idea from Paolo to switch to
a clear 'if' statement for two reasons:

- It is easier to read than having quite a few other commands on a
single line. If only 'dump_stats' function was invoked, that would have
been fine.

- If 'dump_stats' var is not 1 and because this is the last instruction
of the function, chk_subflow_nr will cause the returned code to be != 0.
That's fine here because 'set -e' is not defined and the returned code
is not checked but still, that's not ideal, better to avoid that with a
clear "if" I think.

So I think we should keep the patch as it is, no?

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

Re: [PATCH mptcp-net v2 2/2] selftests: mptcp: stop tests earlier

[Matthieu Baerts]

Hi Geliang,

On 01/02/2023 06:17, Geliang Tang wrote:
> Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
>>
>> These 'endpoint' tests from 'mptcp_join.sh' selftest start a transfer in
>> the background and check the status during this transfer.
>>
>> Once the expected events have been recorded, there is no reason to wait
>> for the data transfer to finish. It can be stopped earlier to reduce the
>> execution time by more than half.
>>
>> For these tests, the exchanged data were not verified. Errors, if any,
>> were ignored but that's fine, plenty of other tests are looking at that.
>> It is then OK to mute stderr now that we are sure errors will be printed
>> (and still ignored) because the transfer is stopped before the end.
>>
>> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> index 52484a7c286a..42e3bd1a05f5 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> @@ -498,6 +498,12 @@ kill_events_pids()
>>         kill_wait $evts_ns2_pid
>>  }
>>
>> +kill_tests_wait()
>> +{
>> +       kill -SIGUSR1 $(ip netns pids $ns2) $(ip netns pids $ns1)
>> +       wait
>> +}
> 
> Can we use the existing function kill_wait() to do this?

Not easily: here, the best is to send SIGUSR1 to mptcp_connect process
to do a graceful stop. We could probably do a SIGTERM but still, in
endpoint_tests(), we don't have the process IDs, we would need to look
for them.

Also, we want to wait for the whole "run_tests" -- launched in the
background -- to finish, not just mptcp_connect but also the validation
part with the different "sleep" before executing the next test.

So I think we should use this new kill_tests_wait() as it is, no?

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

Re: [PATCH mptcp-net v2 1/2] selftests: mptcp: allow more slack for slow test-case

[Geliang Tang]

Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年2月1日周三 18:22写道：
>
> Hi Geliang,
>
> On 01/02/2023 06:14, Geliang Tang wrote:
> > Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
> >>
> >> From: Paolo Abeni <pabeni@redhat.com>
> >>
> >> A test-case is frequently failing on some extremelly slow VMs.
> >> The mptcp transfer completes before the script is able to do
> >> all the required PM manipulation.
> >>
> >> Address the issue in the simplest possible way, making the
> >> transfer even more slow.
> >>
> >> Additionally dump more info in case of failures, to help debugging
> >> similar problems in the future.
> >>
> >> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
> >> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/323
> >> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> >> Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> >> ---
> >>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++++++++--
> >>  1 file changed, 8 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> index 387abdcec011..52484a7c286a 100755
> >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> @@ -1687,6 +1687,7 @@ chk_subflow_nr()
> >>         local subflow_nr=$3
> >>         local cnt1
> >>         local cnt2
> >> +       local dump_stats
> >>
> >>         if [ -n "${need_title}" ]; then
> >>                 printf "%03u %-36s %s" "${TEST_COUNT}" "${TEST_NAME}" "${msg}"
> >> @@ -1704,7 +1705,12 @@ chk_subflow_nr()
> >>                 echo "[ ok ]"
> >>         fi
> >>
> >> -       [ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni | grep token; ip -n $ns1 mptcp endpoint )
> >> +       if [ "${dump_stats}" = 1 ]; then
> >> +               ss -N $ns1 -tOni
> >> +               ss -N $ns1 -tOni | grep token
> >> +               ip -n $ns1 mptcp endpoint
> >> +               dump_stats
> >> +       fi
> >
> > Hi Matt,
>
> Thank you for the review!
>
> > Does this work:
> >
> > [ "${dump_stats}" = 1 ] && ( ss -N $ns1 -tOni ; ss -N $ns1 -tOni |
> > grep token; ip -n $ns1 mptcp endpoint; dump_stats )
> >
> > This is more like the original code.
>
> This works indeed but I think it was a good idea from Paolo to switch to
> a clear 'if' statement for two reasons:
>
> - It is easier to read than having quite a few other commands on a
> single line. If only 'dump_stats' function was invoked, that would have
> been fine.
>
> - If 'dump_stats' var is not 1 and because this is the last instruction
> of the function, chk_subflow_nr will cause the returned code to be != 0.
> That's fine here because 'set -e' is not defined and the returned code
> is not checked but still, that's not ideal, better to avoid that with a
> clear "if" I think.
>
> So I think we should keep the patch as it is, no?

Sure.

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

Re: [PATCH mptcp-net v2 2/2] selftests: mptcp: stop tests earlier

[Geliang Tang]

Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年2月1日周三 18:27写道：
>
> Hi Geliang,
>
> On 01/02/2023 06:17, Geliang Tang wrote:
> > Matthieu Baerts <matthieu.baerts@tessares.net> 于2023年1月31日周二 19:23写道：
> >>
> >> These 'endpoint' tests from 'mptcp_join.sh' selftest start a transfer in
> >> the background and check the status during this transfer.
> >>
> >> Once the expected events have been recorded, there is no reason to wait
> >> for the data transfer to finish. It can be stopped earlier to reduce the
> >> execution time by more than half.
> >>
> >> For these tests, the exchanged data were not verified. Errors, if any,
> >> were ignored but that's fine, plenty of other tests are looking at that.
> >> It is then OK to mute stderr now that we are sure errors will be printed
> >> (and still ignored) because the transfer is stopped before the end.
> >>
> >> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
> >> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> >> ---
> >>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 14 ++++++++++----
> >>  1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> index 52484a7c286a..42e3bd1a05f5 100755
> >> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> >> @@ -498,6 +498,12 @@ kill_events_pids()
> >>         kill_wait $evts_ns2_pid
> >>  }
> >>
> >> +kill_tests_wait()
> >> +{
> >> +       kill -SIGUSR1 $(ip netns pids $ns2) $(ip netns pids $ns1)
> >> +       wait
> >> +}
> >
> > Can we use the existing function kill_wait() to do this?
>
> Not easily: here, the best is to send SIGUSR1 to mptcp_connect process
> to do a graceful stop. We could probably do a SIGTERM but still, in
> endpoint_tests(), we don't have the process IDs, we would need to look
> for them.
>
> Also, we want to wait for the whole "run_tests" -- launched in the
> background -- to finish, not just mptcp_connect but also the validation
> part with the different "sleep" before executing the next test.
>
> So I think we should use this new kill_tests_wait() as it is, no?

I agree.

Thanks,
-Geliang

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

Re: [PATCH mptcp-net v2 0/2] selftests: mptcp: stabilise 'endpoint' Join tests

[Matthieu Baerts]

Hello,

On 31/01/2023 12:23, Matthieu Baerts wrote:
> The first patch is the one of Paolo where I only added the
> initialisation of "dump_stats" var as discussed on the mailing list.
> 
> The second patch is also somehow a fix as it stops the test earlier not
> to wait quite a bit of time for nothing. It is good to have it now as
> after the modification of patch 1/2, the waiting time is longer.

I just applied these two patches after having replied to Geliang's
questions. I hope that's OK

@Geliang: can I add your Acked-by? (I didn't added yet as they were not
explicitly there, thus my script didn't add them automatically :) )

New patches for t/upstream-net:
- 23bbeff9e3c6: selftests: mptcp: allow more slack for slow test-case
- 98d24d468734: selftests: mptcp: stop tests earlier
- Results: b5041e5446ed..d1ac838733b6 (export-net)
- Results: c22959a266e5..8b2ca48295cf (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230202T133022
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230202T133022

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