From: Petr Vorel <pvorel@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/1] net/route: Rewrite route-rmmod to new API
Date: Thu, 2 Sep 2021 18:45:33 +0200 [thread overview]
Message-ID: <YTD/rU/ohgp8p5Zc@pevik> (raw)
In-Reply-To: <a07de84c-5251-9e46-afac-84544247534b@bell-sw.com>
Hi Alexey,
<snip>
> >> BTW, why not using add_macvlan() in route_lib.sh (or gre, vxlan, etc.)
> >> and remove that driver, so that this test can be run with custom setup, and
> >> with remote host setup?
> > Looking into this old patch, it looked to me quite bad approach to move
> > add_macvlan() into tst_net.sh to be reused (you didn't suggested that, that's
> > what I'd do to prevent the duplicity).
> Why not move add_macvlan() to the virt_lib.sh, I think this lib is better
> suited for creating macvlan?
Well, route-change-if.sh and route-change-netlink-if.sh depends on helpers from
route-lib.sh. Using both libraries would require little changes, but sure it's
possible (+ add obviously add $virt_type).
It's just a bit strange to add this special purpose function when there is
virt_add() and virt_add_rhost(). Best will be probably to add macvlan to these
functions and migrate router tests which needs macvlan to use virt_lib.sh before
route-rmmod.sh using it.
In the long term I'd really prefer to add some TST_NET* variable (due doc
generation via docparse), but that can be postponed as another effort after the
release (I thought it'd have to be in tst_net.sh so that tests does not care
about including virt_lib.sh when declaring it, but variable could also have
prefix TST_NET_VIRT_ to make it obvious, that virt_lib.sh must be loaded. But I
still prefer moving to tst_net.sh when implementing this approach).
BTW you consider ok to use macvlan for testing this? I suppose this test was
intended to be used on the real hardware not on virtualization. But I don't have
proper setup and give up on this approach.
Kind regards,
Petr
> > But much better approach would be IMHO to move virt_add() and virt_add_rhost()
> > from virt_lib.sh to tst_net.sh and adjust it not to be too tight to virt_lib.sh.
> > I suppose $virt_type should became $2 (second parameter).
> > Also there could be moved from virt_lib.sh to tst_net.sh: e.g. add flag
> > TST_NET_ADD_VIRT_TYPE (e.g. macvlan, gre, ...) and doing setup and cleanup
> > there. We could reduce code and document which virt drivers are used.
> > route-change-netlink-if is the only test which needs to call tst_init_iface()
> > (to add routes), virt_lib.sh does not need it.
> > Kind regards,
> > Petr
next prev parent reply other threads:[~2021-09-02 16:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 22:03 [LTP] [PATCH 1/1] net/route: Rewrite route-rmmod to new API Petr Vorel
2020-11-17 14:30 ` Alexey Kodanev
2020-11-17 16:16 ` Petr Vorel
2021-09-01 12:15 ` Petr Vorel
2021-09-02 9:07 ` Alexey Kodanev
2021-09-02 16:45 ` Petr Vorel [this message]
2021-09-03 8:53 ` Alexey Kodanev
2021-09-03 8:55 ` 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=YTD/rU/ohgp8p5Zc@pevik \
--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 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.