* [PATCH mptcp-next v4 0/5] add mp_fail testcases
@ 2022-02-09 8:57 Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 1/5] Squash to "mptcp: infinite mapping receiving" Geliang Tang
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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"
mptcp: add mibs for MP_RST
selftests: mptcp: add MP_RST mibs check
selftests: mptcp: add mp_fail testcases
net/mptcp/mib.c | 2 +
net/mptcp/mib.h | 2 +
net/mptcp/options.c | 2 +
net/mptcp/subflow.c | 1 +
tools/testing/selftests/net/mptcp/config | 8 +
.../testing/selftests/net/mptcp/mptcp_join.sh | 217 +++++++++++++++---
6 files changed, 204 insertions(+), 28 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH mptcp-next v4 1/5] Squash to "mptcp: infinite mapping receiving"
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
@ 2022-02-09 8:57 ` Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 2/5] Squash to "selftests: mptcp: add infinite map mibs check" Geliang Tang
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: 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 485f00dcaf84..ae4d855f3c2f 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -962,6 +962,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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH mptcp-next v4 2/5] Squash to "selftests: mptcp: add infinite map mibs check"
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 1/5] Squash to "mptcp: infinite mapping receiving" Geliang Tang
@ 2022-02-09 8:57 ` Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 3/5] mptcp: add mibs for MP_RST Geliang Tang
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: 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 90a6adc36490..3577716cd5e6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -784,27 +784,27 @@ chk_fail_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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH mptcp-next v4 3/5] mptcp: add mibs for MP_RST
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 1/5] Squash to "mptcp: infinite mapping receiving" Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 2/5] Squash to "selftests: mptcp: add infinite map mibs check" Geliang Tang
@ 2022-02-09 8:57 ` Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 4/5] selftests: mptcp: add MP_RST mibs check Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases Geliang Tang
4 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch added two mibs for MP_RST, MPTCP_MIB_MPRSTTX for the MP_RST
sending and MPTCP_MIB_MPRSTRX for the MP_RST receiving.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
net/mptcp/mib.c | 2 ++
net/mptcp/mib.h | 2 ++
net/mptcp/options.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c
index c12251cb0d44..bb7808eecab0 100644
--- a/net/mptcp/mib.c
+++ b/net/mptcp/mib.c
@@ -47,6 +47,8 @@ static const struct snmp_mib mptcp_snmp_list[] = {
SNMP_MIB_ITEM("MPPrioRx", MPTCP_MIB_MPPRIORX),
SNMP_MIB_ITEM("MPFailTx", MPTCP_MIB_MPFAILTX),
SNMP_MIB_ITEM("MPFailRx", MPTCP_MIB_MPFAILRX),
+ SNMP_MIB_ITEM("MPRstTx", MPTCP_MIB_MPRSTTX),
+ SNMP_MIB_ITEM("MPRstRx", MPTCP_MIB_MPRSTRX),
SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
SNMP_MIB_ITEM("SubflowStale", MPTCP_MIB_SUBFLOWSTALE),
SNMP_MIB_ITEM("SubflowRecover", MPTCP_MIB_SUBFLOWRECOVER),
diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h
index 7901f1338d15..37226d8d5339 100644
--- a/net/mptcp/mib.h
+++ b/net/mptcp/mib.h
@@ -40,6 +40,8 @@ enum linux_mptcp_mib_field {
MPTCP_MIB_MPPRIORX, /* Received a MP_PRIO */
MPTCP_MIB_MPFAILTX, /* Transmit a MP_FAIL */
MPTCP_MIB_MPFAILRX, /* Received a MP_FAIL */
+ MPTCP_MIB_MPRSTTX, /* Transmit a MP_RST */
+ MPTCP_MIB_MPRSTRX, /* Received a MP_RST */
MPTCP_MIB_RCVPRUNED, /* Incoming packet dropped due to memory limit */
MPTCP_MIB_SUBFLOWSTALE, /* Subflows entered 'stale' status */
MPTCP_MIB_SUBFLOWRECOVER, /* Subflows returned to active status after being stale */
diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 670364cd455f..556d68e0e50b 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -838,6 +838,7 @@ bool mptcp_established_options(struct sock *sk, struct sk_buff *skb,
if (mptcp_established_options_rst(sk, skb, &opt_size, remaining, opts)) {
*size += opt_size;
remaining -= opt_size;
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPRSTTX);
}
return true;
}
@@ -1159,6 +1160,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
subflow->reset_seen = 1;
subflow->reset_reason = mp_opt.reset_reason;
subflow->reset_transient = mp_opt.reset_transient;
+ MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPRSTRX);
}
if (!(mp_opt.suboptions & OPTION_MPTCP_DSS))
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH mptcp-next v4 4/5] selftests: mptcp: add MP_RST mibs check
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
` (2 preceding siblings ...)
2022-02-09 8:57 ` [PATCH mptcp-next v4 3/5] mptcp: add mibs for MP_RST Geliang Tang
@ 2022-02-09 8:57 ` Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases Geliang Tang
4 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
This patch added a function chk_rst_nr() to check the numbers of the
MP_RST mibs for sending and receiving.
Signed-off-by: Geliang Tang <geliang.tang@suse.com>
---
.../testing/selftests/net/mptcp/mptcp_join.sh | 33 +++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 3577716cd5e6..95d61c97ccad 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -782,6 +782,38 @@ chk_fail_nr()
[ "${dump_stats}" = 1 ] && dump_stats
}
+chk_rst_nr()
+{
+ local rst_tx=$1
+ local rst_rx=$2
+ local count
+ local dump_stats
+
+ printf "%-${nr_blank}s %s" " " "rtx"
+ count=`ip netns exec $ns1 nstat -as | grep MPTcpExtMPRstTx | awk '{print $2}'`
+ [ -z "$count" ] && count=0
+ if [ "$count" != "$rst_tx" ]; then
+ echo "[fail] got $count MP_RST[s] TX expected $rst_tx"
+ ret=1
+ dump_stats=1
+ else
+ echo -n "[ ok ]"
+ fi
+
+ echo -n " - rstrx "
+ count=`ip netns exec $ns2 nstat -as | grep MPTcpExtMPRstRx | awk '{print $2}'`
+ [ -z "$count" ] && count=0
+ if [ "$count" != "$rst_rx" ]; then
+ echo "[fail] got $count MP_RST[s] RX expected $rst_rx"
+ ret=1
+ dump_stats=1
+ else
+ echo "[ ok ]"
+ fi
+
+ [ "${dump_stats}" = 1 ] && dump_stats
+}
+
chk_infi_nr()
{
local infi_tx=$1
@@ -859,6 +891,7 @@ chk_join_nr()
if [ $checksum -eq 1 ]; then
chk_csum_nr
chk_fail_nr 0 0
+ chk_rst_nr 0 0
chk_infi_nr 0 0
fi
}
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
` (3 preceding siblings ...)
2022-02-09 8:57 ` [PATCH mptcp-next v4 4/5] selftests: mptcp: add MP_RST mibs check Geliang Tang
@ 2022-02-09 8:57 ` Geliang Tang
2022-02-09 9:23 ` Matthieu Baerts
4 siblings, 1 reply; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 8:57 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang, Davide Caratti, Matthieu Baerts
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.
These tests used the new test_linkfail value 3 to make 512KB test files for
both the client and server.
Added a new function reset_with_fail(), in it use 'iptables' and 'tc
action pedit' commands to trigger the checksum failures.
Added a new global variable pedit_action to trace whether the tc pedit
action happened during the test. Check and set it in the new function
pedit_action_happened(). Show it in the test description and pass it to
chk_join_nr() to check the numbers of the checksum failures, MP_FAIL
sending and receiving, the infinite mapping sending and receiving.
Added a new global variable validate_checksum to enable checksums for
the MP_FAIL tests without passing the '-C' argument.
Also added the tests needed kernel configures in the 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 | 172 +++++++++++++++---
2 files changed, 158 insertions(+), 22 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 95d61c97ccad..880430058492 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -17,6 +17,8 @@ capture=0
checksum=0
ip_mptcp=0
check_invert=0
+validate_checksum=0
+pedit_action=0
do_all_tests=1
TEST_COUNT=0
@@ -62,6 +64,7 @@ init()
done
check_invert=0
+ validate_checksum=$checksum
# ns1 ns2
# ns1eth1 ns2eth1
@@ -167,6 +170,63 @@ 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
+ pedit_action=0
+ 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
+ if [ $? -ne 0 ];then
+ echo "SKIP: Couldn not add the $tables rule"
+ exit $ksft_skip
+ fi
+
+ tc -n $ns2 qdisc add dev ns2eth$i clsact
+ 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
+}
+
ip -Version > /dev/null 2>&1
if [ $? -ne 0 ];then
echo "SKIP: Could not run test without ip tool"
@@ -185,6 +245,12 @@ if [ $? -ne 0 ];then
exit $ksft_skip
fi
+jq -V > /dev/null 2>&1
+if [ $? -ne 0 ];then
+ echo "SKIP: Could not run all tests without jq tool"
+ exit $ksft_skip
+fi
+
print_file_err()
{
ls -l "$1" 1>&2
@@ -245,6 +311,19 @@ link_failure()
done
}
+pedit_action_happened()
+{
+ local i="$1"
+
+ local action=$(tc -n $ns2 -j -s action show action pedit index 100)
+ local packets=$(echo $action | jq '.[1].actions[0].stats.packets')
+
+ if [ $packets != "null" ] && [ $packets -gt 0 ]; then
+ pedit_action=1
+ fi
+ tc -n $ns2 qdisc del dev ns2eth$i clsact
+}
+
# $1: IP address
is_v6()
{
@@ -446,7 +525,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} \
@@ -466,13 +545,19 @@ do_transfer()
ip netns exec ${connector_ns} \
$mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
$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} \
$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} \
+ $connect_addr > "$cout" &
fi
cpid=$!
@@ -632,7 +717,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"
@@ -681,7 +766,12 @@ run_tests()
# create the input file for the failure test when
# the first failure test run
- if [ "$test_linkfail" -ne 0 -a -z "$cinfail" ]; then
+ if [ "$test_linkfail" -eq 3 ]; then
+ if [ -z "$cinfail" ]; then
+ cinfail=$(mktemp)
+ fi
+ make_file "$cinfail" "client" 512
+ 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
@@ -694,7 +784,12 @@ run_tests()
make_file "$cinfail" "client" $size
fi
- if [ "$test_linkfail" -eq 2 -a -z "$sinfail" ]; then
+ if [ "$test_linkfail" -eq 3 ]; then
+ if [ -z "$sinfail" ]; then
+ sinfail=$(mktemp)
+ fi
+ make_file "$sinfail" "server" 512
+ elif [ "$test_linkfail" -eq 2 -a -z "$sinfail" ]; then
size=$((RANDOM%16))
size=$((size+1))
size=$((size*2048))
@@ -719,6 +814,8 @@ dump_stats()
chk_csum_nr()
{
local msg=${1:-""}
+ local csum_ns1=${2:-0}
+ local csum_ns2=${3:-0}
local count
local dump_stats
@@ -730,8 +827,8 @@ 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"
+ if [ "$count" != $csum_ns1 ]; then
+ echo "[fail] got $count data checksum error[s] expected $csum_ns1"
ret=1
dump_stats=1
else
@@ -740,8 +837,8 @@ chk_csum_nr()
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"
+ if [ "$count" != $csum_ns2 ]; then
+ echo "[fail] got $count data checksum error[s] expected $csum_ns2"
ret=1
dump_stats=1
else
@@ -752,27 +849,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
@@ -852,6 +949,9 @@ chk_join_nr()
local syn_nr=$2
local syn_ack_nr=$3
local ack_nr=$4
+ local fail_nr=${5:-0}
+ local rst_nr=${6:-0}
+ local infi_nr=${7:-0}
local count
local dump_stats
@@ -888,11 +988,11 @@ chk_join_nr()
echo "[ ok ]"
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
+ if [ $validate_checksum -eq 1 ]; then
+ chk_csum_nr "" $fail_nr
+ chk_fail_nr $fail_nr $fail_nr
+ chk_rst_nr $rst_nr $rst_nr
+ chk_infi_nr $infi_nr $infi_nr
fi
}
@@ -2197,6 +2297,29 @@ userspace_tests()
chk_rm_nr 0 0
}
+fail_tests()
+{
+ # multiple subflows
+ reset_with_fail 2
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
+ pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
+ run_tests $ns1 $ns2 10.0.1.1 3
+ pedit_action_happened 2
+ chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
+ $pedit_action $pedit_action
+
+ # single subflow
+ reset_with_fail 1
+ pm_nl_set_limits $ns1 0 2
+ pm_nl_set_limits $ns2 0 2
+ run_tests $ns1 $ns2 10.0.1.1 3
+ pedit_action_happened 1
+ chk_join_nr "MP_FAIL infinite map: $pedit_action pedit action" 0 0 0 \
+ $pedit_action 0 $pedit_action
+}
+
all_tests()
{
subflows_tests
@@ -2215,6 +2338,7 @@ all_tests()
deny_join_id0_tests
fullmesh_tests
userspace_tests
+ fail_tests
}
usage()
@@ -2236,6 +2360,7 @@ usage()
echo " -d deny_join_id0_tests"
echo " -m fullmesh_tests"
echo " -u userspace_tests"
+ echo " -F fail_tests"
echo " -c capture pcap files"
echo " -C enable data checksum"
echo " -i use ip mptcp"
@@ -2275,7 +2400,7 @@ if [ $do_all_tests -eq 1 ]; then
exit $ret
fi
-while getopts 'fesltra64bpkdmuchCSi' opt; do
+while getopts 'fesltra64bpkdmuchCSFi' opt; do
case $opt in
f)
subflows_tests
@@ -2325,6 +2450,9 @@ while getopts 'fesltra64bpkdmuchCSi' opt; do
u)
userspace_tests
;;
+ F)
+ fail_tests
+ ;;
c)
;;
C)
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases
2022-02-09 8:57 ` [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases Geliang Tang
@ 2022-02-09 9:23 ` Matthieu Baerts
2022-02-09 10:55 ` Geliang Tang
0 siblings, 1 reply; 10+ messages in thread
From: Matthieu Baerts @ 2022-02-09 9:23 UTC (permalink / raw)
To: Geliang Tang, mptcp; +Cc: Davide Caratti
Hi Geliang,
On 09/02/2022 09:57, Geliang Tang wrote:
> 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.
Thank you for the new version!
> These tests used the new test_linkfail value 3 to make 512KB test files for
> both the client and server.
>
> Added a new function reset_with_fail(), in it use 'iptables' and 'tc
> action pedit' commands to trigger the checksum failures.
>
> Added a new global variable pedit_action to trace whether the tc pedit
> action happened during the test. Check and set it in the new function
> pedit_action_happened(). Show it in the test description and pass it to
> chk_join_nr() to check the numbers of the checksum failures, MP_FAIL
Related to my comment in the v2: I don't think it is a good idea to pass
it to chk_join_nr(): we expect an MP_FAIL, we should not ignore it if it
was not there.
(...)
> + 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
> + if [ $? -ne 0 ];then
> + echo "SKIP: Couldn not add the $tables rule"
> + exit $ksft_skip
> + fi
Related to my comment in the v2: I think we should exit with an error if
the IPTables command fails.
(...)
-j MARK --set-mark 42 || exit 1
The error message from IPTables should be enough I think.
In Mat's case, it was confusing because the test continued and even
succeeded!
> +
> + tc -n $ns2 qdisc add dev ns2eth$i clsact
> + 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
Same for these two tc commands, we can add "|| exit 1".
(...)
> +pedit_action_happened()
> +{
> + local i="$1"
> +
> + local action=$(tc -n $ns2 -j -s action show action pedit index 100)
> + local packets=$(echo $action | jq '.[1].actions[0].stats.packets')
> +
> + if [ $packets != "null" ] && [ $packets -gt 0 ]; then
> + pedit_action=1
> + fi
> + tc -n $ns2 qdisc del dev ns2eth$i clsact
It is a good idea to keep this just in case there was an issue with TC.
But then, I think we should print the number of packets, that's enough no?
(also we don't need to delete the clsact, it will be removed when the NS
will be removed)
So it would be enough to have:
--------
pedit_action_pkt()
{
tc -n $ns2 -j -s action show action pedit index 100 | \
jq '.[1].actions[0].stats.packets'
}
(...)
chk_join_nr "MP_FAIL MP_RST: $(pedit_action_pkt) corrupted pkt" 2 2 2 1 1
--------
We can even remove the dependence to 'jq' for this simple command:
tc -n $ns2 -j -s action show action pedit index 100 | \
sed 's/.*"packets":\([0-9]\+\),.*/\1/'
WDYT?
> +fail_tests()
> +{
> + # multiple subflows
> + reset_with_fail 2
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 0 2
> + pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
> + pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
> + run_tests $ns1 $ns2 10.0.1.1 3
> + pedit_action_happened 2
> + chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
> + $pedit_action $pedit_action
Related to my comment in the v2: we should not use $pedit_action in the
args we need to verify I think. Instead we should probably then have:
chk_join_nr "(...)" 2 2 2 1 1
> +
> + # single subflow
> + reset_with_fail 1
> + pm_nl_set_limits $ns1 0 2
> + pm_nl_set_limits $ns2 0 2
> + run_tests $ns1 $ns2 10.0.1.1 3
> + pedit_action_happened 1
> + chk_join_nr "MP_FAIL infinite map: $pedit_action pedit action" 0 0 0 \
> + $pedit_action 0 $pedit_action
Same here:
chk_join_nr "(...)" 0 0 0 1 0 1
No?
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases
2022-02-09 9:23 ` Matthieu Baerts
@ 2022-02-09 10:55 ` Geliang Tang
2022-02-09 11:13 ` Matthieu Baerts
0 siblings, 1 reply; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 10:55 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
Hi Matt,
On Wed, Feb 09, 2022 at 10:23:52AM +0100, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 09/02/2022 09:57, Geliang Tang wrote:
> > 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.
>
> Thank you for the new version!
>
> > These tests used the new test_linkfail value 3 to make 512KB test files for
> > both the client and server.
> >
> > Added a new function reset_with_fail(), in it use 'iptables' and 'tc
> > action pedit' commands to trigger the checksum failures.
> >
> > Added a new global variable pedit_action to trace whether the tc pedit
> > action happened during the test. Check and set it in the new function
> > pedit_action_happened(). Show it in the test description and pass it to
> > chk_join_nr() to check the numbers of the checksum failures, MP_FAIL
>
> Related to my comment in the v2: I don't think it is a good idea to pass
> it to chk_join_nr(): we expect an MP_FAIL, we should not ignore it if it
> was not there.
>
> (...)
>
> > + 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
> > + if [ $? -ne 0 ];then
> > + echo "SKIP: Couldn not add the $tables rule"
> > + exit $ksft_skip
> > + fi
>
> Related to my comment in the v2: I think we should exit with an error if
> the IPTables command fails.
>
> (...)
> -j MARK --set-mark 42 || exit 1
>
> The error message from IPTables should be enough I think.
>
> In Mat's case, it was confusing because the test continued and even
> succeeded!
>
> > +
> > + tc -n $ns2 qdisc add dev ns2eth$i clsact
> > + 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
>
> Same for these two tc commands, we can add "|| exit 1".
>
> (...)
>
> > +pedit_action_happened()
> > +{
> > + local i="$1"
> > +
> > + local action=$(tc -n $ns2 -j -s action show action pedit index 100)
> > + local packets=$(echo $action | jq '.[1].actions[0].stats.packets')
> > +
> > + if [ $packets != "null" ] && [ $packets -gt 0 ]; then
> > + pedit_action=1
> > + fi
> > + tc -n $ns2 qdisc del dev ns2eth$i clsact
>
> It is a good idea to keep this just in case there was an issue with TC.
> But then, I think we should print the number of packets, that's enough no?
>
> (also we don't need to delete the clsact, it will be removed when the NS
> will be removed)
>
> So it would be enough to have:
>
> --------
> pedit_action_pkt()
> {
> tc -n $ns2 -j -s action show action pedit index 100 | \
> jq '.[1].actions[0].stats.packets'
> }
>
> (...)
>
> chk_join_nr "MP_FAIL MP_RST: $(pedit_action_pkt) corrupted pkt" 2 2 2 1 1
> --------
>
> We can even remove the dependence to 'jq' for this simple command:
>
> tc -n $ns2 -j -s action show action pedit index 100 | \
> sed 's/.*"packets":\([0-9]\+\),.*/\1/'
>
> WDYT?
>
> > +fail_tests()
> > +{
> > + # multiple subflows
> > + reset_with_fail 2
> > + pm_nl_set_limits $ns1 0 2
> > + pm_nl_set_limits $ns2 0 2
> > + pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
> > + pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
> > + run_tests $ns1 $ns2 10.0.1.1 3
> > + pedit_action_happened 2
> > + chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
> > + $pedit_action $pedit_action
>
> Related to my comment in the v2: we should not use $pedit_action in the
> args we need to verify I think. Instead we should probably then have:
>
> chk_join_nr "(...)" 2 2 2 1 1
Thanks for your review and suggestions. I updated them in v5.
We will get this failure in v5 in rare cases, about one percent
probability:
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
- csum [ ok ]
Server ns stats
TcpPassiveOpens 3 0.0
TcpInSegs 311 0.0
TcpOutSegs 937 0.0
TcpExtTCPPureAcks 155 0.0
TcpExtTCPOrigDataSent 787 0.0
TcpExtTCPDelivered 787 0.0
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 2 0.0
MPTcpExtMPJoinAckRx 2 0.0
Client ns stats
TcpActiveOpens 3 0.0
TcpInSegs 305 0.0
TcpOutSegs 943 0.0
TcpExtTW 3 0.0
TcpExtTCPPureAcks 147 0.0
TcpExtTCPOrigDataSent 785 0.0
TcpExtTCPDelivered 788 0.0
MPTcpExtMPCapableSYNTX 1 0.0
MPTcpExtMPCapableSYNACKRX 1 0.0
MPTcpExtMPJoinSynAckRx 2 0.0
ftx[fail] got 0 MP_FAIL[s] TX expected 1
- failrx[fail] got 0 MP_FAIL[s] RX expected 1
Server ns stats
TcpPassiveOpens 3 0.0
TcpInSegs 311 0.0
TcpOutSegs 937 0.0
TcpExtTCPPureAcks 155 0.0
TcpExtTCPOrigDataSent 787 0.0
TcpExtTCPDelivered 787 0.0
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 2 0.0
MPTcpExtMPJoinAckRx 2 0.0
Client ns stats
TcpActiveOpens 3 0.0
TcpInSegs 305 0.0
TcpOutSegs 943 0.0
TcpExtTW 3 0.0
TcpExtTCPPureAcks 147 0.0
TcpExtTCPOrigDataSent 785 0.0
TcpExtTCPDelivered 788 0.0
MPTcpExtMPCapableSYNTX 1 0.0
MPTcpExtMPCapableSYNACKRX 1 0.0
MPTcpExtMPJoinSynAckRx 2 0.0
rtx[fail] got 0 MP_RST[s] TX expected 1
- rstrx [fail] got 0 MP_RST[s] RX expected 1
Server ns stats
TcpPassiveOpens 3 0.0
TcpInSegs 311 0.0
TcpOutSegs 937 0.0
TcpExtTCPPureAcks 155 0.0
TcpExtTCPOrigDataSent 787 0.0
TcpExtTCPDelivered 787 0.0
MPTcpExtMPCapableSYNRX 1 0.0
MPTcpExtMPCapableACKRX 1 0.0
MPTcpExtMPJoinSynRx 2 0.0
MPTcpExtMPJoinAckRx 2 0.0
Client ns stats
TcpActiveOpens 3 0.0
TcpInSegs 305 0.0
TcpOutSegs 943 0.0
TcpExtTW 3 0.0
TcpExtTCPPureAcks 147 0.0
TcpExtTCPOrigDataSent 785 0.0
TcpExtTCPDelivered 788 0.0
MPTcpExtMPCapableSYNTX 1 0.0
MPTcpExtMPCapableSYNACKRX 1 0.0
MPTcpExtMPJoinSynAckRx 2 0.0
itx[ ok ] - infirx[ ok ]
Thanks,
-Geliang
>
> > +
> > + # single subflow
> > + reset_with_fail 1
> > + pm_nl_set_limits $ns1 0 2
> > + pm_nl_set_limits $ns2 0 2
> > + run_tests $ns1 $ns2 10.0.1.1 3
> > + pedit_action_happened 1
> > + chk_join_nr "MP_FAIL infinite map: $pedit_action pedit action" 0 0 0 \
> > + $pedit_action 0 $pedit_action
>
> Same here:
>
> chk_join_nr "(...)" 0 0 0 1 0 1
>
> No?
>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases
2022-02-09 10:55 ` Geliang Tang
@ 2022-02-09 11:13 ` Matthieu Baerts
2022-02-09 13:52 ` Geliang Tang
0 siblings, 1 reply; 10+ messages in thread
From: Matthieu Baerts @ 2022-02-09 11:13 UTC (permalink / raw)
To: Geliang Tang; +Cc: mptcp
Hi Geliang,
On 09/02/2022 11:55, Geliang Tang wrote:
> On Wed, Feb 09, 2022 at 10:23:52AM +0100, Matthieu Baerts wrote:
>> On 09/02/2022 09:57, Geliang Tang wrote:
>>> +fail_tests()
>>> +{
>>> + # multiple subflows
>>> + reset_with_fail 2
>>> + pm_nl_set_limits $ns1 0 2
>>> + pm_nl_set_limits $ns2 0 2
>>> + pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
>>> + pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
>>> + run_tests $ns1 $ns2 10.0.1.1 3
>>> + pedit_action_happened 2
>>> + chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
>>> + $pedit_action $pedit_action
>>
>> Related to my comment in the v2: we should not use $pedit_action in the
>> args we need to verify I think. Instead we should probably then have:
>>
>> chk_join_nr "(...)" 2 2 2 1 1
>
> Thanks for your review and suggestions. I updated them in v5.
>
> We will get this failure in v5 in rare cases, about one percent
> probability
Do you know why?
We invert a byte so it should always cause an issue, no? (we can always
have 2 same hashes for two different sources but it is a very very rare
case we don't have to consider I think, especially when the content is
so close, no?)
Or maybe we need to improve the IPTables and TC rules?
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases
2022-02-09 11:13 ` Matthieu Baerts
@ 2022-02-09 13:52 ` Geliang Tang
0 siblings, 0 replies; 10+ messages in thread
From: Geliang Tang @ 2022-02-09 13:52 UTC (permalink / raw)
To: Matthieu Baerts; +Cc: mptcp
Hi Matt,
On Wed, Feb 09, 2022 at 12:13:00PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
>
> On 09/02/2022 11:55, Geliang Tang wrote:
> > On Wed, Feb 09, 2022 at 10:23:52AM +0100, Matthieu Baerts wrote:
> >> On 09/02/2022 09:57, Geliang Tang wrote:
> >>> +fail_tests()
> >>> +{
> >>> + # multiple subflows
> >>> + reset_with_fail 2
> >>> + pm_nl_set_limits $ns1 0 2
> >>> + pm_nl_set_limits $ns2 0 2
> >>> + pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags subflow
> >>> + pm_nl_add_endpoint $ns2 10.0.3.2 dev ns2eth3 flags subflow
> >>> + run_tests $ns1 $ns2 10.0.1.1 3
> >>> + pedit_action_happened 2
> >>> + chk_join_nr "MP_FAIL MP_RST: $pedit_action pedit action" 2 2 2 \
> >>> + $pedit_action $pedit_action
> >>
> >> Related to my comment in the v2: we should not use $pedit_action in the
> >> args we need to verify I think. Instead we should probably then have:
> >>
> >> chk_join_nr "(...)" 2 2 2 1 1
> >
> > Thanks for your review and suggestions. I updated them in v5.
> >
> > We will get this failure in v5 in rare cases, about one percent
> > probability
>
> Do you know why?
>
> We invert a byte so it should always cause an issue, no? (we can always
> have 2 same hashes for two different sources but it is a very very rare
> case we don't have to consider I think, especially when the content is
> so close, no?)
>
> Or maybe we need to improve the IPTables and TC rules?
>
I think we should go back to the old version of this patch. We shouldn't
always expect getting 1 MP_FAIL. If no checksum failure occur, we should
expect no MP_FAIL. I just sent a squash-to patch to fix this.
Thanks,
-Geliang
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-02-09 13:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 8:57 [PATCH mptcp-next v4 0/5] add mp_fail testcases Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 1/5] Squash to "mptcp: infinite mapping receiving" Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 2/5] Squash to "selftests: mptcp: add infinite map mibs check" Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 3/5] mptcp: add mibs for MP_RST Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 4/5] selftests: mptcp: add MP_RST mibs check Geliang Tang
2022-02-09 8:57 ` [PATCH mptcp-next v4 5/5] selftests: mptcp: add mp_fail testcases Geliang Tang
2022-02-09 9:23 ` Matthieu Baerts
2022-02-09 10:55 ` Geliang Tang
2022-02-09 11:13 ` Matthieu Baerts
2022-02-09 13:52 ` Geliang Tang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.