From: Simon Horman <horms@kernel.org>
To: Taehee Yoo <ap420073@gmail.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, shuah@kernel.org, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net v2] selftests: net: kill smcrouted in the cleanup logic in amt.sh
Date: Wed, 15 May 2024 10:48:31 +0100 [thread overview]
Message-ID: <20240515094831.GA154012@kernel.org> (raw)
In-Reply-To: <20240513060852.1105380-1-ap420073@gmail.com>
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@gmail.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@gmail.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
>
next prev parent reply other threads:[~2024-05-15 9:48 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-13 6:08 [PATCH net v2] selftests: net: kill smcrouted in the cleanup logic in amt.sh Taehee Yoo
2024-05-15 9:48 ` Simon Horman [this message]
2024-05-15 11:03 ` Taehee Yoo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240515094831.GA154012@kernel.org \
--to=horms@kernel.org \
--cc=ap420073@gmail.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).