netdev - Re: [PATCH net v2] selftests: net: kill smcrouted in the cleanup logic in amt.sh

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240515094831.GA154012@kernel.org>
Date: Wed, 15 May 2024 10:48:31 +0100
From: Simon Horman <horms@...nel.org>
To: Taehee Yoo <ap420073@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, shuah@...nel.org, netdev@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net v2] selftests: net: kill smcrouted in the cleanup
 logic in amt.sh

On Mon, May 13, 2024 at 06:08:52AM +0000, Taehee Yoo wrote:
> The amt.sh requires smcrouted for multicasting routing.
> So, it starts smcrouted before forwarding tests.
> It must be stopped after all tests, but it isn't.
> 
> To fix this issue, it kills smcrouted in the cleanup logic.
> 
> Fixes: c08e8baea78e ("selftests: add amt interface selftest script")
> Signed-off-by: Taehee Yoo <ap420073@...il.com>
> ---
> The v1 patch is here:
> https://lore.kernel.org/netdev/20240508040643.229383-1-ap420073@gmail.com/
> 
> v2
>  - Headline change.
>  - Kill smcrouted process only if amt.pid exists.
>  - Do not remove the return value.
>  - Remove timeout logic because it was already fixed by following commit
>    4c639b6a7b9d ("selftests: net: move amt to socat for better compatibility")
>  - Fix shebang.
> 
>  tools/testing/selftests/net/amt.sh | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/amt.sh b/tools/testing/selftests/net/amt.sh
> index 5175a42cbe8a..d458b45c775b 100755
> --- a/tools/testing/selftests/net/amt.sh
> +++ b/tools/testing/selftests/net/amt.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>  
>  # Author: Taehee Yoo <ap420073@...il.com>

Hi.

I think that the change above is not related to the fix below.
I think it should be in a separate patch targeted at net-next.
And as net-next is currently closed it should be posted once it
reopens, after the 27th May.

The changes below look good to me.
I suggest resending only those changes, targeted at net, as v3.

Thanks.

> @@ -77,6 +77,7 @@ readonly LISTENER=$(mktemp -u listener-XXXXXXXX)
>  readonly GATEWAY=$(mktemp -u gateway-XXXXXXXX)
>  readonly RELAY=$(mktemp -u relay-XXXXXXXX)
>  readonly SOURCE=$(mktemp -u source-XXXXXXXX)
> +readonly SMCROUTEDIR="$(mktemp -d)"
>  ERR=4
>  err=0
>  
> @@ -85,6 +86,11 @@ exit_cleanup()
>  	for ns in "$@"; do
>  		ip netns delete "${ns}" 2>/dev/null || true
>  	done
> +	if [ -f "$SMCROUTEDIR/amt.pid" ]; then
> +		smcpid=$(< $SMCROUTEDIR/amt.pid)
> +		kill $smcpid
> +	fi
> +	rm -rf $SMCROUTEDIR
>  
>  	exit $ERR
>  }
> @@ -167,7 +173,7 @@ setup_iptables()
>  
>  setup_mcast_routing()
>  {
> -	ip netns exec "${RELAY}" smcrouted
> +	ip netns exec "${RELAY}" smcrouted -P $SMCROUTEDIR/amt.pid
>  	ip netns exec "${RELAY}" smcroutectl a relay_src \
>  		172.17.0.2 239.0.0.1 amtr
>  	ip netns exec "${RELAY}" smcroutectl a relay_src \

-- 
pw-bot: changes-requested
>