Re: [LTP] [PATCH v2 2/3] broken_ip-nexthdr.sh: Check IPv6 support before forcing it

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

first, thanks a lot for a review of network shell tests.

> Hi!
> > diff --git a/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh b/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > index ec6643af66..cb4a3dd399 100755
> > --- a/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > +++ b/testcases/network/stress/broken_ip/broken_ip-nexthdr.sh
> > @@ -11,6 +11,7 @@ TST_TESTFUNC="do_test"
> >  do_test()
> >  {
> >  	# not supported on IPv4
> > +	tst_net_require_ipv6
> >  	TST_IPV6=6
> >  	TST_IPVER=6

> I was looking at the code if we can simply instead do:

> [ "$TST_IPVER" == 6 ] && tst_net_require_ipv6
BTW this code was requested by Alexey, originally I suggested different way
to declare TST_NET_IPV{4,6}_ONLY=1 in the test:

https://lore.kernel.org/ltp/20210714140716.1568-3-pvorel@suse.cz/

> in the test library, but it looks like the parameters are parsed in the
> tst_test.sh in the tst_run() function. Which means that the TST_IPVER is
> not actually set until the test starts and the library has no way of
> knowing the variable value beforehand.
Yes.

> I guess that we can actually move the option parsing code in the
> tst_test.sh so that it happens just right after the script is sourced,
> which would make things much easier as the TST_IPVER would end up being
> defined in the tst_network.sh and we coud simply use the statement above
> without any further hacks like this patch adds.
Interesting. Originally I thought I'd need IPv6 check for netns_*.sh tests, but
in the end I realized that better will be if they use tst_net.sh with forcing
netns only.

Sure, I can have look into putting -6 handling into tst_test.sh. It's just a bit
strange to me that tst_test.sh parses it, but it's actually used elsewhere.

> As a side effect we could clean up the test option parsing code since we
> do actually have two different getopts loop in the tst_test.sh library
> and as far as I can tell we can do just with one.
When I looked last time into this and it looked to me that both are needed.
The second one (which is actually run the first - ":hi:$TST_OPTS" is for
ignoring errors when -h is set. But maybe these warnings ("Invalid number of
positional parameters:" and "Unexpected positional arguments" might be really
possible to handle inside of tst_run().

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp