[PATCH mptcp-next v8 0/5] add mp_fail testcases

[PATCH mptcp-next v8 0/5] add mp_fail testcases

[Geliang Tang]

v8:
 - move two MP_RST mibs patches into the "add fastclose testcases" series.
 - allow multiple checksum errors in patch 3.
 - depends on "add fastclose testcases v3".

RESEND:
 - no code modified, just updated the commit logs and comments.
v7:
 This version makes the multiple subflows test more stable.
 - Add delays and drop 'retry' in the multiple subflows test in patch 7.
 - Don't add two addresses for the multiple subflows test in patch 7, add
one address 10.0.2.2 is enough, this make it more stable.
 - Add a comment in patch 6.
 - Rebased to export/20220211T054659.
 - A 500 times loop test log of v7 will be attached.

v6:
 - Split two patches from the last one.
 - Retry the multiple subflows test three times to fix this
"MP_FAIL MP_RST: 0 corrupted pkts" failure reported by me in v5:

Created /tmp/tmp.e4nE5Q14mj (size 1024 KB) containing data sent by client
Created /tmp/tmp.QwpQYClFnm (size 1024 KB) containing data sent by server
001 MP_FAIL MP_RST: 0 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
                                         sum[fail] got 0 data checksum error[s] expected 1
                                         ftx[fail] got 0 MP_FAIL[s] TX expected 1
                                         rtx[fail] got 0 MP_RST[s] TX expected 1
                                         itx[ ok ] - infirx[ ok ]

A test log of running v6 500 times is attached, named v6-loop-500-times.log,
in it, we can see retry happend 8 times (116, 136, 236, 295, 297, 402, 444,
457), and no "0 corrupted pkts" any more.

 - Reduce the single subflow test files size from 1024KB to 128KB to fix
this "file received by client does not match" failure reported by CI and
Matt in v5:

# Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
# Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
# file received by server has inverted byte at 195585
# 100 MP_FAIL MP_RST: 1 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
#                                          sum[ ok ] - csum  [ ok ]
#                                          ftx[ ok ] - failrx[ ok ]
#                                          rtx[ ok ] - rstrx [ ok ]
#                                          itx[ ok ] - infirx[ ok ]
# Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
# Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
# [ FAIL ] file received by client does not match (in, out):
# -rw------- 1 root root 1048604 Feb  9 11:37 /tmp/tmp.jFbZEAnYZa
# Trailing bytes are:
# MPTCP_TEST_FILE_END_MARKER
# -rw------- 1 root root 1048606 Feb  9 11:37 /tmp/tmp.ghV0iWPhu5
# Trailing bytes are:
# MPTCP_TEST_FILE_END_MARKER
# file received by server has inverted byte at 169
# 101 Infinite map: 5 corrupted pkts       syn[ ok ] - synack[ ok ] - ack[ ok ]
#                                          sum[ ok ] - csum  [ ok ]
#                                          ftx[ ok ] - failrx[ ok ]
#                                          rtx[ ok ] - rstrx [ ok ]
#                                          itx[ ok ] - infirx[ ok ]

In the attached v6-loop-500-times.log, no "file received by client does
not match" any more.

I think this v6 is very stable, but there are still 6 tests failed in the
500 time tests log (68 77 97 112 161 243). These failures are all due to
get one more unexpected checksum failure:

 > cat v6-loop-500-times.log  | grep "\[fail"
                                         sum[fail] got 2 data checksum error[s] expected 1
                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
 - failrx[fail] got 2 MP_FAIL[s] RX expected 1
                                         rtx[fail] got 2 MP_RST[s] TX expected 1
 - rstrx [fail] got 2 MP_RST[s] RX expected 1
                                         sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
                                         sum[fail] got 2 data checksum error[s] expected 1
                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
 - failrx[fail] got 2 MP_FAIL[s] RX expected 1
                                         rtx[fail] got 2 MP_RST[s] TX expected 1
 - rstrx [fail] got 2 MP_RST[s] RX expected 1
                                         sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
                                         rtx[fail] got 2 MP_RST[s] TX expected 1
 - rstrx [fail] got 2 MP_RST[s] RX expected 1
                                         sum[fail] got 2 data checksum error[s] expected 1
                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
 - failrx[fail] got 2 MP_FAIL[s] RX expected 1
                                         rtx[fail] got 2 MP_RST[s] TX expected 1
 - rstrx [fail] got 2 MP_RST[s] RX expected 1

These failures are related the checksum bug reported by me, issue #255.
When transferring a larger file, the checksum sometimes fails. Running
"./mptcp_connect.sh -C" in 10 times, we will the MP_FAILs. If we solve
issue #255 in the future, this mp_fail testcases will be more stable.

v5:
 - update patch 5 as Matt suggested.
 - use '|| exit 1'
 - drop jq
 - drop pedit_action

v4:
 - add the mibs for MP_RST
 - patch 4 "selftests: mptcp: add MP_RST mibs check" uses the variable
$nr_blank, so it depends on the commit "selftests: mptcp: adjust output
alignment for more tests".

v3:
 - check the exit code of iptables.
 - add ip6tables support for reset_with_fail too.
 - add the null check for $packets
 - rename nr_mp_fail to pedit_action and get_nr_mp_fail to
pedit_action_happened

This is v12 of the mp_fail testcases with Matt's changes. It works well
and it's very stable.

Geliang Tang (5):
  Squash to "mptcp: infinite mapping receiving"
  Squash to "selftests: mptcp: add infinite map mibs check"
  selftests: mptcp: add more arguments for chk_join_nr
  selftests: mptcp: reuse linkfail to make given size files
  selftests: mptcp: add the MP_FAIL testcases

 net/mptcp/subflow.c                           |   1 +
 tools/testing/selftests/net/mptcp/config      |   8 +
 .../testing/selftests/net/mptcp/mptcp_join.sh | 216 +++++++++++++++---
 3 files changed, 192 insertions(+), 33 deletions(-)

-- 
2.34.1

[PATCH mptcp-next v8 1/5] Squash to "mptcp: infinite mapping receiving"

[Geliang Tang]

Print out the infinite map received info.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 net/mptcp/subflow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index e727d838da0e..8d086641bdc5 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -961,6 +961,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
 
 	data_len = mpext->data_len;
 	if (data_len == 0) {
+		pr_debug("infinite mapping received");
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_INFINITEMAPRX);
 		subflow->map_data_len = 0;
 		return MAPPING_INVALID;
-- 
2.34.1

[PATCH mptcp-next v8 2/5] Squash to "selftests: mptcp: add infinite map mibs check"

[Geliang Tang]

Rename mp_infi_nr_tx, mp_infi_nr_rx and irx.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 00a35601f319..243eb7e14921 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -859,27 +859,27 @@ chk_rst_nr()
 
 chk_infi_nr()
 {
-	local mp_infi_nr_tx=$1
-	local mp_infi_nr_rx=$2
+	local infi_tx=$1
+	local infi_rx=$2
 	local count
 	local dump_stats
 
 	printf "%-${nr_blank}s %s" " " "itx"
 	count=`ip netns exec $ns2 nstat -as | grep InfiniteMapTx | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != "$mp_infi_nr_tx" ]; then
-		echo "[fail] got $count infinite map[s] TX expected $mp_infi_nr_tx"
+	if [ "$count" != "$infi_tx" ]; then
+		echo "[fail] got $count infinite map[s] TX expected $infi_tx"
 		ret=1
 		dump_stats=1
 	else
 		echo -n "[ ok ]"
 	fi
 
-	echo -n " - irx   "
+	echo -n " - infirx"
 	count=`ip netns exec $ns1 nstat -as | grep InfiniteMapRx | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != "$mp_infi_nr_rx" ]; then
-		echo "[fail] got $count infinite map[s] RX expected $mp_infi_nr_rx"
+	if [ "$count" != "$infi_rx" ]; then
+		echo "[fail] got $count infinite map[s] RX expected $infi_rx"
 		ret=1
 		dump_stats=1
 	else
