1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v3 0/8] mptcp: MIB counters for sent MP_JOIN
  4. View patch

[PATCH mptcp-next v3 4/8] selftests: mptcp: join: one line for join check

Matthieu Baerts (NGI0) posted 8 patches 4 months ago
There is a newer version of this series
[PATCH mptcp-next v3 4/8] selftests: mptcp: join: one line for join check
Posted by Matthieu Baerts (NGI0) 4 months ago 
Most tests are checking if the expected number of SYN/SYN+ACK/ACK JOINs
have been received, each of them on one line.

More Join related tests are going to be checked soon, no need to add 5
new lines per test in case of success, just one is enough. In case of
issue, the errors will still be reported like before.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 45 ++++++++++++++++---------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 0401ba1aaf1b..b10bcb1ac970 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -202,6 +202,22 @@ print_skip()
 	mptcp_lib_pr_skip "${@}"
 }
 
+# $1: check name; $2: rc
+print_results()
+{
+	local check="${1}"
+	local rc=${2}
+
+	print_check "${check}"
+	if [ ${rc} = ${KSFT_PASS} ]; then
+		print_ok
+	elif [ ${rc} = ${KSFT_SKIP} ]; then
+		print_skip
+	else
+		fail_test
+	fi
+}
+
 # [ $1: fail msg ]
 mark_as_skipped()
 {
@@ -1326,6 +1342,7 @@ chk_join_nr()
 	local rst_nr=${join_rst_nr:-0}
 	local infi_nr=${join_infi_nr:-0}
 	local corrupted_pkts=${join_corrupted_pkts:-0}
+	local rc=${KSFT_PASS}
 	local count
 	local with_cookie
 
@@ -1333,43 +1350,41 @@ chk_join_nr()
 		print_info "${corrupted_pkts} corrupted pkts"
 	fi
 
-	print_check "syn"
 	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinSynRx")
 	if [ -z "$count" ]; then
-		print_skip
+		rc=${KSFT_SKIP}
 	elif [ "$count" != "$syn_nr" ]; then
+		rc=${KSFT_FAIL}
+		print_check "syn"
 		fail_test "got $count JOIN[s] syn expected $syn_nr"
-	else
-		print_ok
 	fi
 
-	print_check "synack"
 	with_cookie=$(ip netns exec $ns2 sysctl -n net.ipv4.tcp_syncookies)
 	count=$(mptcp_lib_get_counter ${ns2} "MPTcpExtMPJoinSynAckRx")
 	if [ -z "$count" ]; then
-		print_skip
+		rc=${KSFT_SKIP}
 	elif [ "$count" != "$syn_ack_nr" ]; then
 		# simult connections exceeding the limit with cookie enabled could go up to
 		# synack validation as the conn limit can be enforced reliably only after
 		# the subflow creation
-		if [ "$with_cookie" = 2 ] && [ "$count" -gt "$syn_ack_nr" ] && [ "$count" -le "$syn_nr" ]; then
-			print_ok
-		else
+		if [ "$with_cookie" != 2 ] || [ "$count" -le "$syn_ack_nr" ] || [ "$count" -gt "$syn_nr" ]; then
+			rc=${KSFT_FAIL}
+			print_check "synack"
 			fail_test "got $count JOIN[s] synack expected $syn_ack_nr"
 		fi
-	else
-		print_ok
 	fi
 
-	print_check "ack"
 	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinAckRx")
 	if [ -z "$count" ]; then
-		print_skip
+		rc=${KSFT_SKIP}
 	elif [ "$count" != "$ack_nr" ]; then
+		rc=${KSFT_FAIL}
+		print_check "ack"
 		fail_test "got $count JOIN[s] ack expected $ack_nr"
-	else
-		print_ok
 	fi
+
+	print_results "join Rx" ${rc}
+
 	if $validate_checksum; then
 		chk_csum_nr $csum_ns1 $csum_ns2
 		chk_fail_nr $fail_nr $fail_nr

-- 
2.45.2
Re: [PATCH mptcp-next v3 4/8] selftests: mptcp: join: one line for join check
Posted by Geliang Tang 3 months, 3 weeks ago 
On Tue, 2024-08-06 at 13:18 +0200, Matthieu Baerts (NGI0) wrote:
> Most tests are checking if the expected number of SYN/SYN+ACK/ACK
> JOINs
> have been received, each of them on one line.
> 
> More Join related tests are going to be checked soon, no need to add
> 5
> new lines per test in case of success, just one is enough. In case of
> issue, the errors will still be reported like before.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 45
> ++++++++++++++++---------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 0401ba1aaf1b..b10bcb1ac970 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -202,6 +202,22 @@ print_skip()
>  	mptcp_lib_pr_skip "${@}"
>  }
>  
> +# $1: check name; $2: rc
> +print_results()
> +{
> +	local check="${1}"
> +	local rc=${2}
> +
> +	print_check "${check}"
> +	if [ ${rc} = ${KSFT_PASS} ]; then
> +		print_ok
> +	elif [ ${rc} = ${KSFT_SKIP} ]; then
> +		print_skip
> +	else
> +		fail_test

To align error messages, it's better to pass an empty string to
fail_test to make sure an "EOL" is printed out:

		fail_test ""

001 no JOIN
      syn rx                 [FAIL] got 0 JOIN[s] syn rx expected 1
      synack rx              [FAIL] got 0 JOIN[s] synack rx expected 1
      ack rx                 [FAIL] got 0 JOIN[s] ack rx expected 1
      join Rx                syn tx                              [FAIL]
got 0 JOIN[s] syn tx expected 1
      join Tx                002 single subflow, limited by client
      join Rx                             [ OK ]
      join Tx                             [ OK ]

->

001 no JOIN
      syn rx                 [FAIL] got 0 JOIN[s] syn rx 
      synack rx              [FAIL] got 0 JOIN[s] synack rx expected 1
      ack rx                 [FAIL] got 0 JOIN[s] ack rx expected 1
      join Rx                [FAIL]
      syn tx                 [FAIL] got 0 JOIN[s] syn tx expected 1
      join Tx                [FAIL]
002 single subflow, limited by client

WDYT?

Regards,
-Geliang

> +	fi
> +}
> +
>  # [ $1: fail msg ]
>  mark_as_skipped()
>  {
> @@ -1326,6 +1342,7 @@ chk_join_nr()
>  	local rst_nr=${join_rst_nr:-0}
>  	local infi_nr=${join_infi_nr:-0}
>  	local corrupted_pkts=${join_corrupted_pkts:-0}
> +	local rc=${KSFT_PASS}
>  	local count
>  	local with_cookie
>  
> @@ -1333,43 +1350,41 @@ chk_join_nr()
>  		print_info "${corrupted_pkts} corrupted pkts"
>  	fi
>  
> -	print_check "syn"
>  	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinSynRx")
>  	if [ -z "$count" ]; then
> -		print_skip
> +		rc=${KSFT_SKIP}
>  	elif [ "$count" != "$syn_nr" ]; then
> +		rc=${KSFT_FAIL}
> +		print_check "syn"
>  		fail_test "got $count JOIN[s] syn expected $syn_nr"
> -	else
> -		print_ok
>  	fi
>  
> -	print_check "synack"
>  	with_cookie=$(ip netns exec $ns2 sysctl -n
> net.ipv4.tcp_syncookies)
>  	count=$(mptcp_lib_get_counter ${ns2}
> "MPTcpExtMPJoinSynAckRx")
>  	if [ -z "$count" ]; then
> -		print_skip
> +		rc=${KSFT_SKIP}
>  	elif [ "$count" != "$syn_ack_nr" ]; then
>  		# simult connections exceeding the limit with cookie
> enabled could go up to
>  		# synack validation as the conn limit can be
> enforced reliably only after
>  		# the subflow creation
> -		if [ "$with_cookie" = 2 ] && [ "$count" -gt
> "$syn_ack_nr" ] && [ "$count" -le "$syn_nr" ]; then
> -			print_ok
> -		else
> +		if [ "$with_cookie" != 2 ] || [ "$count" -le
> "$syn_ack_nr" ] || [ "$count" -gt "$syn_nr" ]; then
> +			rc=${KSFT_FAIL}
> +			print_check "synack"
>  			fail_test "got $count JOIN[s] synack
> expected $syn_ack_nr"
>  		fi
> -	else
> -		print_ok
>  	fi
>  
> -	print_check "ack"
>  	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtMPJoinAckRx")
>  	if [ -z "$count" ]; then
> -		print_skip
> +		rc=${KSFT_SKIP}
>  	elif [ "$count" != "$ack_nr" ]; then
> +		rc=${KSFT_FAIL}
> +		print_check "ack"
>  		fail_test "got $count JOIN[s] ack expected $ack_nr"
> -	else
> -		print_ok
>  	fi
> +
> +	print_results "join Rx" ${rc}
> +
>  	if $validate_checksum; then
>  		chk_csum_nr $csum_ns1 $csum_ns2
>  		chk_fail_nr $fail_nr $fail_nr
>
Re: [PATCH mptcp-next v3 4/8] selftests: mptcp: join: one line for join check
Posted by Matthieu Baerts 3 months, 3 weeks ago 
Hi Geliang,

On 09/08/2024 04:10, Geliang Tang wrote:
> On Tue, 2024-08-06 at 13:18 +0200, Matthieu Baerts (NGI0) wrote:
>> Most tests are checking if the expected number of SYN/SYN+ACK/ACK
>> JOINs
>> have been received, each of them on one line.
>>
>> More Join related tests are going to be checked soon, no need to add
>> 5
>> new lines per test in case of success, just one is enough. In case of
>> issue, the errors will still be reported like before.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 45
>> ++++++++++++++++---------
>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> index 0401ba1aaf1b..b10bcb1ac970 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>> @@ -202,6 +202,22 @@ print_skip()
>>  	mptcp_lib_pr_skip "${@}"
>>  }
>>  
>> +# $1: check name; $2: rc
>> +print_results()
>> +{
>> +	local check="${1}"
>> +	local rc=${2}
>> +
>> +	print_check "${check}"
>> +	if [ ${rc} = ${KSFT_PASS} ]; then
>> +		print_ok
>> +	elif [ ${rc} = ${KSFT_SKIP} ]; then
>> +		print_skip
>> +	else
>> +		fail_test
> 
> To align error messages, it's better to pass an empty string to
> fail_test to make sure an "EOL" is printed out:
> 
> 		fail_test ""
> 
> 001 no JOIN
>       syn rx                 [FAIL] got 0 JOIN[s] syn rx expected 1
>       synack rx              [FAIL] got 0 JOIN[s] synack rx expected 1
>       ack rx                 [FAIL] got 0 JOIN[s] ack rx expected 1
>       join Rx                syn tx                              [FAIL]
> got 0 JOIN[s] syn tx expected 1
>       join Tx                002 single subflow, limited by client
>       join Rx                             [ OK ]
>       join Tx                             [ OK ]
> 
> ->
> 
> 001 no JOIN
>       syn rx                 [FAIL] got 0 JOIN[s] syn rx 
>       synack rx              [FAIL] got 0 JOIN[s] synack rx expected 1
>       ack rx                 [FAIL] got 0 JOIN[s] ack rx expected 1
>       join Rx                [FAIL]
>       syn tx                 [FAIL] got 0 JOIN[s] syn tx expected 1
>       join Tx                [FAIL]
> 002 single subflow, limited by client
> 
> WDYT?

Good point. We can also print "see above" instead of nothing.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.