On Wed, May 15, 2024 at 6:48 PM Simon Horman <horms@xxxxxxxxxx> wrote:
>
Hi Simon,
Thanks for the review!
> 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@xxxxxxxxx>
> > ---
> > The v1 patch is here:
> > https://lore.kernel.org/netdev/20240508040643.229383-1-ap420073@xxxxxxxxx/
> >
> > 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@xxxxxxxxx>
>
> 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.
>
Okay, I will send a v3 patch that will not change the shebang.
> 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
> >
Thanks a lot!
Taehee Yoo
