[RFC PATCH] selftests: mptcp: tune timeout and delay for simult_flows cases

Paolo Abeni posted 1 patch 2 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/dcaratti/mptcp_net-next tags/patchew/126a63a07de5709e2702abb5f431f9128146c60a.1630922799.git.pabeni@redhat.com
Maintainers: Shuah Khan <shuah@kernel.org>, "David S. Miller" <davem@davemloft.net>, Matthieu Baerts <matthieu.baerts@tessares.net>, Mat Martineau <mathew.j.martineau@linux.intel.com>, Jakub Kicinski <kuba@kernel.org>
.../selftests/net/mptcp/mptcp_connect.c       |  1 -
.../selftests/net/mptcp/simult_flows.sh       | 21 +++++++++++++------
2 files changed, 15 insertions(+), 7 deletions(-)

[RFC PATCH] selftests: mptcp: tune timeout and delay for simult_flows cases

Posted by Paolo Abeni 2 weeks, 5 days ago
We currently have some instabilities in the simult_flows tests-case.
The problem boils down to the unneeded large wait introduced by the
tests to allow for the MPJ handshake to complete, which can also
introduce a quite relevant variance.

Do wait on a single side of the connection, remove the delay at
shutdown time and tune the expected test time with the above.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/137
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Side note: the change in mptcp_connect.c may impact negativelly the
mp_join self-tests. In my local experiments they are still working
reliably, but it would be good have some spins in the slow CI/debug
env for more consistency
---
 .../selftests/net/mptcp/mptcp_connect.c       |  1 -
 .../selftests/net/mptcp/simult_flows.sh       | 21 +++++++++++++------
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index 89c4753c2760..8070e090688d 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -1010,7 +1010,6 @@ static void parse_opts(int argc, char **argv)
 		case 'j':
 			cfg_join = true;
 			cfg_mode = CFG_MODE_POLL;
-			cfg_wait = 400000;
 			break;
 		case 'r':
 			cfg_remove = true;
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 910d8126af8f..8af49014a604 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -51,7 +51,7 @@ setup()
 	sout=$(mktemp)
 	cout=$(mktemp)
 	capout=$(mktemp)
-	size=$((2048 * 4096))
+	size=$((2048 * 4096 * 4))
 	dd if=/dev/zero of=$small bs=4096 count=20 >/dev/null 2>&1
 	dd if=/dev/zero of=$large bs=4096 count=$((size / 4096)) >/dev/null 2>&1
 
@@ -128,7 +128,11 @@ do_transfer()
 	local cin=$1
 	local sin=$2
 	local max_time=$3
+	local reverse=$4
 	local port
+	local srv_args="-j"
+	local cl_args=""
+
 	port=$((10000+$test_cnt))
 	test_cnt=$((test_cnt+1))
 
@@ -159,9 +163,14 @@ do_transfer()
 		sleep 1
 	fi
 
+	if [ "$reverse" = true ]; then
+		srv_args=""
+		cl_args="-j"
+	fi
+
 	timeout ${timeout_test} \
 		ip netns exec ${ns3} \
-			./mptcp_connect -jt ${timeout_poll} -l -p $port \
+			./mptcp_connect $srv_args -t ${timeout_poll} -l -p $port \
 				0.0.0.0 < "$sin" > "$sout" &
 	local spid=$!
 
@@ -171,7 +180,7 @@ do_transfer()
 	start=$(date +%s%3N)
 	timeout ${timeout_test} \
 		ip netns exec ${ns1} \
-			./mptcp_connect -jt ${timeout_poll} -p $port \
+			./mptcp_connect $cl_args -t ${timeout_poll} -p $port \
 				10.0.3.3 < "$cin" > "$cout" &
 	local cpid=$!
 
@@ -244,12 +253,12 @@ run_test()
 	tc -n $ns2 qdisc add dev ns2eth1 root netem rate ${rate1}mbit $delay1
 	tc -n $ns2 qdisc add dev ns2eth2 root netem rate ${rate2}mbit $delay2
 
-	# time is measure in ms
-	local time=$((size * 8 * 1000 / (( $rate1 + $rate2) * 1024 *1024) ))
+	# time is measure in ms, account for headers overhead, with DSS+ACK64 presence
+	local time=$((size * 8 * 1000 * 1514 / (( $rate1 + $rate2) * 1024 * 1024 * 1424) ))
 
 	# mptcp_connect will do some sleeps to allow the mp_join handshake
 	# completion
-	time=$((time + 1350))
+	time=$((time + 350))
 
 	printf "%-50s" "$msg"
 	do_transfer $small $large $((time * 11 / 10))
-- 
2.26.3


Re: [RFC PATCH] selftests: mptcp: tune timeout and delay for simult_flows cases

Posted by Paolo Abeni 2 weeks, 5 days ago
On Mon, 2021-09-06 at 12:08 +0200, Paolo Abeni wrote:
> We currently have some instabilities in the simult_flows tests-case.
> The problem boils down to the unneeded large wait introduced by the
> tests to allow for the MPJ handshake to complete, which can also
> introduce a quite relevant variance.
> 
> Do wait on a single side of the connection, remove the delay at
> shutdown time and tune the expected test time with the above.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/137
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> Side note: the change in mptcp_connect.c may impact negativelly the
> mp_join self-tests. In my local experiments they are still working
> reliably, but it would be good have some spins in the slow CI/debug
> env for more consistency
> ---
>  .../selftests/net/mptcp/mptcp_connect.c       |  1 -
>  .../selftests/net/mptcp/simult_flows.sh       | 21 +++++++++++++------
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> index 89c4753c2760..8070e090688d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
> @@ -1010,7 +1010,6 @@ static void parse_opts(int argc, char **argv)
>  		case 'j':
>  			cfg_join = true;
>  			cfg_mode = CFG_MODE_POLL;
> -			cfg_wait = 400000;
>  			break;
>  		case 'r':
>  			cfg_remove = true;
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index 910d8126af8f..8af49014a604 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -51,7 +51,7 @@ setup()
>  	sout=$(mktemp)
>  	cout=$(mktemp)
>  	capout=$(mktemp)
> -	size=$((2048 * 4096))
> +	size=$((2048 * 4096 * 4))

whooops, this change is unintended, I'll drop it in later version.

@matttbe: could you please drop it in the CI, if you pick this patch
for testing?

Thanks!

/P