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.1Matthieu 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
>
>
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
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
© 2016 - 2025 Red Hat, Inc.