2024-03-05 10:49:08

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 00/13] selftests: mptcp: share code and fix shellcheck warnings

This series cleans MPTCP selftests code.

Patch 1 stops using 'iptables-legacy' if available, but uses 'iptables',
which is likely 'iptables-nft' behind.

Patches 2, 3, 5 and 7 move duplicated code to mptcp_lib.sh. Patch 4 is a
preparation for patch 5, and patch 6 adds generic actions at the
creation and deletion of netns.

Patches 8 to 12 disable a few shellcheck warnings, and fix the rest, so
it is easy to spot real issues later. MPTCP CI is checking that now.

Patch 13 avoids redoing some actions at init time twice, e.g. restarting
the pm events tool.

Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
Geliang Tang (6):
selftests: mptcp: add mptcp_lib_check_tools helper
selftests: mptcp: add mptcp_lib_check_output helper
selftests: mptcp: add local variables rndh
selftests: mptcp: add mptcp_lib_ns_init/exit helpers
selftests: mptcp: more operations in ns_init/exit
selftests: mptcp: add mptcp_lib_events helper

Matthieu Baerts (NGI0) (7):
selftests: mptcp: stop forcing iptables-legacy
selftests: mptcp: diag: fix shellcheck warnings
selftests: mptcp: connect: fix shellcheck warnings
selftests: mptcp: sockopt: fix shellcheck warnings
selftests: mptcp: pm netlink: fix shellcheck warnings
selftests: mptcp: simult flows: fix shellcheck warnings
selftests: userspace pm: avoid relaunching pm events

tools/testing/selftests/net/mptcp/diag.sh | 35 +++----
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 111 +++++++++++----------
tools/testing/selftests/net/mptcp/mptcp_join.sh | 60 ++---------
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 92 +++++++++++++++++
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 55 +++-------
tools/testing/selftests/net/mptcp/pm_netlink.sh | 31 +++---
tools/testing/selftests/net/mptcp/simult_flows.sh | 38 +++----
tools/testing/selftests/net/mptcp/userspace_pm.sh | 56 +++--------
8 files changed, 234 insertions(+), 244 deletions(-)
---
base-commit: 09fcde54776180a76e99cae7f6d51b33c4a06525
change-id: 20240304-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-160fceb5ce44

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



2024-03-05 10:49:29

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 01/13] selftests: mptcp: stop forcing iptables-legacy

Commit 0c4cd3f86a40 ("selftests: mptcp: join: use 'iptables-legacy' if
available") and commit a5a5990c099d ("selftests: mptcp: sockopt: use
'iptables-legacy' if available") forced using iptables-legacy if
available.

This was needed because of some issues that were visible when testing
the kselftests on a v5.15.x with iptables-nft as default backend. It
looks like these errors are no longer present. As mentioned by Pablo [1],
the errors were maybe due to missing kernel config. We can then use
iptables-nft if it is the default one, instead of using a legacy tool.

We can then check the variables iptables and ip6tables are valid. We can
keep the variables to easily change it later or add options.

Link: https://lore.kernel.org/netdev/ZbFiixyMFpQnxzCH@calendula/ [1]
Suggested-by: Pablo Neira Ayuso <[email protected]>
Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_join.sh | 12 ++++--------
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 12 ++++--------
2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 955ee651dcd5..6a4af95cd4c3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -167,15 +167,11 @@ check_tools()
exit $ksft_skip
fi

- # Use the legacy version if available to support old kernel versions
- if iptables-legacy -V &> /dev/null; then
- iptables="iptables-legacy"
- ip6tables="ip6tables-legacy"
- elif ! iptables -V &> /dev/null; then
- echo "SKIP: Could not run all tests without iptables tool"
+ if ! "${iptables}" -V &> /dev/null; then
+ echo "SKIP: Could not run all tests without ${iptables} tool"
exit $ksft_skip
- elif ! ip6tables -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ip6tables tool"
+ elif ! "${ip6tables}" -V &> /dev/null; then
+ echo "SKIP: Could not run all tests without ${ip6tables} tool"
exit $ksft_skip
fi
}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index c643872ddf47..dac8e1fc7143 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -96,15 +96,11 @@ if [ $? -ne 0 ];then
exit $ksft_skip
fi

-# Use the legacy version if available to support old kernel versions
-if iptables-legacy -V &> /dev/null; then
- iptables="iptables-legacy"
- ip6tables="ip6tables-legacy"
-elif ! iptables -V &> /dev/null; then
- echo "SKIP: Could not run all tests without iptables tool"
+if ! "${iptables}" -V &> /dev/null; then
+ echo "SKIP: Could not run all tests without ${iptables} tool"
exit $ksft_skip
-elif ! ip6tables -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ip6tables tool"
+elif ! "${ip6tables}" -V &> /dev/null; then
+ echo "SKIP: Could not run all tests without ${ip6tables} tool"
exit $ksft_skip
fi


--
2.43.0


2024-03-05 10:49:48

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 02/13] selftests: mptcp: add mptcp_lib_check_tools helper

From: Geliang Tang <[email protected]>

This patch exports check_tools() helper from mptcp_join.sh into
mptcp_lib.sh as a public one mptcp_lib_check_tools(). The arguments
"ip", "ss", "iptables" and "ip6tables" are passed into this helper
to indicate whether to check ip tool, ss tool, iptables and ip6tables
tools.

This helper can be used in every scripts.

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 | 12 +--------
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 7 +----
tools/testing/selftests/net/mptcp/mptcp_join.sh | 28 +++----------------
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 31 ++++++++++++++++++++++
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 15 +----------
tools/testing/selftests/net/mptcp/pm_netlink.sh | 7 +----
tools/testing/selftests/net/mptcp/simult_flows.sh | 7 +----
tools/testing/selftests/net/mptcp/userspace_pm.sh | 6 +----
8 files changed, 40 insertions(+), 73 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index e87cf76b3e4a..e42603be03f3 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -34,17 +34,7 @@ cleanup()
}

mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
-fi
-ss -h | grep -q MPTCP
-if [ $? -ne 0 ];then
- echo "SKIP: ss tool does not support MPTCP"
- exit $ksft_skip
-fi
+mptcp_lib_check_tools ip ss

get_msk_inuse()
{
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 7898d62fce0b..ea52110c3fbc 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -147,12 +147,7 @@ cleanup()

mptcp_lib_check_mptcp
mptcp_lib_check_kallsyms
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
-fi
+mptcp_lib_check_tools ip

sin=$(mktemp)
sout=$(mktemp)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 6a4af95cd4c3..aedc5698f26a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -152,34 +152,12 @@ cleanup_partial()
done
}

-check_tools()
-{
- mptcp_lib_check_mptcp
- mptcp_lib_check_kallsyms
-
- if ! ip -Version &> /dev/null; then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
- fi
-
- if ! ss -h | grep -q MPTCP; then
- echo "SKIP: ss tool does not support MPTCP"
- exit $ksft_skip
- fi
-
- if ! "${iptables}" -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ${iptables} tool"
- exit $ksft_skip
- elif ! "${ip6tables}" -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ${ip6tables} tool"
- exit $ksft_skip
- fi
-}
-
init() {
init=1

- check_tools
+ mptcp_lib_check_mptcp
+ mptcp_lib_check_kallsyms
+ mptcp_lib_check_tools ip ss "${iptables}" "${ip6tables}"

sin=$(mktemp)
sout=$(mktemp)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 438f557aac90..b1fe354cfe96 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -342,3 +342,34 @@ mptcp_lib_check_output() {
return 1
fi
}
+
+mptcp_lib_check_tools() {
+ local tool
+
+ for tool in "${@}"; do
+ case "${tool}" in
+ "ip")
+ if ! ip -Version &> /dev/null; then
+ mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
+ exit ${KSFT_SKIP}
+ fi
+ ;;
+ "ss")
+ if ! ss -h | grep -q MPTCP; then
+ mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
+ exit ${KSFT_SKIP}
+ fi
+ ;;
+ "iptables"* | "ip6tables"*)
+ if ! "${tool}" -V &> /dev/null; then
+ mptcp_lib_print_warn "SKIP: Could not run all tests without ${tool}"
+ exit ${KSFT_SKIP}
+ fi
+ ;;
+ *)
+ mptcp_lib_print_err "Internal error: unsupported tool: ${tool}"
+ exit ${KSFT_FAIL}
+ ;;
+ esac
+ done
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index dac8e1fc7143..fd7de1b3dc55 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -89,20 +89,7 @@ cleanup()

mptcp_lib_check_mptcp
mptcp_lib_check_kallsyms
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
-fi
-
-if ! "${iptables}" -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ${iptables} tool"
- exit $ksft_skip
-elif ! "${ip6tables}" -V &> /dev/null; then
- echo "SKIP: Could not run all tests without ${ip6tables} tool"
- exit $ksft_skip
-fi
+mptcp_lib_check_tools ip "${iptables}" "${ip6tables}"

check_mark()
{
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 705106d60db5..1ec9d8622fc9 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -36,12 +36,7 @@ cleanup()
}

mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
-fi
+mptcp_lib_check_tools ip

trap cleanup EXIT

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index ed0165c15a24..dbbb13d1d74e 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -43,12 +43,7 @@ cleanup()
}

mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
- echo "SKIP: Could not run test without ip tool"
- exit $ksft_skip
-fi
+mptcp_lib_check_tools ip

# "$ns1" ns2 ns3
# ns1eth1 ns2eth1 ns2eth3 ns3eth1
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 1b94a75604fe..629fc5d0ecc5 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -17,11 +17,7 @@ if ! mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
echo "userspace pm tests are not supported by the kernel: SKIP"
exit ${KSFT_SKIP}
fi
-
-if ! ip -Version &> /dev/null; then
- echo "SKIP: Cannot not run test without ip tool"
- exit ${KSFT_SKIP}
-fi
+mptcp_lib_check_tools ip

ANNOUNCED=6 # MPTCP_EVENT_ANNOUNCED
REMOVED=7 # MPTCP_EVENT_REMOVED

--
2.43.0


2024-03-05 10:50:06

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 03/13] selftests: mptcp: add mptcp_lib_check_output helper

From: Geliang Tang <[email protected]>

Extract the main part of check() in pm_netlink.sh into a new helper
named mptcp_lib_check_output in mptcp_lib.sh.

This helper will be used for userspace dump addresses tests.

Co-developed-by: Matthieu Baerts (NGI0) <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
Signed-off-by: Geliang Tang <[email protected]>
Reviewed-by: Matthieu Baerts (NGI0) <[email protected]>
Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index b1fe354cfe96..ed86bb0bab49 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -373,3 +373,26 @@ mptcp_lib_check_tools() {
esac
done
}
+
+mptcp_lib_check_output() {
+ local err="${1}"
+ local cmd="${2}"
+ local expected="${3}"
+ local cmd_ret=0
+ local out
+
+ if ! out=$(${cmd} 2>"${err}"); then
+ cmd_ret=${?}
+ fi
+
+ if [ ${cmd_ret} -ne 0 ]; then
+ mptcp_lib_print_err "[FAIL] command execution '${cmd}' stderr"
+ cat "${err}"
+ return 2
+ elif [ "${out}" = "${expected}" ]; then
+ return 0
+ else
+ mptcp_lib_print_err "[FAIL] expected '${expected}' got '${out}'"
+ return 1
+ fi
+}

--
2.43.0


2024-03-05 10:50:25

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 04/13] selftests: mptcp: add local variables rndh

From: Geliang Tang <[email protected]>

This patch adds local variables rndh in do_transfer() functions both in
mptcp_connect.sh and simult_flows.sh, setting it with ${ns1:4}, not the
global variable rndh. The global one is hidden in the next commit.

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/mptcp_connect.sh | 1 +
tools/testing/selftests/net/mptcp/simult_flows.sh | 1 +
2 files changed, 2 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index ea52110c3fbc..b609649311f6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -348,6 +348,7 @@ do_transfer()

if $capture; then
local capuser
+ local rndh="${connector_ns:4}"
if [ -z $SUDO_USER ] ; then
capuser=""
else
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index dbbb13d1d74e..3d08116500a4 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -139,6 +139,7 @@ do_transfer()

if $capture; then
local capuser
+ local rndh="${ns1:4}"
if [ -z $SUDO_USER ] ; then
capuser=""
else

--
2.43.0


2024-03-05 10:51:27

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 07/13] selftests: mptcp: add mptcp_lib_events helper

From: Geliang Tang <[email protected]>

To avoid duplicated code in different MPTCP selftests, we can add and
use helpers defined in mptcp_lib.sh.

This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
and mptcp_join.sh into a helper mptcp_lib_events(). Define it in
mptcp_lib.sh and use it in both scripts.

