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
next prev parent 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