1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next 0/5] selftests: mptcp: more info in case of errors
  4. View patch

[PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh

Matthieu Baerts (NGI0) posted 5 patches 1 week, 6 days ago
[PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh
Posted by Matthieu Baerts (NGI0) 1 week, 6 days ago 
A few MPTCP selftests are using the same code to print stats in case of
error. This code can then be moved to mptcp_lib.sh.

No behaviour changes intended, except to print the error in red and to
stderr, like most error messages.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh |  8 ++------
 tools/testing/selftests/net/mptcp/mptcp_join.sh    |  9 ++-------
 tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 21 +++++++++++++++++++++
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh |  6 +-----
 tools/testing/selftests/net/mptcp/simult_flows.sh  |  8 ++------
 5 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index b48b4e56826a9cfdb3501242b707ae2ebe29b220..bfdaecd0a6a0564020530345daf91bed296bc15c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -445,12 +445,8 @@ do_transfer()
 	printf "(duration %05sms) " "${duration}"
 	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
 		mptcp_lib_pr_fail "client exit code $retc, server $rets"
-		echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port"
-		cat /tmp/${listener_ns}.out
-		echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port"
-		[ ${listener_ns} != ${connector_ns} ] && cat /tmp/${connector_ns}.out
+		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \
+			"/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out"
 
 		echo
 		cat "$capout"
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c07e2bd3a315aac9c422fed85c3196ec46e060f7..13a3b68181ee14eb628a858e5738094c3c936b74 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1039,13 +1039,8 @@ do_transfer()
 
 	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
 		fail_test "client exit code $retc, server $rets"
-		echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port"
-		cat /tmp/${listener_ns}.out
-		echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port"
-		cat /tmp/${connector_ns}.out
-
+		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \
+			"/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out"
 		return 1
 	fi
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d825337978fdba 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -107,6 +107,27 @@ mptcp_lib_pr_info() {
 	mptcp_lib_print_info "INFO: ${*}"
 }
 
+# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector stat file ]
+mptcp_lib_pr_err_stats() {
+	local lns="${1}"
+	local cns="${2}"
+	local port="${3}"
+	local lstat="${4:-}"
+	local cstat="${5:-}"
+
+	echo -en "${MPTCP_LIB_COLOR_RED}"
+	{
+		printf "\nnetns %s (listener) socket stat for %d:\n" "${lns}" "${port}"
+		ip netns exec "${lns}" ss -Menita -o "sport = :${port}"
+		[ -s "${lstat}" ] && cat "${lstat}"
+
+		printf "\nnetns %s (connector) socket stat for %d:\n" "${cns}" "${port}"
+		ip netns exec "${cns}" ss -Menita -o "dport = :${port}"
+		[ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] && cat "${cstat}"
+	} 1>&2
+	echo -en "${MPTCP_LIB_COLOR_RESET}"
+}
+
 # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when validating all
 # features using the last version of the kernel and the selftests to make sure
 # a test is not being skipped by mistake.
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 5e8d5b83e2d092879efc179f1a450542be4e575e..6b366c604a9eeccdb759f260be4feafc380ccb0b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -192,11 +192,7 @@ do_transfer()
 	print_title "Transfer ${ip:2}"
 	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
 		mptcp_lib_pr_fail "client exit code $retc, server $rets"
-		echo -e "\nnetns ${listener_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${listener_ns} ss -Menita 1>&2 -o "sport = :$port"
-
-		echo -e "\nnetns ${connector_ns} socket stat for ${port}:" 1>&2
-		ip netns exec ${connector_ns} ss -Menita 1>&2 -o "dport = :$port"
+		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 
 		mptcp_lib_result_fail "transfer ${ip}"
 
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index e98e5907d52c2d0e9c0152efda82176861905cf1..9c2a415976cbf7a0b56cd4b2fbdd36c9e1ef3c8c 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -204,12 +204,8 @@ do_transfer()
 	fi
 
 	mptcp_lib_pr_fail "client exit code $retc, server $rets"
-	echo -e "\nnetns ${ns3} socket stat for $port:" 1>&2
-	ip netns exec ${ns3} ss -Menita 1>&2 -o "sport = :$port"
-	cat /tmp/${ns3}.out
-	echo -e "\nnetns ${ns1} socket stat for $port:" 1>&2
-	ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port"
-	cat /tmp/${ns1}.out
+	mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}" \
+		"/tmp/${ns3}.out" "/tmp/${ns1}.out"
 	ls -l $sin $cout
 	ls -l $cin $sout
 

-- 
2.47.1
Re: [PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh
Posted by Geliang Tang 1 week, 5 days ago 
Hi Matt,

Thanks for this set.

On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
> A few MPTCP selftests are using the same code to print stats in case
> of
> error. This code can then be moved to mptcp_lib.sh.
> 
> No behaviour changes intended, except to print the error in red and
> to
> stderr, like most error messages.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh |  8 ++------
>  tools/testing/selftests/net/mptcp/mptcp_join.sh    |  9 ++-------
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 21
> +++++++++++++++++++++
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh |  6 +-----
>  tools/testing/selftests/net/mptcp/simult_flows.sh  |  8 ++------
>  5 files changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index
> b48b4e56826a9cfdb3501242b707ae2ebe29b220..bfdaecd0a6a0564020530345daf
> 91bed296bc15c 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -445,12 +445,8 @@ do_transfer()
>  	printf "(duration %05sms) " "${duration}"
>  	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
>  		mptcp_lib_pr_fail "client exit code $retc, server
> $rets"
> -		echo -e "\nnetns ${listener_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${listener_ns} ss -Menita 1>&2 -o
> "sport = :$port"
> -		cat /tmp/${listener_ns}.out
> -		echo -e "\nnetns ${connector_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${connector_ns} ss -Menita 1>&2 -o
> "dport = :$port"
> -		[ ${listener_ns} != ${connector_ns} ] && cat
> /tmp/${connector_ns}.out
> +		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}" \
> +			"/tmp/${listener_ns}.out"
> "/tmp/${connector_ns}.out"
>  
>  		echo
>  		cat "$capout"
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index
> c07e2bd3a315aac9c422fed85c3196ec46e060f7..13a3b68181ee14eb628a858e573
> 8094c3c936b74 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1039,13 +1039,8 @@ do_transfer()
>  
>  	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
>  		fail_test "client exit code $retc, server $rets"
> -		echo -e "\nnetns ${listener_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${listener_ns} ss -Menita 1>&2 -o
> "sport = :$port"
> -		cat /tmp/${listener_ns}.out
> -		echo -e "\nnetns ${connector_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${connector_ns} ss -Menita 1>&2 -o
> "dport = :$port"
> -		cat /tmp/${connector_ns}.out
> -
> +		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}" \
> +			"/tmp/${listener_ns}.out"
> "/tmp/${connector_ns}.out"
>  		return 1
>  	fi
>  
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index
> 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d
> 825337978fdba 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -107,6 +107,27 @@ mptcp_lib_pr_info() {
>  	mptcp_lib_print_info "INFO: ${*}"
>  }
>  
> +# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector
> stat file ]
> +mptcp_lib_pr_err_stats() {
> +	local lns="${1}"
> +	local cns="${2}"
> +	local port="${3}"
> +	local lstat="${4:-}"
> +	local cstat="${5:-}"

I think it's better to add a separate patch to add

	cat /tmp/${listener_ns}.out

and

	cat /tmp/${connector_ns}.out

in mptcp_sockopt.sh, then we can drop the last two arguments "lstat"
and "cstat" of this helper.

WDYT?

Thanks,
-Geliang

> +
> +	echo -en "${MPTCP_LIB_COLOR_RED}"
> +	{
> +		printf "\nnetns %s (listener) socket stat for %d:\n"
> "${lns}" "${port}"
> +		ip netns exec "${lns}" ss -Menita -o "sport =
> :${port}"
> +		[ -s "${lstat}" ] && cat "${lstat}"
> +
> +		printf "\nnetns %s (connector) socket stat for
> %d:\n" "${cns}" "${port}"
> +		ip netns exec "${cns}" ss -Menita -o "dport =
> :${port}"
> +		[ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] &&
> cat "${cstat}"
> +	} 1>&2
> +	echo -en "${MPTCP_LIB_COLOR_RESET}"
> +}
> +
>  # SELFTESTS_MPTCP_LIB_EXPECT_ALL_FEATURES env var can be set when
> validating all
>  # features using the last version of the kernel and the selftests to
> make sure
>  # a test is not being skipped by mistake.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index
> 5e8d5b83e2d092879efc179f1a450542be4e575e..6b366c604a9eeccdb759f260be4
> feafc380ccb0b 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -192,11 +192,7 @@ do_transfer()
>  	print_title "Transfer ${ip:2}"
>  	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
>  		mptcp_lib_pr_fail "client exit code $retc, server
> $rets"
> -		echo -e "\nnetns ${listener_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${listener_ns} ss -Menita 1>&2 -o
> "sport = :$port"
> -
> -		echo -e "\nnetns ${connector_ns} socket stat for
> ${port}:" 1>&2
> -		ip netns exec ${connector_ns} ss -Menita 1>&2 -o
> "dport = :$port"
> +		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}"
>  
>  		mptcp_lib_result_fail "transfer ${ip}"
>  
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
> b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index
> e98e5907d52c2d0e9c0152efda82176861905cf1..9c2a415976cbf7a0b56cd4b2fbd
> d36c9e1ef3c8c 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -204,12 +204,8 @@ do_transfer()
>  	fi
>  
>  	mptcp_lib_pr_fail "client exit code $retc, server $rets"
> -	echo -e "\nnetns ${ns3} socket stat for $port:" 1>&2
> -	ip netns exec ${ns3} ss -Menita 1>&2 -o "sport = :$port"
> -	cat /tmp/${ns3}.out
> -	echo -e "\nnetns ${ns1} socket stat for $port:" 1>&2
> -	ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port"
> -	cat /tmp/${ns1}.out
> +	mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}" \
> +		"/tmp/${ns3}.out" "/tmp/${ns1}.out"
>  	ls -l $sin $cout
>  	ls -l $cin $sout
>  
>
Re: [PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh
Posted by Matthieu Baerts 1 week, 5 days ago 
Hi Geliang,

Thank you for your review!

On 09/01/2025 10:24, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this set.
> 
> On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
>> A few MPTCP selftests are using the same code to print stats in case
>> of
>> error. This code can then be moved to mptcp_lib.sh.
>>
>> No behaviour changes intended, except to print the error in red and
>> to
>> stderr, like most error messages.

(...)

>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> index
>> 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6647d
>> 825337978fdba 100644
>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> @@ -107,6 +107,27 @@ mptcp_lib_pr_info() {
>>  	mptcp_lib_print_info "INFO: ${*}"
>>  }
>>  
>> +# $1-2: listener/connector ns ; $3 port ; [ $4-5 listener/connector
>> stat file ]
>> +mptcp_lib_pr_err_stats() {
>> +	local lns="${1}"
>> +	local cns="${2}"
>> +	local port="${3}"
>> +	local lstat="${4:-}"
>> +	local cstat="${5:-}"
> 
> I think it's better to add a separate patch to add
> 
> 	cat /tmp/${listener_ns}.out
> 
> and
> 
> 	cat /tmp/${connector_ns}.out
> 
> in mptcp_sockopt.sh,

mptcp_sockopt.sh currently doesn't have such file. I hesitated to add
them using the content of nstat like in the other ones, but I didn't
really see how it would help to fix issues with socket options. Or do
you have something in mind?

I guess we could add them "just in case", but...

> then we can drop the last two arguments "lstat"
> and "cstat" of this helper.

... sorry, I'm not sure to understand what you meant here. We still want
to print the content of these files if available.


Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh
Posted by Geliang Tang 1 week, 4 days ago 
Hi Matt,

On Thu, 2025-01-09 at 16:30 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for your review!
> 
> On 09/01/2025 10:24, Geliang Tang wrote:
> > Hi Matt,
> > 
> > Thanks for this set.
> > 
> > On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
> > > A few MPTCP selftests are using the same code to print stats in
> > > case
> > > of
> > > error. This code can then be moved to mptcp_lib.sh.
> > > 
> > > No behaviour changes intended, except to print the error in red
> > > and
> > > to
> > > stderr, like most error messages.
> 
> (...)
> 
> > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > index
> > > 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6
> > > 647d
> > > 825337978fdba 100644
> > > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > @@ -107,6 +107,27 @@ mptcp_lib_pr_info() {
> > >  	mptcp_lib_print_info "INFO: ${*}"
> > >  }
> > >  
> > > +# $1-2: listener/connector ns ; $3 port ; [ $4-5
> > > listener/connector
> > > stat file ]
> > > +mptcp_lib_pr_err_stats() {
> > > +	local lns="${1}"
> > > +	local cns="${2}"
> > > +	local port="${3}"
> > > +	local lstat="${4:-}"
> > > +	local cstat="${5:-}"
> > 
> > I think it's better to add a separate patch to add
> > 
> > 	cat /tmp/${listener_ns}.out
> > 
> > and
> > 
> > 	cat /tmp/${connector_ns}.out
> > 
> > in mptcp_sockopt.sh,
> 
> mptcp_sockopt.sh currently doesn't have such file. I hesitated to add
> them using the content of nstat like in the other ones, but I didn't
> really see how it would help to fix issues with socket options. Or do
> you have something in mind?
> 
> I guess we could add them "just in case", but...

I just sent a patch named "selftests: mptcp: sockopt: save nstat infos"
to do this.

> 
> > then we can drop the last two arguments "lstat"
> > and "cstat" of this helper.
> 
> ... sorry, I'm not sure to understand what you meant here. We still
> want
> to print the content of these files if available.

And a squash-to patch for this.

Thanks,
-Geliang

> 
> 
> Cheers,
> Matt

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.