-- 
2.34.1

[PATCH mptcp-next v8 3/5] selftests: mptcp: add more arguments for chk_join_nr

[Geliang Tang]

This patch added five more arguments for chk_join_nr(). The default
values of them are all zero.

The first two, csum_ns1 and csum_ns1, are passed to chk_csum_nr(), to
check the mib counters of the checksum errors in ns1 and ns2. A '+'
can be added into this two arguments to represent that multiple
checksum errors are allowed when doing this check. For example,

        chk_csum_nr "" +2 +2

indicates that two or more checksum errors are allowed in both ns1 and
ns2.

The remaining three, fail_nr, rst_nr and infi_nr, are passed to
chk_fail_nr(), chk_rst_nr() and chk_infi_nr() respectively, to check
the sending and receiving mib counters of MP_FAIL, MP_RST and the
infinite map.

Also did some cleanups in chk_fail_nr(), renamed two local variables
and updated the output message.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 76 ++++++++++++++-----
 1 file changed, 57 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 243eb7e14921..ec8d750c19eb 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -745,8 +745,21 @@ dump_stats()
 chk_csum_nr()
 {
 	local msg=${1:-""}
+	local csum_ns1=${2:-0}
+	local csum_ns2=${3:-0}
 	local count
 	local dump_stats
+	local allow_multi_errors_ns1=0
+	local allow_multi_errors_ns2=0
+
+	if [[ "${csum_ns1}" = "+"* ]]; then
+		allow_multi_errors_ns1=1
+		csum_ns1=${csum_ns1:1}
+	fi
+	if [[ "${csum_ns2}" = "+"* ]]; then
+		allow_multi_errors_ns2=1
+		csum_ns2=${csum_ns2:1}
+	fi
 
 	if [ ! -z "$msg" ]; then
 		printf "%03u" "$TEST_COUNT"
@@ -756,20 +769,40 @@ chk_csum_nr()
 	printf " %-36s %s" "$msg" "sum"
 	count=`ip netns exec $ns1 nstat -as | grep MPTcpExtDataCsumErr | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != 0 ]; then
-		echo "[fail] got $count data checksum error[s] expected 0"
-		ret=1
-		dump_stats=1
+	if [ "$count" != $csum_ns1 ]; then
+		if [ $allow_multi_errors_ns1 -eq 1 ]; then
+			# allow multiple checksum errors
+			if [ "$count" -lt $csum_ns1 ]; then
+				ret=1
+			fi
+		else
+			ret=1
+		fi
+
+		if [ $ret -eq 1 ]; then
+			echo "[fail] got $count data checksum error[s] expected $csum_ns1"
+			dump_stats=1
+		fi
 	else
 		echo -n "[ ok ]"
 	fi
 	echo -n " - csum  "
 	count=`ip netns exec $ns2 nstat -as | grep MPTcpExtDataCsumErr | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != 0 ]; then
-		echo "[fail] got $count data checksum error[s] expected 0"
-		ret=1
-		dump_stats=1
+	if [ "$count" != $csum_ns2 ]; then
+		if [ $allow_multi_errors_ns2 -eq 1 ]; then
+			# allow multiple checksum errors
+			if [ "$count" -lt $csum_ns2 ]; then
+				ret=1
+			fi
+		else
+			ret=1
+		fi
+
+		if [ $ret -eq 1 ]; then
+			echo "[fail] got $count data checksum error[s] expected $csum_ns2"
+			dump_stats=1
+		fi
 	else
 		echo "[ ok ]"
 	fi
@@ -778,27 +811,27 @@ chk_csum_nr()
 
 chk_fail_nr()
 {
-	local mp_fail_nr_tx=$1
-	local mp_fail_nr_rx=$2
+	local fail_tx=$1
+	local fail_rx=$2
 	local count
 	local dump_stats
 
 	printf "%-${nr_blank}s %s" " " "ftx"
 	count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPFailTx | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != "$mp_fail_nr_tx" ]; then
