Re: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

On Tue, Feb 07, 2017 at 03:17:30PM +0000, Van Haaren, Harry wrote:
> > -----Original Message-----
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > Sent: Monday, February 6, 2017 5:24 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; hemant.agrawal@nxp.com; Eads, Gage
> > <gage.eads@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>; Jerin Jacob
> > <jerin.jacob@caviumnetworks.com>
> > Subject: [dpdk-dev] [PATCH 4/4] app/test: unit test case to exercise eventdev vdev uninit
> > 
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> 
> Comments inline,
> 
> >  app/test/test_eventdev.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/app/test/test_eventdev.c b/app/test/test_eventdev.c
> > index 042a446..e817838 100644
> > --- a/app/test/test_eventdev.c
> > +++ b/app/test/test_eventdev.c
> > @@ -51,7 +51,7 @@ testsuite_setup(void)
> >  	if (!count) {
> >  		printf("Failed to find a valid event device,"
> >  			" testing with event_skeleton device\n");
> > -		return rte_eal_vdev_init("event_skeleton", NULL);
> > +		return rte_eal_vdev_init("event_skeleton0", NULL);
> 
> I think these hard-coded eventdev names need to be removed from the testing framework, and we should let the app/test >> propt accept the name of the vdev to use instead. That would allow running tests on each device by:
> 
> RTE>> eventdev_common_autotest event_skeleton0
> 
> RTE>> eventdev_common_autotest event_sw0

It checks for "count = rte_event_dev_count()" before the calling rte_eal_vdev_init.
That's would translate to use any eventdev
driver if it is available in the EAL command line argument.So I think, we
are good here and in case none of the drivers provided in EAL argument
it choose the skeleton driver as default.

sudo ./build/app/test -c 2 --vdev='event_sw0'
sudo ./build/app/test -c 2 --vdev='event_skeleton0'

> 
> 
> >  	}
> >  	return TEST_SUCCESS;
> >  }
> > @@ -720,6 +720,62 @@ test_eventdev_close(void)
> >  	return rte_event_dev_close(TEST_DEV_ID);
> >  }
> > 
> > +#define TEST_EVENTDEV_COUNT 8
> > +
> > +static int
> > +test_eventdev_create_destroy(void)
> > +{
> > +#ifdef RTE_LIBRTE_PMD_SKELETON_EVENTDEV
> 
> Same issue as above, compile-time selection of the tests to be run isn't flexible enough to allow testing multiple eventdevs.
> I think using a parameter to eventdev_common_autotest to pass the "name" to use would work again? I am open to other solutions if there's a better way :)

In general I agree with your comment. But in this specific case,

1) Device can be PCI device too. But,This test case will be applicable only to vdev
device. So does it make sense to generalize.
2) Currently app/test won't accepts the command line arguments for a specific
test case.Is there any other alternatives? If none, I will check for possibility
of adding that change to app/test first.

Another options could be,
1) environment variable
2) new eventdev APIs to get driver name from dev_id.But does not makes much
sense in case PCIe device.

Thoughts?

> 
> 
> > +	int i, ret;
> > +	uint8_t curr_count;
> > +	char name[RTE_EVENTDEV_NAME_MAX_LEN];
> > +
> > +	curr_count = rte_event_dev_count();
> > +
> > +	/* Start from one to avoid overlap with active event_skeleton0 dev */
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_init(name, NULL);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> > +					name);
> > +	}
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_uninit(name);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> > +					name);
> > +	}
> > +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> > +			 "init/uninit pairs count mismatch");
> > +
> > +	/* Test in reverse order */
> > +	for (i = 1; i <= TEST_EVENTDEV_COUNT; i++) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_init(name, NULL);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to init event_skeleton dev %s",
> > +					name);
> > +	}
> > +	for (i = TEST_EVENTDEV_COUNT; i > 0; i--) {
> > +		ret = snprintf(name, RTE_EVENTDEV_NAME_MAX_LEN,
> > +				 "%s%u", "event_skeleton", i);
> > +		TEST_ASSERT(ret >= 0, "Expected >0, %d", ret);
> > +		ret = rte_eal_vdev_uninit(name);
> > +		TEST_ASSERT_SUCCESS(ret, "Failed to uninit skeleton dev %s",
> > +					name);
> > +	}
> > +	TEST_ASSERT(curr_count == rte_event_dev_count(),
> > +			 "init/uninit pairs count mismatch");
> > +#else
> > +	printf("Skipping eventdev_create_destroy test due to unavailability of event skeleton
> > device\n");
> > +#endif
> > +	return TEST_SUCCESS;
> > +}
> >  static struct unit_test_suite eventdev_common_testsuite  = {
> >  	.suite_name = "eventdev common code unit test suite",
> >  	.setup = testsuite_setup,
> > @@ -765,6 +821,8 @@ static struct unit_test_suite eventdev_common_testsuite  = {
> >  			test_eventdev_link_get),
> >  		TEST_CASE_ST(eventdev_setup_device, NULL,
> >  			test_eventdev_close),
> > +		TEST_CASE_ST(NULL, NULL,
> > +			test_eventdev_create_destroy),
> >  		TEST_CASES_END() /**< NULL terminate unit test array */
> >  	}
> >  };
> > --
> > 2.5.5
>