[PATCH mptcp-next 0/4] selftests: mptcp: get stats just before timing out

[PATCH mptcp-next 0/4] selftests: mptcp: get stats just before timing out

[Matthieu Baerts (NGI0)]

A few patches to improve the selftests in case of issues:

- Patch 1: a fix to properly kill background tasks.

- Patch 2: avoid taking the same packet trace twice.

- Patch 3: wait for an event instead of a fix time.

- Patch 4: instead of using 'timeout' and print the stats after, another
  internal timeout is used: if it fires, it will print stats, then stop
  everything. This avoids confusions around stats in case of timeout.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (4):
      selftests: mptcp: join: properly kill background tasks
      selftests: mptcp: connect: avoid double packet traces
      selftests: mptcp: wait for port instead of sleep
      selftests: mptcp: get stats just before timing out

 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 31 ++++++++------
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 47 +++++++++++-----------
 tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 34 ++++++++++++++++
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 23 ++++++-----
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 24 ++++++-----
 tools/testing/selftests/net/mptcp/userspace_pm.sh  |  3 +-
 6 files changed, 106 insertions(+), 56 deletions(-)
---
base-commit: e3f3a4ba83a02423a3cb045f7485340b713fb549
change-id: 20251106-slft-timeout-stats-d061de7fcebe

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

[PATCH mptcp-next 1/4] selftests: mptcp: join: properly kill background tasks

[Matthieu Baerts (NGI0)]

The 'run_tests' function is executed in the background, but killing its
associated PID would not kill the children tasks running in the
background.

To properly kill all background tasks, 'kill -- -PID' could be used, but
this requires kill from procps-ng. Instead, all children tasks are
listed using 'ps', and 'kill' is called with all PIDs of this group.

Fixes: 31ee4ad86afd ("selftests: mptcp: join: stop transfer when check is done (part 1)")
Fixes: 04b57c9e096a ("selftests: mptcp: join: stop transfer when check is done (part 2)")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 +++++++++---------
 tools/testing/selftests/net/mptcp/mptcp_lib.sh  | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 6afcf6ad2788..8be8bfa71946 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3995,7 +3995,7 @@ userspace_tests()
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		kill_events_pids
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 
 	# userspace pm create destroy subflow
@@ -4023,7 +4023,7 @@ userspace_tests()
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
 		kill_events_pids
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 
 	# userspace pm create id 0 subflow
@@ -4044,7 +4044,7 @@ userspace_tests()
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2
 		kill_events_pids
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 
 	# userspace pm remove initial subflow
@@ -4068,7 +4068,7 @@ userspace_tests()
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 1 1
 		kill_events_pids
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 
 	# userspace pm send RM_ADDR for ID 0
@@ -4094,7 +4094,7 @@ userspace_tests()
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 1 1
 		kill_events_pids
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 }
 
@@ -4124,7 +4124,7 @@ endpoint_tests()
 		pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
 		pm_nl_check_endpoint "modif is allowed" \
 			$ns2 10.0.2.2 id 1 flags signal
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 	fi
 
 	if reset_with_tcp_filter "delete and re-add" ns2 10.0.3.2 REJECT OUTPUT &&
@@ -4179,7 +4179,7 @@ endpoint_tests()
 			chk_mptcp_info subflows 3 subflows 3
 		done
 
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 
 		kill_events_pids
 		chk_evt_nr ns1 MPTCP_LIB_EVENT_LISTENER_CREATED 1
@@ -4253,7 +4253,7 @@ endpoint_tests()
 		wait_mpj $ns2
 		chk_subflow_nr "after re-re-add ID 0" 3
 		chk_mptcp_info subflows 3 subflows 3
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 
 		kill_events_pids
 		chk_evt_nr ns1 MPTCP_LIB_EVENT_LISTENER_CREATED 1
@@ -4301,7 +4301,7 @@ endpoint_tests()
 		wait_mpj $ns2
 		pm_nl_add_endpoint $ns1 10.0.3.1 id 2 flags signal
 		wait_mpj $ns2
-		mptcp_lib_kill_wait $tests_pid
+		mptcp_lib_kill_group_wait $tests_pid
 
 		join_syn_tx=3 join_connect_err=1 \
 			chk_join_nr 2 2 2
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index ce6c92826be7..91ec75ddcb96 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -359,6 +359,27 @@ mptcp_lib_kill_wait() {
 	wait "${1}" 2>/dev/null
 }
 
