diff mbox

[dpdk-dev,v2,10/11] test: register eventdev selftest

Message ID 20171214150138.25667-11-pbhagavatula@caviumnetworks.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Pavan Nikhilesh Dec. 14, 2017, 3:01 p.m. UTC 
  Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
---
 test/test/test_eventdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Van Haaren, Harry Dec. 19, 2017, 3:27 p.m. UTC | #1 
> From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Thursday, December 14, 2017 3:02 PM
> To: jerin.jacob@caviumnetworks.com; santosh.shukla@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>

[Side note: we shouldn't put everybody on CC all the time..]

> Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> Subject: [dpdk-dev] [PATCH v2 10/11] test: register eventdev selftest
> 
> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>


> diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c
> index 1ed2a1ddd..b86efab01 100644
> --- a/test/test/test_eventdev.c
> +++ b/test/test/test_eventdev.c
> @@ -1009,4 +1009,11 @@ test_eventdev_common(void)
>  	return unit_test_suite_runner(&eventdev_common_testsuite);
>  }
> 
> +static int
> +test_eventdev_selftest(void)
> +{
> +	return rte_event_dev_selftest(TEST_DEV_ID);
> +}
> +
>  REGISTER_TEST_COMMAND(eventdev_common_autotest, test_eventdev_common);
> +REGISTER_TEST_COMMAND(eventdev_selftest, test_eventdev_selftest);


Currently when running the test app, we don't pass any arguments. Running the "eventdev_sw_autotest" command, it will create the required event_sw0 PMD vdev, and launch the tests then.

Given the selftest is PMD agnostic, does it makes sense to have a single string "sw" or "octeontx" to run the tests against? Right now it requires that we pass ./app/test --vdev event_sw0  which I think is a burden, particularly when automating this with the meson test infrastructure down the line.

Summary; Please add a string parameter that indicates the PMD to run the self-test on.
Pavan Nikhilesh Dec. 19, 2017, 6:44 p.m. UTC | #2 
Hi Harry,

Thanks for the review, comments below.

On Tue, Dec 19, 2017 at 03:27:25PM +0000, Van Haaren, Harry wrote:
> > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > Sent: Thursday, December 14, 2017 3:02 PM
> > To: jerin.jacob@caviumnetworks.com; santosh.shukla@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>
>
> [Side note: we shouldn't put everybody on CC all the time..]
>
> > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH v2 10/11] test: register eventdev selftest
> >
> > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
>
>
> > diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c
> > index 1ed2a1ddd..b86efab01 100644
> > --- a/test/test/test_eventdev.c
> > +++ b/test/test/test_eventdev.c
> > @@ -1009,4 +1009,11 @@ test_eventdev_common(void)
> >  	return unit_test_suite_runner(&eventdev_common_testsuite);
> >  }
> >
> > +static int
> > +test_eventdev_selftest(void)
> > +{
> > +	return rte_event_dev_selftest(TEST_DEV_ID);
> > +}
> > +
> >  REGISTER_TEST_COMMAND(eventdev_common_autotest, test_eventdev_common);
> > +REGISTER_TEST_COMMAND(eventdev_selftest, test_eventdev_selftest);
>
>
> Currently when running the test app, we don't pass any arguments. Running the "eventdev_sw_autotest" command, it will create the required event_sw0 PMD vdev, and launch the tests then.
>
> Given the selftest is PMD agnostic, does it makes sense to have a single string "sw" or "octeontx" to run the tests against? Right now it requires that we pass ./app/test --vdev event_sw0  which I think is a burden, particularly when automating this with the meson test infrastructure down the line.
>
> Summary; Please add a string parameter that indicates the PMD to run the self-test on.
>

We can't pass extra parameter while running test (it will only accept test name).
So, I will register pmd specific test test_eventdev_sw/octeontx which will
create the eventdev and call the selftest API.

Also, I have mistakenly changed the name from event_sw0 to event_sw do you want
me to revert it? or retain the change.

