All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: Wei Gao <wegao@suse.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] [PATCH v1] tst_net.sh: Add more tst_require_cmds check
Date: Fri, 20 Jan 2023 09:38:43 +0100	[thread overview]
Message-ID: <Y8pTE3I38qTV2Kez@pevik> (raw)
In-Reply-To: <Y8Zg+PIzLUySx2BI@aa>

> On Tue, Jan 17, 2023 at 09:33:14AM +0100, Petr Vorel wrote:
> > Hi Wei,

> > > More strict check for ns_xxx etc will help avoid following issue:
> > > https://github.com/linux-test-project/ltp/issues/991

> > > Signed-off-by: Wei Gao <wegao@suse.com>
> > > Suggested-by: Petr Vorel <pvorel@suse.cz>
> > > ---
> > >  testcases/lib/tst_net.sh | 2 ++
> > >  1 file changed, 2 insertions(+)

> > > diff --git a/testcases/lib/tst_net.sh b/testcases/lib/tst_net.sh
> > > index ceb45c98d..ec915ad74 100644
> > > --- a/testcases/lib/tst_net.sh
> > > +++ b/testcases/lib/tst_net.sh
> > > @@ -205,6 +205,7 @@ tst_rhost_run()
> > >  	sh_cmd="$pre_cmd $cmd $post_cmd"

> > >  	if [ -n "${TST_USE_NETNS:-}" ]; then
> > > +		tst_require_cmds ip ns_create ns_exec ns_ifmove
> > Why this? none of these commands is used here.
> This line is the key to avoid wrong configuration(such as add ip inteface etc..) continue even ns_xxx not exist,
> you can check following code, line 222 will execute ns_xxx with error but not exit test immediately, so if we add 
> require_cmds here then we can avoid it. (line 222 $rcmd will contain command ns_xxx)

>  218                 tst_res_ TINFO "tst_rhost_run: cmd: $cmd"
>  219                 tst_res_ TINFO "$use: $rcmd \"$sh_cmd\" $out 2>&1"
>  220         fi
>  221
>  222         output=$($rcmd "$sh_cmd" $out 2>&1 || echo 'RTERR')
>  223
>  224         echo "$output" | grep -q 'RTERR$' && ret=1
>  225         if [ $ret -eq 1 ]; then

Long time ago the first run code for netns was init_ltp_netspace.
I added IPv6 check quite recently. (I originally submitted it as run after, but
in the end it is run before it.)
I need to have a deeper look how tst_net_detect_ipv6 rhost works, calling
tst_rhost_run before init_ltp_netspace should not work (interfaces aren't
configured yet).

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.

Kind regards,
Petr

> > >  		use="NETNS"
> > >  		rcmd="$LTP_NETNS sh -c"
> > >  	else
> > > @@ -1006,6 +1007,7 @@ IPV6_RHOST="${IPV6_RHOST:-fd00:1:1:1::1/64}"
> > >  # tst_net_iface_prefix -h
> > >  # tst_net_vars -h
> > >  if [ -z "$_tst_net_parse_variables" ]; then
> > > +	tst_require_cmds tst_net_ip_prefix
> > This is correct.

> > Kind regards,
> > Petr

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

  reply	other threads:[~2023-01-20  8:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-17  4:01 [LTP] [PATCH v1] tst_net.sh: Add more tst_require_cmds check Wei Gao via ltp
2023-01-17  8:33 ` Petr Vorel
2023-01-17  8:48   ` Wei Gao via ltp
2023-01-20  8:38     ` Petr Vorel [this message]
2023-01-20 11:01       ` Wei Gao via ltp
2023-01-22 21:58         ` Petr Vorel
2023-01-26 22:04           ` Petr Vorel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y8pTE3I38qTV2Kez@pevik \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    --cc=wegao@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.