-		echo "[fail] got $count MP_FAIL[s] TX expected $mp_fail_nr_tx"
+	if [ "$count" != "$fail_tx" ]; then
+		echo "[fail] got $count MP_FAIL[s] TX expected $fail_tx"
 		ret=1
 		dump_stats=1
 	else
 		echo -n "[ ok ]"
 	fi
 
-	echo -n " - frx   "
+	echo -n " - failrx"
 	count=`ip netns exec $ns2 nstat -as | grep MPTcpExtMPFailRx | awk '{print $2}'`
 	[ -z "$count" ] && count=0
-	if [ "$count" != "$mp_fail_nr_rx" ]; then
-		echo "[fail] got $count MP_FAIL[s] RX expected $mp_fail_nr_rx"
+	if [ "$count" != "$fail_rx" ]; then
+		echo "[fail] got $count MP_FAIL[s] RX expected $fail_rx"
 		ret=1
 		dump_stats=1
 	else
@@ -927,6 +960,11 @@ chk_join_nr()
 	local syn_nr=$2
 	local syn_ack_nr=$3
 	local ack_nr=$4
+	local csum_ns1=${5:-0}
+	local csum_ns2=${6:-0}
+	local fail_nr=${7:-0}
+	local rst_nr=${8:-0}
+	local infi_nr=${9:-0}
 	local count
 	local dump_stats
 	local with_cookie
@@ -973,10 +1011,10 @@ chk_join_nr()
 	fi
 	[ "${dump_stats}" = 1 ] && dump_stats
 	if [ $checksum -eq 1 ]; then
-		chk_csum_nr
-		chk_fail_nr 0 0
-		chk_rst_nr 0 0
-		chk_infi_nr 0 0
+		chk_csum_nr "" $csum_ns1 $csum_ns2
+		chk_fail_nr $fail_nr $fail_nr
+		chk_rst_nr $rst_nr $rst_nr
+		chk_infi_nr $infi_nr $infi_nr
 	fi
 }
 
-- 
2.34.1

[PATCH mptcp-next v8 4/5] selftests: mptcp: reuse linkfail to make given size files

[Geliang Tang]

This patch reused the test_linkfail values above 2 to make test files with
the given sizes (KB) for both the client side and the server side. It's
useful for the test cases using different file sizes.

Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index ec8d750c19eb..26829fbbc67c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -475,7 +475,7 @@ do_transfer()
 		local_addr="0.0.0.0"
 	fi
 
-	if [ "$test_link_fail" -eq 2 ];then
+	if [ "$test_link_fail" -gt 1 ];then
 		timeout ${timeout_test} \
 			ip netns exec ${listener_ns} \
 				./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
@@ -495,13 +495,19 @@ do_transfer()
 			ip netns exec ${connector_ns} \
 				./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
 					$extra_args $connect_addr < "$cin" > "$cout" &
-	else
+	elif [ "$test_link_fail" -eq 1 ] || [ "$test_link_fail" -eq 2 ];then
 		( cat "$cinfail" ; sleep 2; link_failure $listener_ns ; cat "$cinfail" ) | \
 			tee "$cinsent" | \
 			timeout ${timeout_test} \
 				ip netns exec ${connector_ns} \
 					./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
 						$extra_args $connect_addr > "$cout" &
+	else
+		cat "$cinfail" | tee "$cinsent" | \
+			timeout ${timeout_test} \
+				ip netns exec ${connector_ns} \
+					./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
+						$extra_args $connect_addr > "$cout" &
 	fi
 	cpid=$!
 
@@ -661,7 +667,7 @@ do_transfer()
 		return 1
 	fi
 
-	if [ "$test_link_fail" -eq 2 ];then
+	if [ "$test_link_fail" -gt 1 ];then
 		check_transfer $sinfail $cout "file received by client"
 	else
 		check_transfer $sin $cout "file received by client"
