Linux Test Project
 help / color / mirror / Atom feed
From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v4 0/7] net/route: rewrite route-change-{dst, gw, if} into new API
Date: Thu, 12 Sep 2019 15:48:26 +0200	[thread overview]
Message-ID: <20190912134826.GB22246@dell5510> (raw)
In-Reply-To: <df197557-8f95-52d5-813e-a5abc73a1af6@oracle.com>

Hi Alexey,

first, thanks for your review.

..
> > this version address functionality problems you pointed out.
> > But yet there might need to be another version as I'm not sure about
> > tst_ipaddr_un() API changes.

> > Changes v3-v4:
> > * enhanced tst_ipaddr_un() with -b, -h -l, -m, -n and -p options (-p was
> > previous -m)
> > * use tst_ipaddr_un() options to fix address clash on host_id, net_id clash fix
> > by adjusting $1 in test (this is really inconsistent, see note at 3/7).
> > * added tests for tst_ipaddr_un()
> > * quiet EXPECT_PASS ping$TST_IPV6


> The patch-set looks good, the only concerns are:

> * complicated tst_ipaddr_un(), may be add another high-level function
>   to use raw tst_ipadd_un(), if -b, -f, -n options are really needed?
Sure. I'll keep only -l and -m and drop -b, -f, -n and -h as not needed ATM.
And add them via high-level function only if needed.

In that case I'd like to have -l and -m functionality also for NET_ID (already
needed in route-change-dst.sh). But not sure which options would be for it
(it'd be easy with long opts, but we don't want to depend on /usr/bin/getopt,
nor to parse it manually although of course doable)

Another option is to have single option for adding both MAX and MIN:

-h MIN,MAX # for HOST_ID
-n MIN,MAX # for NET_ID

(e.g. -n5,255 -h1,255)

But it looks to me a bit uncomfortable having to always to add both min and max.

> * if NS_TIMES is large, there are a lot of messages in a test output
>   (it seems NS_TIMES * 4), could we minimize the number of them?

I can add -q option to tst_add_ipaddr() which usage will make it NS_TIMES * 2. (1)
If it's still too much I'll drop "testing route over ..." (2)
Other option would be to print only even Nth iteration (I don't think it's a
good idea) or even has only single TPASS/TFAIL.

BTW don't we want end testing on first failure?


Kind regards,
Petr

  reply	other threads:[~2019-09-12 13:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-03 14:16 [LTP] [PATCH v4 0/7] net/route: rewrite route-change-{dst, gw, if} into new API Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 1/7] tst_net.sh: enhance tst_add_ipaddr(), add tst_del_ipaddr() Petr Vorel
2019-09-12 12:12   ` Alexey Kodanev
2019-09-12 13:26     ` Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 2/7] tst_net.sh: Add -p option to return prefix in tst_ipaddr_un() Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 3/7] tst_net.sh: Add (mostly) HOST_ID related options to tst_ipaddr_un Petr Vorel
2019-09-12 12:41   ` Alexey Kodanev
2019-09-03 14:16 ` [LTP] [PATCH v4 4/7] net: Add tst_ipaddr_un.sh test Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 5/7] network/route: Rewrite route{4, 6}-change-dst into new shell API Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 6/7] network/route: Rewrite route{4, 6}-change-gw " Petr Vorel
2019-09-03 14:16 ` [LTP] [PATCH v4 7/7] network/route: Rewrite route{4, 6}-change-if into new API Petr Vorel
2019-09-12 12:56 ` [LTP] [PATCH v4 0/7] net/route: rewrite route-change-{dst, gw, if} " Alexey Kodanev
2019-09-12 13:48   ` Petr Vorel [this message]
2019-09-12 14:18     ` Alexey Kodanev
2019-09-12 15:07       ` 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=20190912134826.GB22246@dell5510 \
    --to=pvorel@suse.cz \
    --cc=ltp@lists.linux.it \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox