From 7c2eac649054c04007f61fcc90a42f6800727efc Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:42:50 +0100 Subject: [PATCH 01/12] 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 Reviewed-by: Mat Martineau Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-1-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 From 3fb8c33ef4b90f4cc83e635ca716a831d5251bc7 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 6 Mar 2024 10:42:51 +0100 Subject: [PATCH 02/12] selftests: mptcp: add mptcp_lib_check_tools helper 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 Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-2-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/diag.sh | 12 +------ .../selftests/net/mptcp/mptcp_connect.sh | 7 +---- .../testing/selftests/net/mptcp/mptcp_join.sh | 28 ++--------------- .../testing/selftests/net/mptcp/mptcp_lib.sh | 31 +++++++++++++++++++ .../selftests/net/mptcp/mptcp_sockopt.sh | 15 +-------- .../testing/selftests/net/mptcp/pm_netlink.sh | 7 +---- .../selftests/net/mptcp/simult_flows.sh | 7 +---- .../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 cc8bdf95b104..16c115709a37 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 From 4214aac14e51f931e19e39083432e93abb938ab0 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 6 Mar 2024 10:42:52 +0100 Subject: [PATCH 03/12] selftests: mptcp: add local variables rndh 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 Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-3-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 From 3a0f9bed3c28811d692f59b2db35e6c3e259ab43 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 6 Mar 2024 10:42:53 +0100 Subject: [PATCH 04/12] selftests: mptcp: add mptcp_lib_ns_init/exit helpers 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 Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-4-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/diag.sh | 9 +++----- .../selftests/net/mptcp/mptcp_connect.sh | 23 ++++++++----------- .../testing/selftests/net/mptcp/mptcp_join.sh | 11 ++------- .../testing/selftests/net/mptcp/mptcp_lib.sh | 21 +++++++++++++++++ .../selftests/net/mptcp/mptcp_sockopt.sh | 17 +++++--------- .../testing/selftests/net/mptcp/pm_netlink.sh | 9 +++----- .../selftests/net/mptcp/simult_flows.sh | 16 ++++--------- .../selftests/net/mptcp/userspace_pm.sh | 14 ++++------- 8 files changed, 53 insertions(+), 67 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh index 16c115709a37..c3b69b64042c 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 b1fe354cfe96..17d609368603 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh @@ -373,3 +373,24 @@ mptcp_lib_check_tools() { esac done } +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 From df8d3ba55b4fa1d6aed8449971ee50757cb0732f Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 6 Mar 2024 10:42:54 +0100 Subject: [PATCH 05/12] selftests: mptcp: more operations in ns_init/exit 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 Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-5-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 17d609368603..5f44a4ebd185 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh @@ -385,6 +385,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 } @@ -392,5 +395,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 From 35bc143a8514ee72b2e9d6b8b385468608b93a53 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 6 Mar 2024 10:42:55 +0100 Subject: [PATCH 06/12] selftests: mptcp: add mptcp_lib_events helper 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 Reviewed-by: Matthieu Baerts (NGI0) Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-6-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 5f44a4ebd185..5d071b6eb780 100644 --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh @@ -398,3 +398,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 From 97633aa74d93cefabc0943f6b0d4cfdeb39177cf Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:42:56 +0100 Subject: [PATCH 07/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-7-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 c3b69b64042c..afe862895946 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 \ From e3aae1098f109f0bd33c971deff1926f4e4441d0 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:42:57 +0100 Subject: [PATCH 08/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-8-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- .../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)) From 5751c291349deb9c1fa59455df3c9e1be30137a5 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:42:58 +0100 Subject: [PATCH 09/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-9-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 } From 21781b42f2f35d24b502205a3d8987adcfd05636 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:42:59 +0100 Subject: [PATCH 10/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-10-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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 From 2aebd3579d90e12cadc1bf93c524e2f422cd1c49 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:43:00 +0100 Subject: [PATCH 11/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-11-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- 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" From c66fb480a3302577f657a5d4f5308312bf1b52f8 Mon Sep 17 00:00:00 2001 From: "Matthieu Baerts (NGI0)" Date: Wed, 6 Mar 2024 10:43:01 +0100 Subject: [PATCH 12/12] 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 Signed-off-by: Matthieu Baerts (NGI0) Link: https://lore.kernel.org/r/20240306-upstream-net-next-20240304-selftests-mptcp-shared-code-shellcheck-v2-12-bc79e6e5e6a0@kernel.org Signed-off-by: Jakub Kicinski --- .../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" \