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
> > is
> > another solution (or another problem).


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

This would be an easiest fix, but OTOH it's another hack which depends on the
implementation (I would prefer to get rid of the need to use struct tst_test
for these tools instead).

Anyway, I'll send a patch to fix tst_net.sh, but ideas how to improve and unify
testcases/lib are welcome.

Kind regards,
Petr

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