diff mbox series

[mptcp-next,v2] selftests: mptcp: add link failure test case

Message ID 20201009151117.6672-1-fw@strlen.de
State Accepted, archived
Commit 36221baa871249fd1e5c8e2d7580b5e4ee773170
Delegated to: Matthieu Baerts
Headers show
Series [mptcp-next,v2] selftests: mptcp: add link failure test case | expand

Commit Message

Florian Westphal Oct. 9, 2020, 3:11 p.m. UTC 
Add a test case where a link fails with multiple subflows.
The expectation is that MPTCP will transmit any data that
could not be delivered via the failed link on another subflow.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 v2: rebase and add 'link loss' to test-case comment.
     make sure we exit with 1 if file was corrupt or server/client
     exited nonzero.

 .../testing/selftests/net/mptcp/mptcp_join.sh | 135 +++++++++++++-----
 1 file changed, 97 insertions(+), 38 deletions(-)

Comments

Mat Martineau Oct. 15, 2020, 12:09 a.m. UTC | #1 
On Fri, 9 Oct 2020, Florian Westphal wrote:

> Add a test case where a link fails with multiple subflows.
> The expectation is that MPTCP will transmit any data that
> could not be delivered via the failed link on another subflow.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> v2: rebase and add 'link loss' to test-case comment.
>     make sure we exit with 1 if file was corrupt or server/client
>     exited nonzero.
>
> .../testing/selftests/net/mptcp/mptcp_join.sh | 135 +++++++++++++-----
> 1 file changed, 97 insertions(+), 38 deletions(-)
>

Thanks for the v2, looks good for the export branch.

--
Mat Martineau
Intel
Matthieu Baerts Oct. 17, 2020, 7:31 a.m. UTC | #2 
Hi Florian, Mat,

On 15/10/2020 02:09, Mat Martineau wrote:
> On Fri, 9 Oct 2020, Florian Westphal wrote:
> 
>> Add a test case where a link fails with multiple subflows.
>> The expectation is that MPTCP will transmit any data that
>> could not be delivered via the failed link on another subflow.
>>
>> Signed-off-by: Florian Westphal <fw@strlen.de>
>> ---
>> v2: rebase and add 'link loss' to test-case comment.
>>     make sure we exit with 1 if file was corrupt or server/client
>>     exited nonzero.
>>
>> .../testing/selftests/net/mptcp/mptcp_join.sh | 135 +++++++++++++-----
>> 1 file changed, 97 insertions(+), 38 deletions(-)
>>
> 
> Thanks for the v2, looks good for the export branch.

Thank you for the patch and the review!

Just applied with Mat's Reviewed-by.

- 36221baa8712: selftests: mptcp: add link failure test case
- Results: 3fd01a5ab17b..0e70ef7be280

Tests and export will be started soon!

Cheers,
Matt
Matthieu Baerts Oct. 24, 2020, 10:05 a.m. UTC | #3 
Hi Florian,

On 09/10/2020 17:11, Florian Westphal wrote:
> Add a test case where a link fails with multiple subflows.
> The expectation is that MPTCP will transmit any data that
> could not be delivered via the failed link on another subflow.

Thank you for these modifications!

(...)

> @@ -279,14 +308,32 @@ run_tests()
>   	listener_ns="$1"
>   	connector_ns="$2"
>   	connect_addr="$3"
> -	rm_nr_ns1="${4:-0}"
> -	rm_nr_ns2="${5:-0}"
> -	speed="${6:-fast}"
> +	test_linkfail="$4"
> +	rm_nr_ns1="${5:-0}"
> +	rm_nr_ns2="${6:-0}"
> +	speed="${7:-fast}"
>   	lret=0
> +	oldin=""
> +
> +	if [ "$test_linkfail" -eq 1 ];then
> +		size=$((RANDOM%1024))
> +		size=$((size+1))
> +		size=$((size*128)) > +
> +		oldin=$(mktemp)
> +		cp "$cin" "$oldin"
> +		make_file "$cin" "client" $size
This will create one file up to 1024*128 KB = ~131MB when later using 
this command:


   dd if=/dev/urandom of="$name" bs=1024 count=$size


