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

[Matthieu Baerts <matttbe@kernel.org>]

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.