[PATCH mptcp-next 0/5] selftests: mptcp: more info in case of errors

[PATCH mptcp-next 0/5] selftests: mptcp: more info in case of errors

[Matthieu Baerts (NGI0)]

Here are just a bunch of small improvements:

- MIB and 'ss -Me' in simult_flows.sh
- move common code to lib.sh
- use 'ss' with '-m'
- remove unused variable
- show only the size instead of size + filename

Nothing exiting, but the 3rd one can help investigating some issues like
https://github.com/multipath-tcp/mptcp_net-next/issues/499

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (5):
      selftests: mptcp: simult_flows: unify errors msgs
      selftests: mptcp: move stats info in case of errors to lib.sh
      selftests: mptcp: add -m with ss in case of errors
      selftests: mptcp: connect: remove unused variable
      selftests: mptcp: connect: better display the files size

 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 13 ++++---------
 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  | 21 ++++++++++++++-------
 5 files changed, 42 insertions(+), 28 deletions(-)
---
base-commit: f289bc80702ce603593169e80f339e9ffc48cafc
change-id: 20250108-slft-mptcp-conn-misc-impr-0cc5537a8bf2

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH mptcp-next 1/5] selftests: mptcp: simult_flows: unify errors msgs

[Matthieu Baerts (NGI0)]

In order to unify what is printed in case of error, similar to what is
done in mptcp_connect.sh and mptcp_join.sh, it is interesting to do the
following modifications in simult_flows.sh:

- Print the rc errors at the end of the line.

- Print the MIB counters.

- Use the same ss options: add -M (MPTCP sockets) and -e (detailed
  socket information).

While at it, also print of the 'max' time only in case of success,
because 'mptcp_connect.c' will already print this info in case of error,
e.g.:

  transfer slower than expected! runtime 11948 ms, expected 11921 ms

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

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 8fa77c8e9b651171a34c89bfd5c9ded0288a5bde..e98e5907d52c2d0e9c0152efda82176861905cf1 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -155,6 +155,11 @@ do_transfer()
 		sleep 1
 	fi
 
+	NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
+		nstat -n
+	NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
+		nstat -n
+
 	timeout ${timeout_test} \
 		ip netns exec ${ns3} \
 			./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \
@@ -180,25 +185,31 @@ do_transfer()
 		kill ${cappid_connector}
 	fi
 
+	NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
+		nstat | grep Tcp > /tmp/${ns3}.out
+	NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
+		nstat | grep Tcp > /tmp/${ns1}.out
+
 	cmp $sin $cout > /dev/null 2>&1
 	local cmps=$?
 	cmp $cin $sout > /dev/null 2>&1
 	local cmpc=$?
 
-	printf "%-16s" " max $max_time "
 	if [ $retc -eq 0 ] && [ $rets -eq 0 ] && \
 	   [ $cmpc -eq 0 ] && [ $cmps -eq 0 ]; then
+		printf "%-16s" " max $max_time "
 		mptcp_lib_pr_ok
 		cat "$capout"
 		return 0
 	fi
 
-	mptcp_lib_pr_fail
-	echo "client exit code $retc, server $rets" 1>&2
+	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 -nita 1>&2 -o "sport = :$port"
+	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 -nita 1>&2 -o "dport = :$port"
+	ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port"
+	cat /tmp/${ns1}.out
 	ls -l $sin $cout
 	ls -l $cin $sout
 

-- 
2.47.1

Re: [PATCH mptcp-next 1/5] selftests: mptcp: simult_flows: unify errors msgs

[Geliang Tang]

Hi Matt,

On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
> In order to unify what is printed in case of error, similar to what
> is
> done in mptcp_connect.sh and mptcp_join.sh, it is interesting to do
> the
> following modifications in simult_flows.sh:
> 
> - Print the rc errors at the end of the line.
> 
> - Print the MIB counters.
> 
> - Use the same ss options: add -M (MPTCP sockets) and -e (detailed
>   socket information).
> 
> While at it, also print of the 'max' time only in case of success,
> because 'mptcp_connect.c' will already print this info in case of
> error,
> e.g.:
> 
>   transfer slower than expected! runtime 11948 ms, expected 11921 ms
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/simult_flows.sh | 21
> ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
> b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index
> 8fa77c8e9b651171a34c89bfd5c9ded0288a5bde..e98e5907d52c2d0e9c0152efda8
> 2176861905cf1 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -155,6 +155,11 @@ do_transfer()
>   sleep 1
>   fi
>  
> + NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
> + nstat -n
> + NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
> + nstat -n
> +
>   timeout ${timeout_test} \
>   ip netns exec ${ns3} \
>   ./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \
> @@ -180,25 +185,31 @@ do_transfer()
>   kill ${cappid_connector}
>   fi
>  
> + NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
> + nstat | grep Tcp > /tmp/${ns3}.out
> + NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
> + nstat | grep Tcp > /tmp/${ns1}.out

