[PATCH mptcp-next] selftests: mptcp: sockopt: save nstat infos

Geliang Tang posted 1 patch 1 week, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/5ff48cd1eed9a8b4c51a9ba5d7f9e0d0e8b0389b.1736477880.git.tanggeliang@kylinos.cn
tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH mptcp-next] selftests: mptcp: sockopt: save nstat infos
Posted by Geliang Tang 1 week, 4 days ago
From: Geliang Tang <tanggeliang@kylinos.cn>

Similar to the way nstat information is stored in mptcp_connect.sh
and mptcp_join.sh scripts, this patch adds a similar way for
mptcp_sockopt.sh and displays the nstat information when errors
occur.

Please apply this patch before the commit

"selftests: mptcp: simult_flows: unify errors msgs"

when merging it.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 5e8d5b83e2d0..9a78bfdc3d5e 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -169,6 +169,11 @@ do_transfer()
 		cmsg+=",TCPINQ"
 	fi
 
+	NSTAT_HISTORY=/tmp/${listener_ns}.nstat ip netns exec ${listener_ns} \
+		nstat -n
+	NSTAT_HISTORY=/tmp/${connector_ns}.nstat ip netns exec ${connector_ns} \
+		nstat -n
+
 	timeout ${timeout_test} \
 		ip netns exec ${listener_ns} \
 			$mptcp_connect -t ${timeout_poll} -l -M 1 -p $port -s ${srv_proto} -c "${cmsg}" \
@@ -189,14 +194,20 @@ do_transfer()
 	wait $spid
 	local rets=$?
 
+	NSTAT_HISTORY=/tmp/${listener_ns}.nstat ip netns exec ${listener_ns} \
+		nstat | grep Tcp > /tmp/${listener_ns}.out
+	NSTAT_HISTORY=/tmp/${connector_ns}.nstat ip netns exec ${connector_ns} \
+		nstat | grep Tcp > /tmp/${connector_ns}.out
+
 	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"
-
+		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_result_fail "transfer ${ip}"
 
-- 
2.45.2
Re: [PATCH mptcp-next] selftests: mptcp: sockopt: save nstat infos
Posted by Matthieu Baerts 1 week, 4 days ago
Hi Geliang,

On 10/01/2025 04:02, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Similar to the way nstat information is stored in mptcp_connect.sh
> and mptcp_join.sh scripts, this patch adds a similar way for
> mptcp_sockopt.sh and displays the nstat information when errors
> occur.

Thank you for the patch! Even if I'm not sure if it will really help
debugging issues, at least it is uniformed, and it looks good to me:

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

> Please apply this patch before the commit
> 
> "selftests: mptcp: simult_flows: unify errors msgs"
> 
> when merging it.

Will do! Or after, it might be easier with the automated scripts.

Can I add your Reviewed-by tag on my other patches from this series?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
[PATCH mptcp-next] Squash to "selftests: mptcp: move stats info in case of errors to lib.sh"
Posted by Geliang Tang 1 week, 4 days ago
From: Geliang Tang <tanggeliang@kylinos.cn>

Drop the last two arguments "lstat" and "cstat" of mptcp_lib_pr_err_stats.

Based-on: <20250108-slft-mptcp-conn-misc-impr-v1-0-bdfbdba48a1f@kernel.org>

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 3 +--
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 3 +--
 tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 6 ++----
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 3 +--
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 5e3c56253274..d3bf0ec25bfa 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -444,8 +444,7 @@ do_transfer()
 	printf "(duration %05sms) " "${duration}"
 	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
 		mptcp_lib_pr_fail "client exit code $retc, server $rets"
-		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \
-			"/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out"
+		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 
 		echo
 		cat "$capout"
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 13a3b68181ee..6365555568ec 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1039,8 +1039,7 @@ do_transfer()
 
 	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
 		fail_test "client exit code $retc, server $rets"
-		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}" \
-			"/tmp/${listener_ns}.out" "/tmp/${connector_ns}.out"
+		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 		return 1
 	fi
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 1f797d5f5156..633e7d776c3d 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -112,18 +112,16 @@ 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 -Menitam -o "sport = :${port}"
-		[ -s "${lstat}" ] && cat "${lstat}"
+		cat /tmp/"${lns}".out
 
 		printf "\nnetns %s (connector) socket stat for %d:\n" "${cns}" "${port}"
 		ip netns exec "${cns}" ss -Menitam -o "dport = :${port}"
-		[ "${lstat}" != "${cstat}" ] && [ -s "${cstat}" ] && cat "${cstat}"
+		[ "${lns}" != "${cns}" ] && cat /tmp/"${cns}".out
 	} 1>&2
 	echo -en "${MPTCP_LIB_COLOR_RESET}"
 }
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 9c2a415976cb..f60a27b913d0 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -204,8 +204,7 @@ do_transfer()
 	fi
 
 	mptcp_lib_pr_fail "client exit code $retc, server $rets"
-	mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}" \
-		"/tmp/${ns3}.out" "/tmp/${ns1}.out"
+	mptcp_lib_pr_err_stats "${ns3}" "${ns1}" "${port}"
 	ls -l $sin $cout
 	ls -l $cin $sout
 
-- 
2.45.2
Re: [PATCH mptcp-next] Squash to "selftests: mptcp: move stats info in case of errors to lib.sh"
Posted by Matthieu Baerts 1 week, 4 days ago
Hi Geliang,

10 Jan 2025 04:02:53 Geliang Tang <geliang@kernel.org>:

> Drop the last two arguments "lstat" and "cstat" of mptcp_lib_pr_err_stats.

I think it might be better to keep these arguments, not to have the lib assuming the files will be located there, e.g. just in case we move them elsewhere later, to be clear that when reading the code in the tests that these files will be used there, etc. no?

Cheers,
Matt
Re: [PATCH mptcp-next] Squash to "selftests: mptcp: move stats info in case of errors to lib.sh"
Posted by Matthieu Baerts 1 week, 4 days ago
Hi Geliang,

On 10/01/2025 08:41, Matthieu Baerts wrote:
> Hi Geliang,
> 
> 10 Jan 2025 04:02:53 Geliang Tang <geliang@kernel.org>:
> 
>> Drop the last two arguments "lstat" and "cstat" of mptcp_lib_pr_err_stats.
> 
> I think it might be better to keep these arguments, not to have the lib assuming the files will be located there, e.g. just in case we move them elsewhere later, to be clear that when reading the code in the tests that these files will be used there, etc. no?

Note that, with your previous patch (sockopt: save nstat infos), the 4
and 5 parameters can no longer be optional, and I can remove the size
checks ('[ -s <file> ]'). I can do that when applying the patches.


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