+# $1: PID
+mptcp_lib_pid_list_children() {
+	local curr="${1}"
+	# evoke 'ps' only once
+	local pids="${2:-"$(ps o pid,ppid)"}"
+
+	echo "${curr}"
+
+	local pid
+	for pid in $(echo "${pids}" | awk "\$2 == ${curr} { print \$1 }"); do
+		mptcp_lib_pid_list_children "${pid}" "${pids}"
+	done
+}
+
+# $1: PID
+mptcp_lib_kill_group_wait() {
+	# Some users might not have procps-ng: cannot use "kill -- -PID"
+	mptcp_lib_pid_list_children "${1}" | xargs -r kill &>/dev/null
+	wait "${1}" 2>/dev/null
+}
+
 # $1: IP address
 mptcp_lib_is_v6() {
 	[ -z "${1##*:*}" ]

-- 
2.51.0

[PATCH mptcp-next 2/4] selftests: mptcp: connect: avoid double packet traces

[Matthieu Baerts (NGI0)]

When the same netns is used for the listener and the connector, no need
to take exactly the same packet trace twice, one is enough.

This avoids confusions when the traces are the same, and wasting
resources which might not help reproducing an issue.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 3a804abebd2c..1149bd150d6a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -378,8 +378,10 @@ do_transfer()
 		ip netns exec ${listener_ns}  tcpdump ${capopt} -w "${capfile}-listener.pcap"  >> "${capout}" 2>&1 &
 		local cappid_listener=$!
 
-		ip netns exec ${connector_ns} tcpdump ${capopt} -w "${capfile}-connector.pcap" >> "${capout}" 2>&1 &
-		local cappid_connector=$!
+		if [ ${listener_ns} != ${connector_ns} ]; then
+			ip netns exec ${connector_ns} tcpdump ${capopt} -w "${capfile}-connector.pcap" >> "${capout}" 2>&1 &
+			local cappid_connector=$!
+		fi
 
 		sleep 1
 	fi
@@ -416,7 +418,9 @@ do_transfer()
 	if $capture; then
 		sleep 1
 		kill ${cappid_listener}
-		kill ${cappid_connector}
+		if [ ${listener_ns} != ${connector_ns} ]; then
+			kill ${cappid_connector}
+		fi
 	fi
 
 	mptcp_lib_nstat_get "${listener_ns}"

-- 
2.51.0

[PATCH mptcp-next 3/4] selftests: mptcp: wait for port instead of sleep

[Matthieu Baerts (NGI0)]

After having started mptcp_connect in listening mode,
'mptcp_lib_wait_local_port_listen' can be used to wait for the listening
socket to be ready.

This is better than using the 'sleep' command, not to pause for a fixed
amount of time, but waiting for an event. This helper is used in all
other MPTCP selftests, but not in these two.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 2 +-
 tools/testing/selftests/net/mptcp/userspace_pm.sh  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 42d533b95ec7..6cde7429104b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -178,7 +178,7 @@ do_transfer()
 				${local_addr} < "$sin" > "$sout" &
 	local spid=$!
 
-	sleep 1
+	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
 
 	timeout ${timeout_test} \
 		ip netns exec ${connector_ns} \
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 87323942cb8a..e9ae1806ab07 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -211,7 +211,8 @@ make_connection()
 	ip netns exec "$ns1" \
 	   ./mptcp_connect -s MPTCP -w 300 -p $app_port -l $listen_addr > /dev/null 2>&1 &
 	local server_pid=$!
-	sleep 0.5
+
+	mptcp_lib_wait_local_port_listen "${ns1}" "${port}"
 
 	# Run the client, transfer $file and stay connected to the server
 	# to conduct tests

-- 
2.51.0

[PATCH mptcp-next 4/4] selftests: mptcp: get stats just before timing out

[Matthieu Baerts (NGI0)]

Recently, some debugging happened around a test that was timing out. The
stats were showing connections being closed which was confusing because
the closing state was caused by the timeout stopping the transfer.

To avoid such confusion, the timeout is no longer done per mptcp_connect
process, but separately. In case of timeout, the stats are now printed,
then the apps are killed.

The stats will still be printed after the kill, but that's fine, and
this might even be useful, just in case. Timeout should be exceptional.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 1149bd150d6a..6bff77ed0a74 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -391,26 +391,29 @@ do_transfer()
 		mptcp_lib_nstat_init "${connector_ns}"
 	fi
 
-	timeout ${timeout_test} \
-		ip netns exec ${listener_ns} \
-			./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
-				$extra_args $local_addr < "$sin" > "$sout" &
+	ip netns exec ${listener_ns} \
+		./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
+			$extra_args $local_addr < "$sin" > "$sout" &
 	local spid=$!
 
 	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
 
 	local start
 	start=$(date +%s%3N)
-	timeout ${timeout_test} \
-		ip netns exec ${connector_ns} \
-			./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
-				$extra_args $connect_addr < "$cin" > "$cout" &
+	ip netns exec ${connector_ns} \
+		./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
+			$extra_args $connect_addr < "$cin" > "$cout" &
 	local cpid=$!
 
+	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
+		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
+	local timeout_pid=$!
+
 	wait $cpid
 	local retc=$?
 	wait $spid
 	local rets=$?
+	kill $timeout_pid 2>/dev/null && timeout_pid=0
 
 	local stop
 	stop=$(date +%s%3N)
@@ -431,7 +434,7 @@ do_transfer()
 	local duration
 	duration=$((stop-start))
 	printf "(duration %05sms) " "${duration}"
-	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
+	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [ ${timeout_pid} -ne 0 ]; then
 		mptcp_lib_pr_fail "client exit code $retc, server $rets"
 		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 8be8bfa71946..9ce386b3056c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1041,38 +1041,38 @@ do_transfer()
 	if [ "$test_linkfail" -gt 1 ];then
 		listener_in="${sinfail}"
 	fi
-	timeout ${timeout_test} \
-		ip netns exec ${listener_ns} \
-			./mptcp_connect -t ${timeout_poll} -l -p ${port} -s ${srv_proto} \
-				${extra_srv_args} "${bind_addr}" < "${listener_in}" > "${sout}" &
+	ip netns exec ${listener_ns} \
+		./mptcp_connect -t ${timeout_poll} -l -p ${port} -s ${srv_proto} \
+			${extra_srv_args} "${bind_addr}" < "${listener_in}" > "${sout}" &
 	local spid=$!
 
 	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
 
 	extra_cl_args="$extra_args $extra_cl_args"
 	if [ "$test_linkfail" -eq 0 ];then
-		timeout ${timeout_test} \
-			ip netns exec ${connector_ns} \
-				./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
-					$extra_cl_args $connect_addr < "$cin" > "$cout" &
+		ip netns exec ${connector_ns} \
+			./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
+				$extra_cl_args $connect_addr < "$cin" > "$cout" &
 	elif [ "$test_linkfail" -eq 1 ] || [ "$test_linkfail" -eq 2 ];then
 		connector_in="${cinsent}"
 		( cat "$cinfail" ; sleep 2; link_failure $listener_ns ; cat "$cinfail" ) | \
 			tee "$cinsent" | \
-			timeout ${timeout_test} \
 				ip netns exec ${connector_ns} \
 					./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
 						$extra_cl_args $connect_addr > "$cout" &
 	else
 		connector_in="${cinsent}"
 		tee "$cinsent" < "$cinfail" | \
-			timeout ${timeout_test} \
-				ip netns exec ${connector_ns} \
-					./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
-						$extra_cl_args $connect_addr > "$cout" &
+			ip netns exec ${connector_ns} \
+				./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
+					$extra_cl_args $connect_addr > "$cout" &
 	fi
 	local cpid=$!
 
+	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
+		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
+	local timeout_pid=$!
+
 	pm_nl_set_endpoint $listener_ns $connector_ns $connect_addr
 	check_cestab $listener_ns $connector_ns
 
@@ -1080,13 +1080,14 @@ do_transfer()
 	local retc=$?
 	wait $spid
 	local rets=$?
+	kill $timeout_pid 2>/dev/null && timeout_pid=0
 
 	cond_stop_capture
 
 	mptcp_lib_nstat_get "${listener_ns}"
 	mptcp_lib_nstat_get "${connector_ns}"
 
-	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
+	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [ ${timeout_pid} -ne 0 ]; then
 		fail_test "client exit code $retc, server $rets"
 		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 		return 1
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 91ec75ddcb96..5fea7e7df628 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -350,6 +350,19 @@ mptcp_lib_evts_get_info() {
 		mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
 }
 
+mptcp_lib_wait_timeout() {
+	local timeout_test="${1}"
+	local listener_ns="${2}"
+	local connector_ns="${3}"
+	local port="${4}"
+	shift 4 # rest are PIDs
+
+	sleep "${timeout_test}"
+	mptcp_lib_print_err "timeout"
+	mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
+	kill "${@}" 2>/dev/null
+}
+
 # $1: PID
 mptcp_lib_kill_wait() {
 	[ "${1}" -eq 0 ] && return 0
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 6cde7429104b..326466f2f6eb 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -172,31 +172,34 @@ do_transfer()
 	mptcp_lib_nstat_init "${listener_ns}"
 	mptcp_lib_nstat_init "${connector_ns}"
 
-	timeout ${timeout_test} \
-		ip netns exec ${listener_ns} \
-			$mptcp_connect -t ${timeout_poll} -l -M 1 -p $port -s ${srv_proto} -c "${cmsg}" \
-				${local_addr} < "$sin" > "$sout" &
+	ip netns exec ${listener_ns} \
+		$mptcp_connect -t ${timeout_poll} -l -M 1 -p $port -s ${srv_proto} -c "${cmsg}" \
+			${local_addr} < "$sin" > "$sout" &
 	local spid=$!
 
 	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
 
-	timeout ${timeout_test} \
-		ip netns exec ${connector_ns} \
-			$mptcp_connect -t ${timeout_poll} -M 2 -p $port -s ${cl_proto} -c "${cmsg}" \
-				$connect_addr < "$cin" > "$cout" &
+	ip netns exec ${connector_ns} \
+		$mptcp_connect -t ${timeout_poll} -M 2 -p $port -s ${cl_proto} -c "${cmsg}" \
+			$connect_addr < "$cin" > "$cout" &
 
 	local cpid=$!
 
+	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
+		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
+	local timeout_pid=$!
+
 	wait $cpid
 	local retc=$?
 	wait $spid
 	local rets=$?
+	kill $timeout_pid 2>/dev/null && timeout_pid=0
 
 	mptcp_lib_nstat_get "${listener_ns}"
 	mptcp_lib_nstat_get "${connector_ns}"
 
 	print_title "Transfer ${ip:2}"
-	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
+	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [ ${timeout_pid} -ne 0 ]; then
 		mptcp_lib_pr_fail "client exit code $retc, server $rets"
 		mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}" "${port}"
 
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 503cb59571a4..cc4d40d149e2 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -158,24 +158,27 @@ do_transfer()
 	mptcp_lib_nstat_init "${ns3}"
 	mptcp_lib_nstat_init "${ns1}"
 
-	timeout ${timeout_test} \
-		ip netns exec ${ns3} \
-			./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \
-				0.0.0.0 < "$sin" > "$sout" &
+	ip netns exec ${ns3} \
+		./mptcp_connect -jt ${timeout_poll} -l -p $port -T $max_time \
+			0.0.0.0 < "$sin" > "$sout" &
 	local spid=$!
 
 	mptcp_lib_wait_local_port_listen "${ns3}" "${port}"
 
-	timeout ${timeout_test} \
-		ip netns exec ${ns1} \
-			./mptcp_connect -jt ${timeout_poll} -p $port -T $max_time \
-				10.0.3.3 < "$cin" > "$cout" &
+	ip netns exec ${ns1} \
+		./mptcp_connect -jt ${timeout_poll} -p $port -T $max_time \
+			10.0.3.3 < "$cin" > "$cout" &
 	local cpid=$!
 
+	mptcp_lib_wait_timeout "${timeout_test}" "${ns3}" "${ns1}" "${port}" \
+		"${cpid}" "${spid}" &
+	local timeout_pid=$!
+
 	wait $cpid
 	local retc=$?
 	wait $spid
 	local rets=$?
+	kill $timeout_pid 2>/dev/null && timeout_pid=0
 
 	if $capture; then
 		sleep 1
@@ -191,8 +194,9 @@ do_transfer()
 	cmp $cin $sout > /dev/null 2>&1
 	local cmpc=$?
 
-	if [ $retc -eq 0 ] && [ $rets -eq 0 ] && \
-	   [ $cmpc -eq 0 ] && [ $cmps -eq 0 ]; then
+	if [ $retc -eq 0 ] && [ $rets -eq 0 ] &&
+	   [ $cmpc -eq 0 ] && [ $cmps -eq 0 ] &&
+	   [ $timeout_pid -eq 0 ]; then
 		printf "%-16s" " max $max_time "
 		mptcp_lib_pr_ok
 		cat "$capout"

-- 
2.51.0

Re: [PATCH mptcp-next 0/4] selftests: mptcp: get stats just before timing out

[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 (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Unstable: 1 failed test(s): selftest_mptcp_connect_mmap 🔴
- KVM Validation: debug (only selftest_mptcp_join): Critical: KMemLeak ❌
- 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/19194469113

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


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 1/4] selftests: mptcp: join: properly kill background tasks

[Geliang Tang]

Hi Matt,

This for this fix.

On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
> The 'run_tests' function is executed in the background, but killing
> its
> associated PID would not kill the children tasks running in the
> background.
> 
> To properly kill all background tasks, 'kill -- -PID' could be used,
> but
> this requires kill from procps-ng. Instead, all children tasks are
> listed using 'ps', and 'kill' is called with all PIDs of this group.
> 
> Fixes: 31ee4ad86afd ("selftests: mptcp: join: stop transfer when
> check is done (part 1)")
> Fixes: 04b57c9e096a ("selftests: mptcp: join: stop transfer when
> check is done (part 2)")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 18 +++++++++------
> ---
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh  | 21
> +++++++++++++++++++++
>  2 files changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 6afcf6ad2788..8be8bfa71946 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3995,7 +3995,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 0 subflows 0
>  		chk_subflows_total 1 1
>  		kill_events_pids
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  
>  	# userspace pm create destroy subflow
> @@ -4023,7 +4023,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 0 subflows 0
>  		chk_subflows_total 1 1
>  		kill_events_pids
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  
>  	# userspace pm create id 0 subflow
> @@ -4044,7 +4044,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 2 2
>  		kill_events_pids
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  
>  	# userspace pm remove initial subflow
> @@ -4068,7 +4068,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 1 1
>  		kill_events_pids
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  
>  	# userspace pm send RM_ADDR for ID 0
> @@ -4094,7 +4094,7 @@ userspace_tests()
>  		chk_mptcp_info subflows 1 subflows 1
>  		chk_subflows_total 1 1
>  		kill_events_pids
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  }
>  
> @@ -4124,7 +4124,7 @@ endpoint_tests()
>  		pm_nl_add_endpoint $ns2 10.0.2.2 flags signal
>  		pm_nl_check_endpoint "modif is allowed" \
>  			$ns2 10.0.2.2 id 1 flags signal
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  	fi
>  
>  	if reset_with_tcp_filter "delete and re-add" ns2 10.0.3.2
> REJECT OUTPUT &&
> @@ -4179,7 +4179,7 @@ endpoint_tests()
>  			chk_mptcp_info subflows 3 subflows 3
>  		done
>  
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  
>  		kill_events_pids
>  		chk_evt_nr ns1 MPTCP_LIB_EVENT_LISTENER_CREATED 1
> @@ -4253,7 +4253,7 @@ endpoint_tests()
>  		wait_mpj $ns2
>  		chk_subflow_nr "after re-re-add ID 0" 3
>  		chk_mptcp_info subflows 3 subflows 3
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  
>  		kill_events_pids
>  		chk_evt_nr ns1 MPTCP_LIB_EVENT_LISTENER_CREATED 1
> @@ -4301,7 +4301,7 @@ endpoint_tests()
>  		wait_mpj $ns2
>  		pm_nl_add_endpoint $ns1 10.0.3.1 id 2 flags signal
>  		wait_mpj $ns2
> -		mptcp_lib_kill_wait $tests_pid
> +		mptcp_lib_kill_group_wait $tests_pid
>  
>  		join_syn_tx=3 join_connect_err=1 \
>  			chk_join_nr 2 2 2
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index ce6c92826be7..91ec75ddcb96 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -359,6 +359,27 @@ mptcp_lib_kill_wait() {
>  	wait "${1}" 2>/dev/null
>  }
>  
> +# $1: PID
> +mptcp_lib_pid_list_children() {
> +	local curr="${1}"
> +	# evoke 'ps' only once
> +	local pids="${2:-"$(ps o pid,ppid)"}"
> +
> +	echo "${curr}"
> +
> +	local pid
> +	for pid in $(echo "${pids}" | awk "\$2 == ${curr} { print
> \$1 }"); do
> +		mptcp_lib_pid_list_children "${pid}" "${pids}"

To be honest, I don't understand why this mptcp_lib_pid_list_children
function is calling itself here. Is this recursion?

The other three patches all look good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> +	done
> +}
> +
> +# $1: PID
> +mptcp_lib_kill_group_wait() {
> +	# Some users might not have procps-ng: cannot use "kill -- -
> PID"
> +	mptcp_lib_pid_list_children "${1}" | xargs -r kill
> &>/dev/null
> +	wait "${1}" 2>/dev/null
> +}
> +
>  # $1: IP address
>  mptcp_lib_is_v6() {
>  	[ -z "${1##*:*}" ]
> 

Re: [PATCH mptcp-next 1/4] selftests: mptcp: join: properly kill background tasks

[Matthieu Baerts]

Hi Geliang,

Thank you for the reviews.

11 Nov 2025 03:46:06 Geliang Tang <geliang@kernel.org>:

> Hi Matt,
>
> This for this fix.
>
> On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
>> The 'run_tests' function is executed in the background, but killing
>> its
>> associated PID would not kill the children tasks running in the
>> background.
>>
>> To properly kill all background tasks, 'kill -- -PID' could be used,
>> but
>> this requires kill from procps-ng. Instead, all children tasks are
>> listed using 'ps', and 'kill' is called with all PIDs of this group.

(...)

>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> index ce6c92826be7..91ec75ddcb96 100644
>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>> @@ -359,6 +359,27 @@ mptcp_lib_kill_wait() {
>>     wait "${1}" 2>/dev/null
>>  }
>>  
>> +# $1: PID
>> +mptcp_lib_pid_list_children() {
>> +   local curr="${1}"
>> +   # evoke 'ps' only once
>> +   local pids="${2:-"$(ps o pid,ppid)"}"
>> +
>> +   echo "${curr}"
>> +
>> +   local pid
>> +   for pid in $(echo "${pids}" | awk "\$2 == ${curr} { print
>> \$1 }"); do
>> +       mptcp_lib_pid_list_children "${pid}" "${pids}"
>
> To be honest, I don't understand why this mptcp_lib_pid_list_children
> function is calling itself here. Is this recursion?

Yes it is a recursion: children processes can have children ones.

mptcp_lib_pid_list_children() will print the PID it is called with, and call
itself with all children (can be none).

In our case with mptcp_join.sh, it means getting the run_tests job plus
the other background tasks: mptcp_connect (x2), the timeout, and
eventually the task modifying the endpoints.

Cheers,
Matt

Re: [PATCH mptcp-next 1/4] selftests: mptcp: join: properly kill background tasks

[Geliang Tang]

Hi Matt,

On Tue, 2025-11-11 at 07:53 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for the reviews.
> 
> 11 Nov 2025 03:46:06 Geliang Tang <geliang@kernel.org>:
> 
> > Hi Matt,
> > 
> > This for this fix.
> > 
> > On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
> > > The 'run_tests' function is executed in the background, but
> > > killing
> > > its
> > > associated PID would not kill the children tasks running in the
> > > background.
> > > 
> > > To properly kill all background tasks, 'kill -- -PID' could be
> > > used,
> > > but
> > > this requires kill from procps-ng. Instead, all children tasks
> > > are
> > > listed using 'ps', and 'kill' is called with all PIDs of this
> > > group.
> 
> (...)
> 
> > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > index ce6c92826be7..91ec75ddcb96 100644
> > > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > @@ -359,6 +359,27 @@ mptcp_lib_kill_wait() {
> > >     wait "${1}" 2>/dev/null
> > >  }
> > >  
> > > +# $1: PID
> > > +mptcp_lib_pid_list_children() {
> > > +   local curr="${1}"
> > > +   # evoke 'ps' only once
> > > +   local pids="${2:-"$(ps o pid,ppid)"}"
> > > +
> > > +   echo "${curr}"
> > > +
> > > +   local pid
> > > +   for pid in $(echo "${pids}" | awk "\$2 == ${curr} { print
> > > \$1 }"); do
> > > +       mptcp_lib_pid_list_children "${pid}" "${pids}"
> > 
> > To be honest, I don't understand why this
> > mptcp_lib_pid_list_children
> > function is calling itself here. Is this recursion?
> 
> Yes it is a recursion: children processes can have children ones.
> 
> mptcp_lib_pid_list_children() will print the PID it is called with,
> and call
> itself with all children (can be none).
> 
> In our case with mptcp_join.sh, it means getting the run_tests job
> plus
> the other background tasks: mptcp_connect (x2), the timeout, and
> eventually the task modifying the endpoints.

Thanks for your explanation, this patch looks good to me.

I'll reply my RB tag in the cover letter.

And for patch 6 of "selftests: mptcp: join: fix some flaky tests"
sending to -net too.

-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next 1/4] selftests: mptcp: join: properly kill background tasks

[Matthieu Baerts]

Hi Geliang,

On 11/11/2025 09:17, Geliang Tang wrote:
> Hi Matt,
> 
> On Tue, 2025-11-11 at 07:53 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> Thank you for the reviews.
>>
>> 11 Nov 2025 03:46:06 Geliang Tang <geliang@kernel.org>:
>>
>>> Hi Matt,
>>>
>>> This for this fix.
>>>
>>> On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
>>>> The 'run_tests' function is executed in the background, but
>>>> killing
>>>> its
>>>> associated PID would not kill the children tasks running in the
>>>> background.
>>>>
>>>> To properly kill all background tasks, 'kill -- -PID' could be
>>>> used,
>>>> but
>>>> this requires kill from procps-ng. Instead, all children tasks
>>>> are
>>>> listed using 'ps', and 'kill' is called with all PIDs of this
>>>> group.
>>
>> (...)
>>
>>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>>> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>>> index ce6c92826be7..91ec75ddcb96 100644
>>>> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
>>>> @@ -359,6 +359,27 @@ mptcp_lib_kill_wait() {
>>>>     wait "${1}" 2>/dev/null
>>>>  }
>>>>  
>>>> +# $1: PID
>>>> +mptcp_lib_pid_list_children() {
>>>> +   local curr="${1}"
>>>> +   # evoke 'ps' only once
>>>> +   local pids="${2:-"$(ps o pid,ppid)"}"
>>>> +
>>>> +   echo "${curr}"
>>>> +
>>>> +   local pid
>>>> +   for pid in $(echo "${pids}" | awk "\$2 == ${curr} { print
>>>> \$1 }"); do
>>>> +       mptcp_lib_pid_list_children "${pid}" "${pids}"
>>>
>>> To be honest, I don't understand why this
>>> mptcp_lib_pid_list_children
>>> function is calling itself here. Is this recursion?
>>
>> Yes it is a recursion: children processes can have children ones.
>>
>> mptcp_lib_pid_list_children() will print the PID it is called with,
>> and call
>> itself with all children (can be none).
>>
>> In our case with mptcp_join.sh, it means getting the run_tests job
>> plus
>> the other background tasks: mptcp_connect (x2), the timeout, and
>> eventually the task modifying the endpoints.
> 
> Thanks for your explanation, this patch looks good to me.
> 
> I'll reply my RB tag in the cover letter.
> 
> And for patch 6 of "selftests: mptcp: join: fix some flaky tests"
> sending to -net too.

Thank you for that! Note that I sent it directly to 'net' because it was
"simple", and other patches depend on it: we already have quite a few
patches in the queue, probably better not to delay some even longer.

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

Re: [PATCH mptcp-next 1/4] selftests: mptcp: join: properly kill background tasks

[Matthieu Baerts]

Hi Geliang,

On 11/11/2025 09:17, Geliang Tang wrote:
> Thanks for your explanation, this patch looks good to me.
> 
> I'll reply my RB tag in the cover letter.

I don't see this RB tag: was there an issue with the email, or do you
simply need more time?

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

Re: [PATCH mptcp-next 0/4] selftests: mptcp: get stats just before timing out

[Geliang Tang]

Hi Matt,

On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
> A few patches to improve the selftests in case of issues:
> 
> - Patch 1: a fix to properly kill background tasks.
> 
> - Patch 2: avoid taking the same packet trace twice.
> 
> - Patch 3: wait for an event instead of a fix time.
> 
> - Patch 4: instead of using 'timeout' and print the stats after,
> another
>   internal timeout is used: if it fires, it will print stats, then
> stop
>   everything. This avoids confusions around stats in case of timeout.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This set looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> ---
> Matthieu Baerts (NGI0) (4):
>       selftests: mptcp: join: properly kill background tasks
>       selftests: mptcp: connect: avoid double packet traces
>       selftests: mptcp: wait for port instead of sleep
>       selftests: mptcp: get stats just before timing out
> 
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 31 ++++++++----
> --
>  tools/testing/selftests/net/mptcp/mptcp_join.sh    | 47 +++++++++++-
> ----------
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 34
> ++++++++++++++++
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 23 ++++++-----
>  tools/testing/selftests/net/mptcp/simult_flows.sh  | 24 ++++++-----
>  tools/testing/selftests/net/mptcp/userspace_pm.sh  |  3 +-
>  6 files changed, 106 insertions(+), 56 deletions(-)
> ---
> base-commit: e3f3a4ba83a02423a3cb045f7485340b713fb549
> change-id: 20251106-slft-timeout-stats-d061de7fcebe
> 
> Best regards,

Re: [PATCH mptcp-next 0/4] selftests: mptcp: get stats just before timing out

[Matthieu Baerts]

Hi Geliang,

On 12/11/2025 11:02, Geliang Tang wrote:
> Hi Matt,
> 
> On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
>> A few patches to improve the selftests in case of issues:
>>
>> - Patch 1: a fix to properly kill background tasks.
>>
>> - Patch 2: avoid taking the same packet trace twice.
>>
>> - Patch 3: wait for an event instead of a fix time.
>>
>> - Patch 4: instead of using 'timeout' and print the stats after,
>> another
>>   internal timeout is used: if it fires, it will print stats, then
>> stop
>>   everything. This avoids confusions around stats in case of timeout.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> This set looks good to me.
> 
>     Reviewed-by: Geliang Tang <geliang@kernel.org>

Thank you for the review!

Now in our tree:

New patches for t/upstream:
- 5bdd05d66b06: selftests: mptcp: connect: avoid double packet traces
- 070800d3509c: selftests: mptcp: wait for port instead of sleep
- 7bf5a769dde9: selftests: mptcp: get stats just before timing out
- Results: ee7fdf18febf..6ed800e9ef97 (export)

Tests are now in progress:

- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/03bd0df93bb84cb9894db56697eea33b27d2f1d9/checks

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

Re: [PATCH mptcp-next 4/4] selftests: mptcp: get stats just before timing out

[Geliang Tang]

Hi Matt,

On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
> Recently, some debugging happened around a test that was timing out.
> The
> stats were showing connections being closed which was confusing
> because
> the closing state was caused by the timeout stopping the transfer.
> 
> To avoid such confusion, the timeout is no longer done per
> mptcp_connect
> process, but separately. In case of timeout, the stats are now
> printed,
> then the apps are killed.
> 
> The stats will still be printed after the kill, but that's fine, and
> this might even be useful, just in case. Timeout should be
> exceptional.

Today I found that this patch has increased the runtime of the
selftests, for example:

# 66 ns1 MPTCP -> ns1 (dead:beef:1::1:20004) MPTCP     (duration  
390ms) [ OK ]
# 67 ns1 MPTCP -> ns1 (dead:beef:1::1:20005) TCP       (duration  
415ms) [ OK ]
# 68 ns1 TCP   -> ns1 (dead:beef:1::1:20006) MPTCP     (duration  
380ms) [ OK ]
# Time: 38 seconds
ok 1 test: selftest_mptcp_connect
# time=99

The mptcp_connect test ended after 38 seconds, but it waited an
additional 61 seconds before finally stopping at 99 seconds.

This needs to be fixed, or we should revert this patch first.

Thanks,
-Geliang

> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 21 +++++++++---
> ----
>  tools/testing/selftests/net/mptcp/mptcp_join.sh    | 29 +++++++++++-
> ----------
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 13 ++++++++++
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 21 +++++++++---
> ----
>  tools/testing/selftests/net/mptcp/simult_flows.sh  | 24 ++++++++++--
> ------
>  5 files changed, 66 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 1149bd150d6a..6bff77ed0a74 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -391,26 +391,29 @@ do_transfer()
>  		mptcp_lib_nstat_init "${connector_ns}"
>  	fi
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${listener_ns} \
> -			./mptcp_connect -t ${timeout_poll} -l -p
> $port -s ${srv_proto} \
> -				$extra_args $local_addr < "$sin" >
> "$sout" &
> +	ip netns exec ${listener_ns} \
> +		./mptcp_connect -t ${timeout_poll} -l -p $port -s
> ${srv_proto} \
> +			$extra_args $local_addr < "$sin" > "$sout" &
>  	local spid=$!
>  
>  	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>  
>  	local start
>  	start=$(date +%s%3N)
> -	timeout ${timeout_test} \
> -		ip netns exec ${connector_ns} \
> -			./mptcp_connect -t ${timeout_poll} -p $port
> -s ${cl_proto} \
> -				$extra_args $connect_addr < "$cin" >
> "$cout" &
> +	ip netns exec ${connector_ns} \
> +		./mptcp_connect -t ${timeout_poll} -p $port -s
> ${cl_proto} \
> +			$extra_args $connect_addr < "$cin" > "$cout"
> &
>  	local cpid=$!
>  
> +	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
> +		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
> +	local timeout_pid=$!
> +
>  	wait $cpid
>  	local retc=$?
>  	wait $spid
>  	local rets=$?
> +	kill $timeout_pid 2>/dev/null && timeout_pid=0
>  
>  	local stop
>  	stop=$(date +%s%3N)
> @@ -431,7 +434,7 @@ do_transfer()
>  	local duration
>  	duration=$((stop-start))
>  	printf "(duration %05sms) " "${duration}"
> -	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
> +	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [
> ${timeout_pid} -ne 0 ]; then
>  		mptcp_lib_pr_fail "client exit code $retc, server
> $rets"
>  		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}"
>  
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 8be8bfa71946..9ce386b3056c 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1041,38 +1041,38 @@ do_transfer()
>  	if [ "$test_linkfail" -gt 1 ];then
>  		listener_in="${sinfail}"
>  	fi
> -	timeout ${timeout_test} \
> -		ip netns exec ${listener_ns} \
> -			./mptcp_connect -t ${timeout_poll} -l -p
> ${port} -s ${srv_proto} \
> -				${extra_srv_args} "${bind_addr}" <
> "${listener_in}" > "${sout}" &
> +	ip netns exec ${listener_ns} \
> +		./mptcp_connect -t ${timeout_poll} -l -p ${port} -s
> ${srv_proto} \
> +			${extra_srv_args} "${bind_addr}" <
> "${listener_in}" > "${sout}" &
>  	local spid=$!
>  
>  	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>  
>  	extra_cl_args="$extra_args $extra_cl_args"
>  	if [ "$test_linkfail" -eq 0 ];then
> -		timeout ${timeout_test} \
> -			ip netns exec ${connector_ns} \
> -				./mptcp_connect -t ${timeout_poll} -
> p $port -s ${cl_proto} \
> -					$extra_cl_args $connect_addr
> < "$cin" > "$cout" &
> +		ip netns exec ${connector_ns} \
> +			./mptcp_connect -t ${timeout_poll} -p $port
> -s ${cl_proto} \
> +				$extra_cl_args $connect_addr <
> "$cin" > "$cout" &
>  	elif [ "$test_linkfail" -eq 1 ] || [ "$test_linkfail" -eq 2
> ];then
>  		connector_in="${cinsent}"
>  		( cat "$cinfail" ; sleep 2; link_failure
> $listener_ns ; cat "$cinfail" ) | \
>  			tee "$cinsent" | \
> -			timeout ${timeout_test} \
>  				ip netns exec ${connector_ns} \
>  					./mptcp_connect -t
> ${timeout_poll} -p $port -s ${cl_proto} \
>  						$extra_cl_args
> $connect_addr > "$cout" &
>  	else
>  		connector_in="${cinsent}"
>  		tee "$cinsent" < "$cinfail" | \
> -			timeout ${timeout_test} \
> -				ip netns exec ${connector_ns} \
> -					./mptcp_connect -t
> ${timeout_poll} -p $port -s ${cl_proto} \
> -						$extra_cl_args
> $connect_addr > "$cout" &
> +			ip netns exec ${connector_ns} \
> +				./mptcp_connect -t ${timeout_poll} -
> p $port -s ${cl_proto} \
> +					$extra_cl_args $connect_addr
> > "$cout" &
>  	fi
>  	local cpid=$!
>  
> +	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
> +		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
> +	local timeout_pid=$!
> +
>  	pm_nl_set_endpoint $listener_ns $connector_ns $connect_addr
>  	check_cestab $listener_ns $connector_ns
>  
> @@ -1080,13 +1080,14 @@ do_transfer()
>  	local retc=$?
>  	wait $spid
>  	local rets=$?
> +	kill $timeout_pid 2>/dev/null && timeout_pid=0
>  
>  	cond_stop_capture
>  
>  	mptcp_lib_nstat_get "${listener_ns}"
>  	mptcp_lib_nstat_get "${connector_ns}"
>  
> -	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
> +	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [
> ${timeout_pid} -ne 0 ]; then
>  		fail_test "client exit code $retc, server $rets"
>  		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}"
>  		return 1
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 91ec75ddcb96..5fea7e7df628 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -350,6 +350,19 @@ mptcp_lib_evts_get_info() {
>  		mptcp_lib_get_info_value "${1}" "^type:${3:-1},"
>  }
>  
> +mptcp_lib_wait_timeout() {
> +	local timeout_test="${1}"
> +	local listener_ns="${2}"
> +	local connector_ns="${3}"
> +	local port="${4}"
> +	shift 4 # rest are PIDs
> +
> +	sleep "${timeout_test}"
> +	mptcp_lib_print_err "timeout"
> +	mptcp_lib_pr_err_stats "${listener_ns}" "${connector_ns}"
> "${port}"
> +	kill "${@}" 2>/dev/null
> +}
> +
>  # $1: PID
>  mptcp_lib_kill_wait() {
>  	[ "${1}" -eq 0 ] && return 0
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index 6cde7429104b..326466f2f6eb 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -172,31 +172,34 @@ do_transfer()
>  	mptcp_lib_nstat_init "${listener_ns}"
>  	mptcp_lib_nstat_init "${connector_ns}"
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${listener_ns} \
> -			$mptcp_connect -t ${timeout_poll} -l -M 1 -p
> $port -s ${srv_proto} -c "${cmsg}" \
> -				${local_addr} < "$sin" > "$sout" &
> +	ip netns exec ${listener_ns} \
> +		$mptcp_connect -t ${timeout_poll} -l -M 1 -p $port -
> s ${srv_proto} -c "${cmsg}" \
> +			${local_addr} < "$sin" > "$sout" &
>  	local spid=$!
>  
>  	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${connector_ns} \
> -			$mptcp_connect -t ${timeout_poll} -M 2 -p
> $port -s ${cl_proto} -c "${cmsg}" \
> -				$connect_addr < "$cin" > "$cout" &
> +	ip netns exec ${connector_ns} \
> +		$mptcp_connect -t ${timeout_poll} -M 2 -p $port -s
> ${cl_proto} -c "${cmsg}" \
> +			$connect_addr < "$cin" > "$cout" &
>  
>  	local cpid=$!
>  
> +	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
> +		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
> +	local timeout_pid=$!
> +
>  	wait $cpid
>  	local retc=$?
>  	wait $spid
>  	local rets=$?
> +	kill $timeout_pid 2>/dev/null && timeout_pid=0
>  
>  	mptcp_lib_nstat_get "${listener_ns}"
>  	mptcp_lib_nstat_get "${connector_ns}"
>  
>  	print_title "Transfer ${ip:2}"
> -	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ]; then
> +	if [ ${rets} -ne 0 ] || [ ${retc} -ne 0 ] || [
> ${timeout_pid} -ne 0 ]; then
>  		mptcp_lib_pr_fail "client exit code $retc, server
> $rets"
>  		mptcp_lib_pr_err_stats "${listener_ns}"
> "${connector_ns}" "${port}"
>  
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh
> b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index 503cb59571a4..cc4d40d149e2 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -158,24 +158,27 @@ do_transfer()
>  	mptcp_lib_nstat_init "${ns3}"
>  	mptcp_lib_nstat_init "${ns1}"
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${ns3} \
> -			./mptcp_connect -jt ${timeout_poll} -l -p
> $port -T $max_time \
> -				0.0.0.0 < "$sin" > "$sout" &
> +	ip netns exec ${ns3} \
> +		./mptcp_connect -jt ${timeout_poll} -l -p $port -T
> $max_time \
> +			0.0.0.0 < "$sin" > "$sout" &
>  	local spid=$!
>  
>  	mptcp_lib_wait_local_port_listen "${ns3}" "${port}"
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${ns1} \
> -			./mptcp_connect -jt ${timeout_poll} -p $port
> -T $max_time \
> -				10.0.3.3 < "$cin" > "$cout" &
> +	ip netns exec ${ns1} \
> +		./mptcp_connect -jt ${timeout_poll} -p $port -T
> $max_time \
> +			10.0.3.3 < "$cin" > "$cout" &
>  	local cpid=$!
>  
> +	mptcp_lib_wait_timeout "${timeout_test}" "${ns3}" "${ns1}"
> "${port}" \
> +		"${cpid}" "${spid}" &
> +	local timeout_pid=$!
> +
>  	wait $cpid
>  	local retc=$?
>  	wait $spid
>  	local rets=$?
> +	kill $timeout_pid 2>/dev/null && timeout_pid=0
>  
>  	if $capture; then
>  		sleep 1
> @@ -191,8 +194,9 @@ do_transfer()
>  	cmp $cin $sout > /dev/null 2>&1
>  	local cmpc=$?
>  
> -	if [ $retc -eq 0 ] && [ $rets -eq 0 ] && \
> -	   [ $cmpc -eq 0 ] && [ $cmps -eq 0 ]; then
> +	if [ $retc -eq 0 ] && [ $rets -eq 0 ] &&
> +	   [ $cmpc -eq 0 ] && [ $cmps -eq 0 ] &&
> +	   [ $timeout_pid -eq 0 ]; then
>  		printf "%-16s" " max $max_time "
>  		mptcp_lib_pr_ok
>  		cat "$capout"
> 

Re: [PATCH mptcp-next 4/4] selftests: mptcp: get stats just before timing out

[Paolo Abeni]

On 11/8/25 3:20 PM, Matthieu Baerts (NGI0) wrote:
> Recently, some debugging happened around a test that was timing out. The
> stats were showing connections being closed which was confusing because
> the closing state was caused by the timeout stopping the transfer.
> 
> To avoid such confusion, the timeout is no longer done per mptcp_connect
> process, but separately. In case of timeout, the stats are now printed,
> then the apps are killed.
> 
> The stats will still be printed after the kill, but that's fine, and
> this might even be useful, just in case. Timeout should be exceptional.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This is very useful, thanks!

> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 21 +++++++++-------
>  tools/testing/selftests/net/mptcp/mptcp_join.sh    | 29 +++++++++++-----------
>  tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 13 ++++++++++
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 21 +++++++++-------
>  tools/testing/selftests/net/mptcp/simult_flows.sh  | 24 ++++++++++--------
>  5 files changed, 66 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 1149bd150d6a..6bff77ed0a74 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -391,26 +391,29 @@ do_transfer()
>  		mptcp_lib_nstat_init "${connector_ns}"
>  	fi
>  
> -	timeout ${timeout_test} \
> -		ip netns exec ${listener_ns} \
> -			./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
> -				$extra_args $local_addr < "$sin" > "$sout" &
> +	ip netns exec ${listener_ns} \
> +		./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
> +			$extra_args $local_addr < "$sin" > "$sout" &
>  	local spid=$!
>  
>  	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>  
>  	local start
>  	start=$(date +%s%3N)
> -	timeout ${timeout_test} \
> -		ip netns exec ${connector_ns} \
> -			./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
> -				$extra_args $connect_addr < "$cin" > "$cout" &
> +	ip netns exec ${connector_ns} \
> +		./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
> +			$extra_args $connect_addr < "$cin" > "$cout" &
>  	local cpid=$!
>  
> +	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
> +		"${connector_ns}" "${port}" "${cpid}" "${spid}" &

I'm still debugging some timeouts caused by `${timeout_poll}`. In such
cases the process still terminates and closes the relevant fd. I'm
wondering additionally setting ${timeout_test} to a lower value of
${timeout_poll}?

BTW I'm testing with CONFIG_PREEMPT=y and possibly I'm seeing the
timeout due to such setting triggering some race we don't observe with
vng defaults: it's very easily reproducible on mptcp_connect.sh
"disconnect" case.

/P

Re: [PATCH mptcp-next 4/4] selftests: mptcp: get stats just before timing out

[Matthieu Baerts]

Hi Geliang,

On 13/11/2025 08:15, Geliang Tang wrote:
> Hi Matt,
> 
> On Sat, 2025-11-08 at 15:20 +0100, Matthieu Baerts (NGI0) wrote:
>> Recently, some debugging happened around a test that was timing out.
>> The
>> stats were showing connections being closed which was confusing
>> because
>> the closing state was caused by the timeout stopping the transfer.
>>
>> To avoid such confusion, the timeout is no longer done per
>> mptcp_connect
>> process, but separately. In case of timeout, the stats are now
>> printed,
>> then the apps are killed.
>>
>> The stats will still be printed after the kill, but that's fine, and
>> this might even be useful, just in case. Timeout should be
>> exceptional.
> 
> Today I found that this patch has increased the runtime of the
> selftests, for example:
> 
> # 66 ns1 MPTCP -> ns1 (dead:beef:1::1:20004) MPTCP     (duration  
> 390ms) [ OK ]
> # 67 ns1 MPTCP -> ns1 (dead:beef:1::1:20005) TCP       (duration  
> 415ms) [ OK ]
> # 68 ns1 TCP   -> ns1 (dead:beef:1::1:20006) MPTCP     (duration  
> 380ms) [ OK ]
> # Time: 38 seconds
> ok 1 test: selftest_mptcp_connect
> # time=99
> 
> The mptcp_connect test ended after 38 seconds, but it waited an
> additional 61 seconds before finally stopping at 99 seconds.