Note that mptcp_lib_kill_wait is now call before starting 'events' for
mptcp_join.sh as well, but that's fine: each test is started from a new
netns, so there will not be any existing pid there, and nothing is done
when mptcp_lib_kill_wait is called with 0.

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/mptcp_join.sh | 10 ++++------
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 12 ++++++++++++
tools/testing/selftests/net/mptcp/userspace_pm.sh | 14 ++------------
3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 2d9cf6f3bbf3..1df2d24979a0 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -430,12 +430,8 @@ reset_with_events()
{
reset "${1}" || return 1

- :> "$evts_ns1"
- :> "$evts_ns2"
- ip netns exec $ns1 ./pm_nl_ctl events >> "$evts_ns1" 2>&1 &
- evts_ns1_pid=$!
- ip netns exec $ns2 ./pm_nl_ctl events >> "$evts_ns2" 2>&1 &
- evts_ns2_pid=$!
+ mptcp_lib_events "${ns1}" "${evts_ns1}" evts_ns1_pid
+ mptcp_lib_events "${ns2}" "${evts_ns2}" evts_ns2_pid
}

reset_with_tcp_filter()
@@ -608,7 +604,9 @@ wait_mpj()
kill_events_pids()
{
mptcp_lib_kill_wait $evts_ns1_pid
+ evts_ns1_pid=0
mptcp_lib_kill_wait $evts_ns2_pid
+ evts_ns2_pid=0
}

pm_nl_set_limits()
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index f98cde130869..5e8a3c4700e4 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -422,3 +422,15 @@ mptcp_lib_ns_exit() {
rm -f /tmp/"${netns}".{nstat,out}
done
}
+
+mptcp_lib_events() {
+ local ns="${1}"
+ local evts="${2}"
+ declare -n pid="${3}"
+
+ :>"${evts}"
+
+ mptcp_lib_kill_wait "${pid:-0}"
+ ip netns exec "${ns}" ./pm_nl_ctl events >> "${evts}" 2>&1 &
+ pid=$!
+}
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 6d71bf36a1b9..3200d0b96d53 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -178,21 +178,11 @@ make_connection()
if [ -z "$client_evts" ]; then
client_evts=$(mktemp)
fi
- :>"$client_evts"
- if [ $client_evts_pid -ne 0 ]; then
- mptcp_lib_kill_wait $client_evts_pid
- fi
- ip netns exec "$ns2" ./pm_nl_ctl events >> "$client_evts" 2>&1 &
- client_evts_pid=$!
+ mptcp_lib_events "${ns2}" "${client_evts}" client_evts_pid
if [ -z "$server_evts" ]; then
server_evts=$(mktemp)
fi
- :>"$server_evts"
- if [ $server_evts_pid -ne 0 ]; then
- mptcp_lib_kill_wait $server_evts_pid
- fi
- ip netns exec "$ns1" ./pm_nl_ctl events >> "$server_evts" 2>&1 &
- server_evts_pid=$!
+ mptcp_lib_events "${ns1}" "${server_evts}" server_evts_pid
sleep 0.5

# Run the server

--
2.43.0


2024-03-05 10:51:46

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 08/13] selftests: mptcp: diag: fix shellcheck warnings

shellcheck recently helped to prevent issues. It is then good to fix the
other harmless issues in order to spot "real" ones later.

Here, two categories of warnings are now ignored:

- SC2317: Command appears to be unreachable. The cleanup() function is
invoked indirectly via the EXIT trap.

- SC2086: Double quote to prevent globbing and word splitting. This is
recommended, but the current usage is correct and there is no need to
do all these modifications to be compliant with this rule.

For the modifications:

- SC2034: ksft_skip appears unused.
- SC2046: Quote '$(get_msk_inuse)' to prevent word splitting.
- SC2006: Use $(...) notation instead of legacy backticks `...`.

Now this script is shellcheck (0.9.0) compliant. We can easily spot new
issues.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/diag.sh | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 0ee93915bccb..d8f080f934ac 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -1,10 +1,14 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

+# Double quotes to prevent globbing and word splitting is recommended in new
+# code but we accept it, especially because there were too many before having
+# address all other issues detected by shellcheck.
+#shellcheck disable=SC2086
+
. "$(dirname "${0}")/mptcp_lib.sh"

ns=""
-ksft_skip=4
test_cnt=1
timeout_poll=30
timeout_test=$((timeout_poll * 2 + 1))
@@ -24,6 +28,8 @@ flush_pids()
done
}

+# This function is used in the cleanup trap
+#shellcheck disable=SC2317
cleanup()
{
ip netns pids "${ns}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
@@ -174,7 +180,7 @@ chk_msk_inuse()
expected=$((expected + listen_nr))

for _ in $(seq 10); do
- if [ $(get_msk_inuse) -eq $expected ];then
+ if [ "$(get_msk_inuse)" -eq $expected ]; then
break
fi
sleep 0.1
@@ -260,7 +266,7 @@ chk_msk_inuse 0 "1->0"
chk_msk_cestab 0 "1->0"

NR_CLIENTS=100
-for I in `seq 1 $NR_CLIENTS`; do
+for I in $(seq 1 $NR_CLIENTS); do
echo "a" | \
timeout ${timeout_test} \
ip netns exec $ns \
@@ -269,7 +275,7 @@ for I in `seq 1 $NR_CLIENTS`; do
done
mptcp_lib_wait_local_port_listen $ns $((NR_CLIENTS + 10001))

-for I in `seq 1 $NR_CLIENTS`; do
+for I in $(seq 1 $NR_CLIENTS); do
echo "b" | \
timeout ${timeout_test} \
ip netns exec $ns \

--
2.43.0


2024-03-05 10:52:09

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 09/13] selftests: mptcp: connect: fix shellcheck warnings

shellcheck recently helped to prevent issues. It is then good to fix the
other harmless issues in order to spot "real" ones later.

Here, two categories of warnings are now ignored:

- SC2317: Command appears to be unreachable. The cleanup() function is
invoked indirectly via the EXIT trap.

- SC2086: Double quote to prevent globbing and word splitting. This is
recommended, but the current usage is correct and there is no need to
do all these modifications to be compliant with this rule.

For the modifications:

- SC2034: ksft_skip appears unused.
- SC2181: Check exit code directly with e.g. 'if mycmd;', not
indirectly with $?.
- SC2004: $/${} is unnecessary on arithmetic variables.
- SC2155: Declare and assign separately to avoid masking return
values.
- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
- SC2059: Don't use variables in the printf format string. Use printf
'..%s..' "$foo".

Now this script is shellcheck (0.9.0) compliant. We can easily spot new
issues.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 76 +++++++++++++---------
1 file changed, 47 insertions(+), 29 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index b53ae64ec08c..0ca2960c9099 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -1,6 +1,11 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