@@ -706,9 +712,16 @@ run_tests()
 	speed="${7:-fast}"
 	sflags="${8:-""}"
 
+	# The values above 2 are reused to make test files
+	# with the given sizes (KB)
+	if [ "$test_linkfail" -gt 2 ]; then
+		if [ -z "$cinfail" ]; then
+			cinfail=$(mktemp)
+		fi
+		make_file "$cinfail" "client" $test_linkfail
 	# create the input file for the failure test when
 	# the first failure test run
-	if [ "$test_linkfail" -ne 0 -a -z "$cinfail" ]; then
+	elif [ "$test_linkfail" -ne 0 -a -z "$cinfail" ]; then
 		# the client file must be considerably larger
 		# of the maximum expected cwin value, or the
 		# link utilization will be not predicable
@@ -721,7 +734,12 @@ run_tests()
 		make_file "$cinfail" "client" $size
 	fi
 
-	if [ "$test_linkfail" -eq 2 -a -z "$sinfail" ]; then
+	if [ "$test_linkfail" -gt 2 ]; then
+		if [ -z "$sinfail" ]; then
+			sinfail=$(mktemp)
+		fi
+		make_file "$sinfail" "server" $test_linkfail
+	elif [ "$test_linkfail" -eq 2 -a -z "$sinfail" ]; then
 		size=$((RANDOM%16))
 		size=$((size+1))
 		size=$((size*2048))
-- 
2.34.1

[PATCH mptcp-next v8 5/5] selftests: mptcp: add the MP_FAIL testcases

[Geliang Tang]

Added the test cases for MP_FAIL, the multiple subflows test for the
MP_RST case and the single subflow one for the infinite mapping case.
The former used the test_linkfail value to make 1024KB test files,
and the latter 128KB.

Added a new function reset_with_fail(), in it use 'iptables' and 'tc
action pedit' rules to produce the bit flips to trigger the checksum
failures. Added a new function pedit_action_pkts() to get the numbers
of the packets edited by the tc pedit actions.

Added a new global variable validate_checksum to enable checksums for
the MP_FAIL tests without passing the '-C' argument.

Also added the needed kernel configures in the selftests config file.

Suggested-by: Davide Caratti <dcaratti@redhat.com>
Co-developed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
 tools/testing/selftests/net/mptcp/config      |  8 ++
 .../testing/selftests/net/mptcp/mptcp_join.sh | 98 ++++++++++++++++++-
 2 files changed, 104 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/config b/tools/testing/selftests/net/mptcp/config
index d36b7da5082a..38021a0dd527 100644
--- a/tools/testing/selftests/net/mptcp/config
+++ b/tools/testing/selftests/net/mptcp/config
@@ -12,6 +12,9 @@ CONFIG_NF_TABLES=m
 CONFIG_NFT_COMPAT=m
 CONFIG_NETFILTER_XTABLES=m
 CONFIG_NETFILTER_XT_MATCH_BPF=m
+CONFIG_NETFILTER_XT_MATCH_LENGTH=m
+CONFIG_NETFILTER_XT_MATCH_STATISTIC=m
+CONFIG_NETFILTER_XT_TARGET_MARK=m
 CONFIG_NF_TABLES_INET=y
 CONFIG_NFT_TPROXY=m
 CONFIG_NFT_SOCKET=m
@@ -19,3 +22,8 @@ CONFIG_IP_ADVANCED_ROUTER=y
 CONFIG_IP_MULTIPLE_TABLES=y
 CONFIG_IP_NF_TARGET_REJECT=m
 CONFIG_IPV6_MULTIPLE_TABLES=y
+CONFIG_NET_ACT_CSUM=m
+CONFIG_NET_ACT_PEDIT=m
+CONFIG_NET_CLS_ACT=y
+CONFIG_NET_CLS_FW=m
+CONFIG_NET_SCH_INGRESS=m
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 26829fbbc67c..f00b1ae835a9 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -16,6 +16,7 @@ capture=0
 checksum=0
 ip_mptcp=0
 check_invert=0