Here I got "nstat: history is aged out, resetting" warnings, and I
don't know how to fix it:

Selftest Test: ./simult_flows.sh
TAP version 13
1..1
[    1.022664] ip (155) used greatest stack depth: 12280 bytes left
[    1.316990] netem: version 1.3
# 01 balanced bwidth                                            
7401nstat: history is aged out, resetting
# nstat: history is aged out, resetting
#  max 7906       [ OK ]
# 02 balanced bwidth - reverse direction                         7394
max 7906       [ OK ]

> +
>   cmp $sin $cout > /dev/null 2>&1
>   local cmps=$?
>   cmp $cin $sout > /dev/null 2>&1
>   local cmpc=$?
>  
> - printf "%-16s" " max $max_time "
>   if [ $retc -eq 0 ] && [ $rets -eq 0 ] && \
>      [ $cmpc -eq 0 ] && [ $cmps -eq 0 ]; then
> + printf "%-16s" " max $max_time "
>   mptcp_lib_pr_ok
>   cat "$capout"
>   return 0
>   fi
>  
> - mptcp_lib_pr_fail
> - echo "client exit code $retc, server $rets" 1>&2
> + 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 -nita 1>&2 -o "sport = :$port"
> + 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 -nita 1>&2 -o "dport = :$port"
> + ip netns exec ${ns1} ss -Menita 1>&2 -o "dport = :$port"
> + cat /tmp/${ns1}.out
>   ls -l $sin $cout
>   ls -l $cin $sout
>  
> 

Re: [PATCH mptcp-next 1/5] selftests: mptcp: simult_flows: unify errors msgs

[Matthieu Baerts]

Hi Geliang,

On 10/01/2025 09:16, Geliang Tang wrote:
> Hi Matt,
> 
> On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
>> In order to unify what is printed in case of error, similar to what
>> is
>> done in mptcp_connect.sh and mptcp_join.sh, it is interesting to do
>> the
>> following modifications in simult_flows.sh:
>>
>> - Print the rc errors at the end of the line.
>>
>> - Print the MIB counters.
>>
>> - Use the same ss options: add -M (MPTCP sockets) and -e (detailed
>>   socket information).
>>
>> While at it, also print of the 'max' time only in case of success,
>> because 'mptcp_connect.c' will already print this info in case of
>> error,
>> e.g.:
>>
>>   transfer slower than expected! runtime 11948 ms, expected 11921 ms
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  tools/testing/selftests/net/mptcp/simult_flows.sh | 21
>> ++++++++++++++++-----
>>  1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
>> b/tools/testing/selftests/net/mptcp/simult_flows.sh
>> index
>> 8fa77c8e9b651171a34c89bfd5c9ded0288a5bde..e98e5907d52c2d0e9c0152efda8
>> 2176861905cf1 100755
>> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
>> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
>> @@ -155,6 +155,11 @@ do_transfer()
>>   sleep 1
>>   fi
>>  
>> + NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
>> + nstat -n
>> + NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
>> + nstat -n
>> +
>>   timeout ${timeout_test} \
>>   ip netns exec ${ns3} \
>>   ./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \
>> @@ -180,25 +185,31 @@ do_transfer()
>>   kill ${cappid_connector}
>>   fi
>>  
>> + NSTAT_HISTORY=/tmp/${ns3}.nstat ip netns exec ${ns3} \
>> + nstat | grep Tcp > /tmp/${ns3}.out
>> + NSTAT_HISTORY=/tmp/${ns1}.nstat ip netns exec ${ns1} \
>> + nstat | grep Tcp > /tmp/${ns1}.out
> 
> Here I got "nstat: history is aged out, resetting" warnings, and I
> don't know how to fix it:
> 
> Selftest Test: ./simult_flows.sh
> TAP version 13
> 1..1
> [    1.022664] ip (155) used greatest stack depth: 12280 bytes left
> [    1.316990] netem: version 1.3
> # 01 balanced bwidth                                            
> 7401nstat: history is aged out, resetting
> # nstat: history is aged out, resetting
> #  max 7906       [ OK ]
> # 02 balanced bwidth - reverse direction                         7394
> max 7906       [ OK ]

Interesting: I already saw this message on NIPA, but I could not
reproduce it on my side.

Could you eventually try to find out what it is? Maybe the stats from
files in /tmp are wrong inside your VM?

The code of nstat is in the iproute2 repo:

https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/tree/misc/nstat.c#n691

  if (!ignore_history) {
      FILE *tfp;
      long uptime = -1;

      if ((tfp = fopen("/proc/uptime", "r")) != NULL) {
          if (fscanf(tfp, "%ld", &uptime) != 1)
              uptime = -1;
          fclose(tfp);
      }
      if (uptime >= 0 && time(NULL) >= stb.st_mtime+uptime) {
          fprintf(stderr, "nstat: history is aged out, resetting\n");
          if (ftruncate(fileno(hist_fp), 0) < 0)
              perror("nstat: ftruncate");
      }
  }

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

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