+# Double quotes to prevent globbing and word splitting is recommended in new
+# code but we accept it, especially because there were too many before having
+# address all other issues detected by shellcheck.
+#shellcheck disable=SC2086
+
. "$(dirname "${0}")/mptcp_lib.sh"

time_start=$(date +%s)
@@ -13,7 +18,6 @@ sout=""
cin_disconnect=""
cin=""
cout=""
-ksft_skip=4
capture=false
timeout_poll=30
timeout_test=$((timeout_poll * 2 + 1))
@@ -129,6 +133,8 @@ ns4=""
TEST_COUNT=0
TEST_GROUP=""

+# This function is used in the cleanup trap
+#shellcheck disable=SC2317
cleanup()
{
rm -f "$cin_disconnect" "$cout_disconnect"
@@ -211,8 +217,9 @@ set_ethtool_flags() {
local dev="$2"
local flags="$3"

- ip netns exec $ns ethtool -K $dev $flags 2>/dev/null
- [ $? -eq 0 ] && echo "INFO: set $ns dev $dev: ethtool -K $flags"
+ if ip netns exec $ns ethtool -K $dev $flags 2>/dev/null; then
+ echo "INFO: set $ns dev $dev: ethtool -K $flags"
+ fi
}

set_random_ethtool_flags() {
@@ -307,7 +314,7 @@ do_transfer()
local extra_args="$7"

local port
- port=$((10000+$TEST_COUNT))
+ port=$((10000+TEST_COUNT))
TEST_COUNT=$((TEST_COUNT+1))

if [ "$rcvbuf" -gt 0 ]; then
@@ -365,12 +372,18 @@ do_transfer()
nstat -n
fi

- local stat_synrx_last_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableSYNRX")
- local stat_ackrx_last_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableACKRX")
- local stat_cookietx_last=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesSent")
- local stat_cookierx_last=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesRecv")
- local stat_csum_err_s=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtDataCsumErr")
- local stat_csum_err_c=$(mptcp_lib_get_counter "${connector_ns}" "MPTcpExtDataCsumErr")
+ local stat_synrx_last_l
+ local stat_ackrx_last_l
+ local stat_cookietx_last
+ local stat_cookierx_last
+ local stat_csum_err_s
+ local stat_csum_err_c
+ stat_synrx_last_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableSYNRX")
+ stat_ackrx_last_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableACKRX")
+ stat_cookietx_last=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesSent")
+ stat_cookierx_last=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesRecv")
+ stat_csum_err_s=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtDataCsumErr")
+ stat_csum_err_c=$(mptcp_lib_get_counter "${connector_ns}" "MPTcpExtDataCsumErr")

timeout ${timeout_test} \
ip netns exec ${listener_ns} \
@@ -433,11 +446,16 @@ do_transfer()
mptcp_lib_check_transfer $cin $sout "file received by server"
rets=$?

- local stat_synrx_now_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableSYNRX")
- local stat_ackrx_now_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableACKRX")
- local stat_cookietx_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesSent")
- local stat_cookierx_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesRecv")
- local stat_ooo_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtTCPOFOQueue")
+ local stat_synrx_now_l
+ local stat_ackrx_now_l
+ local stat_cookietx_now
+ local stat_cookierx_now
+ local stat_ooo_now
+ stat_synrx_now_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableSYNRX")
+ stat_ackrx_now_l=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtMPCapableACKRX")
+ stat_cookietx_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesSent")
+ stat_cookierx_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtSyncookiesRecv")
+ stat_ooo_now=$(mptcp_lib_get_counter "${listener_ns}" "TcpExtTCPOFOQueue")

expect_synrx=$((stat_synrx_last_l))
expect_ackrx=$((stat_ackrx_last_l))
@@ -446,8 +464,8 @@ do_transfer()
cookies=${cookies##*=}

if [ ${cl_proto} = "MPTCP" ] && [ ${srv_proto} = "MPTCP" ]; then
- expect_synrx=$((stat_synrx_last_l+$connect_per_transfer))
- expect_ackrx=$((stat_ackrx_last_l+$connect_per_transfer))
+ expect_synrx=$((stat_synrx_last_l+connect_per_transfer))
+ expect_ackrx=$((stat_ackrx_last_l+connect_per_transfer))
fi

if [ ${stat_synrx_now_l} -lt ${expect_synrx} ]; then
@@ -455,7 +473,7 @@ do_transfer()
"${stat_synrx_now_l}" "${expect_synrx}" 1>&2
retc=1
fi
- if [ ${stat_ackrx_now_l} -lt ${expect_ackrx} -a ${stat_ooo_now} -eq 0 ]; then
+ if [ ${stat_ackrx_now_l} -lt ${expect_ackrx} ] && [ ${stat_ooo_now} -eq 0 ]; then
if [ ${stat_ooo_now} -eq 0 ]; then
printf "[ FAIL ] lower MPC ACK rx (%d) than expected (%d)\n" \
"${stat_ackrx_now_l}" "${expect_ackrx}" 1>&2
@@ -466,18 +484,20 @@ do_transfer()
fi

if $checksum; then
- local csum_err_s=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtDataCsumErr")
- local csum_err_c=$(mptcp_lib_get_counter "${connector_ns}" "MPTcpExtDataCsumErr")
+ local csum_err_s
+ local csum_err_c
+ csum_err_s=$(mptcp_lib_get_counter "${listener_ns}" "MPTcpExtDataCsumErr")
+ csum_err_c=$(mptcp_lib_get_counter "${connector_ns}" "MPTcpExtDataCsumErr")

local csum_err_s_nr=$((csum_err_s - stat_csum_err_s))
if [ $csum_err_s_nr -gt 0 ]; then
- printf "[ FAIL ]\nserver got $csum_err_s_nr data checksum error[s]"
+ printf "[ FAIL ]\nserver got %d data checksum error[s]" ${csum_err_s_nr}
rets=1
fi

local csum_err_c_nr=$((csum_err_c - stat_csum_err_c))
if [ $csum_err_c_nr -gt 0 ]; then
- printf "[ FAIL ]\nclient got $csum_err_c_nr data checksum error[s]"
+ printf "[ FAIL ]\nclient got %d data checksum error[s]" ${csum_err_c_nr}
retc=1
fi
fi
@@ -645,7 +665,7 @@ run_test_transparent()
return
fi

-ip netns exec "$listener_ns" nft -f /dev/stdin <<"EOF"
+ if ! ip netns exec "$listener_ns" nft -f /dev/stdin <<"EOF"
flush ruleset
table inet mangle {
chain divert {
@@ -656,7 +676,7 @@ table inet mangle {
}
}
EOF
- if [ $? -ne 0 ]; then
+ then
echo "SKIP: $msg, could not load nft ruleset"
mptcp_lib_fail_if_expected_feature "nft rules"
mptcp_lib_result_skip "${TEST_GROUP}"
@@ -671,8 +691,7 @@ EOF
local_addr="0.0.0.0"
fi

- ip -net "$listener_ns" $r6flag rule add fwmark 1 lookup 100
- if [ $? -ne 0 ]; then
+ if ! ip -net "$listener_ns" $r6flag rule add fwmark 1 lookup 100; then
ip netns exec "$listener_ns" nft flush ruleset
echo "SKIP: $msg, ip $r6flag rule failed"
mptcp_lib_fail_if_expected_feature "ip rule"
@@ -680,8 +699,7 @@ EOF
return
fi

- ip -net "$listener_ns" route add local $local_addr/0 dev lo table 100
- if [ $? -ne 0 ]; then
+ if ! ip -net "$listener_ns" route add local $local_addr/0 dev lo table 100; then
ip netns exec "$listener_ns" nft flush ruleset
ip -net "$listener_ns" $r6flag rule del fwmark 1 lookup 100
echo "SKIP: $msg, ip route add local $local_addr failed"
@@ -844,7 +862,7 @@ stop_if_error "Could not even run ping tests"
echo -n "INFO: Using loss of $tc_loss "
test "$tc_delay" -gt 0 && echo -n "delay $tc_delay ms "

-reorder_delay=$(($tc_delay / 4))
+reorder_delay=$((tc_delay / 4))

if [ -z "${tc_reorder}" ]; then
reorder1=$((RANDOM%10))

--
2.43.0


2024-03-05 10:52:25

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 10/13] selftests: mptcp: sockopt: fix shellcheck warnings

shellcheck recently helped to prevent issues. It is then good to fix the
other harmless issues in order to spot "real" ones later.

Here, two categories of warnings are now ignored:

- SC2317: Command appears to be unreachable. The cleanup() function is
invoked indirectly via the EXIT trap.

- SC2086: Double quote to prevent globbing and word splitting. This is
recommended, but the current usage is correct and there is no need to
do all these modifications to be compliant with this rule.

For the modifications:

- SC2034: ksft_skip appears unused.
- SC2006: Use $(...) notation instead of legacy backticks `...`.
- SC2145: Argument mixes string and array. Use * or separate argument.

Now this script is shellcheck (0.9.0) compliant. We can easily spot new
issues.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 7dd0e5467d35..6ed4aa32222f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -1,6 +1,11 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

+# Double quotes to prevent globbing and word splitting is recommended in new
+# code but we accept it, especially because there were too many before having
+# address all other issues detected by shellcheck.
+#shellcheck disable=SC2086
+
. "$(dirname "${0}")/mptcp_lib.sh"

ret=0
@@ -8,7 +13,6 @@ sin=""
sout=""
cin=""
cout=""
-ksft_skip=4
timeout_poll=30
timeout_test=$((timeout_poll * 2 + 1))
iptables="iptables"
@@ -41,7 +45,7 @@ init()
mptcp_lib_ns_init ns1 ns2 ns_sbox

local i
- for i in `seq 1 4`; do
+ for i in $(seq 1 4); do
ip link add ns1eth$i netns "$ns1" type veth peer name ns2eth$i netns "$ns2"
ip -net "$ns1" addr add 10.0.$i.1/24 dev ns1eth$i
ip -net "$ns1" addr add dead:beef:$i::1/64 dev ns1eth$i nodad
@@ -68,6 +72,8 @@ init()
add_mark_rules $ns2 2
}

+# This function is used in the cleanup trap
+#shellcheck disable=SC2317
cleanup()
{
mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns_sbox}"
@@ -257,12 +263,12 @@ do_tcpinq_test()
local lret=$?
if [ $lret -ne 0 ];then
ret=$lret
- echo "FAIL: mptcp_inq $@" 1>&2
+ echo "FAIL: mptcp_inq $*" 1>&2
mptcp_lib_result_fail "TCP_INQ: $*"
return $lret
fi

- echo "PASS: TCP_INQ cmsg/ioctl $@"
+ echo "PASS: TCP_INQ cmsg/ioctl $*"
mptcp_lib_result_pass "TCP_INQ: $*"
return $lret
}

--
2.43.0


2024-03-05 10:52:49

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 11/13] selftests: mptcp: pm netlink: fix shellcheck warnings