Cheers,
Pavan.
Van Haaren, Harry Dec. 20, 2017, 11:07 a.m. UTC | #3 
> From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> Sent: Tuesday, December 19, 2017 6:45 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>;
> jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 10/11] test: register eventdev selftest
> 
> Hi Harry,
> 
> Thanks for the review, comments below.
> 
> On Tue, Dec 19, 2017 at 03:27:25PM +0000, Van Haaren, Harry wrote:
> > > From: Pavan Nikhilesh [mailto:pbhagavatula@caviumnetworks.com]
> > > Sent: Thursday, December 14, 2017 3:02 PM
> > > To: jerin.jacob@caviumnetworks.com; santosh.shukla@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>
> >
> > [Side note: we shouldn't put everybody on CC all the time..]
> >
> > > Cc: dev@dpdk.org; Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > Subject: [dpdk-dev] [PATCH v2 10/11] test: register eventdev selftest
> > >
> > > Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> >
> >
> > > diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c
> > > index 1ed2a1ddd..b86efab01 100644
> > > --- a/test/test/test_eventdev.c
> > > +++ b/test/test/test_eventdev.c
> > > @@ -1009,4 +1009,11 @@ test_eventdev_common(void)
> > >  	return unit_test_suite_runner(&eventdev_common_testsuite);
> > >  }
> > >
> > > +static int
> > > +test_eventdev_selftest(void)
> > > +{
> > > +	return rte_event_dev_selftest(TEST_DEV_ID);
> > > +}
> > > +
> > >  REGISTER_TEST_COMMAND(eventdev_common_autotest, test_eventdev_common);
> > > +REGISTER_TEST_COMMAND(eventdev_selftest, test_eventdev_selftest);
> >
> >
> > Currently when running the test app, we don't pass any arguments. Running
> the "eventdev_sw_autotest" command, it will create the required event_sw0
> PMD vdev, and launch the tests then.
> >
> > Given the selftest is PMD agnostic, does it makes sense to have a single
> string "sw" or "octeontx" to run the tests against? Right now it requires
> that we pass ./app/test --vdev event_sw0  which I think is a burden,
> particularly when automating this with the meson test infrastructure down
> the line.
> >
> > Summary; Please add a string parameter that indicates the PMD to run the
> self-test on.
> >
> 
> We can't pass extra parameter while running test (it will only accept test
> name).
> So, I will register pmd specific test test_eventdev_sw/octeontx which will
> create the eventdev and call the selftest API.

Sure that works for me. Its probably best to break out the create/run into
a helper function, just to save code-duplication:

int eventdev_selftest_impl(const char *pmd, const char *opts)
{
    int id = rte_vdev_create(pmd, opts);
    return rte_eventdev_selftest(id);
}

int eventdev_selftest_sw()
{
    eventdev_selftest_impl("event_sw", "");
}

int eventdev_selftest_octeontx()
{
    eventdev_selftest_impl("event_octeontx", "");
}


> Also, I have mistakenly changed the name from event_sw0 to event_sw do you
> want
> me to revert it? or retain the change.

I think it actually makes more sense to use "event_sw" over "event_sw0".
The PMD name is constant, while the 0 is an arbitrary instance name.
So I think we should keep the change.
diff mbox

Patch

diff --git a/test/test/test_eventdev.c b/test/test/test_eventdev.c
index 1ed2a1ddd..b86efab01 100644
--- a/test/test/test_eventdev.c
+++ b/test/test/test_eventdev.c
@@ -1009,4 +1009,11 @@  test_eventdev_common(void)
 	return unit_test_suite_runner(&eventdev_common_testsuite);
 }
 
+static int
+test_eventdev_selftest(void)
+{
+	return rte_event_dev_selftest(TEST_DEV_ID);
+}
+
 REGISTER_TEST_COMMAND(eventdev_common_autotest, test_eventdev_common);
+REGISTER_TEST_COMMAND(eventdev_selftest, test_eventdev_selftest);