Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases

["Gaëtan Rivet" <grive@u256.net>]

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.

-- 
Gaetan Rivet