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

[Geliang Tang <geliang@kernel.org>]

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