[Matthieu Baerts (NGI0)]

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

[Geliang Tang]

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

[Matthieu Baerts]

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

[Geliang Tang]

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

[PATCH mptcp-next 3/5] selftests: mptcp: add -m with ss in case of errors

[Matthieu Baerts (NGI0)]

Recently, we had an issue where getting info about the memory would have
helped better understanding what went wrong.

Let add it just in case for later.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 50ca64357053b14cd4989e6647d825337978fdba..1f797d5f5156ebc99e5e0be5ea27483bc9cb4c65 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -118,11 +118,11 @@ mptcp_lib_pr_err_stats() {
 	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}"
+		ip netns exec "${lns}" ss -Menitam -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}"
+		ip netns exec "${cns}" ss -Menitam -o "dport = :${port}"
 		[ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] && cat "${cstat}"
 	} 1>&2
 	echo -en "${MPTCP_LIB_COLOR_RESET}"

-- 
2.47.1

[PATCH mptcp-next 4/5] selftests: mptcp: connect: remove unused variable

[Matthieu Baerts (NGI0)]

'cin_disconnect' is used in run_tests_disconnect(), but not
'cout_disconnect', so it is safe to drop it.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index bfdaecd0a6a0564020530345daf91bed296bc15c..e508d356fcdaebbfb95750bba0fa834a8463e32a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -137,7 +137,7 @@ TEST_GROUP=""
 #shellcheck disable=SC2317
 cleanup()
 {
-	rm -f "$cin_disconnect" "$cout_disconnect"
+	rm -f "$cin_disconnect"
 	rm -f "$cin" "$cout"
 	rm -f "$sin" "$sout"
 	rm -f "$capout"
@@ -155,7 +155,6 @@ cin=$(mktemp)
 cout=$(mktemp)
 capout=$(mktemp)
 cin_disconnect="$cin".disconnect
-cout_disconnect="$cout".disconnect
 trap cleanup EXIT
 
 mptcp_lib_ns_init ns1 ns2 ns3 ns4

-- 
2.47.1

[PATCH mptcp-next 5/5] selftests: mptcp: connect: better display the files size

[Matthieu Baerts (NGI0)]

'du' will print the name of the file, which was already displayed
before, e.g.

  Created /tmp/tmp.UOyy0ghfmQ (size 4703740/tmp/tmp.UOyy0ghfmQ) containing data sent by client
  Created /tmp/tmp.xq3zvFinGo (size 1391724/tmp/tmp.xq3zvFinGo) containing data sent by server

'stat' can be used instead, to display this instead:

  Created /tmp/tmp.UOyy0ghfmQ (size 4703740 B) containing data sent by client
  Created /tmp/tmp.xq3zvFinGo (size 1391724 B) containing data sent by server

So easier to spot the file sizes.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index e508d356fcdaebbfb95750bba0fa834a8463e32a..5e3c56253274a1f938d2ed9986c4290fcea8b96b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -582,7 +582,7 @@ make_file()
 	mptcp_lib_make_file $name 1024 $ksize
 	dd if=/dev/urandom conv=notrunc of="$name" oflag=append bs=1 count=$rem 2> /dev/null
 
-	echo "Created $name (size $(du -b "$name")) containing data sent by $who"
+	echo "Created $name (size $(stat -c "%s" "$name") B) containing data sent by $who"
 }
 
 run_tests_lo()

-- 
2.47.1

Re: [PATCH mptcp-next 0/5] selftests: mptcp: more info in case of errors

[MPTCP CI]

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/12677226592

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/75c3a9a72f6a
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=923491


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

Re: [PATCH mptcp-next 0/5] selftests: mptcp: more info in case of errors

[Matthieu Baerts]

Hi Geliang,

On 08/01/2025 19:40, Matthieu Baerts (NGI0) wrote:
> Here are just a bunch of small improvements:
> 
> - MIB and 'ss -Me' in simult_flows.sh
> - move common code to lib.sh
> - use 'ss' with '-m'
> - remove unused variable
> - show only the size instead of size + filename
> 
> Nothing exiting, but the 3rd one can help investigating some issues like
> https://github.com/multipath-tcp/mptcp_net-next/issues/499

Now in our tree (feat. for net-next) with Geliang's RvB tag (I hope
that's OK),

New patches for t/upstream:
- 5731da7ccbbf: selftests: mptcp: simult_flows: unify errors msgs
- 668a6c97c2b1: selftests: mptcp: sockopt: save nstat infos
- 1cb853be929c: selftests: mptcp: move stats info in case of errors to
lib.sh
- a97277439285: selftests: mptcp: add -m with ss in case of errors
- 70019b947cf7: selftests: mptcp: connect: remove unused variable
- c94beebdce72: selftests: mptcp: connect: better display the files size
- Results: 17aabfa75062..632bb7330ffe (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/3d444e3a3d41f6228c9bcdc5dbf649cc6857c7d3/checks

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