[LTP] [PATCH v1] tst_net.sh: Add more tst_require_cmds check

Petr Vorel pvorel@suse.cz
Thu Jan 26 23:04:20 CET 2023


Hi Wei,

> ...
> > > But even if "ip ns_create ns_exec ns_ifmove" are needed to be checked to fix
> > > tst_rhost_run (not yet convinced), why to check them each time tst_rhost_run is
> > > called? It should be checked before first tst_rhost_run call to avoid useless
> > > repeating.

> > tst_rhost_run already be called everywhere so if you need check before call
> > tst_rhost_run, means lot of place/cases maybe need review and update. 

> No, the check would be on single place at tst_net.sh.

> > Furthermore, check each time in tst_rhost_run can make this function more robust, the 
> > old code also do tst_require_cmds if go ssh logic in line 211 for example.

> Well, repeated check for ssh is not ideal either (how likely is that ssh will
> disappear during testing?), but it's a single command which is used here.
> And it's definitely less awkward than checking "ip ns_create ns_exec ns_ifmove"
> which aren't used there. tst_net.sh needs cleanup, not more unneeded code.

> We could document the reason to make this check more obvious, but the code is
> wrong, because for netns (the default) running "tst_net_detect_ipv6 rhost"
> before init_ltp_netspace does not make sense, because interfaces aren't defined
> (probably error code is not detected, I need to recheck that).

> It's actually enough for netns to test IPv6 support only on localhost (it will
> be the same on rhost), I'll send a patch to fix that.

I'm sorry, I was wrong. tst_net_detect_ipv6() checks [ -f /proc/net/if_inet6 ],
which is ok even before interfaces are set. It's a question whether this needs
to be run on netns also on rhost (IMHO the result will be always the same).
We should investigate that.

FYI I changed your patch to use just check in $_tst_net_parse_variables and put
it into larger cleanup. I haven't addressed move tools to testcases/lib/ yet,
nor the case when tools are missing. Instead I fixed few real problems with IPv6
disabled and did cleanup.

https://lore.kernel.org/ltp/20230126215401.29101-1-pvorel@suse.cz/
https://patchwork.ozlabs.org/project/ltp/list/?series=338700&state=*

Kind regards,
Petr


More information about the ltp mailing list