2024-03-01 17:12:28

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net 0/2] selftests: mptcp: fixes for diag.sh

Here are two patches fixing issues in MPTCP diag.sh kselftest:

- Patch 1 makes sure the exit code is '1' in case of error, and not the
test ID, not to return an exit code that would be wrongly interpreted
by the ksefltests framework, e.g. '4' means 'skip'.

- Patch 2 avoids waiting for unnecessary conditions, which can cause
timeouts in some very slow environments.

Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
Geliang Tang (1):
selftests: mptcp: diag: return KSFT_FAIL not test_cnt

Matthieu Baerts (NGI0) (1):
selftests: mptcp: diag: avoid extra waiting

tools/testing/selftests/net/mptcp/diag.sh | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
---
base-commit: 1c61728be22c1cb49c1be88693e72d8c06b1c81e
change-id: 20240301-upstream-net-20240301-selftests-mptcp-diag-exit-timeout-207d7925b7c0

Best regards,
--
Matthieu Baerts (NGI0) <[email protected]>



2024-03-01 17:18:58

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net 1/2] selftests: mptcp: diag: return KSFT_FAIL not test_cnt

From: Geliang Tang <[email protected]>

The test counter 'test_cnt' should not be returned in diag.sh, e.g. what
if only the 4th test fail? Will do 'exit 4' which is 'exit ${KSFT_SKIP}',
the whole test will be marked as skipped instead of 'failed'!

So we should do ret=${KSFT_FAIL} instead.

Fixes: df62f2ec3df6 ("selftests/mptcp: add diag interface tests")
Cc: [email protected]
Fixes: 42fb6cddec3b ("selftests: mptcp: more stable diag tests")
Signed-off-by: Geliang Tang <[email protected]>
Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/diag.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index f300f4e1eb59..18d37d4695c1 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -69,7 +69,7 @@ __chk_nr()
else
echo "[ fail ] expected $expected found $nr"
mptcp_lib_result_fail "${msg}"
- ret=$test_cnt
+ ret=${KSFT_FAIL}
fi
else
echo "[ ok ]"
@@ -124,11 +124,11 @@ wait_msk_nr()
if [ $i -ge $timeout ]; then
echo "[ fail ] timeout while expecting $expected max $max last $nr"
mptcp_lib_result_fail "${msg} # timeout"
- ret=$test_cnt
+ ret=${KSFT_FAIL}
elif [ $nr != $expected ]; then
echo "[ fail ] expected $expected found $nr"
mptcp_lib_result_fail "${msg} # unexpected result"
- ret=$test_cnt
+ ret=${KSFT_FAIL}
else
echo "[ ok ]"
mptcp_lib_result_pass "${msg}"

--
2.43.0


2024-03-01 17:21:36

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net 2/2] selftests: mptcp: diag: avoid extra waiting

When creating a lot of listener sockets, it is enough to wait only for
the last one, like we are doing before in diag.sh for other subtests.

If we do a check for each listener sockets, each time listing all
available sockets, it can take a very long time in very slow
environments, at the point we can reach some timeout.

When using the debug kconfig, the waiting time switches from more than
8 sec to 0.1 sec on my side. In slow/busy environments, and with a poll
timeout set to 30 ms, the waiting time could go up to ~100 sec because
the listener socket would timeout and stop, while the script would still
be checking one by one if all sockets are ready. The result is that
after having waited for everything to be ready, all sockets have been
stopped due to a timeout, and it is too late for the script to check how
many there were.

While at it, also removed ss options we don't need: we only need the
filtering options, to count how many listener sockets have been created.
We don't need to ask ss to display internal TCP information, and the
memory if the output is dropped by the 'wc -l' command anyway.

Fixes: b4b51d36bbaa ("selftests: mptcp: explicitly trigger the listener diag code-path")
Reported-by: Jakub Kicinski <[email protected]>
Closes: https://lore.kernel.org/r/[email protected]
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
Notes:
- The 'Fixes' commit was will be in the future 6.8-rc6, but not marked
to be backported. Because of that, the stable ML has not been added
in Cc.
---
tools/testing/selftests/net/mptcp/diag.sh | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 18d37d4695c1..75fc95675e2d 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -96,8 +96,8 @@ chk_listener_nr()
local expected=$1
local msg="$2"

- __chk_nr "ss -inmlHMON $ns | wc -l" "$expected" "$msg - mptcp" 0
- __chk_nr "ss -inmlHtON $ns | wc -l" "$expected" "$msg - subflows"
+ __chk_nr "ss -nlHMON $ns | wc -l" "$expected" "$msg - mptcp" 0
+ __chk_nr "ss -nlHtON $ns | wc -l" "$expected" "$msg - subflows"
}

wait_msk_nr()
@@ -304,10 +304,7 @@ for I in $(seq 1 $NR_SERVERS); do
ip netns exec $ns ./mptcp_connect -p $((I + 20001)) \
-t ${timeout_poll} -l 0.0.0.0 >/dev/null 2>&1 &
done
-
-for I in $(seq 1 $NR_SERVERS); do
- mptcp_lib_wait_local_port_listen $ns $((I + 20001))
-done
+mptcp_lib_wait_local_port_listen $ns $((NR_SERVERS + 20001))

chk_listener_nr $NR_SERVERS "many listener sockets"


--
2.43.0


2024-03-04 13:12:11

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net 0/2] selftests: mptcp: fixes for diag.sh

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Fri, 01 Mar 2024 18:11:21 +0100 you wrote:
> Here are two patches fixing issues in MPTCP diag.sh kselftest:
>
> - Patch 1 makes sure the exit code is '1' in case of error, and not the
> test ID, not to return an exit code that would be wrongly interpreted
> by the ksefltests framework, e.g. '4' means 'skip'.
>
> - Patch 2 avoids waiting for unnecessary conditions, which can cause
> timeouts in some very slow environments.
>
> [...]

Here is the summary with links:
- [net,1/2] selftests: mptcp: diag: return KSFT_FAIL not test_cnt
https://git.kernel.org/netdev/net/c/45bcc0346561
- [net,2/2] selftests: mptcp: diag: avoid extra waiting
https://git.kernel.org/netdev/net/c/f05d2283d111

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html