[PATCH mptcp-net] selftests: mptcp: fix mibit vs mbit mix up

Matthieu Baerts posted 1 patch 1 year, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20221110160017.3641527-1-matthieu.baerts@tessares.net
Maintainers: Mat Martineau <mathew.j.martineau@linux.intel.com>, Matthieu Baerts <matthieu.baerts@tessares.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
tools/testing/selftests/net/mptcp/simult_flows.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH mptcp-net] selftests: mptcp: fix mibit vs mbit mix up
Posted by Matthieu Baerts 1 year, 4 months ago
The estimated time was supposing the rate was expressed in mibit
(bit * 1024^2) but it is in mbit (bit * 1000^2).

This makes the threshold higher but in a more realistic way to avoid
false positives reported by CI instances.

Before this patch, the thresholds were at 7561/4005ms and now they are
at 7906/4178ms.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/310
Fixes: 1a418cb8e888 ("mptcp: simult flow self-tests")
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/simult_flows.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index af70c14e0bf9..34362c4644ff 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -249,7 +249,8 @@ run_test()
 
 	# time is measured in ms, account for transfer size, affegated link speed
 	# and header overhead (10%)
-	local time=$((size * 8 * 1000 * 10 / (( $rate1 + $rate2) * 1024 *1024 * 9) ))
+	#              ms    byte -> bit   10%        mbit      -> kbit -> bit  10%
+	local time=$((1000 * size  *  8  * 10 / ((rate1 + rate2) * 1000 * 1000 * 9) ))
 
 	# mptcp_connect will do some sleeps to allow the mp_join handshake
 	# completion (see mptcp_connect): 200ms on each side, add some slack

base-commit: 9c6870ffc2198874360447d04eb79c5f578b0f26
-- 
2.37.2
Re: selftests: mptcp: fix mibit vs mbit mix up: Tests Results
Posted by MPTCP CI 1 year, 4 months ago
Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): mptcp_connect_mmap 🔴:
  - Task: https://cirrus-ci.com/task/4553270654926848
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4553270654926848/summary/summary.txt

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

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


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] selftests: mptcp: fix mibit vs mbit mix up
Posted by Mat Martineau 1 year, 4 months ago
On Thu, 10 Nov 2022, Matthieu Baerts wrote:

> The estimated time was supposing the rate was expressed in mibit
> (bit * 1024^2) but it is in mbit (bit * 1000^2).
>
> This makes the threshold higher but in a more realistic way to avoid
> false positives reported by CI instances.
>
> Before this patch, the thresholds were at 7561/4005ms and now they are
> at 7906/4178ms.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/310
> Fixes: 1a418cb8e888 ("mptcp: simult flow self-tests")
> Suggested-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Looks good to me, if you can make one small adjustment when applying! (see 
below)

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

> ---
> tools/testing/selftests/net/mptcp/simult_flows.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index af70c14e0bf9..34362c4644ff 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -249,7 +249,8 @@ run_test()
>
> 	# time is measured in ms, account for transfer size, affegated link speed
                                                              ^^^^^^^^^
Think this is supposed to be "aggregated"? Might as well fix.

> 	# and header overhead (10%)
> -	local time=$((size * 8 * 1000 * 10 / (( $rate1 + $rate2) * 1024 *1024 * 9) ))
> +	#              ms    byte -> bit   10%        mbit      -> kbit -> bit  10%
> +	local time=$((1000 * size  *  8  * 10 / ((rate1 + rate2) * 1000 * 1000 * 9) ))
>
> 	# mptcp_connect will do some sleeps to allow the mp_join handshake
> 	# completion (see mptcp_connect): 200ms on each side, add some slack
>
> base-commit: 9c6870ffc2198874360447d04eb79c5f578b0f26
> -- 
> 2.37.2
>
>
>

--
Mat Martineau
Intel
Re: [PATCH mptcp-net] selftests: mptcp: fix mibit vs mbit mix up
Posted by Matthieu Baerts 1 year, 4 months ago
Hi Mat,

On 11/11/2022 02:13, Mat Martineau wrote:
> On Thu, 10 Nov 2022, Matthieu Baerts wrote:
> 
>> The estimated time was supposing the rate was expressed in mibit
>> (bit * 1024^2) but it is in mbit (bit * 1000^2).
>>
>> This makes the threshold higher but in a more realistic way to avoid
>> false positives reported by CI instances.
>>
>> Before this patch, the thresholds were at 7561/4005ms and now they are
>> at 7906/4178ms.
>>
>> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/310
>> Fixes: 1a418cb8e888 ("mptcp: simult flow self-tests")
>> Suggested-by: Paolo Abeni <pabeni@redhat.com>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> 
> Looks good to me, if you can make one small adjustment when applying!
> (see below)
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> 
>> ---
>> tools/testing/selftests/net/mptcp/simult_flows.sh | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
>> b/tools/testing/selftests/net/mptcp/simult_flows.sh
>> index af70c14e0bf9..34362c4644ff 100755
>> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
>> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
>> @@ -249,7 +249,8 @@ run_test()
>>
>>     # time is measured in ms, account for transfer size, affegated
>> link speed
>                                                              ^^^^^^^^^
> Think this is supposed to be "aggregated"? Might as well fix.

Good point, I didn't even notice the typo before, I must have the
auto-correction on! :)

Thank you for the review!

Now applied in our tree (fix for -net) with the suggested fix:

New patches for t/upstream-net and t/upstream:
- 1e7c2317154b: selftests: mptcp: fix mibit vs mbit mix up
- Results: 4bc0063504d3..a89943a1df52 (export-net)
- Results: e33b4398ef80..bd41b66dc9e6 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20221111T173159
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20221111T173159

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
Re: selftests: mptcp: fix mibit vs mbit mix up: Tests Results
Posted by MPTCP CI 1 year, 4 months ago
Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): packetdrill_add_addr 🔴:
  - Task: https://cirrus-ci.com/task/6122416422780928
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6122416422780928/summary/summary.txt

- KVM Validation: debug:
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5559466469359616
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5559466469359616/summary/summary.txt

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


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)