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

[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.
> 
> 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##*:*}" ]
>