+validate_checksum=0
 do_all_tests=1
 init=0
 
@@ -62,6 +63,7 @@ init_partial()
 	done
 
 	check_invert=0
+	validate_checksum=$checksum
 
 	#  ns1              ns2
 	# ns1eth1    ns2eth1
@@ -207,6 +209,58 @@ reset_with_allow_join_id0()
 	ip netns exec $ns2 sysctl -q net.mptcp.allow_join_initial_addr_port=$ns2_enable
 }
 
+# Modify TCP payload without corrupting the TCP packet
+#
+# This rule inverts a 8-bit word at byte offset 148 for the 2nd TCP ACK packets
+# carrying enough data.
+# Once it is done, the TCP Checksum field is updated so the packet is still
+# considered as valid at the TCP level.
+# Because the MPTCP checksum, covering the TCP options and data, has not been
+# updated, the modification will be detected and an MP_FAIL will be emitted:
+# what we want to validate here without corrupting "random" MPTCP options.
+#
+# To avoid having tc producing this pr_info() message for each TCP ACK packets
+# not carrying enough data:
+#
+#     tc action pedit offset 162 out of bounds
+#
+# Netfilter is used to mark packets with enough data.
+reset_with_fail()
+{
+	reset
+
+	ip netns exec $ns1 sysctl -q net.mptcp.checksum_enabled=1
+	ip netns exec $ns2 sysctl -q net.mptcp.checksum_enabled=1
+
+	check_invert=1
+	validate_checksum=1
+	local i="$1"
+	local ip="${2:-4}"
+	local tables
+
+	tables="iptables"
+	if [ $ip -eq 6 ]; then
+		tables="ip6tables"
+	fi
+
+	ip netns exec $ns2 $tables \
+		-t mangle \
+		-A OUTPUT \
+		-o ns2eth$i \
+		-p tcp \
+		-m length --length 150:9999 \
+		-m statistic --mode nth --packet 1 --every 99999 \
+		-j MARK --set-mark 42 || exit 1
+
+	tc -n $ns2 qdisc add dev ns2eth$i clsact || exit 1
+	tc -n $ns2 filter add dev ns2eth$i egress \
+		protocol ip prio 1000 \
+		handle 42 fw \
+		action pedit munge offset 148 u8 invert \
+		pipe csum tcp \
+		index 100 || exit 1
+}
+
 print_file_err()
 {
 	ls -l "$1" 1>&2
@@ -1028,7 +1082,7 @@ chk_join_nr()
 		echo "[ ok ]"
 	fi
 	[ "${dump_stats}" = 1 ] && dump_stats
-	if [ $checksum -eq 1 ]; then
+	if [ $validate_checksum -eq 1 ]; then
 		chk_csum_nr "" $csum_ns1 $csum_ns2
 		chk_fail_nr $fail_nr $fail_nr
 		chk_rst_nr $rst_nr $rst_nr
@@ -2362,6 +2416,41 @@ fastclose_tests()
 	chk_rst_nr 2 2 invert
 }
 
+pedit_action_pkts()
+{
+	tc -n $ns2 -j -s action show action pedit index 100 | \
+		sed 's/.*"packets":\([0-9]\+\),.*/\1/'
+}
+
+fail_tests()
+{
+	# multiple subflows
+	reset_with_fail 2
+	tc -n $ns2 qdisc add dev ns2eth1 root netem rate 20mbit delay 1
+	pm_nl_set_limits $ns1 0 1
+	pm_nl_set_limits $ns2 0 1
+	pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
+	run_tests $ns1 $ns2 10.0.1.1 1024
+	chk_join_nr "MP_FAIL MP_RST: $(pedit_action_pkts) corrupted pkts" 1 1 1 \
+									  +1 +0 \
+									  1 \
+									  1
+
+	# single subflow
+	reset_with_fail 1
+	run_tests $ns1 $ns2 10.0.1.1 128
+									# syn_nr syn_ack_nr ack_nr
+									# csum_ns1 csum_ns2
+									# fail_nr
+									# rst_nr
+									# infi_nr
+	chk_join_nr "Infinite map: $(pedit_action_pkts) corrupted pkts" 0 0 0 \
+									+1 +0 \
+									1 \
+									0 \
+									1
+}
+
 all_tests()
 {
 	subflows_tests
@@ -2381,6 +2470,7 @@ all_tests()
 	fullmesh_tests
 	userspace_tests
 	fastclose_tests
+	fail_tests
 }
 
 # [$1: error message]