Good catch, indeed, thanks, I will check that!

I guess I should use mptcp_lib_kill_group_wait() to kill the "sleep"
command as well.

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

Re: [PATCH mptcp-next 4/4] selftests: mptcp: get stats just before timing out

[Matthieu Baerts]

Hi Paolo,

On 13/11/2025 08:25, Paolo Abeni wrote:
> On 11/8/25 3:20 PM, Matthieu Baerts (NGI0) wrote:
>> Recently, some debugging happened around a test that was timing out. The
>> stats were showing connections being closed which was confusing because
>> the closing state was caused by the timeout stopping the transfer.
>>
>> To avoid such confusion, the timeout is no longer done per mptcp_connect
>> process, but separately. In case of timeout, the stats are now printed,
>> then the apps are killed.
>>
>> The stats will still be printed after the kill, but that's fine, and
>> this might even be useful, just in case. Timeout should be exceptional.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> 
> This is very useful, thanks!
> 
>> ---
>>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 21 +++++++++-------
>>  tools/testing/selftests/net/mptcp/mptcp_join.sh    | 29 +++++++++++-----------
>>  tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 13 ++++++++++
>>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 21 +++++++++-------
>>  tools/testing/selftests/net/mptcp/simult_flows.sh  | 24 ++++++++++--------
>>  5 files changed, 66 insertions(+), 42 deletions(-)
>>
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> index 1149bd150d6a..6bff77ed0a74 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
>> @@ -391,26 +391,29 @@ do_transfer()
>>  		mptcp_lib_nstat_init "${connector_ns}"
>>  	fi
>>  
>> -	timeout ${timeout_test} \
>> -		ip netns exec ${listener_ns} \
>> -			./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
>> -				$extra_args $local_addr < "$sin" > "$sout" &
>> +	ip netns exec ${listener_ns} \
>> +		./mptcp_connect -t ${timeout_poll} -l -p $port -s ${srv_proto} \
>> +			$extra_args $local_addr < "$sin" > "$sout" &
>>  	local spid=$!
>>  
>>  	mptcp_lib_wait_local_port_listen "${listener_ns}" "${port}"
>>  
>>  	local start
>>  	start=$(date +%s%3N)
>> -	timeout ${timeout_test} \
>> -		ip netns exec ${connector_ns} \
>> -			./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
>> -				$extra_args $connect_addr < "$cin" > "$cout" &
>> +	ip netns exec ${connector_ns} \
>> +		./mptcp_connect -t ${timeout_poll} -p $port -s ${cl_proto} \
>> +			$extra_args $connect_addr < "$cin" > "$cout" &
>>  	local cpid=$!
>>  
>> +	mptcp_lib_wait_timeout "${timeout_test}" "${listener_ns}" \
>> +		"${connector_ns}" "${port}" "${cpid}" "${spid}" &
> 
> I'm still debugging some timeouts caused by `${timeout_poll}`. In such
> cases the process still terminates and closes the relevant fd. I'm
> wondering additionally setting ${timeout_test} to a lower value of
> ${timeout_poll}?
${timeout_test} is supposed to be a safeguard, just in case
${timeout_poll} is not enough. I think it might be better to call 'ss'
from mptcp_connect.c in case of timeout: WDYT?

> BTW I'm testing with CONFIG_PREEMPT=y and possibly I'm seeing the
> timeout due to such setting triggering some race we don't observe with
> vng defaults: it's very easily reproducible on mptcp_connect.sh
> "disconnect" case.

Arf, maybe not a new issue... Thank you for looking at that!

The MPTCP CI and my syzkaller instances are all using
CONFIG_PREEMPT_VOLUNTARY=y. Should I change that? Or at least have a mix
in the Syzkaller instances?

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