* [PATCH mptcp-next 1/6] selftests: mptcp: diag: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
@ 2024-02-22 16:27 ` Matthieu Baerts (NGI0)
2024-02-22 16:27 ` [PATCH mptcp-next 2/6] selftests: mptcp: connect: " Matthieu Baerts (NGI0)
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:27 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH mptcp-next 2/6] selftests: mptcp: connect: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
2024-02-22 16:27 ` [PATCH mptcp-next 1/6] selftests: mptcp: diag: " Matthieu Baerts (NGI0)
@ 2024-02-22 16:27 ` Matthieu Baerts (NGI0)
2024-02-22 16:27 ` [PATCH mptcp-next 3/6] selftests: mptcp: sockopt: " Matthieu Baerts (NGI0)
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:27 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH mptcp-next 3/6] selftests: mptcp: sockopt: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
2024-02-22 16:27 ` [PATCH mptcp-next 1/6] selftests: mptcp: diag: " Matthieu Baerts (NGI0)
2024-02-22 16:27 ` [PATCH mptcp-next 2/6] selftests: mptcp: connect: " Matthieu Baerts (NGI0)
@ 2024-02-22 16:27 ` Matthieu Baerts (NGI0)
2024-02-22 16:28 ` [PATCH mptcp-next 4/6] selftests: mptcp: pm netlink: " Matthieu Baerts (NGI0)
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:27 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH mptcp-next 4/6] selftests: mptcp: pm netlink: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
` (2 preceding siblings ...)
2024-02-22 16:27 ` [PATCH mptcp-next 3/6] selftests: mptcp: sockopt: " Matthieu Baerts (NGI0)
@ 2024-02-22 16:28 ` Matthieu Baerts (NGI0)
2024-02-22 16:28 ` [PATCH mptcp-next 5/6] selftests: mptcp: simult flows: " Matthieu Baerts (NGI0)
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:28 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH mptcp-next 5/6] selftests: mptcp: simult flows: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
` (3 preceding siblings ...)
2024-02-22 16:28 ` [PATCH mptcp-next 4/6] selftests: mptcp: pm netlink: " Matthieu Baerts (NGI0)
@ 2024-02-22 16:28 ` Matthieu Baerts (NGI0)
2024-02-22 16:28 ` [PATCH mptcp-next 6/6] selftests: userspace pm: avoid relaunching pm events Matthieu Baerts (NGI0)
2024-02-22 20:01 ` [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Mat Martineau
6 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:28 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
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 bc8f107357ac..467feb17e07b 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH mptcp-next 6/6] selftests: userspace pm: avoid relaunching pm events
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
` (4 preceding siblings ...)
2024-02-22 16:28 ` [PATCH mptcp-next 5/6] selftests: mptcp: simult flows: " Matthieu Baerts (NGI0)
@ 2024-02-22 16:28 ` Matthieu Baerts (NGI0)
2024-02-22 17:18 ` selftests: userspace pm: avoid relaunching pm events: Tests Results MPTCP CI
2024-02-22 20:01 ` [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Mat Martineau
6 siblings, 1 reply; 10+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-02-22 16:28 UTC (permalink / raw)
To: mptcp; +Cc: Matthieu Baerts (NGI0)
'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.
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Notes:
- we could backport this to stable, but it also works without issue
without this patch, and it would be a bit annoying to backport, due
to the use of the new mptcp_lib_xxx helpers (make file, events).
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings
2024-02-22 16:27 [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Matthieu Baerts (NGI0)
` (5 preceding siblings ...)
2024-02-22 16:28 ` [PATCH mptcp-next 6/6] selftests: userspace pm: avoid relaunching pm events Matthieu Baerts (NGI0)
@ 2024-02-22 20:01 ` Mat Martineau
2024-02-23 13:35 ` Matthieu Baerts
6 siblings, 1 reply; 10+ messages in thread
From: Mat Martineau @ 2024-02-22 20:01 UTC (permalink / raw)
To: Matthieu Baerts (NGI0); +Cc: mptcp
On Thu, 22 Feb 2024, Matthieu Baerts (NGI0) wrote:
> shellcheck recently helped to prevent issues. It is then good to fix the
> other harmless issues in order to spot "real" ones later.
>
> Note that 3 scripts were already compliant: mptcp_join.sh, mptcp_lib.sh,
> and userspace_pm.sh. mptcp_lib.sh was compliant from the beginning.
>
> The modifications are minor, mainly because SC2086 has been ignored:
> 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. New files, like
> mptcp-lib.sh, should not have that rule ideally. Ideally new code other
> existing files should use double quotes when needed.
>
> Once applied, I'm planning to add a check on the CI side to catch new
> issues, not to have to check that manually and report issues when
> applying patches, like I had to do a few times very recently.
>
> The last patch is not related to shellcheck, but I found this while
> looking at the scripts: in userspace_pm.sh, we launch 'pm_nl_ctl events'
> once, then kill it, then relaunch it using the same temp files.
>
Regarding your note in patch 6, I don't think it's a big enough issue to
backport it.
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Matthieu Baerts (NGI0) (6):
> 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 | 14 ++--
> tools/testing/selftests/net/mptcp/mptcp_connect.sh | 76 +++++++++++++---------
> tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 14 ++--
> tools/testing/selftests/net/mptcp/pm_netlink.sh | 14 ++--
> tools/testing/selftests/net/mptcp/simult_flows.sh | 10 ++-
> tools/testing/selftests/net/mptcp/userspace_pm.sh | 29 ++++-----
> 6 files changed, 98 insertions(+), 59 deletions(-)
> ---
> base-commit: 482712b4b5fc44b30dcd54409cbab4ec7e405294
> change-id: 20240222-selftests-mptcp-shellcheck-80338a0300c5
Series LGTM:
Reviewed-by: Mat Martineau <martineau@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings
2024-02-22 20:01 ` [PATCH mptcp-next 0/6] selftests: mptcp: fix shellcheck warnings Mat Martineau
@ 2024-02-23 13:35 ` Matthieu Baerts
0 siblings, 0 replies; 10+ messages in thread
From: Matthieu Baerts @ 2024-02-23 13:35 UTC (permalink / raw)
To: Mat Martineau; +Cc: mptcp
Hi Mat,
On 22/02/2024 9:01 pm, Mat Martineau wrote:
> On Thu, 22 Feb 2024, Matthieu Baerts (NGI0) wrote:
>
>> shellcheck recently helped to prevent issues. It is then good to fix the
>> other harmless issues in order to spot "real" ones later.
>>
>> Note that 3 scripts were already compliant: mptcp_join.sh, mptcp_lib.sh,
>> and userspace_pm.sh. mptcp_lib.sh was compliant from the beginning.
>>
>> The modifications are minor, mainly because SC2086 has been ignored:
>> 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. New files, like
>> mptcp-lib.sh, should not have that rule ideally. Ideally new code other
>> existing files should use double quotes when needed.
>>
>> Once applied, I'm planning to add a check on the CI side to catch new
>> issues, not to have to check that manually and report issues when
>> applying patches, like I had to do a few times very recently.
>>
>> The last patch is not related to shellcheck, but I found this while
>> looking at the scripts: in userspace_pm.sh, we launch 'pm_nl_ctl events'
>> once, then kill it, then relaunch it using the same temp files.
>>
>
> Regarding your note in patch 6, I don't think it's a big enough issue to
> backport it.
Good, easier not to backport it.
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>> Matthieu Baerts (NGI0) (6):
>> 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 | 14 ++--
>> tools/testing/selftests/net/mptcp/mptcp_connect.sh | 76
>> +++++++++++++---------
>> tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 14 ++--
>> tools/testing/selftests/net/mptcp/pm_netlink.sh | 14 ++--
>> tools/testing/selftests/net/mptcp/simult_flows.sh | 10 ++-
>> tools/testing/selftests/net/mptcp/userspace_pm.sh | 29 ++++-----
>> 6 files changed, 98 insertions(+), 59 deletions(-)
>> ---
>> base-commit: 482712b4b5fc44b30dcd54409cbab4ec7e405294
>> change-id: 20240222-selftests-mptcp-shellcheck-80338a0300c5
>
> Series LGTM:
>
> Reviewed-by: Mat Martineau <martineau@kernel.org>
Thank you for the review!
Now in our tree (feat. for next):
New patches for t/upstream:
- 235be4449155: selftests: mptcp: diag: fix shellcheck warnings
- 07f49a9477f8: selftests: mptcp: connect: fix shellcheck warnings
- a20c511f445b: selftests: mptcp: sockopt: fix shellcheck warnings
- e2f73e8be7b1: selftests: mptcp: pm netlink: fix shellcheck warnings
- 581e384d1545: selftests: mptcp: simult flows: fix shellcheck warnings
- 46330f024779: selftests: userspace pm: avoid relaunching pm events
- Results: 80144e8d21c6..7f64b53905ca (export)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 10+ messages in thread