[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <185851852.53771.1597772937306@webmail.appsuite.proximus.be>
Date: Tue, 18 Aug 2020 19:48:57 +0200 (CEST)
From: Fabian Frederick <fabf@...net.be>
To: Stefano Brivio <sbrivio@...hat.com>
Cc: pablo@...filter.org, kadlec@...filter.org, fw@...len.de,
netfilter-devel@...r.kernel.org, coreteam@...filter.org,
netdev@...r.kernel.org
Subject: Re: [PATCH 2/2 nf] selftests: netfilter: exit on invalid parameters
> On 18/08/2020 02:39 Stefano Brivio <sbrivio@...hat.com> wrote:
>
>
> Hi Fabian,
>
> On Fri, 14 Aug 2020 20:55:44 +0200
> Fabian Frederick <fabf@...net.be> wrote:
>
> > exit script with comments when parameters are wrong during address
> > addition. No need for a message when trying to change MTU with lower
> > values: output is self-explanatory
> >
> > Signed-off-by: Fabian Frederick <fabf@...net.be>
> > ---
> > tools/testing/selftests/netfilter/nft_flowtable.sh | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/tools/testing/selftests/netfilter/nft_flowtable.sh b/tools/testing/selftests/netfilter/nft_flowtable.sh
> > index 28e32fddf9b2c..c3617d0037f2e 100755
> > --- a/tools/testing/selftests/netfilter/nft_flowtable.sh
> > +++ b/tools/testing/selftests/netfilter/nft_flowtable.sh
> > @@ -97,9 +97,17 @@ do
> > done
> >
> > ip -net nsr1 link set veth0 mtu $omtu
> > +if [ $? -ne 0 ]; then
> > + exit 1
> > +fi
> > +
>
> As some of your recent patches are also clean-ups, perhaps you get some
> assistance from 'shellcheck' (https://www.shellcheck.net/). For
> example, this could be written as:
>
> ip -net nsr1 link set veth0 mtu $omtu || exit 1
>
> or, I'm not sure it's doable, you could get all those checks for free
> by setting the -e flag for the entire script. You would then need to
> take care explicitly of commands that can legitimately fail.
Hi Stefano,
Thanks a lot for the tip. I'll let original script developer decide wether that kind of syntax is interesting or not and resend this one if necessary (a lot of ip tests are already done this way in the script). Idea behind my recent patches was to enable testing with different MTU but other feature ideas are welcome :)
Best regards,
Fabian
>
> --
> Stefano
Powered by blists - more mailing lists