shellcheck recently helped to prevent issues. It is then good to fix the
other harmless issues in order to spot "real" ones later.

Here, two categories of warnings are now ignored:

- SC2317: Command appears to be unreachable. The cleanup() function is
invoked indirectly via the EXIT trap.

- SC2086: Double quote to prevent globbing and word splitting. This is
recommended, but the current usage is correct and there is no need to
do all these modifications to be compliant with this rule.

For the modifications:

- SC2034: ksft_skip appears unused.
- SC2154: optstring is referenced but not assigned.
- SC2006: Use $(...) notation instead of legacy backticks `...`.

Now this script is shellcheck (0.9.0) compliant. We can easily spot new
issues.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/pm_netlink.sh | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index c7c46152f6fd..427fc5c70b3c 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -1,16 +1,20 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

+# Double quotes to prevent globbing and word splitting is recommended in new
+# code but we accept it, especially because there were too many before having
+# address all other issues detected by shellcheck.
+#shellcheck disable=SC2086
+
. "$(dirname "${0}")/mptcp_lib.sh"

-ksft_skip=4
ret=0

usage() {
echo "Usage: $0 [ -h ]"
}

-
+optstring=h
while getopts "$optstring" option;do
case "$option" in
"h")
@@ -27,6 +31,8 @@ done
ns1=""
err=$(mktemp)

+# This function is used in the cleanup trap
+#shellcheck disable=SC2317
cleanup()
{
rm -f $err
@@ -91,14 +97,14 @@ check "ip netns exec $ns1 ./pm_nl_ctl get 4" "" "duplicate addr"
ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.4 flags signal
check "ip netns exec $ns1 ./pm_nl_ctl get 4" "id 4 flags signal 10.0.1.4" "id addr increment"

-for i in `seq 5 9`; do
+for i in $(seq 5 9); do
ip netns exec $ns1 ./pm_nl_ctl add 10.0.1.$i flags signal >/dev/null 2>&1
done
check "ip netns exec $ns1 ./pm_nl_ctl get 9" "id 9 flags signal 10.0.1.9" "hard addr limit"
check "ip netns exec $ns1 ./pm_nl_ctl get 10" "" "above hard addr limit"

ip netns exec $ns1 ./pm_nl_ctl del 9
-for i in `seq 10 255`; do
+for i in $(seq 10 255); do
ip netns exec $ns1 ./pm_nl_ctl add 10.0.0.9 id $i
ip netns exec $ns1 ./pm_nl_ctl del $i
done

--
2.43.0


2024-03-05 10:53:22

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 12/13] selftests: mptcp: simult flows: fix shellcheck warnings