@@ -2409,6 +2499,7 @@ usage()
 	echo "  -m fullmesh_tests"
 	echo "  -u userspace_tests"
 	echo "  -z fastclose_tests"
+	echo "  -F fail_tests"
 	echo "  -c capture pcap files"
 	echo "  -C enable data checksum"
 	echo "  -i use ip mptcp"
@@ -2440,7 +2531,7 @@ if [ $do_all_tests -eq 1 ]; then
 	exit $ret
 fi
 
-while getopts 'fesltra64bpkdmuchzCSi' opt; do
+while getopts 'fesltra64bpkdmuchzCSFi' opt; do
 	case $opt in
 		f)
 			subflows_tests
@@ -2493,6 +2584,9 @@ while getopts 'fesltra64bpkdmuchzCSi' opt; do
 		z)
 			fastclose_tests
 			;;
+		F)
+			fail_tests
+			;;
 		c)
 			;;
 		C)
-- 
2.34.1

Re: [PATCH mptcp-next v8 0/5] add mp_fail testcases

[Mat Martineau]

On Tue, 15 Feb 2022, Geliang Tang wrote:

> v8:
> - move two MP_RST mibs patches into the "add fastclose testcases" series.
> - allow multiple checksum errors in patch 3.
> - depends on "add fastclose testcases v3".
>

