From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8AE6FA0C43; Wed, 20 Oct 2021 16:34:53 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5DF804118F; Wed, 20 Oct 2021 16:34:53 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id 0E15A40142 for ; Wed, 20 Oct 2021 16:34:51 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10143"; a="314996759" X-IronPort-AV: E=Sophos;i="5.87,167,1631602800"; d="scan'208";a="314996759" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Oct 2021 07:34:51 -0700 X-IronPort-AV: E=Sophos;i="5.87,167,1631602800"; d="scan'208";a="444383038" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.29.221]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 20 Oct 2021 07:34:49 -0700 Date: Wed, 20 Oct 2021 15:34:45 +0100 From: Bruce Richardson To: =?iso-8859-1?Q?Ga=EBtan?= Rivet Cc: "Xueming(Steven) Li" , "dev@dpdk.org" , David Marchand , Lior Margalit , Thomas Monjalon Message-ID: References: <20211005123012.264727-1-xuemingl@nvidia.com> <20211020111230.2441949-1-xuemingl@nvidia.com> <20211020111230.2441949-4-xuemingl@nvidia.com> <4aefeffc-35a9-4f1f-acc3-4d4682587b81@www.fastmail.com> <7b7e93e7-1eae-405a-a905-b151f16ed731@www.fastmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <7b7e93e7-1eae-405a-a905-b151f16ed731@www.fastmail.com> Subject: Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > >> > --- > >> > 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.