Re: [LTP] [PATCH 2/2] testcases/lib: tst_net.sh: Do not use stderr in tst_rhost_run()

Petr Vorel
2025-01-17T14:16:34Z

> Hi!
> > I'm not sure about this. Maybe it's safe, because stderr gets to the output.
> > But in case some network test need to verify something which is defined on
> > stderr it will fails (when tst_rhost_run is run with -s).

> That's why I send the first patch that I know is 100% safe.

+1

> > FYI *without* this patch:

> > # route-redirect.sh
> > ...
> > route-redirect 1 TINFO: timeout per run is 0h 5m 0s
> > route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': 'sh: 1: ns-icmp_redirector: not found'
> > route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': 'ns-icmp_redirector: no process found'
> > ns-icmp_redirector: no process found

> > and *with* this patch:

> > # route-redirect.sh
> > ...
> > route-redirect 1 TINFO: timeout per run is 0h 5m 0s
> > sh: 1: ns-icmp_redirector: not found
> > route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on '': ''
> > ns-icmp_redirector: no process found
> > route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on '': ''

> > Info is there, but not on the same line (second '').

> > Also with the change below it would be more meaningful (fixing first empty ''):

> > route-redirect 1 TBROK: 'ns-icmp_redirector -I ltp_ns_veth1 -b' failed on NETNS: 'sh: 1: ns-icmp_redirector: not found'
> > route-redirect 1 TWARN: 'killall -SIGHUP ns-icmp_redirector' failed on NETNS: 'ns-icmp_redirector: no process found'
> > ns-icmp_redirector: no process found

> > I also wrote conditional quiet on stderr (not sent to ML), but actually if we
> > merge this and need sometimes to parse stderr, the opposite would be needed.

> I supose that we would need to explicitly tell the tst_rhost_run() if we
> want stderr included or not. That's probably something to be done after
> the release.

Generally stderr was always needed. It got broken when tst_ns_exec, which runs
rhost commands on network namespaces, e.g. not the command it runs tst_rhost_run
with -c parameter, but how exactly it is executed. ATM I'm aware only about
tst_rhost_run runs in tst_net_setup_network(), which got broken due eval.

I can send a patch, but IMHO we should concentrate (after the release) on
rewriting as much as we can from tst_net.sh to single C binary + help to
simplify the architecture.

Kind regards,
Petr

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

tst_rhost_run()
{
...
while getopts :bc:su: opt; do
case "$opt" in
b) [ "${TST_USE_NETNS:-}" ] && pre_cmd= || pre_cmd="nohup"
post_cmd=" > /dev/null 2>&1 &"
out="1> /dev/null"
;;
c) cmd="$OPTARG" ;;
s) safe=1 ;;
u) user="$OPTARG" ;;
*) tst_brk_ TBROK "tst_rhost_run: unknown option: $OPTARG" ;;
esac
done
...
sh_cmd="$pre_cmd $cmd $post_cmd"

if [ -n "${TST_USE_NETNS:-}" ]; then
use="NETNS"
rcmd="$LTP_NETNS sh -c"
else
tst_require_cmds ssh
use="SSH"
rcmd="ssh -nq $user@$RHOST"
fi
...
output=$($rcmd "$sh_cmd" $out 2>&1 || echo 'RTERR')

tst_net_setup_network()
{
...
eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV4_RHOST \
|| echo "exit $?")
...
eval $(tst_net_iface_prefix $IPV6_LHOST || echo "exit $?")
eval $(tst_rhost_run -c 'tst_net_iface_prefix -r '$IPV6_RHOST \