Thanks Geliang. This series (plus the two associated squash-to patches in 
patchwork) look good to me. Matthieu, note the dependency on the fastclose 
test series that has a pending question about intermittent failures.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> RESEND:
> - no code modified, just updated the commit logs and comments.
> v7:
> This version makes the multiple subflows test more stable.
> - Add delays and drop 'retry' in the multiple subflows test in patch 7.
> - Don't add two addresses for the multiple subflows test in patch 7, add
> one address 10.0.2.2 is enough, this make it more stable.
> - Add a comment in patch 6.
> - Rebased to export/20220211T054659.
> - A 500 times loop test log of v7 will be attached.
>
> v6:
> - Split two patches from the last one.
> - Retry the multiple subflows test three times to fix this
> "MP_FAIL MP_RST: 0 corrupted pkts" failure reported by me in v5:
>
> Created /tmp/tmp.e4nE5Q14mj (size 1024 KB) containing data sent by client
> Created /tmp/tmp.QwpQYClFnm (size 1024 KB) containing data sent by server
> 001 MP_FAIL MP_RST: 0 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
>                                         sum[fail] got 0 data checksum error[s] expected 1
>                                         ftx[fail] got 0 MP_FAIL[s] TX expected 1
>                                         rtx[fail] got 0 MP_RST[s] TX expected 1
>                                         itx[ ok ] - infirx[ ok ]
>
> A test log of running v6 500 times is attached, named v6-loop-500-times.log,
> in it, we can see retry happend 8 times (116, 136, 236, 295, 297, 402, 444,
> 457), and no "0 corrupted pkts" any more.
>
> - Reduce the single subflow test files size from 1024KB to 128KB to fix
> this "file received by client does not match" failure reported by CI and
> Matt in v5:
>
> # Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
> # Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
> # file received by server has inverted byte at 195585
> # 100 MP_FAIL MP_RST: 1 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
> #                                          sum[ ok ] - csum  [ ok ]
> #                                          ftx[ ok ] - failrx[ ok ]
> #                                          rtx[ ok ] - rstrx [ ok ]
> #                                          itx[ ok ] - infirx[ ok ]
> # Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
> # Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
> # [ FAIL ] file received by client does not match (in, out):
> # -rw------- 1 root root 1048604 Feb  9 11:37 /tmp/tmp.jFbZEAnYZa
> # Trailing bytes are:
> # MPTCP_TEST_FILE_END_MARKER
> # -rw------- 1 root root 1048606 Feb  9 11:37 /tmp/tmp.ghV0iWPhu5
> # Trailing bytes are:
> # MPTCP_TEST_FILE_END_MARKER
> # file received by server has inverted byte at 169
> # 101 Infinite map: 5 corrupted pkts       syn[ ok ] - synack[ ok ] - ack[ ok ]
> #                                          sum[ ok ] - csum  [ ok ]
> #                                          ftx[ ok ] - failrx[ ok ]
> #                                          rtx[ ok ] - rstrx [ ok ]
> #                                          itx[ ok ] - infirx[ ok ]
>
> In the attached v6-loop-500-times.log, no "file received by client does
> not match" any more.
>
> I think this v6 is very stable, but there are still 6 tests failed in the
> 500 time tests log (68 77 97 112 161 243). These failures are all due to
> get one more unexpected checksum failure:
>
> > cat v6-loop-500-times.log  | grep "\[fail"
>                                         sum[fail] got 2 data checksum error[s] expected 1
>                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
> - failrx[fail] got 2 MP_FAIL[s] RX expected 1
>                                         rtx[fail] got 2 MP_RST[s] TX expected 1
> - rstrx [fail] got 2 MP_RST[s] RX expected 1
>                                         sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
>                                         sum[fail] got 2 data checksum error[s] expected 1
>                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
> - failrx[fail] got 2 MP_FAIL[s] RX expected 1
>                                         rtx[fail] got 2 MP_RST[s] TX expected 1
> - rstrx [fail] got 2 MP_RST[s] RX expected 1
>                                         sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
>                                         rtx[fail] got 2 MP_RST[s] TX expected 1
> - rstrx [fail] got 2 MP_RST[s] RX expected 1
>                                         sum[fail] got 2 data checksum error[s] expected 1
>                                         ftx[fail] got 2 MP_FAIL[s] TX expected 1
> - failrx[fail] got 2 MP_FAIL[s] RX expected 1
>                                         rtx[fail] got 2 MP_RST[s] TX expected 1
> - rstrx [fail] got 2 MP_RST[s] RX expected 1
>
> These failures are related the checksum bug reported by me, issue #255.
> When transferring a larger file, the checksum sometimes fails. Running
> "./mptcp_connect.sh -C" in 10 times, we will the MP_FAILs. If we solve
> issue #255 in the future, this mp_fail testcases will be more stable.
>
> v5:
> - update patch 5 as Matt suggested.
> - use '|| exit 1'
> - drop jq
> - drop pedit_action
>
> v4:
> - add the mibs for MP_RST
> - patch 4 "selftests: mptcp: add MP_RST mibs check" uses the variable
> $nr_blank, so it depends on the commit "selftests: mptcp: adjust output
> alignment for more tests".
>
> v3:
> - check the exit code of iptables.
> - add ip6tables support for reset_with_fail too.
> - add the null check for $packets
> - rename nr_mp_fail to pedit_action and get_nr_mp_fail to
> pedit_action_happened
>
> This is v12 of the mp_fail testcases with Matt's changes. It works well
> and it's very stable.
>
> Geliang Tang (5):
>  Squash to "mptcp: infinite mapping receiving"
>  Squash to "selftests: mptcp: add infinite map mibs check"
>  selftests: mptcp: add more arguments for chk_join_nr
>  selftests: mptcp: reuse linkfail to make given size files
>  selftests: mptcp: add the MP_FAIL testcases
>
> net/mptcp/subflow.c                           |   1 +
> tools/testing/selftests/net/mptcp/config      |   8 +
> .../testing/selftests/net/mptcp/mptcp_join.sh | 216 +++++++++++++++---
> 3 files changed, 192 insertions(+), 33 deletions(-)
>
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel