[PATCH 2/2 nf] selftests: netfilter: exit on invalid parameters

[PATCH 2/2 nf] selftests: netfilter: exit on invalid parameters

[Fabian Frederick]

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@skynet.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
+
 ip -net ns1 link set eth0 mtu $omtu
 
 ip -net nsr2 link set veth1 mtu $rmtu
+if [ $? -ne 0 ]; then
+	exit 1
+fi
+
 ip -net ns2 link set eth0 mtu $rmtu
 
 # transfer-net between nsr1 and nsr2.
@@ -119,6 +127,11 @@ for i in 1 2; do
   ip -net ns$i addr add 10.0.$i.99/24 dev eth0
   ip -net ns$i route add default via 10.0.$i.1
   ip -net ns$i addr add dead:$i::99/64 dev eth0
+  if [ $? -ne 0 ]; then
+	echo "ERROR: Check Originator/Responder values (problem during address addition)" 1>&2
+	exit 1
+  fi
+
   ip -net ns$i route add default via dead:$i::1
   ip netns exec ns$i sysctl net.ipv4.tcp_no_metrics_save=1 > /dev/null
 
-- 
2.27.0

Re: [PATCH 2/2 nf] selftests: netfilter: exit on invalid parameters

[Stefano Brivio]

Hi Fabian,

On Fri, 14 Aug 2020 20:55:44 +0200
Fabian Frederick <fabf@skynet.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@skynet.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.

-- 
Stefano

Re: [PATCH 2/2 nf] selftests: netfilter: exit on invalid parameters

[Fabian Frederick]

> On 18/08/2020 02:39 Stefano Brivio <sbrivio@redhat.com> wrote:
> 
>  
> Hi Fabian,
> 
> On Fri, 14 Aug 2020 20:55:44 +0200
> Fabian Frederick <fabf@skynet.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@skynet.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