When deleting netns, it is possible to still have some tasks running,
e.g. background tasks like tcpdump running in the background, not
stopped because the test has been interrupted.

Before deleting the netns, it is then safer to kill all attached PIDs,
if any. That should reduce some noises after the end of some tests, and
help with the debugging of some issues. That's why this modification is
seen as a "fix".

Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/lib.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index d0219032f773..8ee4489238ca 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -146,6 +146,7 @@ cleanup_ns()
 
 	for ns in "$@"; do
 		[ -z "${ns}" ] && continue
+		ip netns pids "${ns}" 2> /dev/null | xargs -r kill || true
 		ip netns delete "${ns}" &> /dev/null || true
 		if ! busywait $BUSYWAIT_TIMEOUT ip netns list \| grep -vq "^$ns$" &> /dev/null; then
 			echo "Warn: Failed to remove namespace $ns"

---
base-commit: 3b0e1975962a388844508be0b3e61e759837fa3e
change-id: 20240802-selftests-net-lib-cleanup-kill-f546a233572e

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

On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:

> When deleting netns, it is possible to still have some tasks running,
> e.g. background tasks like tcpdump running in the background, not
> stopped because the test has been interrupted.
>
> Before deleting the netns, it is then safer to kill all attached PIDs,
> if any. That should reduce some noises after the end of some tests, and
> help with the debugging of some issues. That's why this modification is
> seen as a "fix".
>
> Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> tools/testing/selftests/net/lib.sh | 1 +
> 1 file changed, 1 insertion(+)
>

Sounds like a good idea to me:

Reviewed-by: Mat Martineau <martineau@kernel.org>


> diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> index d0219032f773..8ee4489238ca 100644
> --- a/tools/testing/selftests/net/lib.sh
> +++ b/tools/testing/selftests/net/lib.sh
> @@ -146,6 +146,7 @@ cleanup_ns()
>
> 	for ns in "$@"; do
> 		[ -z "${ns}" ] && continue
> +		ip netns pids "${ns}" 2> /dev/null | xargs -r kill || true
> 		ip netns delete "${ns}" &> /dev/null || true
> 		if ! busywait $BUSYWAIT_TIMEOUT ip netns list \| grep -vq "^$ns$" &> /dev/null; then
> 			echo "Warn: Failed to remove namespace $ns"
>
> ---
> base-commit: 3b0e1975962a388844508be0b3e61e759837fa3e
> change-id: 20240802-selftests-net-lib-cleanup-kill-f546a233572e
>
> Best regards,
> -- 
> Matthieu Baerts (NGI0) <matttbe@kernel.org>
>
>
>

On Fri, Aug 2, 2024 at 10:43 AM Mat Martineau <martineau@kernel.org> wrote:
>
> On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:
>
> > When deleting netns, it is possible to still have some tasks running,
> > e.g. background tasks like tcpdump running in the background, not
> > stopped because the test has been interrupted.
> >
> > Before deleting the netns, it is then safer to kill all attached PIDs,
> > if any. That should reduce some noises after the end of some tests, and
> > help with the debugging of some issues. That's why this modification is
> > seen as a "fix".
> >
> > Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > ---
> > tools/testing/selftests/net/lib.sh | 1 +
> > 1 file changed, 1 insertion(+)
> >
>
> Sounds like a good idea to me:
>
> Reviewed-by: Mat Martineau <martineau@kernel.org>

Oops, it's in the net/ lib, not net/mptcp! I should have said:

Acked-by: Mat Martineau <martineau@kernel.org>

>
>
> > diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
> > index d0219032f773..8ee4489238ca 100644
> > --- a/tools/testing/selftests/net/lib.sh
> > +++ b/tools/testing/selftests/net/lib.sh
> > @@ -146,6 +146,7 @@ cleanup_ns()
> >
> >       for ns in "$@"; do
> >               [ -z "${ns}" ] && continue
> > +             ip netns pids "${ns}" 2> /dev/null | xargs -r kill || true
> >               ip netns delete "${ns}" &> /dev/null || true
> >               if ! busywait $BUSYWAIT_TIMEOUT ip netns list \| grep -vq "^$ns$" &> /dev/null; then
> >                       echo "Warn: Failed to remove namespace $ns"
> >
> > ---
> > base-commit: 3b0e1975962a388844508be0b3e61e759837fa3e
> > change-id: 20240802-selftests-net-lib-cleanup-kill-f546a233572e
> >
> > Best regards,
> > --
> > Matthieu Baerts (NGI0) <matttbe@kernel.org>
> >
> >
> >

Hi Mat,

On 02/08/2024 19:52, Mat Martineau wrote:
> On Fri, Aug 2, 2024 at 10:43 AM Mat Martineau <martineau@kernel.org> wrote:
>>
>> On Fri, 2 Aug 2024, Matthieu Baerts (NGI0) wrote:
>>
>>> When deleting netns, it is possible to still have some tasks running,
>>> e.g. background tasks like tcpdump running in the background, not
>>> stopped because the test has been interrupted.
>>>
>>> Before deleting the netns, it is then safer to kill all attached PIDs,
>>> if any. That should reduce some noises after the end of some tests, and
>>> help with the debugging of some issues. That's why this modification is
>>> seen as a "fix".
>>>
>>> Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>>> ---
>>> tools/testing/selftests/net/lib.sh | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>
>> Sounds like a good idea to me:
>>
>> Reviewed-by: Mat Martineau <martineau@kernel.org>
> 
> Oops, it's in the net/ lib, not net/mptcp! I should have said:
> 
> Acked-by: Mat Martineau <martineau@kernel.org>

Thank you for the review!

Now in our tree:

New patches for t/upstream-net and t/upstream:
- 8ebb566eeb44: selftests: net: lib: kill PIDs before del netns
- Results: 4636b17d0a66..35242dba0f3b (export-net)
- Results: 3b0e1975962a..2b058d91b960 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/37ea6923481f76518c4ec25ce70981cc77a5dfca/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/5faa935e1c80da7b646c37ff0ea876778b8c984a/checks

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