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
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
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
>
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
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
>
© 2016 - 2026 Red Hat, Inc.