Re: [LTP] [REGRESSION] Broken tests using tst_net.sh by 893ca0abe7 (was: [PATCH 1/2] lib: multiply the timeout if detect slow kconfigs)

[Petr Vorel <pvorel@suse.cz>]

> On Thu, Jan 16, 2025 at 6:42 AM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li, Cyril, all,

> > ...
> > > +++ b/lib/tst_test.c
> > > @@ -555,9 +555,6 @@ static int multiply_runtime(int max_runtime)

> > >       parse_mul(&runtime_mul, "LTP_RUNTIME_MUL", 0.0099, 100);

> > > -     if (tst_has_slow_kconfig())
> > > -             max_runtime *= 4;
> > > -
> > >       return max_runtime * runtime_mul;
> > >  }

> > > @@ -1706,6 +1703,9 @@ unsigned int tst_multiply_timeout(unsigned int
> > timeout)
> > >       if (timeout < 1)
> > >               tst_brk(TBROK, "timeout must to be >= 1! (%d)", timeout);

> > > +     if (tst_has_slow_kconfig())
> > > +             timeout *= 4;

> > FYI this change, merged as 893ca0abe7 ("lib: multiply the timeout if
> > detect slow
> > kconfigs") caused a regression on *all* tests which use tst_net.sh.

> > First, it detects slow config on everything which uses struct tst_test,
> > which are unfortunately some tools at testcases/lib/:

> > $ git grep -l "struct tst_test" testcases/lib/*.c
> > testcases/lib/tst_device.c // not obvious reason, might be removed
> > testcases/lib/tst_get_free_pids.c // force messages to be printed from new
> > library
> > testcases/lib/tst_ns_create.c // .forks_child = 1, Needed by SAFE_CLONE
> > testcases/lib/tst_ns_exec.c  // .forks_child = 1, Needed by SAFE_CLONE
> > testcases/lib/tst_run_shell.c // not obvious reason, might be removed

> > Besides unimportant fact that slow config detection is an unnecessary
> > slowdown
> > on these tools, the problem is that it prints messages to stderr, which
> > causes
> > breakage.

> > Previously tst_ns_exec.c run just command passed by -c parameter:

> > $ ./tst_ns_exec 14536 net,mnt sh -c " cat
> > /proc/sys/net/ipv6/conf/ltp_ns_veth1/disable_ipv6"
> > 0

> > Now it prints TINFO:
> > $ ./tst_ns_exec 14536 net,mnt sh -c " cat
> > /proc/sys/net/ipv6/conf/ltp_ns_veth1/disable_ipv6"
> > tst_kconfig.c:88: TINFO: Parsing kernel config '/proc/config.gz'
> > tst_kconfig.c:667: TINFO: CONFIG_LATENCYTOP kernel option detected which
> > might slow the execution
> > 0

> > tst_rhost_run() in tst_net.sh runs binary on remote host:

> >         output=$($rcmd "$sh_cmd" $out 2>&1 || echo 'RTERR')

> > Redirect stderr to stdout (2>&1) is likely needed for some tests. But
> > tst_rhost_run() output is often parsed including therefore certain tools in
> > testcases/lib/ have to print only expected output:

> > init_ltp_netspace()
> > {
> >         ...
> >                 pid="$(ROD tst_ns_create net,mnt)"
> >         ...
> >         export LTP_NETNS="${LTP_NETNS:-tst_ns_exec $pid net,mnt}"

> > I will probably solve it by adding yet another parameter to
> > tst_rhost_run(),
> > which ignores stderr and use it for tst_ns_{create,exec}. But maybe there

Actually tst_ns_exec.c needs to be quiet to not clutter the output of the
command it runs via tst_rhost_run() for netns (via $LTP_NETNS). Therefore adding
an quiet option to tst_rhost_run() is not a solution.

> > is
> > another solution (or another problem).


> Or, another way is just to set 'tst_test->timeout == TST_UNLIMITED_TIMEOUT'
> in those testcase/lib tools.

Unfortunately this does not work. In the end I probably filter out 'TINFO:'
messages in tst_rhost_run().

It'd be safer to avoid using library for tst_ns_exec.c, but it's also used in
containers shell tests (netns_lib.sh, netns_sysfs.sh) and fs_bind tests
(fs_bind_cloneNS*.sh).

Also, tst_ns_exec.c (and tst_ns_create.c) could be for netns replaced by
'ip netns' (revert back change in 2017 3fb501e04c ("net: use LTP ns_create/ns_exec")

Maybe other tools which use tst_ns_{create,exec,ifmove}.c could be replaced by
some tools from util-linux (unshare ?). We consider having own binary as better
than external dependency, but binary needs to be simple to avoid problems when
LTP library changes.

Kind regards,
Petr

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