[dpdk-dev,1/7] event/octeontx: move eventdev octeontx test to driver
Checks
Commit Message
Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
driver/event/octeontx.
Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
---
.../event/octeontx/selftest_octeontx.c | 0
test/test/Makefile | 1 -
2 files changed, 1 deletion(-)
rename test/test/test_eventdev_octeontx.c => drivers/event/octeontx/selftest_octeontx.c (100%)
Comments
> -----Original Message-----
> From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Tuesday, December 12, 2017 7:27 PM
> To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> <bruce.richardson@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> <liang.j.ma@intel.com>
> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> to driver
>
> Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> driver/event/octeontx.
<snip patch content>
Replying to 1st patch, as no cover letter;
Summary of patchset:
- Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
- Enable self tests to run when passed the vdev arg "self-test=1"
A few comments on this change;
1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
Specific comments will be sent as replies to the patches. Cheers, -Harry
On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > Sent: Tuesday, December 12, 2017 7:27 PM
> > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > <bruce.richardson@intel.com>; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > <liang.j.ma@intel.com>
> > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > to driver
> >
> > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > driver/event/octeontx.
>
> <snip patch content>
>
> Replying to 1st patch, as no cover letter;
>
> Summary of patchset:
> - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> - Enable self tests to run when passed the vdev arg "self-test=1"
>
>
> A few comments on this change;
>
> 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
>
> From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
>
> 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
>
>
> Specific comments will be sent as replies to the patches. Cheers, -Harry
What I gather from a cursory glance at this set is that the self tests
are designed to be triggered via devargs to the device driver, correct?
I'm not sure I like this approach, though I do agree with having the
tests inside the individual drivers.
What I think I would prefer to see is the self-tests being called via an
API rather than via devargs. I think we should add a
"rte_event_dev_self_test()" API to the eventdev library, and have that
then call into the driver-provided tests. This means that self-tests can
only be called by applications which are set up to allow the tests to be
called, e.g. the autotest binary, while also avoiding the issue of
having lots of driver specifics clutter up test binaries.
Regards,
/Bruce
On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > Sent: Tuesday, December 12, 2017 7:27 PM
> > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > <bruce.richardson@intel.com>; Van Haaren, Harry
> > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > <liang.j.ma@intel.com>
> > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > to driver
> >
> > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > driver/event/octeontx.
>
> <snip patch content>
>
> Replying to 1st patch, as no cover letter;
>
> Summary of patchset:
> - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> - Enable self tests to run when passed the vdev arg "self-test=1"
>
>
> A few comments on this change;
>
> 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
>
> From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
>
> 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
>
I initially thought of abstracting the macros but couldnt find a suitable file
to place them in we have two options here, one is to use CFLAGS and include
test.h directly (dirty) or have rte_assert/test in eal/common/inlcude.
Thoughts?
>
> Specific comments will be sent as replies to the patches. Cheers, -Harry
On Wed, Dec 13, 2017 at 10:34:28AM +0000, Bruce Richardson wrote:
> On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > > -----Original Message-----
> > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > Sent: Tuesday, December 12, 2017 7:27 PM
> > > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Van Haaren, Harry
> > > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > > <liang.j.ma@intel.com>
> > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > > to driver
> > >
> > > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > > driver/event/octeontx.
> >
> > <snip patch content>
> >
> > Replying to 1st patch, as no cover letter;
> >
> > Summary of patchset:
> > - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> > - Enable self tests to run when passed the vdev arg "self-test=1"
> >
> >
> > A few comments on this change;
> >
> > 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
> >
> > From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
> >
> > 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
> >
> >
> > Specific comments will be sent as replies to the patches. Cheers, -Harry
>
> What I gather from a cursory glance at this set is that the self tests
> are designed to be triggered via devargs to the device driver, correct?
> I'm not sure I like this approach, though I do agree with having the
> tests inside the individual drivers.
>
> What I think I would prefer to see is the self-tests being called via an
> API rather than via devargs. I think we should add a
> "rte_event_dev_self_test()" API to the eventdev library, and have that
> then call into the driver-provided tests. This means that self-tests can
> only be called by applications which are set up to allow the tests to be
> called, e.g. the autotest binary, while also avoiding the issue of
> having lots of driver specifics clutter up test binaries.
Agreed, will modify it to ops based scheme so that application can call driver
specific `event_dev_self_test` and register selftest in
test/test/test_eventdev.c.
Although we would like to retain devargs selftest scheme for event_octeontx. I
will remove it for event_sw. Does that sound good?
>
> Regards,
> /Bruce
Regards,
Pavan
On Wed, Dec 13, 2017 at 04:54:29PM +0530, Pavan Nikhilesh Bhagavatula wrote:
> On Wed, Dec 13, 2017 at 10:34:28AM +0000, Bruce Richardson wrote:
> > On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > > > -----Original Message-----
> > > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > > Sent: Tuesday, December 12, 2017 7:27 PM
> > > > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; Van Haaren, Harry
> > > > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > > > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > > > <liang.j.ma@intel.com>
> > > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > > > to driver
> > > >
> > > > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > > > driver/event/octeontx.
> > >
> > > <snip patch content>
> > >
> > > Replying to 1st patch, as no cover letter;
> > >
> > > Summary of patchset:
> > > - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> > > - Enable self tests to run when passed the vdev arg "self-test=1"
> > >
> > >
> > > A few comments on this change;
> > >
> > > 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
> > >
> > > From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
> > >
> > > 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
> > >
> > >
> > > Specific comments will be sent as replies to the patches. Cheers, -Harry
> >
> > What I gather from a cursory glance at this set is that the self tests
> > are designed to be triggered via devargs to the device driver, correct?
> > I'm not sure I like this approach, though I do agree with having the
> > tests inside the individual drivers.
> >
> > What I think I would prefer to see is the self-tests being called via an
> > API rather than via devargs. I think we should add a
> > "rte_event_dev_self_test()" API to the eventdev library, and have that
> > then call into the driver-provided tests. This means that self-tests can
> > only be called by applications which are set up to allow the tests to be
> > called, e.g. the autotest binary, while also avoiding the issue of
> > having lots of driver specifics clutter up test binaries.
>
> Agreed, will modify it to ops based scheme so that application can call driver
> specific `event_dev_self_test` and register selftest in
> test/test/test_eventdev.c.
>
> Although we would like to retain devargs selftest scheme for event_octeontx. I
> will remove it for event_sw. Does that sound good?
>
Sure, sounds good to me.
Thanks,
/Bruce
On Wed, Dec 13, 2017 at 04:49:47PM +0530, Pavan Nikhilesh Bhagavatula wrote:
> On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > > -----Original Message-----
> > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > Sent: Tuesday, December 12, 2017 7:27 PM
> > > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Van Haaren, Harry
> > > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > > <liang.j.ma@intel.com>
> > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > > to driver
> > >
> > > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > > driver/event/octeontx.
> >
> > <snip patch content>
> >
> > Replying to 1st patch, as no cover letter;
> >
> > Summary of patchset:
> > - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> > - Enable self tests to run when passed the vdev arg "self-test=1"
> >
> >
> > A few comments on this change;
> >
> > 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
> >
> > From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
> >
> > 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
> >
>
> I initially thought of abstracting the macros but couldnt find a suitable file
> to place them in we have two options here, one is to use CFLAGS and include
> test.h directly (dirty) or have rte_assert/test in eal/common/inlcude.
>
> Thoughts?
>
If other device types, e.g. ethdev or cryptodev, also take the approach of
having a self_test API (something I think would be a good thing, and I
actually hacked together when working on the i40e rx and tx code), I
think we should look to have an rte_test.h header file for such macros
to avoid duplication.
At this point, moving them to an EAL include may not be worth it for
just eventdev.
/Bruce
On Wed, Dec 13, 2017 at 11:41:55AM +0000, Bruce Richardson wrote:
> On Wed, Dec 13, 2017 at 04:49:47PM +0530, Pavan Nikhilesh Bhagavatula wrote:
> > On Wed, Dec 13, 2017 at 10:19:51AM +0000, Van Haaren, Harry wrote:
> > > > -----Original Message-----
> > > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > > Sent: Tuesday, December 12, 2017 7:27 PM
> > > > To: jerin.jacob@caviumnetworks.com; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; Van Haaren, Harry
> > > > <harry.van.haaren@intel.com>; Eads, Gage <gage.eads@intel.com>;
> > > > hemant.agrawal@nxp.com; nipun.gupta@nxp.com; Ma, Liang J
> > > > <liang.j.ma@intel.com>
> > > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > > Subject: [dpdk-dev] [PATCH 1/7] event/octeontx: move eventdev octeontx test
> > > > to driver
> > > >
> > > > Move octeontx eventdev specific test (test_eventdev_octeontx.c) to
> > > > driver/event/octeontx.
> > >
> > > <snip patch content>
> > >
> > > Replying to 1st patch, as no cover letter;
> > >
> > > Summary of patchset:
> > > - Move tests for a specific Eventdev PMD into the PMD dir: drivers/event/x/x_selftest.c
> > > - Enable self tests to run when passed the vdev arg "self-test=1"
> > >
> > >
> > > A few comments on this change;
> > >
> > > 1) We should not lose the capability to run tests as part of the existing unit testing infrastructure. We should not fragment the testing tool - requiring multiple binaries to test a single component.
> > >
> > > From discussion on #IRC, it seems reasonable to call rte_eal_vdev_init() with "self-test=1" from the test/test/ code, and then we can continue to use the existing test infrastructure despite that the actual tests are now part of each PMD.
> > >
> > > 2) We should not copy/paste TEST_ASSERT macros into new test files. Abstracting the TEST_ASSERT and other macros out to a header file would solve this duplication.
> > >
> >
> > I initially thought of abstracting the macros but couldnt find a suitable file
> > to place them in we have two options here, one is to use CFLAGS and include
> > test.h directly (dirty) or have rte_assert/test in eal/common/inlcude.
> >
> > Thoughts?
> >
> If other device types, e.g. ethdev or cryptodev, also take the approach of
> having a self_test API (something I think would be a good thing, and I
> actually hacked together when working on the i40e rx and tx code), I
> think we should look to have an rte_test.h header file for such macros
> to avoid duplication.
> At this point, moving them to an EAL include may not be worth it for
> just eventdev.
>
I think atleast having a basic version of rte_test.h (macros used by
test_event_octeontx) would be good rather than taking a hacky path.
Later on more stuff could be added when other devices make use of it.
> /Bruce
>
Thanks,
Pavan.
This patchset aims to remove pmd specific unit test clutter from the common
tests directory by moving them into the respective pmd folder.
- Patch [1/11] moves basic asserts into eal area so that they can be reused
by other devices.
- Patch [2/11] introduces new API that an application can use to run the self
test.
- Patch set [3-6/11] cleans up event_octeontx specific test, updates the
selftest ops and provides a devarg 'selftest' to run selftest from any given
application after probe is complete.
- Patch set [7-9/11] cleans up event_sw specific test and updates the selftest
ops.
- Patch [10/11] registers selftest command to the common unit test area.
Pavan Nikhilesh (11):
eal: add common test assert macros
eventdev: add API to perform self test
event/octeontx: move eventdev octeontx test to driver
event/octeontx: modify octeontx eventdev test
event/octeontx: update octeontx eventdev selftest ops
event/octeontx: add selftest to device arguments
event/sw: move eventdev sw test to driver
event/sw: modify eventdev sw test
event/sw: update software eventdev selftest ops
test: register eventdev selftest
doc: update eventdev documentation
doc/guides/eventdevs/octeontx.rst | 13 +
drivers/event/octeontx/Makefile | 3 +-
.../event/octeontx/octeontx_evdev_selftest.c | 427 +++++++++++----------
drivers/event/octeontx/ssovf_evdev.c | 45 +++
drivers/event/octeontx/ssovf_evdev.h | 6 +
drivers/event/sw/Makefile | 2 +
drivers/event/sw/sw_evdev.c | 2 +
drivers/event/sw/sw_evdev.h | 1 +
.../event/sw/sw_evdev_selftest.c | 70 ++--
lib/librte_eal/common/Makefile | 2 +-
lib/librte_eal/common/include/rte_test.h | 97 +++++
lib/librte_eventdev/rte_eventdev.c | 10 +
lib/librte_eventdev/rte_eventdev.h | 12 +
lib/librte_eventdev/rte_eventdev_pmd.h | 11 +
lib/librte_eventdev/rte_eventdev_version.map | 6 +
test/test/Makefile | 2 -
test/test/test_eventdev.c | 7 +
17 files changed, 485 insertions(+), 231 deletions(-)
rename test/test/test_eventdev_octeontx.c => drivers/event/octeontx/octeontx_evdev_selftest.c (78%)
rename test/test/test_eventdev_sw.c => drivers/event/sw/sw_evdev_selftest.c (99%)
create mode 100644 lib/librte_eal/common/include/rte_test.h
--
2.14.1
similarity index 100%
rename from test/test/test_eventdev_octeontx.c
rename to drivers/event/octeontx/selftest_octeontx.c
@@ -211,7 +211,6 @@ SRCS-y += test_eventdev.c
SRCS-y += test_event_ring.c
SRCS-y += test_event_eth_rx_adapter.c
SRCS-$(CONFIG_RTE_LIBRTE_PMD_SW_EVENTDEV) += test_eventdev_sw.c
-SRCS-$(CONFIG_RTE_LIBRTE_PMD_OCTEONTX_SSOVF) += test_eventdev_octeontx.c
endif
SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) += test_kvargs.c