From: Bruce Richardson <bruce.richardson@intel.com>
To: "Gaëtan Rivet" <grive@u256.net>
Cc: "Xueming(Steven) Li" <xuemingl@nvidia.com>,
"dev@dpdk.org" <dev@dpdk.org>,
David Marchand <david.marchand@redhat.com>,
Lior Margalit <lmargalit@nvidia.com>,
Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases
Date: Wed, 20 Oct 2021 15:34:45 +0100 [thread overview]
Message-ID: <YXApBQN1qSRD+8Tg@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <7b7e93e7-1eae-405a-a905-b151f16ed731@www.fastmail.com>
On Wed, Oct 20, 2021 at 04:22:54PM +0200, Gaëtan Rivet wrote:
>
>
> On Wed, Oct 20, 2021, at 15:53, Xueming(Steven) Li wrote:
> > On Wed, 2021-10-20 at 13:55 +0200, Gaëtan Rivet wrote:
> >> On Wed, Oct 20, 2021, at 13:12, Xueming Li wrote:
> >> > Initial version to test global devargs syntax.
> >> >
> >> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> >> > ---
> >> > app/test/meson.build | 5 ++
> >> > app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++
> >> > 2 files changed, 189 insertions(+)
> >> > create mode 100644 app/test/test_devargs.c
> >> >
> >> > diff --git a/app/test/meson.build b/app/test/meson.build
> >> > index a16374b7a10..c4b0241010d 100644
> >> > --- a/app/test/meson.build
> >> > +++ b/app/test/meson.build
> >> > @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING')
> >> > fast_tests += [['latencystats_autotest', true]]
> >> > fast_tests += [['pdump_autotest', true]]
> >> > endif
> >> > +if dpdk_conf.has('RTE_NET_VIRTIO')
> >> > + test_deps += 'net_virtio'
> >> > + test_sources += 'test_devargs.c'
> >> > + fast_tests += [['devargs_autotest', true]]
> >> > +endif
> >>
> >> Hi Steven,
> >>
> >> Thanks for adding new use-cases and expanding the expect.
> >> The dep check for the test can be improved I think.
> >>
> >> When the build is shared, not all built drivers will be available, even if part of the meson config. It might generate false negative when running your test.
> >>
> >> Additionally, even if net_virtio is not built, a subset of your test remains valid.
> >>
> >> Finally, net_virtio might not be generic enough as a driver. A simpler one could be used, such as net_ring maybe.
> >>
> >> Instead of checking the dependencies in the meson file, you could detect support in preamble
> >> of the test:
> >>
> >> bool pci_bus_avail = (rte_bus_find_by_name("pci") != NULL);
> >> bool vdev_bus_avail = (rte_bus_find_by_name("vdev") != NULL);
> >> bool eth_class_avail = [...]
> >>
> >> Then during the test case, depending on the expect part of list[i],
> >> you can skip the case if its deps were not found. In that case, am INFO or DEBUG level
> >> message could be printed to say that the test was skipped.
> >>
> >
> > For vdev supported drivers, currently no API to find it. Macro worked
> > for me:
> >
> > #ifdef RTE_NET_VIRTIO
> > { "net_virtio_user0,iface=test,path=/dev/vhost-
> > net,queues=1",
> > 0, 0, 3, "vdev", "net_virtio_user0", NULL },
> > {
> > "net_virtio_user0,iface=test,path=/class/bus/,queues=1",
> > 0, 0, 3, "vdev", "net_virtio_user0", NULL },
> > #endif
> >
> > PCI, vdev and eth is enabled by default, seems we don't need any flag.
> > But macros can be used as well to be safe, how do you think?
>
> With the macro you will go back to the same issue the meson-based dep check has:
> the macro will be set, but it's possible the relevant .so won't be loaded when
> executing the test.
>
> For Busses and classes, similarly is there a chance that even though they are built,
> they won't be loaded at runtime and make the test result incorrect?
>
> What do you think about switching from net_virtio to net_ring instead otherwise?
> It seems a lighter, utility driver that might be present even in minimal builds.
> It is used in EAL tests, I think it makes more sense than net_virtio for unit tests.
>
> The virtio-specific path-based parameters are irrelevant there, the actual
> driver probe won't be executed anyway so any parameter looking like a path will
> test properly.
>
> Checkout app/test/test_eal_flags.c for examples of net_ring dummy devargs.
>
I'd also recommend making use of the "TEST_SKIPPED" return value rather
than erroring out if a necessary dependency can't be found.
next prev parent reply other threads:[~2021-10-20 14:34 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-05 12:30 ` [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-05 12:30 ` [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-05 14:01 ` David Marchand
2021-10-05 15:56 ` Xueming(Steven) Li
2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li
2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-19 14:57 ` Gaëtan Rivet
2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-19 15:19 ` Gaëtan Rivet
2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-19 15:07 ` Gaëtan Rivet
2021-10-20 7:20 ` Xueming(Steven) Li
2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li
2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-20 7:38 ` David Marchand
2021-10-20 8:23 ` Xueming(Steven) Li
2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li
2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-20 8:22 ` [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-20 9:08 ` David Marchand
2021-10-20 9:40 ` Xueming(Steven) Li
2021-10-20 10:41 ` Bruce Richardson
2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li
2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-20 11:55 ` Gaëtan Rivet
2021-10-20 13:53 ` Xueming(Steven) Li
2021-10-20 14:22 ` Gaëtan Rivet
2021-10-20 14:34 ` Bruce Richardson [this message]
2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li
2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments Xueming Li
2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 2/3] devargs: make bus key parsing optional Xueming Li
2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li
2021-10-21 9:03 ` Gaëtan Rivet
2021-10-23 6:17 ` David Marchand
2021-10-23 12:20 ` Xueming(Steven) Li
2021-10-21 9:22 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax David Marchand
2021-10-21 10:43 ` Xueming(Steven) Li
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=YXApBQN1qSRD+8Tg@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=grive@u256.net \
--cc=lmargalit@nvidia.com \
--cc=thomas@monjalon.net \
--cc=xuemingl@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).