shellcheck recently helped to prevent issues. It is then good to fix the
other harmless issues in order to spot "real" ones later.

Here, two categories of warnings are now ignored:

- SC2317: Command appears to be unreachable. The cleanup() function is
invoked indirectly via the EXIT trap.

- SC2086: Double quote to prevent globbing and word splitting. This is
recommended, but the current usage is correct and there is no need to
do all these modifications to be compliant with this rule.

For the modifications:

- SC2034: ksft_skip appears unused.
- SC2004: $/${} is unnecessary on arithmetic variables.

Now this script is shellcheck (0.9.0) compliant. We can easily spot new
issues.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/simult_flows.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 5a4b83cdaaa9..365fb3d6ef55 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -1,13 +1,17 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

+# Double quotes to prevent globbing and word splitting is recommended in new
+# code but we accept it, especially because there were too many before having
+# address all other issues detected by shellcheck.
+#shellcheck disable=SC2086
+
. "$(dirname "${0}")/mptcp_lib.sh"

ns1=""
ns2=""
ns3=""
capture=false
-ksft_skip=4
timeout_poll=30
timeout_test=$((timeout_poll * 2 + 1))
test_cnt=1
@@ -28,6 +32,8 @@ usage() {
echo -e "\t-d: debug this script"
}

+# This function is used in the cleanup trap
+#shellcheck disable=SC2317
cleanup()
{
rm -f "$cout" "$sout"
@@ -120,7 +126,7 @@ do_transfer()
local sin=$2
local max_time=$3
local port
- port=$((10000+$test_cnt))
+ port=$((10000+test_cnt))
test_cnt=$((test_cnt+1))

:> "$cout"

--
2.43.0


2024-03-05 10:53:43

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 13/13] selftests: userspace pm: avoid relaunching pm events

'make_connection' is launched twice: once for IPv4, once for IPv6.

But then, the "pm_nl_ctl events" was launched a first time, killed, then
relaunched after for no particular reason.

We can then move this code, and the generation of the temp file to
exchange, to the init part, and remove extra conditions that no longer
needed.

Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts (NGI0) <[email protected]>
---
tools/testing/selftests/net/mptcp/userspace_pm.sh | 29 ++++++++++-------------
1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 3200d0b96d53..b0cce8f065d8 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -149,17 +149,23 @@ ip -net "$ns2" addr add dead:beef:1::2/64 dev ns2eth1 nodad
ip -net "$ns2" addr add dead:beef:2::2/64 dev ns2eth1 nodad
ip -net "$ns2" link set ns2eth1 up

+file=$(mktemp)
+mptcp_lib_make_file "$file" 2 1
+
+# Capture netlink events over the two network namespaces running
+# the MPTCP client and server
+client_evts=$(mktemp)
+mptcp_lib_events "${ns2}" "${client_evts}" client_evts_pid
+server_evts=$(mktemp)
+mptcp_lib_events "${ns1}" "${server_evts}" server_evts_pid
+sleep 0.5
+
print_title "Init"
print_test "Created network namespaces ns1, ns2"
test_pass

make_connection()
{
- if [ -z "$file" ]; then
- file=$(mktemp)
- fi
- mptcp_lib_make_file "$file" 2 1
-
local is_v6=$1
local app_port=$app4_port
local connect_addr="10.0.1.1"
@@ -173,17 +179,8 @@ make_connection()
is_v6="v4"
fi

- # Capture netlink events over the two network namespaces running
- # the MPTCP client and server
- if [ -z "$client_evts" ]; then
- client_evts=$(mktemp)
- fi
- mptcp_lib_events "${ns2}" "${client_evts}" client_evts_pid
- if [ -z "$server_evts" ]; then
- server_evts=$(mktemp)
- fi
- mptcp_lib_events "${ns1}" "${server_evts}" server_evts_pid
- sleep 0.5
+ :>"$client_evts"
+ :>"$server_evts"

# Run the server
ip netns exec "$ns1" \

--
2.43.0


2024-03-05 10:53:48

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 05/13] selftests: mptcp: add mptcp_lib_ns_init/exit helpers

From: Geliang Tang <[email protected]>

Add helpers mptcp_lib_ns_init() and mptcp_lib_ns_exit() in mptcp_lib.sh
to initialize and delete the given namespaces. Then every test script
can invoke these helpers and use all namespaces.

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 | 9 +++------
tools/testing/selftests/net/mptcp/mptcp_connect.sh | 23 +++++++++-------------
tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++---------
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 22 +++++++++++++++++++++
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 17 ++++++----------
tools/testing/selftests/net/mptcp/pm_netlink.sh | 9 +++------
tools/testing/selftests/net/mptcp/simult_flows.sh | 16 +++++----------
tools/testing/selftests/net/mptcp/userspace_pm.sh | 14 ++++---------
8 files changed, 54 insertions(+), 67 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index e42603be03f3..0ee93915bccb 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -3,9 +3,7 @@

. "$(dirname "${0}")/mptcp_lib.sh"

-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns="ns1-$rndh"
+ns=""
ksft_skip=4
test_cnt=1
timeout_poll=30
@@ -30,7 +28,7 @@ cleanup()
{
ip netns pids "${ns}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null

- ip netns del $ns
+ mptcp_lib_ns_exit "${ns}"
}

mptcp_lib_check_mptcp
@@ -214,8 +212,7 @@ wait_connected()
}

trap cleanup EXIT
-ip netns add $ns
-ip -n $ns link set dev lo up
+mptcp_lib_ns_init ns

echo "a" | \
timeout ${timeout_test} \
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index b609649311f6..92f6260ba9f3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -121,12 +121,10 @@ while getopts "$optstring" option;do
esac
done

-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns3="ns3-$rndh"
-ns4="ns4-$rndh"
+ns1=""
+ns2=""
+ns3=""
+ns4=""

TEST_COUNT=0
TEST_GROUP=""
@@ -140,9 +138,9 @@ cleanup()

local netns
for netns in "$ns1" "$ns2" "$ns3" "$ns4";do
- ip netns del $netns
rm -f /tmp/$netns.{nstat,out}
done
+ mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns3}" "${ns4}"
}

