selftests: mptcp: get stats just before timing out

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

Posted by Matthieu Baerts (NGI0) 2 weeks, 4 days ago

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

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

Posted by Geliang Tang 2 weeks, 2 days ago

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

Posted by Matthieu Baerts 2 weeks, 2 days ago

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

Posted by Geliang Tang 2 weeks, 2 days ago

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

Posted by Matthieu Baerts 2 weeks, 1 day ago

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

Posted by Matthieu Baerts 2 weeks, 2 days ago

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.