From d17b968b98761e30b94177f0f9a2f5d9aae15da2 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Thu, 17 Feb 2022 19:03:05 -0800 Subject: [PATCH 1/7] selftests: mptcp: increase timeout to 20 minutes With the increase number of tests, one CI instance, using a debug kernel config and not recent hardware, takes around 10 minutes to execute the slowest MPTCP test: mptcp_join.sh. Even if most CIs don't take that long to execute these tests -- typically max 10 minutes to run all selftests -- it will help some of them if the timeout is increased. The timeout could be disabled but it is always good to have an extra safeguard, just in case. Please note that on slow public CIs with kernel debug settings, it has been observed it can easily take up to 45 minutes to execute all tests in this very slow environment with other jobs running in parallel. The slowest test, mptcp_join.sh takes ~30 minutes in this case. In such environments, the selftests timeout set in the 'settings' file is disabled because this environment is known as being exceptionnally slow. It has been decided not to take such exceptional environments into account and set the timeout to 20min. Signed-off-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/settings | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/settings b/tools/testing/selftests/net/mptcp/settings index a62d2fa1275c..79b65bdf05db 100644 --- a/tools/testing/selftests/net/mptcp/settings +++ b/tools/testing/selftests/net/mptcp/settings @@ -1 +1 @@ -timeout=600 +timeout=1200 From bccefb7624395183e5602d168f4343b9ddbb72b9 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 17 Feb 2022 19:03:06 -0800 Subject: [PATCH 2/7] selftests: mptcp: simplify pm_nl_change_endpoint This patch simplified pm_nl_change_endpoint(), using id-based address lookups only. And dropped the fragile way of parsing 'addr' and 'id' from the output of pm_nl_show_endpoints(). Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- .../testing/selftests/net/mptcp/mptcp_join.sh | 37 ++++--------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 18bb0d0cf4bd..bbcacaaf81ce 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -240,16 +240,6 @@ is_v6() [ -z "${1##*:*}" ] } -is_addr() -{ - [ -z "${1##*[.:]*}" ] -} - -is_number() -{ - [[ $1 == ?(-)+([0-9]) ]] -} - # $1: ns, $2: port wait_local_port_listen() { @@ -379,16 +369,13 @@ pm_nl_show_endpoints() pm_nl_change_endpoint() { local ns=$1 - local flags=$2 - local id=$3 - local addr=$4 - local port="" + local id=$2 + local flags=$3 if [ $ip_mptcp -eq 1 ]; then ip -n $ns mptcp endpoint change id $id ${flags//","/" "} else - if [ $5 -ne 0 ]; then port="port $5"; fi - ip netns exec $ns ./pm_nl_ctl set $addr flags $flags $port + ip netns exec $ns ./pm_nl_ctl set id $id flags $flags fi } @@ -591,24 +578,16 @@ do_transfer() for netns in "$ns1" "$ns2"; do pm_nl_show_endpoints $netns | while read line; do local arr=($line) - local addr - local port=0 + local nr=0 local id for i in ${arr[@]}; do - if is_addr $i; then - addr=$i - elif is_number $i; then - # The minimum expected port number is 10000 - if [ $i -gt 10000 ]; then - port=$i - # The maximum id number is 255 - elif [ $i -lt 255 ]; then - id=$i - fi + if [ $i = "id" ]; then + id=${arr[$nr+1]} fi + let nr+=1 done - pm_nl_change_endpoint $netns $sflags $id $addr $port + pm_nl_change_endpoint $netns $id $sflags done done fi From 22514d52962b58771ac3eb61f8c4573617d1d73d Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Thu, 17 Feb 2022 19:03:07 -0800 Subject: [PATCH 3/7] selftests: mptcp: join: exit after usage() With an error if it is an unknown option. Signed-off-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index bbcacaaf81ce..1a881a21e7ef 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -2077,8 +2077,14 @@ all_tests() fullmesh_tests } +# [$1: error message] usage() { + if [ -n "${1}" ]; then + echo "${1}" + ret=1 + fi + echo "mptcp_join usage:" echo " -f subflows_tests" echo " -e subflows_error_tests" @@ -2099,6 +2105,8 @@ usage() echo " -C enable data checksum" echo " -i use ip mptcp" echo " -h help" + + exit ${ret} } sin=$(mktemp) @@ -2187,9 +2195,12 @@ while getopts 'fesltra64bpkdmchCSi' opt; do ;; i) ;; - h | *) + h) usage ;; + *) + usage "Unknown option: -${opt}" + ;; esac done From 0a40e273be0416a9a00ecea89b7f61c841382b3e Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Thu, 17 Feb 2022 19:03:08 -0800 Subject: [PATCH 4/7] selftests: mptcp: join: remove unused vars Shellcheck found that these variables were set but never used. Note that rndh is no longer prefixed with '0-' but it doesn't change anything. Signed-off-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/mptcp/mptcp_join.sh | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 1a881a21e7ef..c6379093f38a 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -42,7 +42,7 @@ init() { capout=$(mktemp) - rndh=$(printf %x $sec)-$(mktemp -u XXXXXX) + rndh=$(mktemp -u XXXXXX) ns1="ns1-$rndh" ns2="ns2-$rndh" @@ -665,8 +665,6 @@ run_tests() addr_nr_ns2="${6:-0}" speed="${7:-fast}" sflags="${8:-""}" - lret=0 - oldin="" # create the input file for the failure test when # the first failure test run @@ -694,7 +692,6 @@ run_tests() do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr} \ ${test_linkfail} ${addr_nr_ns1} ${addr_nr_ns2} ${speed} ${sflags} - lret=$? } dump_stats() From 93827ad58f6296bf15fa72265e55fb0f335fcf84 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Thu, 17 Feb 2022 19:03:09 -0800 Subject: [PATCH 5/7] selftests: mptcp: join: create tmp files only if needed These tmp files will only be created when a test will be launched. This avoid 'dd' output when '-h' is used for example. While at it, also avoid creating netns that will be removed when starting the first test. Signed-off-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- .../testing/selftests/net/mptcp/mptcp_join.sh | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index c6379093f38a..63340bb76920 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -17,6 +17,7 @@ capture=0 checksum=0 ip_mptcp=0 do_all_tests=1 +init=0 TEST_COUNT=0 @@ -38,7 +39,7 @@ CBPF_MPTCP_SUBOPTION_ADD_ADDR="14, 6 0 0 65535, 6 0 0 0" -init() +init_partial() { capout=$(mktemp) @@ -98,6 +99,21 @@ cleanup_partial() done } +init() { + init=1 + + sin=$(mktemp) + sout=$(mktemp) + cin=$(mktemp) + cinsent=$(mktemp) + cout=$(mktemp) + + trap cleanup EXIT + + make_file "$cin" "client" 1 + make_file "$sin" "server" 1 +} + cleanup() { rm -f "$cin" "$cout" "$sinfail" @@ -107,8 +123,13 @@ cleanup() reset() { - cleanup_partial - init + if [ "${init}" != "1" ]; then + init + else + cleanup_partial + fi + + init_partial } reset_with_cookies() @@ -2106,16 +2127,6 @@ usage() exit ${ret} } -sin=$(mktemp) -sout=$(mktemp) -cin=$(mktemp) -cinsent=$(mktemp) -cout=$(mktemp) -init -make_file "$cin" "client" 1 -make_file "$sin" "server" 1 -trap cleanup EXIT - for arg in "$@"; do # check for "capture/checksum" args before launching tests if [[ "${arg}" =~ ^"-"[0-9a-zA-Z]*"c"[0-9a-zA-Z]*$ ]]; then From 87154755d90ed60919cc5709e322b397701e4f58 Mon Sep 17 00:00:00 2001 From: Matthieu Baerts Date: Thu, 17 Feb 2022 19:03:10 -0800 Subject: [PATCH 6/7] selftests: mptcp: join: check for tools only if needed To allow showing the 'help' menu even if these tools are not available. While at it, also avoid launching the command then checking $?. Instead, the check is directly done in the 'if'. Signed-off-by: Matthieu Baerts Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- .../testing/selftests/net/mptcp/mptcp_join.sh | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 63340bb76920..725924012b41 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -99,9 +99,29 @@ cleanup_partial() done } +check_tools() +{ + if ! ip -Version &> /dev/null; 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 + fi + + if ! ip6tables -V &> /dev/null; then + echo "SKIP: Could not run all tests without ip6tables tool" + exit $ksft_skip + fi +} + init() { init=1 + check_tools + sin=$(mktemp) sout=$(mktemp) cin=$(mktemp) @@ -183,24 +203,6 @@ reset_with_allow_join_id0() ip netns exec $ns2 sysctl -q net.mptcp.allow_join_initial_addr_port=$ns2_enable } -ip -Version > /dev/null 2>&1 -if [ $? -ne 0 ];then - echo "SKIP: Could not run test without ip tool" - exit $ksft_skip -fi - -iptables -V > /dev/null 2>&1 -if [ $? -ne 0 ];then - echo "SKIP: Could not run all tests without iptables tool" - exit $ksft_skip -fi - -ip6tables -V > /dev/null 2>&1 -if [ $? -ne 0 ];then - echo "SKIP: Could not run all tests without ip6tables tool" - exit $ksft_skip -fi - print_file_err() { ls -l "$1" 1>&2 From 24720d7452df2dff2e539d9dff28904e25bb1c6d Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 17 Feb 2022 19:03:11 -0800 Subject: [PATCH 7/7] selftests: mptcp: add csum mib check for mptcp_connect This patch added the data checksum error mib counters check for the script mptcp_connect.sh when the data checksum is enabled. In do_transfer(), got the mib counters twice, before and after running the mptcp_connect commands. The latter minus the former is the actual number of the data checksum mib counter. The output looks like this: ns1 MPTCP -> ns2 (dead:beef:1::2:10007) MPTCP (duration 86ms) [ OK ] ns1 MPTCP -> ns2 (10.0.2.1:10008 ) MPTCP (duration 66ms) [ FAIL ] server got 1 data checksum error[s] Fixes: 94d66ba1d8e48 ("selftests: mptcp: enable checksum in mptcp_connect.sh") Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/255 Signed-off-by: Geliang Tang Signed-off-by: Mat Martineau Signed-off-by: Jakub Kicinski --- .../selftests/net/mptcp/mptcp_connect.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh index cb5809b89081..5b7a40d73253 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh @@ -432,6 +432,8 @@ do_transfer() local stat_ackrx_last_l=$(get_mib_counter "${listener_ns}" "MPTcpExtMPCapableACKRX") local stat_cookietx_last=$(get_mib_counter "${listener_ns}" "TcpExtSyncookiesSent") local stat_cookierx_last=$(get_mib_counter "${listener_ns}" "TcpExtSyncookiesRecv") + local stat_csum_err_s=$(get_mib_counter "${listener_ns}" "MPTcpExtDataCsumErr") + local stat_csum_err_c=$(get_mib_counter "${connector_ns}" "MPTcpExtDataCsumErr") timeout ${timeout_test} \ ip netns exec ${listener_ns} \ @@ -524,6 +526,23 @@ do_transfer() fi fi + if $checksum; then + local csum_err_s=$(get_mib_counter "${listener_ns}" "MPTcpExtDataCsumErr") + local csum_err_c=$(get_mib_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]" + 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]" + retc=1 + fi + fi + if [ $retc -eq 0 ] && [ $rets -eq 0 ]; then printf "[ OK ]" fi