From: Geliang Tang <tanggeliang@kylinos.cn>
To unify the output formats of all test scripts, this patch adds
four more helpers:
mptcp_lib_pr_ok()
mptcp_lib_pr_skip()
mptcp_lib_pr_fail()
mptcp_lib_pr_info()
to print out [ OK ], [SKIP], [FAIL] and 'INFO: ' with colors. Use them
in all scripts to print the "ok/skip/fail/info' using the same 'format'.
Having colors helps to quickly identify issues when looking at a long
list of output logs and results.
Note that the mptcp_join.sh tests will now print these keywords with
capital letters, like most of the other tests.
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
tools/testing/selftests/net/mptcp/diag.sh | 12 ++--
.../selftests/net/mptcp/mptcp_connect.sh | 61 +++++++++----------
.../testing/selftests/net/mptcp/mptcp_join.sh | 6 +-
.../testing/selftests/net/mptcp/mptcp_lib.sh | 35 ++++++++---
.../selftests/net/mptcp/mptcp_sockopt.sh | 25 ++++----
.../testing/selftests/net/mptcp/pm_netlink.sh | 2 +-
.../selftests/net/mptcp/simult_flows.sh | 4 +-
.../selftests/net/mptcp/userspace_pm.sh | 18 ++----
8 files changed, 85 insertions(+), 78 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 74b19b89d6e6..f7a729dfc555 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -57,15 +57,15 @@ __chk_nr()
mptcp_lib_print_title "$msg"
if [ "$nr" != "$expected" ]; then
if [ "$nr" = "$skip" ] && ! mptcp_lib_expect_all_features; then
- echo "[ skip ] Feature probably not supported"
+ mptcp_lib_pr_skip "Feature probably not supported"
mptcp_lib_result_skip "${msg}"
else
- echo "[ fail ] expected $expected found $nr"
+ mptcp_lib_pr_fail "expected $expected found $nr"
mptcp_lib_result_fail "${msg}"
ret=${KSFT_FAIL}
fi
else
- echo "[ ok ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "${msg}"
fi
}
@@ -114,15 +114,15 @@ wait_msk_nr()
mptcp_lib_print_title "$msg"
if [ $i -ge $timeout ]; then
- echo "[ fail ] timeout while expecting $expected max $max last $nr"
+ mptcp_lib_pr_fail "timeout while expecting $expected max $max last $nr"
mptcp_lib_result_fail "${msg} # timeout"
ret=${KSFT_FAIL}
elif [ $nr != $expected ]; then
- echo "[ fail ] expected $expected found $nr"
+ mptcp_lib_pr_fail "expected $expected found $nr"
mptcp_lib_result_fail "${msg} # unexpected result"
ret=${KSFT_FAIL}
else
- echo "[ ok ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "${msg}"
fi
}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 096ff8941c5b..70acbb19f568 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -218,7 +218,7 @@ set_ethtool_flags() {
local flags="$3"
if ip netns exec $ns ethtool -K $dev $flags 2>/dev/null; then
- echo "INFO: set $ns dev $dev: ethtool -K $flags"
+ mptcp_lib_pr_info "set $ns dev $dev: ethtool -K $flags"
fi
}
@@ -255,7 +255,7 @@ check_mptcp_disabled()
mptcp_lib_print_title "New MPTCP socket can be blocked via sysctl"
# net.mptcp.enabled should be enabled by default
if [ "$(ip netns exec ${disabled_ns} sysctl net.mptcp.enabled | awk '{ print $3 }')" -ne 1 ]; then
- echo -e "net.mptcp.enabled sysctl is not 1 by default\t\t[ FAIL ]"
+ mptcp_lib_pr_fail "net.mptcp.enabled sysctl is not 1 by default"
mptcp_lib_result_fail "net.mptcp.enabled sysctl is not 1 by default"
ret=1
return 1
@@ -268,13 +268,13 @@ check_mptcp_disabled()
mptcp_lib_ns_exit "${disabled_ns}"
if [ ${err} -eq 0 ]; then
- echo -e "New MPTCP socket cannot be blocked via sysctl\t\t[ FAIL ]"
+ mptcp_lib_pr_fail "New MPTCP socket cannot be blocked via sysctl"
mptcp_lib_result_fail "New MPTCP socket cannot be blocked via sysctl"
ret=1
return 1
fi
- echo "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "New MPTCP socket can be blocked via sysctl"
return 0
}
@@ -295,7 +295,7 @@ do_ping()
ip netns exec ${connector_ns} ping ${ping_args} $connect_addr >/dev/null || rc=1
if [ $rc -ne 0 ] ; then
- echo "$listener_ns -> $connect_addr connectivity [ FAIL ]" 1>&2
+ mptcp_lib_pr_fail "$listener_ns -> $connect_addr connectivity"
ret=1
return 1
@@ -330,7 +330,7 @@ do_transfer()
fi
if [ -n "$extra_args" ] && $options_log; then
- echo "INFO: extra options: $extra_args"
+ mptcp_lib_pr_info "extra options: $extra_args"
fi
options_log=false
@@ -427,7 +427,7 @@ do_transfer()
result_msg+=" # time=${duration}ms"
printf "(duration %05sms) " "${duration}"
if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
- echo "[ FAIL ] client exit code $retc, server $rets" 1>&2
+ mptcp_lib_pr_fail "client exit code $retc, server $rets"
echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port"
cat /tmp/${listener_ns}.out
@@ -469,14 +469,14 @@ do_transfer()
fi
if [ ${stat_synrx_now_l} -lt ${expect_synrx} ]; then
- printf "[ FAIL ] lower MPC SYN rx (%d) than expected (%d)\n" \
- "${stat_synrx_now_l}" "${expect_synrx}" 1>&2
+ mptcp_lib_pr_fail "lower MPC SYN rx (${stat_synrx_now_l})" \
+ "than expected (${expect_synrx})"
retc=1
fi
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
+ mptcp_lib_pr_fail "lower MPC ACK rx (${stat_ackrx_now_l})" \
+ "than expected (${expect_ackrx})"
rets=1
else
printf "[ Note ] fallback due to TCP OoO"
@@ -491,19 +491,19 @@ do_transfer()
local csum_err_s_nr=$((csum_err_s - stat_csum_err_s))
if [ $csum_err_s_nr -gt 0 ]; then
- printf "[ FAIL ]\nserver got %d data checksum error[s]" ${csum_err_s_nr}
+ mptcp_lib_pr_fail "server got ${csum_err_s_nr} data checksum error[s]"
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 %d data checksum error[s]" ${csum_err_c_nr}
+ mptcp_lib_pr_fail "client got ${csum_err_c_nr} data checksum error[s]"
retc=1
fi
fi
if [ $retc -eq 0 ] && [ $rets -eq 0 ]; then
- printf "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "${TEST_GROUP}: ${result_msg}"
else
mptcp_lib_result_fail "${TEST_GROUP}: ${result_msg}"
@@ -534,7 +534,6 @@ do_transfer()
"${expect_ackrx}" "${stat_ackrx_now_l}"
fi
- echo
cat "$capout"
[ $retc -eq 0 ] && [ $rets -eq 0 ]
}
@@ -660,7 +659,7 @@ run_test_transparent()
# following function has been exported (T). Not great but better than
# checking for a specific kernel version.
if ! mptcp_lib_kallsyms_has "T __ip_sock_set_tos$"; then
- echo "INFO: ${msg} not supported by the kernel: SKIP"
+ mptcp_lib_pr_skip "INFO: ${msg} not supported by the kernel"
mptcp_lib_result_skip "${TEST_GROUP}"
return
fi
@@ -677,7 +676,7 @@ table inet mangle {
}
EOF
then
- echo "SKIP: $msg, could not load nft ruleset"
+ mptcp_lib_pr_skip "$msg, could not load nft ruleset"
mptcp_lib_fail_if_expected_feature "nft rules"
mptcp_lib_result_skip "${TEST_GROUP}"
return
@@ -693,7 +692,7 @@ EOF
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_pr_skip "$msg, ip $r6flag rule failed"
mptcp_lib_fail_if_expected_feature "ip rule"
mptcp_lib_result_skip "${TEST_GROUP}"
return
@@ -702,13 +701,13 @@ EOF
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"
+ mptcp_lib_pr_skip "$msg, ip route add local $local_addr failed"
mptcp_lib_fail_if_expected_feature "ip route"
mptcp_lib_result_skip "${TEST_GROUP}"
return
fi
- echo "INFO: test $msg"
+ mptcp_lib_pr_info "test $msg"
PORT=10000
local extra_args="-o TRANSPARENT"
@@ -721,12 +720,12 @@ EOF
ip -net "$listener_ns" route del local $local_addr/0 dev lo table 100
if [ $lret -ne 0 ]; then
- echo "FAIL: $msg, mptcp connection error" 1>&2
+ mptcp_lib_pr_fail "$msg, mptcp connection error"
ret=$lret
return 1
fi
- echo "PASS: $msg"
+ mptcp_lib_pr_info "PASS: $msg"
return 0
}
@@ -735,7 +734,7 @@ run_tests_peekmode()
local peekmode="$1"
TEST_GROUP="peek mode: ${peekmode}"
- echo "INFO: with peek mode: ${peekmode}"
+ mptcp_lib_pr_info "with peek mode: ${peekmode}"
run_tests_lo "$ns1" "$ns1" 10.0.1.1 1 "-P ${peekmode}"
run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1 "-P ${peekmode}"
}
@@ -745,12 +744,12 @@ run_tests_mptfo()
TEST_GROUP="MPTFO"
if ! mptcp_lib_kallsyms_has "mptcp_fastopen_"; then
- echo "INFO: TFO not supported by the kernel: SKIP"
+ mptcp_lib_pr_skip "TFO not supported by the kernel"
mptcp_lib_result_skip "${TEST_GROUP}"
return
fi
- echo "INFO: with MPTFO start"
+ mptcp_lib_pr_info "with MPTFO start"
ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=2
ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=1
@@ -762,7 +761,7 @@ run_tests_mptfo()
ip netns exec "$ns1" sysctl -q net.ipv4.tcp_fastopen=0
ip netns exec "$ns2" sysctl -q net.ipv4.tcp_fastopen=0
- echo "INFO: with MPTFO end"
+ mptcp_lib_pr_info "with MPTFO end"
}
run_tests_disconnect()
@@ -773,7 +772,7 @@ run_tests_disconnect()
TEST_GROUP="full disconnect"
if ! mptcp_lib_kallsyms_has "mptcp_pm_data_reset$"; then
- echo "INFO: Full disconnect not supported: SKIP"
+ mptcp_lib_pr_skip "Full disconnect not supported"
mptcp_lib_result_skip "${TEST_GROUP}"
return
fi
@@ -786,7 +785,7 @@ run_tests_disconnect()
cin_disconnect="$old_cin"
connect_per_transfer=3
- echo "INFO: disconnect"
+ mptcp_lib_pr_info "disconnect"
run_tests_lo "$ns1" "$ns1" 10.0.1.1 1 "-I 3 -i $old_cin"
run_tests_lo "$ns1" "$ns1" dead:beef:1::1 1 "-I 3 -i $old_cin"
@@ -810,7 +809,7 @@ log_if_error()
local msg="$1"
if [ ${ret} -ne 0 ]; then
- echo "FAIL: ${msg}" 1>&2
+ mptcp_lib_pr_fail "${msg}"
final_ret=${ret}
ret=0
@@ -858,11 +857,11 @@ done
mptcp_lib_result_code "${ret}" "ping tests"
stop_if_error "Could not even run ping tests"
-echo "[ OK ]"
+mptcp_lib_pr_ok
MPTCP_LIB_TEST_FORMAT="%02u %-50s"
[ -n "$tc_loss" ] && tc -net "$ns2" qdisc add dev ns2eth3 root netem loss random $tc_loss delay ${tc_delay}ms
-echo -n "INFO: Using loss of $tc_loss "
+mptcp_lib_pr_info "Using loss of $tc_loss "
test "$tc_delay" -gt 0 && echo -n "delay $tc_delay ms "
reorder_delay=$((tc_delay / 4))
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 7acc6064eb17..c3654eb45870 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -190,17 +190,17 @@ print_info()
print_ok()
{
- mptcp_lib_print_ok "[ ok ]${1:+ ${*}}"
+ mptcp_lib_pr_ok "${@}"
}
print_fail()
{
- mptcp_lib_print_err "[fail]${1:+ ${*}}"
+ mptcp_lib_pr_fail "${@}"
}
print_skip()
{
- mptcp_lib_print_warn "[skip]${1:+ ${*}}"
+ mptcp_lib_pr_skip "${@}"
}
# [ $1: fail msg ]
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 358d5b77fc0f..53867f9d212d 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -48,6 +48,23 @@ mptcp_lib_print_err() {
mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
}
+#shellcheck disable=SC2120
+mptcp_lib_pr_ok() {
+ mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
+}
+
+mptcp_lib_pr_skip() {
+ mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"
+}
+
+mptcp_lib_pr_fail() {
+ mptcp_lib_print_err "[FAIL]${1:+ ${*}}"
+}
+
+mptcp_lib_pr_info() {
+ mptcp_lib_print_info "INFO: ${*}"
+}
+
# SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all
# features using the last version of the kernel and the selftests to make sure
# a test is not being skipped by mistake.
@@ -78,14 +95,14 @@ mptcp_lib_has_file() {
mptcp_lib_check_mptcp() {
if ! mptcp_lib_has_file "/proc/sys/net/mptcp/enabled"; then
- echo "SKIP: MPTCP support is not available"
+ mptcp_lib_pr_skip "MPTCP support is not available"
exit ${KSFT_SKIP}
fi
}
mptcp_lib_check_kallsyms() {
if ! mptcp_lib_has_file "/proc/kallsyms"; then
- echo "SKIP: CONFIG_KALLSYMS is missing"
+ mptcp_lib_pr_skip "CONFIG_KALLSYMS is missing"
exit ${KSFT_SKIP}
fi
}
@@ -292,7 +309,7 @@ mptcp_lib_check_transfer() {
local what="${3}"
if ! cmp "$in" "$out" > /dev/null 2>&1; then
- echo "[ FAIL ] $what does not match (in, out):"
+ mptcp_lib_pr_fail "$what does not match (in, out):"
mptcp_lib_print_file_err "$in"
mptcp_lib_print_file_err "$out"
@@ -327,24 +344,24 @@ mptcp_lib_check_tools() {
case "${tool}" in
"ip")
if ! ip -Version &> /dev/null; then
- mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
+ mptcp_lib_pr_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"
+ mptcp_lib_pr_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}"
+ mptcp_lib_pr_skip "Could not run all tests without ${tool}"
exit ${KSFT_SKIP}
fi
;;
*)
- mptcp_lib_print_err "Internal error: unsupported tool: ${tool}"
+ mptcp_lib_pr_fail "Internal error: unsupported tool: ${tool}"
exit ${KSFT_FAIL}
;;
esac
@@ -363,13 +380,13 @@ mptcp_lib_check_output() {
fi
if [ ${cmd_ret} -ne 0 ]; then
- mptcp_lib_print_err "[FAIL] command execution '${cmd}' stderr"
+ mptcp_lib_pr_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}'"
+ mptcp_lib_pr_fail "expected '${expected}' got '${out}'"
return 1
fi
}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index a797e13d3fe7..d695561fbf46 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -103,7 +103,8 @@ check_mark()
local v
for v in $values; do
if [ $v -ne 0 ]; then
- echo "FAIL: got $tables $values in ns $ns , not 0 - not all expected packets marked" 1>&2
+ mptcp_lib_pr_fail "got $tables $values in ns $ns," \
+ "not 0 - not all expected packets marked"
ret=1
return 1
fi
@@ -175,7 +176,7 @@ do_transfer()
ret=1
return 1
fi
- echo "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_print_title "Mark ${ip}"
if [ $local_addr = "::" ];then
@@ -193,10 +194,10 @@ do_transfer()
mptcp_lib_result_code "${rets}" "transfer ${ip}"
if [ $retc -eq 0 ] && [ $rets -eq 0 ];then
- echo "[ OK ]"
+ mptcp_lib_pr_ok
return 0
fi
- echo "FAIL: Mark ${ip}"
+ mptcp_lib_pr_fail "Mark ${ip}"
return 1
}
@@ -217,7 +218,7 @@ do_mptcp_sockopt_tests()
local lret=0
if ! mptcp_lib_kallsyms_has "mptcp_diag_fill_info$"; then
- echo "INFO: MPTCP sockopt not supported: SKIP"
+ mptcp_lib_pr_skip "MPTCP sockopt not supported"
mptcp_lib_result_skip "sockopt"
return
fi
@@ -227,12 +228,12 @@ do_mptcp_sockopt_tests()
mptcp_lib_print_title "SOL_MPTCP sockopt v4"
if [ $lret -ne 0 ]; then
- echo "FAIL: SOL_MPTCP getsockopt" 1>&2
+ mptcp_lib_pr_fail "SOL_MPTCP getsockopt"
mptcp_lib_result_fail "sockopt v4"
ret=$lret
return
fi
- echo "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "sockopt v4"
ip netns exec "$ns_sbox" ./mptcp_sockopt -6
@@ -240,12 +241,12 @@ do_mptcp_sockopt_tests()
mptcp_lib_print_title "SOL_MPTCP sockopt v6"
if [ $lret -ne 0 ]; then
- echo "FAIL: SOL_MPTCP getsockopt (v6)" 1>&2
+ mptcp_lib_pr_fail "SOL_MPTCP getsockopt (v6)"
mptcp_lib_result_fail "sockopt v6"
ret=$lret
return
fi
- echo "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "sockopt v6"
}
@@ -273,12 +274,12 @@ do_tcpinq_test()
local lret=$?
if [ $lret -ne 0 ];then
ret=$lret
- echo "FAIL: mptcp_inq $*" 1>&2
+ mptcp_lib_pr_fail "mptcp_inq $*"
mptcp_lib_result_fail "TCP_INQ: $*"
return $lret
fi
- echo "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "TCP_INQ: $*"
return $lret
}
@@ -288,7 +289,7 @@ do_tcpinq_tests()
local lret=0
if ! mptcp_lib_kallsyms_has "mptcp_ioctl$"; then
- echo "INFO: TCP_INQ not supported: SKIP"
+ mptcp_lib_pr_skip "INFO: TCP_INQ not supported"
mptcp_lib_result_skip "TCP_INQ"
return
fi
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 5b9bc25dfef4..69ffff8b076b 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -190,7 +190,7 @@ else
for st in fullmesh nofullmesh backup,fullmesh; do
st=" (${st})"
mptcp_lib_print_title "${st}"
- echo "[SKIP]"
+ mptcp_lib_pr_skip
mptcp_lib_result_skip "${st}"
done
fi
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 212782301c6f..c5dbe083e47f 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -185,12 +185,12 @@ do_transfer()
printf "%-10s" " max $max_time "
if [ $retc -eq 0 ] && [ $rets -eq 0 ] && \
[ $cmpc -eq 0 ] && [ $cmps -eq 0 ]; then
- echo "[ OK ]"
+ mptcp_lib_pr_ok
cat "$capout"
return 0
fi
- echo " [ fail ]"
+ mptcp_lib_pr_fail
echo "client exit code $retc, server $rets" 1>&2
echo -e "\nnetns ${ns3} socket stat for $port:" 1>&2
ip netns exec ${ns3} ss -nita 1>&2 -o "sport = :$port"
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 62e059e3fb24..0c5ba35f6af4 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -61,7 +61,7 @@ _printf() {
print_title()
{
- _printf "INFO: %s\n" "${1}"
+ mptcp_lib_pr_info "${1}"
}
# $1: test name
@@ -73,33 +73,23 @@ print_test()
mptcp_lib_print_title "${test_name}"
}
-print_results()
-{
- _printf "[%s]\n" "${1}"
-}
-
test_pass()
{
- print_results " OK "
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "${test_name}"
}
test_skip()
{
- print_results "SKIP"
+ mptcp_lib_pr_skip
mptcp_lib_result_skip "${test_name}"
}
# $1: msg
test_fail()
{
- print_results "FAIL"
+ mptcp_lib_pr_fail "${@}"
ret=1
-
- if [ -n "${1}" ]; then
- _printf "\t%s\n" "${1}"
- fi
-
mptcp_lib_result_fail "${test_name}"
}
--
2.40.1On 29/02/2024 10:51, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> To unify the output formats of all test scripts, this patch adds
> four more helpers:
>
> mptcp_lib_pr_ok()
> mptcp_lib_pr_skip()
> mptcp_lib_pr_fail()
> mptcp_lib_pr_info()
>
> to print out [ OK ], [SKIP], [FAIL] and 'INFO: ' with colors. Use them
> in all scripts to print the "ok/skip/fail/info' using the same 'format'.
>
> Having colors helps to quickly identify issues when looking at a long
> list of output logs and results.
>
> Note that the mptcp_join.sh tests will now print these keywords with
> capital letters, like most of the other tests.
I think it is not just mptcp_join.sh that was using different keywords.
Note that now all print the same keywords, which was not the case
before, but it is good to uniform that.
(...)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 096ff8941c5b..70acbb19f568 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
(...)
> @@ -534,7 +534,6 @@ do_transfer()
> "${expect_ackrx}" "${stat_ackrx_now_l}"
> fi
>
> - echo
Just to be sure: do we still have something good in case of error?
I mean: why were we not printing a new line before? Maybe there was no
reason, or maybe sometimes we print more details? (syn cookies?)
Maybe that's fine with the new output, it is just to know if you checked
that.
> cat "$capout"
> [ $retc -eq 0 ] && [ $rets -eq 0 ]
> }
(...)
> @@ -810,7 +809,7 @@ log_if_error()
> local msg="$1"
>
> if [ ${ret} -ne 0 ]; then
> - echo "FAIL: ${msg}" 1>&2
> + mptcp_lib_pr_fail "${msg}"
Good to mention in the commit message that all errors messages are
printed to 'stdout' now, as part of the uniformation. (or do that in an
dedicated commit)
>
> final_ret=${ret}
> ret=0
> @@ -858,11 +857,11 @@ done
> mptcp_lib_result_code "${ret}" "ping tests"
>
> stop_if_error "Could not even run ping tests"
> -echo "[ OK ]"
> +mptcp_lib_pr_ok
> MPTCP_LIB_TEST_FORMAT="%02u %-50s"
>
> [ -n "$tc_loss" ] && tc -net "$ns2" qdisc add dev ns2eth3 root netem loss random $tc_loss delay ${tc_delay}ms
> -echo -n "INFO: Using loss of $tc_loss "
Here, it was using 'echo -n' ...
> +mptcp_lib_pr_info "Using loss of $tc_loss "
Now a new line will be printed at the end (with a trailing whitespace)
> test "$tc_delay" -gt 0 && echo -n "delay $tc_delay ms "
And this will be printed in a new line, it will look strange I think
Maybe use a variable to store all the info?
tc_info="loss of $tc_loss "
test "$tc_delay" -gt 0 && tc_info+="delay $tc_delay ms "
(...)
mptcp_lib_pr_info "Using ${tc_info}"
>
> reorder_delay=$((tc_delay / 4))
(...)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 358d5b77fc0f..53867f9d212d 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -48,6 +48,23 @@ mptcp_lib_print_err() {
> mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
> }
>
> +#shellcheck disable=SC2120
Add a space after '#' and add a comment justifying the exception, like
the one I shared in patch 1/9 from v6:
# shellcheck disable=SC2120 # parameters are optional
Out of curiosity, we don't need this for 'skip' and 'fail'?
> +mptcp_lib_pr_ok() {
> + mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> +}
> +
> +mptcp_lib_pr_skip() {
> + mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"
> +}
> +
> +mptcp_lib_pr_fail() {
> + mptcp_lib_print_err "[FAIL]${1:+ ${*}}"
> +}
> +
> +mptcp_lib_pr_info() {
> + mptcp_lib_print_info "INFO: ${*}"
> +}
> +
> # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all
> # features using the last version of the kernel and the selftests to make sure
> # a test is not being skipped by mistake.
(...)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index a797e13d3fe7..d695561fbf46 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
(...)
> @@ -217,7 +218,7 @@ do_mptcp_sockopt_tests()
> local lret=0
>
> if ! mptcp_lib_kallsyms_has "mptcp_diag_fill_info$"; then
> - echo "INFO: MPTCP sockopt not supported: SKIP"
> + mptcp_lib_pr_skip "MPTCP sockopt not supported"
> mptcp_lib_result_skip "sockopt"
> return
> fi
> @@ -227,12 +228,12 @@ do_mptcp_sockopt_tests()
>
> mptcp_lib_print_title "SOL_MPTCP sockopt v4"
> if [ $lret -ne 0 ]; then
> - echo "FAIL: SOL_MPTCP getsockopt" 1>&2
> + mptcp_lib_pr_fail "SOL_MPTCP getsockopt"
No need to repeat what is already in the title. (I didn't check them
all, but I guess some other subtests here repeat stuff already in the
title, e.g. the same for v6)
> mptcp_lib_result_fail "sockopt v4"
> ret=$lret
> return
> fi
> - echo "[ OK ]"
> + mptcp_lib_pr_ok
> mptcp_lib_result_pass "sockopt v4"
>
> ip netns exec "$ns_sbox" ./mptcp_sockopt -6
(...)
> @@ -288,7 +289,7 @@ do_tcpinq_tests()
> local lret=0
>
> if ! mptcp_lib_kallsyms_has "mptcp_ioctl$"; then
> - echo "INFO: TCP_INQ not supported: SKIP"
> + mptcp_lib_pr_skip "INFO: TCP_INQ not supported"
You can remove 'INFO: ' I suppose.
> mptcp_lib_result_skip "TCP_INQ"
> return
> fi
(...)
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
On Thu, 2024-02-29 at 14:04 +0100, Matthieu Baerts wrote:
> On 29/02/2024 10:51, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> >
> > To unify the output formats of all test scripts, this patch adds
> > four more helpers:
> >
> > mptcp_lib_pr_ok()
> > mptcp_lib_pr_skip()
> > mptcp_lib_pr_fail()
> > mptcp_lib_pr_info()
> >
> > to print out [ OK ], [SKIP], [FAIL] and 'INFO: ' with colors. Use
> > them
> > in all scripts to print the "ok/skip/fail/info' using the same
> > 'format'.
> >
> > Having colors helps to quickly identify issues when looking at a
> > long
> > list of output logs and results.
> >
> > Note that the mptcp_join.sh tests will now print these keywords
> > with
> > capital letters, like most of the other tests.
>
> I think it is not just mptcp_join.sh that was using different
> keywords.
>
> Note that now all print the same keywords, which was not the case
> before, but it is good to uniform that.
>
> (...)
>
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > index 096ff8941c5b..70acbb19f568 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>
> (...)
>
> > @@ -534,7 +534,6 @@ do_transfer()
> > "${expect_ackrx}" "${stat_ackrx_now_l}"
> > fi
> >
> > - echo
>
> Just to be sure: do we still have something good in case of error?
>
> I mean: why were we not printing a new line before? Maybe there was
> no
> reason, or maybe sometimes we print more details? (syn cookies?)
>
> Maybe that's fine with the new output, it is just to know if you
> checked
> that.
This 'echo' does need to be dropped. Here use mptcp_lib_pr_ok instead
of 'printf "[ OK ]"'. 'printf "[ OK ]"' needs a new line but
mptcp_lib_pr_ok don't need:
if [ $retc -eq 0 ] && [ $rets -eq 0 ]; then
- printf "[ OK ]"
+ mptcp_lib_pr_ok
mptcp_lib_result_pass "${TEST_GROUP}: ${result_msg}"
else
mptcp_lib_result_fail "${TEST_GROUP}: ${result_msg}"
@@ -540,7 +540,6 @@ do_transfer()
"${expect_ackrx}" "${stat_ackrx_now_l}"
fi
- echo
cat "$capout"
>
> > cat "$capout"
> > [ $retc -eq 0 ] && [ $rets -eq 0 ]
> > }
>
> (...)
>
> > @@ -810,7 +809,7 @@ log_if_error()
> > local msg="$1"
> >
> > if [ ${ret} -ne 0 ]; then
> > - echo "FAIL: ${msg}" 1>&2
> > + mptcp_lib_pr_fail "${msg}"
>
> Good to mention in the commit message that all errors messages are
> printed to 'stdout' now, as part of the uniformation. (or do that in
> an
> dedicated commit)
A new commit added in v9.
>
> >
> > final_ret=${ret}
> > ret=0
> > @@ -858,11 +857,11 @@ done
> > mptcp_lib_result_code "${ret}" "ping tests"
> >
> > stop_if_error "Could not even run ping tests"
> > -echo "[ OK ]"
> > +mptcp_lib_pr_ok
> > MPTCP_LIB_TEST_FORMAT="%02u %-50s"
> >
> > [ -n "$tc_loss" ] && tc -net "$ns2" qdisc add dev ns2eth3 root
> > netem loss random $tc_loss delay ${tc_delay}ms
> > -echo -n "INFO: Using loss of $tc_loss "
>
> Here, it was using 'echo -n' ...
I keep this 'echo -n' unchanged.
>
> > +mptcp_lib_pr_info "Using loss of $tc_loss "
>
> Now a new line will be printed at the end (with a trailing
> whitespace)
>
> > test "$tc_delay" -gt 0 && echo -n "delay $tc_delay ms "
>
> And this will be printed in a new line, it will look strange I think
>
> Maybe use a variable to store all the info?
>
> tc_info="loss of $tc_loss "
> test "$tc_delay" -gt 0 && tc_info+="delay $tc_delay ms "
> (...)
> mptcp_lib_pr_info "Using ${tc_info}"
>
> >
> > reorder_delay=$((tc_delay / 4))
>
> (...)
>
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > index 358d5b77fc0f..53867f9d212d 100644
> > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > @@ -48,6 +48,23 @@ mptcp_lib_print_err() {
> > mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
> > }
> >
> > +#shellcheck disable=SC2120
>
> Add a space after '#' and add a comment justifying the exception,
> like
> the one I shared in patch 1/9 from v6:
>
> # shellcheck disable=SC2120 # parameters are optional
>
> Out of curiosity, we don't need this for 'skip' and 'fail'?
Not a single parameter was passed to mptcp_lib_pr_ok at a time.But 'skip' or 'fail' sometimes has parameters passed in.
Thanks,
-Geliang
>
> > +mptcp_lib_pr_ok() {
> > + mptcp_lib_print_ok "[ OK ]${1:+ ${*}}"
> > +}
> > +
> > +mptcp_lib_pr_skip() {
> > + mptcp_lib_print_warn "[SKIP]${1:+ ${*}}"
> > +}
> > +
> > +mptcp_lib_pr_fail() {
> > + mptcp_lib_print_err "[FAIL]${1:+ ${*}}"
> > +}
> > +
> > +mptcp_lib_pr_info() {
> > + mptcp_lib_print_info "INFO: ${*}"
> > +}
> > +
> > # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when
> > validating all
> > # features using the last version of the kernel and the selftests
> > to make sure
> > # a test is not being skipped by mistake.
>
> (...)
>
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > index a797e13d3fe7..d695561fbf46 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>
> (...)
>
> > @@ -217,7 +218,7 @@ do_mptcp_sockopt_tests()
> > local lret=0
> >
> > if ! mptcp_lib_kallsyms_has "mptcp_diag_fill_info$"; then
> > - echo "INFO: MPTCP sockopt not supported: SKIP"
> > + mptcp_lib_pr_skip "MPTCP sockopt not supported"
> > mptcp_lib_result_skip "sockopt"
> > return
> > fi
> > @@ -227,12 +228,12 @@ do_mptcp_sockopt_tests()
> >
> > mptcp_lib_print_title "SOL_MPTCP sockopt v4"
> > if [ $lret -ne 0 ]; then
> > - echo "FAIL: SOL_MPTCP getsockopt" 1>&2
> > + mptcp_lib_pr_fail "SOL_MPTCP getsockopt"
>
> No need to repeat what is already in the title. (I didn't check them
> all, but I guess some other subtests here repeat stuff already in the
> title, e.g. the same for v6)
>
> > mptcp_lib_result_fail "sockopt v4"
> > ret=$lret
> > return
> > fi
> > - echo "[ OK ]"
> > + mptcp_lib_pr_ok
> > mptcp_lib_result_pass "sockopt v4"
> >
> > ip netns exec "$ns_sbox" ./mptcp_sockopt -6
>
> (...)
>
> > @@ -288,7 +289,7 @@ do_tcpinq_tests()
> > local lret=0
> >
> > if ! mptcp_lib_kallsyms_has "mptcp_ioctl$"; then
> > - echo "INFO: TCP_INQ not supported: SKIP"
> > + mptcp_lib_pr_skip "INFO: TCP_INQ not supported"
>
> You can remove 'INFO: ' I suppose.
>
> > mptcp_lib_result_skip "TCP_INQ"
> > return
> > fi
>
> (...)
>
> Cheers,
> Matt
Hi Geliang,
On 01/03/2024 08:16, Geliang Tang wrote:
> On Thu, 2024-02-29 at 14:04 +0100, Matthieu Baerts wrote:
>> On 29/02/2024 10:51, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> To unify the output formats of all test scripts, this patch adds
>>> four more helpers:
>>>
>>> mptcp_lib_pr_ok()
>>> mptcp_lib_pr_skip()
>>> mptcp_lib_pr_fail()
>>> mptcp_lib_pr_info()
>>>
>>> to print out [ OK ], [SKIP], [FAIL] and 'INFO: ' with colors. Use
>>> them
>>> in all scripts to print the "ok/skip/fail/info' using the same
>>> 'format'.
>>>
>>> Having colors helps to quickly identify issues when looking at a
>>> long
>>> list of output logs and results.
>>>
>>> Note that the mptcp_join.sh tests will now print these keywords
>>> with
>>> capital letters, like most of the other tests.
>>
>> I think it is not just mptcp_join.sh that was using different
>> keywords.
>>
>> Note that now all print the same keywords, which was not the case
>> before, but it is good to uniform that.
>>
>> (...)
>>
>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>>> b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>>> index 096ff8941c5b..70acbb19f568 100755
>>> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>>
>> (...)
>>
>>> @@ -534,7 +534,6 @@ do_transfer()
>>> "${expect_ackrx}" "${stat_ackrx_now_l}"
>>> fi
>>>
>>> - echo
>>
>> Just to be sure: do we still have something good in case of error?
>>
>> I mean: why were we not printing a new line before? Maybe there was
>> no
>> reason, or maybe sometimes we print more details? (syn cookies?)
>>
>> Maybe that's fine with the new output, it is just to know if you
>> checked
>> that.
>
> This 'echo' does need to be dropped. Here use mptcp_lib_pr_ok instead
> of 'printf "[ OK ]"'. 'printf "[ OK ]"' needs a new line but
> mptcp_lib_pr_ok don't need:
Yes, but my point was: I guess there was a reason to have:
print "[ OK ]"
(...)
echo
and not just one line with:
echo "[ OK ]"
After a quick look, it looks like that's because we want to add info
depending on some conditions, see all the "WARN" messages in between.
I guess we should instead move the "pr_ok" to the end, something like:
local extra
(...)
### <= the 'printf [ OK ]' was here, now moved to the end ↓
if (...); then
extra+=" WARN (...)"
fi
(...)
if [ $retc -eq 0 ] && [ $rets -eq 0 ]; then
mptcp_lib_pr_ok "${extra:1}"
mptcp_lib_result_pass "${TEST_GROUP}: ${result_msg}"
else
if [ -n "${extra}" ]; then
mptcp_lib_print_warn "${extra:1}"
fi
mptcp_lib_result_fail "${TEST_GROUP}: ${result_msg}"
fi
cat "$capout"
[ $retc -eq 0 ] && [ $rets -eq 0 ]
no?
>
> if [ $retc -eq 0 ] && [ $rets -eq 0 ]; then
> - printf "[ OK ]"
> + mptcp_lib_pr_ok
> mptcp_lib_result_pass "${TEST_GROUP}: ${result_msg}"
> else
> mptcp_lib_result_fail "${TEST_GROUP}: ${result_msg}"
> @@ -540,7 +540,6 @@ do_transfer()
> "${expect_ackrx}" "${stat_ackrx_now_l}"
> fi
>
> - echo
> cat "$capout"
>
>>
>>> cat "$capout"
>>> [ $retc -eq 0 ] && [ $rets -eq 0 ]
>>> }
>>
>> (...)
>>
>>> @@ -810,7 +809,7 @@ log_if_error()
>>> local msg="$1"
>>>
>>> if [ ${ret} -ne 0 ]; then
>>> - echo "FAIL: ${msg}" 1>&2
>>> + mptcp_lib_pr_fail "${msg}"
>>
>> Good to mention in the commit message that all errors messages are
>> printed to 'stdout' now, as part of the uniformation. (or do that in
>> an
>> dedicated commit)
>
> A new commit added in v9.
Thanks!
>>
>>>
>>> final_ret=${ret}
>>> ret=0
>>> @@ -858,11 +857,11 @@ done
>>> mptcp_lib_result_code "${ret}" "ping tests"
>>>
>>> stop_if_error "Could not even run ping tests"
>>> -echo "[ OK ]"
>>> +mptcp_lib_pr_ok
>>> MPTCP_LIB_TEST_FORMAT="%02u %-50s"
>>>
>>> [ -n "$tc_loss" ] && tc -net "$ns2" qdisc add dev ns2eth3 root
>>> netem loss random $tc_loss delay ${tc_delay}ms
>>> -echo -n "INFO: Using loss of $tc_loss "
>>
>> Here, it was using 'echo -n' ...
>
> I keep this 'echo -n' unchanged.
You don't use pr_info with the appended message, as suggested below?
>
>>
>>> +mptcp_lib_pr_info "Using loss of $tc_loss "
>>
>> Now a new line will be printed at the end (with a trailing
>> whitespace)
>>
>>> test "$tc_delay" -gt 0 && echo -n "delay $tc_delay ms "
>>
>> And this will be printed in a new line, it will look strange I think
>>
>> Maybe use a variable to store all the info?
>>
>> tc_info="loss of $tc_loss "
>> test "$tc_delay" -gt 0 && tc_info+="delay $tc_delay ms "
>> (...)
>> mptcp_lib_pr_info "Using ${tc_info}"
>>
>>>
>>> reorder_delay=$((tc_delay / 4))
>>
>> (...)
>>
>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> index 358d5b77fc0f..53867f9d212d 100644
>>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>> @@ -48,6 +48,23 @@ mptcp_lib_print_err() {
>>> mptcp_lib_print_color "${MPTCP_LIB_COLOR_RED}${*}"
>>> }
>>>
>>> +#shellcheck disable=SC2120
>>
>> Add a space after '#' and add a comment justifying the exception,
>> like
>> the one I shared in patch 1/9 from v6:
>>
>> # shellcheck disable=SC2120 # parameters are optional
>>
>> Out of curiosity, we don't need this for 'skip' and 'fail'?
>
> Not a single parameter was passed to mptcp_lib_pr_ok at a time.
> But 'skip' or 'fail' sometimes has parameters passed in.
OK, I see. It is just strange to put it only on one, and it would not
hurt to put it on the others as well, but OK, anyway a false positive
from shellcheck.
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
© 2016 - 2024 Red Hat, Inc.