This file will be duplicated a few times later when doing the transfer:


   ( cat "$cin" ; (...) ; cat "$cin" ) | \
       tee "$cinsent" | \
       (...) $mptcp_connect (...) > "$cout"


If I am not mistaken, we then have:

- $cin = max 131MB: initial file
- $cinsent = 2 times $cin
- $sout = $cinsent

→ total: 5*$cin = max 655MB

Was it the intension? Could we reduce the maximum size while keeping the 
test valid?

On my side, /tmp is mounted in tmpfs and currently limited to 490M. Just 
to know if I need to increase the size of modify the selftest script :)

Cheers,
Matt
Florian Westphal Oct. 24, 2020, 12:17 p.m. UTC | #4 
Matthieu Baerts <matthieu.baerts@tessares.net> wrote:
>   ( cat "$cin" ; (...) ; cat "$cin" ) | \
>       tee "$cinsent" | \
>       (...) $mptcp_connect (...) > "$cout"
> 
> 
> If I am not mistaken, we then have:
> 
> - $cin = max 131MB: initial file
> - $cinsent = 2 times $cin
> - $sout = $cinsent
> 
> → total: 5*$cin = max 655MB
> 
> Was it the intension? Could we reduce the maximum size while keeping the
> test valid?

Yes, I think its safe to reduce the size though.

Another option is to use /dev/zero as source and set the "sparse" flag for dd,
this would also reduce /tmp usage.
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 3767acb9bb12..b3fe9b4b0a16 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -5,6 +5,7 @@  ret=0
 sin=""
 sout=""
 cin=""
+cinsent=""
 cout=""
 ksft_skip=4
 timeout=30
@@ -79,7 +80,7 @@  cleanup_partial()
 cleanup()
 {
 	rm -f "$cin" "$cout"
-	rm -f "$sin" "$sout"
+	rm -f "$cin" "$cout" "$cinsent"
 	cleanup_partial
 }
 
@@ -122,6 +123,12 @@  if [ $? -ne 0 ];then
 	exit $ksft_skip
 fi
 
+print_file_err()
+{
+	ls -l "$1" 1>&2
+	echo "Trailing bytes are: "
+	tail -c 27 "$1"
+}
 
 check_transfer()
 {
@@ -134,6 +141,7 @@  check_transfer()
 		echo "[ FAIL ] $what does not match (in, out):"
 		print_file_err "$in"
 		print_file_err "$out"
+		ret=1
 
 		return 1
 	fi
@@ -154,6 +162,17 @@  do_ping()
 	fi
 }
 