mptcp_lib_check_mptcp
@@ -158,10 +156,7 @@ cin_disconnect="$cin".disconnect
cout_disconnect="$cout".disconnect
trap cleanup EXIT

-for i in "$ns1" "$ns2" "$ns3" "$ns4";do
- ip netns add $i || exit $ksft_skip
- ip -net $i link set lo up
-done
+mptcp_lib_ns_init ns1 ns2 ns3 ns4

# "$ns1" ns2 ns3 ns4
# ns1eth2 ns2eth1 ns2eth3 ns3eth2 ns3eth4 ns4eth3
@@ -251,8 +246,8 @@ fi

check_mptcp_disabled()
{
- local disabled_ns="ns_disabled-$rndh"
- ip netns add ${disabled_ns} || exit $ksft_skip
+ local disabled_ns
+ mptcp_lib_ns_init disabled_ns

# net.mptcp.enabled should be enabled by default
if [ "$(ip netns exec ${disabled_ns} sysctl net.mptcp.enabled | awk '{ print $3 }')" -ne 1 ]; then
@@ -266,7 +261,7 @@ check_mptcp_disabled()
local err=0
LC_ALL=C ip netns exec ${disabled_ns} ./mptcp_connect -p 10000 -s MPTCP 127.0.0.1 < "$cin" 2>&1 | \
grep -q "^socket: Protocol not available$" && err=1
- ip netns delete ${disabled_ns}
+ mptcp_lib_ns_exit "${disabled_ns}"

if [ ${err} -eq 0 ]; then
echo -e "New MPTCP socket cannot be blocked via sysctl\t\t[ FAIL ]"
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index aedc5698f26a..612470244c58 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -86,17 +86,10 @@ init_partial()
{
capout=$(mktemp)

- local sec rndh
- sec=$(date +%s)
- rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-
- ns1="ns1-$rndh"
- ns2="ns2-$rndh"
+ mptcp_lib_ns_init ns1 ns2

local netns
for netns in "$ns1" "$ns2"; do
- ip netns add $netns || exit $ksft_skip
- ip -net $netns link set lo up
ip netns exec $netns sysctl -q net.mptcp.enabled=1
ip netns exec $netns sysctl -q net.mptcp.pm_type=0 2>/dev/null || true
ip netns exec $netns sysctl -q net.ipv4.conf.all.rp_filter=0
@@ -147,9 +140,9 @@ cleanup_partial()

local netns
for netns in "$ns1" "$ns2"; do
- ip netns del $netns
rm -f /tmp/$netns.{nstat,out}
done
+ mptcp_lib_ns_exit "${ns1}" "${ns2}"
}

init() {
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index ed86bb0bab49..2fcf31277e28 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -396,3 +396,25 @@ mptcp_lib_check_output() {
return 1
fi
}
+
+mptcp_lib_ns_init() {
+ local sec rndh
+
+ sec=$(date +%s)
+ rndh=$(printf %x "${sec}")-$(mktemp -u XXXXXX)
+
+ local netns
+ for netns in "${@}"; do
+ eval "${netns}=${netns}-${rndh}"
+
+ ip netns add "${!netns}" || exit ${KSFT_SKIP}
+ ip -net "${!netns}" link set lo up
+ done
+}
+
+mptcp_lib_ns_exit() {
+ local netns
+ for netns in "${@}"; do
+ ip netns del "${netns}"
+ done
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index fd7de1b3dc55..5fa5fa8cab71 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -14,11 +14,9 @@ timeout_test=$((timeout_poll * 2 + 1))
iptables="iptables"
ip6tables="ip6tables"

-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns_sbox="ns_sbox-$rndh"
+ns1=""
+ns2=""
+ns_sbox=""

add_mark_rules()
{
@@ -40,10 +38,10 @@ add_mark_rules()

init()
{
+ mptcp_lib_ns_init ns1 ns2 ns_sbox
+
local netns
for netns in "$ns1" "$ns2" "$ns_sbox";do
- ip netns add $netns || exit $ksft_skip
- ip -net $netns link set lo up
ip netns exec $netns sysctl -q net.mptcp.enabled=1
ip netns exec $netns sysctl -q net.ipv4.conf.all.rp_filter=0
ip netns exec $netns sysctl -q net.ipv4.conf.default.rp_filter=0
@@ -79,10 +77,7 @@ init()

cleanup()
{
- local netns
- for netns in "$ns1" "$ns2" "$ns_sbox"; do
- ip netns del $netns
- done
+ mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns_sbox}"
rm -f "$cin" "$cout"
rm -f "$sin" "$sout"
}
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 1ec9d8622fc9..30ec0ec3d68f 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -24,15 +24,13 @@ while getopts "$optstring" option;do
esac
done

-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
+ns1=""
err=$(mktemp)

cleanup()
{
rm -f $err
- ip netns del $ns1
+ mptcp_lib_ns_exit "${ns1}"
}

mptcp_lib_check_mptcp
@@ -40,8 +38,7 @@ mptcp_lib_check_tools ip

trap cleanup EXIT

-ip netns add $ns1 || exit $ksft_skip
-ip -net $ns1 link set lo up
+mptcp_lib_ns_init ns1
ip netns exec $ns1 sysctl -q net.mptcp.enabled=1

check()
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 3d08116500a4..2aeebb80da07 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -3,11 +3,9 @@

. "$(dirname "${0}")/mptcp_lib.sh"

-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns3="ns3-$rndh"
+ns1=""
+ns2=""
+ns3=""
capture=false
ksft_skip=4
timeout_poll=30
@@ -36,10 +34,7 @@ cleanup()
rm -f "$large" "$small"
rm -f "$capout"

- local netns
- for netns in "$ns1" "$ns2" "$ns3";do
- ip netns del $netns
- done
+ mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns3}"
}

mptcp_lib_check_mptcp
@@ -65,9 +60,8 @@ setup()

trap cleanup EXIT

+ mptcp_lib_ns_init ns1 ns2 ns3
for i in "$ns1" "$ns2" "$ns3";do
- ip netns add $i || exit $ksft_skip
- ip -net $i link set lo up
ip netns exec $i sysctl -q net.ipv4.conf.all.rp_filter=0
ip netns exec $i sysctl -q net.ipv4.conf.default.rp_filter=0
done
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 629fc5d0ecc5..e3092696d2de 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -50,10 +50,8 @@ app6_port=50004
client_addr_id=${RANDOM:0:2}
server_addr_id=${RANDOM:0:2}

-sec=$(date +%s)
-rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
+ns1=""
+ns2=""
ret=0
test_name=""

@@ -118,10 +116,7 @@ cleanup()
mptcp_lib_kill_wait $pid
done

- local netns
- for netns in "$ns1" "$ns2" ;do
- ip netns del "$netns"
- done
+ mptcp_lib_ns_exit "${ns1}" "${ns2}"

rm -rf $file $client_evts $server_evts

@@ -131,9 +126,8 @@ cleanup()
trap cleanup EXIT

# Create and configure network namespaces for testing
+mptcp_lib_ns_init ns1 ns2
for i in "$ns1" "$ns2" ;do
- ip netns add "$i" || exit 1
- ip -net "$i" link set lo up
ip netns exec "$i" sysctl -q net.mptcp.enabled=1
ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
done

--
2.43.0


2024-03-05 10:54:07

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: [PATCH net-next 06/13] selftests: mptcp: more operations in ns_init/exit

From: Geliang Tang <[email protected]>

Set more the default sysctl values in mptcp_lib_ns_init(). It is fine to
do that everywhere, because they could be overridden latter if needed.

mptcp_lib_ns_exit() now also try to remove temp netns files used for the
stats even for selftests not using them. That's fine to do that because
these files have a unique name.

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/mptcp_connect.sh | 4 ----
tools/testing/selftests/net/mptcp/mptcp_join.sh | 7 -------
tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++++
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 7 -------
tools/testing/selftests/net/mptcp/pm_netlink.sh | 1 -
tools/testing/selftests/net/mptcp/simult_flows.sh | 4 ----
tools/testing/selftests/net/mptcp/userspace_pm.sh | 1 -
7 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 92f6260ba9f3..b53ae64ec08c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -136,10 +136,6 @@ cleanup()
rm -f "$sin" "$sout"
rm -f "$capout"

- local netns
- for netns in "$ns1" "$ns2" "$ns3" "$ns4";do
- rm -f /tmp/$netns.{nstat,out}
- done
mptcp_lib_ns_exit "${ns1}" "${ns2}" "${ns3}" "${ns4}"
}

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 612470244c58..2d9cf6f3bbf3 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -90,10 +90,7 @@ init_partial()

local netns
for netns in "$ns1" "$ns2"; do
- ip netns exec $netns sysctl -q net.mptcp.enabled=1
ip netns exec $netns sysctl -q net.mptcp.pm_type=0 2>/dev/null || true
- ip netns exec $netns sysctl -q net.ipv4.conf.all.rp_filter=0
- ip netns exec $netns sysctl -q net.ipv4.conf.default.rp_filter=0
if $checksum; then
ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
fi
@@ -138,10 +135,6 @@ cleanup_partial()
{
rm -f "$capout"

- local netns
- for netns in "$ns1" "$ns2"; do
- rm -f /tmp/$netns.{nstat,out}
- done
mptcp_lib_ns_exit "${ns1}" "${ns2}"
}

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 2fcf31277e28..f98cde130869 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -409,6 +409,9 @@ mptcp_lib_ns_init() {

ip netns add "${!netns}" || exit ${KSFT_SKIP}
ip -net "${!netns}" link set lo up
+ ip netns exec "${!netns}" sysctl -q net.mptcp.enabled=1
+ ip netns exec "${!netns}" sysctl -q net.ipv4.conf.all.rp_filter=0
+ ip netns exec "${!netns}" sysctl -q net.ipv4.conf.default.rp_filter=0
done
}

@@ -416,5 +419,6 @@ mptcp_lib_ns_exit() {
local netns
for netns in "${@}"; do
ip netns del "${netns}"
+ rm -f /tmp/"${netns}".{nstat,out}
done
}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 5fa5fa8cab71..7dd0e5467d35 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -40,13 +40,6 @@ init()
{
mptcp_lib_ns_init ns1 ns2 ns_sbox

- local netns
- for netns in "$ns1" "$ns2" "$ns_sbox";do
- ip netns exec $netns sysctl -q net.mptcp.enabled=1
- ip netns exec $netns sysctl -q net.ipv4.conf.all.rp_filter=0
- ip netns exec $netns sysctl -q net.ipv4.conf.default.rp_filter=0
- done
-
local i
for i in `seq 1 4`; do
ip link add ns1eth$i netns "$ns1" type veth peer name ns2eth$i netns "$ns2"
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 30ec0ec3d68f..c7c46152f6fd 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -39,7 +39,6 @@ mptcp_lib_check_tools ip
trap cleanup EXIT

mptcp_lib_ns_init ns1
-ip netns exec $ns1 sysctl -q net.mptcp.enabled=1

check()
{
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 2aeebb80da07..5a4b83cdaaa9 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -61,10 +61,6 @@ setup()
trap cleanup EXIT

mptcp_lib_ns_init ns1 ns2 ns3
- for i in "$ns1" "$ns2" "$ns3";do
- ip netns exec $i sysctl -q net.ipv4.conf.all.rp_filter=0
- ip netns exec $i sysctl -q net.ipv4.conf.default.rp_filter=0
- done

ip link add ns1eth1 netns "$ns1" type veth peer name ns2eth1 netns "$ns2"
ip link add ns1eth2 netns "$ns1" type veth peer name ns2eth2 netns "$ns2"
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index e3092696d2de..6d71bf36a1b9 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -128,7 +128,6 @@ trap cleanup EXIT
# Create and configure network namespaces for testing
mptcp_lib_ns_init ns1 ns2
for i in "$ns1" "$ns2" ;do
- ip netns exec "$i" sysctl -q net.mptcp.enabled=1
ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
done


--
2.43.0


2024-03-05 11:21:03

by Matthieu Baerts (NGI0)

[permalink] [raw]
Subject: Re: [PATCH net-next 03/13] selftests: mptcp: add mptcp_lib_check_output helper

Hello,

On 05/03/2024 11:42, Matthieu Baerts (NGI0) wrote:
> From: Geliang Tang <[email protected]>
>
> Extract the main part of check() in pm_netlink.sh into a new helper
> named mptcp_lib_check_output in mptcp_lib.sh.
>
> This helper will be used for userspace dump addresses tests.

Arf, I just noticed that when the MPTCP tree got rebased on top of
net-next, Git didn't drop this patch, but resolved the conflicts by
duplicating mptcp_lib_check_output() function. Of course, redefining the
function is OK in Bash, and I didn't notice the issue when running the
tests...

I will resend this series without this patch tomorrow.

Sorry for the noise :-/

pw-bot: changes-requested

Cheers,
Matt
--
Sponsored by the NGI0 Core fund.