diff mbox series

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

Message ID 20250108-slft-mptcp-conn-misc-impr-v1-1-bdfbdba48a1f@kernel.org (mailing list archive)
State Accepted
Commit 5731da7ccbbffa9ae28431d04fccd756aac97cf4
Delegated to: Matthieu Baerts
Headers show
Series selftests: mptcp: more info in case of errors | expand

Checks

Context Check Description
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅
matttbe/build success Build and static analysis OK
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 47 lines checked
matttbe/shellcheck success No ShellCheck issues

Commit Message

Matthieu Baerts Jan. 8, 2025, 6:40 p.m. UTC 
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(-)

Comments

Geliang Tang Jan. 10, 2025, 8:16 a.m. UTC | #1 
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
>  
>
Matthieu Baerts Jan. 10, 2025, 10:35 a.m. UTC | #2 
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
diff mbox series

Patch

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