+link_failure()
+{
+	ns="$1"
+
+	l=$((RANDOM%4))
+	l=$((l+1))
+
+	veth="ns1eth$l"
+	ip -net "$ns" link set "$veth" down
+}
+
 do_transfer()
 {
 	listener_ns="$1"
@@ -161,9 +180,10 @@  do_transfer()
 	cl_proto="$3"
 	srv_proto="$4"
 	connect_addr="$5"
-	rm_nr_ns1="$6"
-	rm_nr_ns2="$7"
-	speed="$8"
+	test_link_fail="$6"
+	rm_nr_ns1="$7"
+	rm_nr_ns2="$8"
+	speed="$9"
 
 	port=$((10000+$TEST_COUNT))
 	TEST_COUNT=$((TEST_COUNT+1))
@@ -199,7 +219,12 @@  do_transfer()
 
 	sleep 1
 
-	ip netns exec ${connector_ns} $mptcp_connect -t $timeout -p $port -s ${cl_proto} $connect_addr < "$cin" > "$cout" &
+	if [ "$test_link_fail" -eq 0 ];then
+		ip netns exec ${connector_ns} $mptcp_connect -t $timeout -p $port -s ${cl_proto} $connect_addr < "$cin" > "$cout" &
+	else
+		( cat "$cin" ; sleep 2; link_failure $listener_ns ; cat "$cin" ) | tee "$cinsent" | \
+		ip netns exec ${connector_ns} $mptcp_connect -t $timeout -p $port -s ${cl_proto} $connect_addr > "$cout" &
+	fi
 	cpid=$!
 
 	if [ $rm_nr_ns1 -gt 0 ]; then
@@ -244,12 +269,17 @@  do_transfer()
 		ip netns exec ${connector_ns} ss -nita 1>&2 -o "dport = :$port"
 
 		cat "$capout"
+		ret=1
 		return 1
 	fi
 
 	check_transfer $sin $cout "file received by client"
 	retc=$?
-	check_transfer $cin $sout "file received by server"
+	if [ "$test_link_fail" -eq 0 ];then
+		check_transfer $cin $sout "file received by server"
+	else
+		check_transfer $cinsent $sout "file received by server"
+	fi
 	rets=$?
 
 	if [ $retc -eq 0 ] && [ $rets -eq 0 ];then
@@ -265,13 +295,12 @@  make_file()
 {
 	name=$1
 	who=$2
+	size=$3
 
-	SIZE=1
-
-	dd if=/dev/urandom of="$name" bs=1024 count=$SIZE 2> /dev/null
+	dd if=/dev/urandom of="$name" bs=1024 count=$size 2> /dev/null
 	echo -e "\nMPTCP_TEST_FILE_END_MARKER" >> "$name"
 
-	echo "Created $name (size $SIZE KB) containing data sent by $who"
+	echo "Created $name (size $size KB) containing data sent by $who"
 }
 
 run_tests()
@@ -279,14 +308,32 @@  run_tests()
 	listener_ns="$1"
 	connector_ns="$2"
 	connect_addr="$3"
-	rm_nr_ns1="${4:-0}"
-	rm_nr_ns2="${5:-0}"
-	speed="${6:-fast}"
+	test_linkfail="$4"
+	rm_nr_ns1="${5:-0}"
+	rm_nr_ns2="${6:-0}"
+	speed="${7:-fast}"
 	lret=0
+	oldin=""
+
+	if [ "$test_linkfail" -eq 1 ];then
+		size=$((RANDOM%1024))
+		size=$((size+1))
+		size=$((size*128))
+
+		oldin=$(mktemp)
+		cp "$cin" "$oldin"
+		make_file "$cin" "client" $size
+	fi
 
 	do_transfer ${listener_ns} ${connector_ns} MPTCP MPTCP ${connect_addr} \
-		${rm_nr_ns1} ${rm_nr_ns2} ${speed}
+		${test_linkfail} ${rm_nr_ns1} ${rm_nr_ns2} ${speed}
 	lret=$?
+
+	if [ "$test_linkfail" -eq 1 ];then
+		cp "$oldin" "$cin"
+		rm -f "$oldin"
+	fi
+
 	if [ $lret -ne 0 ]; then
 		ret=$lret
 		return
@@ -419,26 +466,27 @@  chk_rm_nr()
 sin=$(mktemp)
 sout=$(mktemp)
 cin=$(mktemp)
+cinsent=$(mktemp)
 cout=$(mktemp)
 init
-make_file "$cin" "client"
-make_file "$sin" "server"
+make_file "$cin" "client" 1
+make_file "$sin" "server" 1
 trap cleanup EXIT
 
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "no JOIN" "0" "0" "0"
 
 # subflow limted by client
 reset
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "single subflow, limited by client" 0 0 0
 
 # subflow limted by server
 reset
 ip netns exec $ns2 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "single subflow, limited by server" 1 1 0
 
 # subflow
@@ -446,7 +494,7 @@  reset
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "single subflow" 1 1 1
 
 # multiple subflows
@@ -455,7 +503,7 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "multiple subflows" 2 2 2
 
 # multiple subflows limited by serverf
@@ -464,13 +512,13 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "multiple subflows, limited by server" 2 2 1
 
 # add_address, unused
 reset
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "unused signal address" 0 0 0
 chk_add_nr 1 1
 
@@ -479,7 +527,7 @@  reset
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 1 1
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "signal address" 1 1 1
 chk_add_nr 1 1
 
@@ -492,7 +540,7 @@  ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl limits 1 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "subflow and signal" 2 2 2
 chk_add_nr 1 1
 
@@ -503,16 +551,27 @@  ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns2 ./pm_nl_ctl limits 1 3
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.4.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "multiple subflows and signal" 3 3 3
 chk_add_nr 1 1
 
+# accept and use add_addr with additional subflows and link loss
+reset
+ip netns exec $ns1 ./pm_nl_ctl limits 0 3
+ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
+ip netns exec $ns2 ./pm_nl_ctl limits 1 3
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
+ip netns exec $ns2 ./pm_nl_ctl add 10.0.4.2 flags subflow
+run_tests $ns1 $ns2 10.0.1.1 1
+chk_join_nr "multiple flows, signal, link failure" 3 3 3
+chk_add_nr 1 1
+
 # add_addr timeout
 reset_with_add_addr_timeout
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 1 1
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
-run_tests $ns1 $ns2 10.0.1.1 0 0 slow
+run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow
 chk_join_nr "signal address, ADD_ADDR timeout" 1 1 1
 chk_add_nr 4 0
 
@@ -521,7 +580,7 @@  reset
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1 0 1 slow
+run_tests $ns1 $ns2 10.0.1.1 0 0 1 slow
 chk_join_nr "remove single subflow" 1 1 1
 chk_rm_nr 1 1
 
@@ -531,7 +590,7 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1 0 2 slow
+run_tests $ns1 $ns2 10.0.1.1 0 0 2 slow
 chk_join_nr "remove multiple subflows" 2 2 2
 chk_rm_nr 2 2
 
@@ -540,7 +599,7 @@  reset
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns2 ./pm_nl_ctl limits 1 1
-run_tests $ns1 $ns2 10.0.1.1 1 0 slow
+run_tests $ns1 $ns2 10.0.1.1 0 1 0 slow
 chk_join_nr "remove single address" 1 1 1
 chk_add_nr 1 1
 chk_rm_nr 0 0
@@ -551,7 +610,7 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns2 ./pm_nl_ctl limits 1 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1 1 1 slow
+run_tests $ns1 $ns2 10.0.1.1 0 1 1 slow
 chk_join_nr "remove subflow and signal" 2 2 2
 chk_add_nr 1 1
 chk_rm_nr 1 1
@@ -563,7 +622,7 @@  ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns2 ./pm_nl_ctl limits 1 3
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.4.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1 1 2 slow
+run_tests $ns1 $ns2 10.0.1.1 0 1 2 slow
 chk_join_nr "remove subflows and signal" 3 3 3
 chk_add_nr 1 1
 chk_rm_nr 2 2
@@ -573,7 +632,7 @@  reset_with_cookies
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "single subflow with syn cookies" 1 1 1
 
 # multiple subflows with syn cookies
@@ -582,7 +641,7 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "multiple subflows with syn cookies" 2 2 2
 
 # multiple subflows limited by server
@@ -591,7 +650,7 @@  ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.2.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "subflows limited by server w cookies" 2 2 1
 
 # test signal address with cookies
@@ -599,7 +658,7 @@  reset_with_cookies
 ip netns exec $ns1 ./pm_nl_ctl limits 0 1
 ip netns exec $ns2 ./pm_nl_ctl limits 1 1
 ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "signal address with syn cookies" 1 1 1
 chk_add_nr 1 1
 
@@ -609,7 +668,7 @@  ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 ip netns exec $ns2 ./pm_nl_ctl limits 1 2
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "subflow and signal w cookies" 2 2 2
 chk_add_nr 1 1
 
@@ -620,7 +679,7 @@  ip netns exec $ns1 ./pm_nl_ctl add 10.0.2.1 flags signal
 ip netns exec $ns2 ./pm_nl_ctl limits 1 3
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.3.2 flags subflow
 ip netns exec $ns2 ./pm_nl_ctl add 10.0.4.2 flags subflow
-run_tests $ns1 $ns2 10.0.1.1
+run_tests $ns1 $ns2 10.0.1.1 0
 chk_join_nr "subflows and signal w. cookies" 3 3 3
 chk_add_nr 1 1