[dpdk-dev] [PATCH 5/6] service core: add unit tests

[Van Haaren, Harry]

> From: Jerin Jacob [mailto:jerin.jacob at caviumnetworks.com]
> Sent: Monday, June 26, 2017 2:06 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: dev at dpdk.org; thomas at monjalon.net; Wiles, Keith <keith.wiles at intel.com>; Richardson,
> Bruce <bruce.richardson at intel.com>
> Subject: Re: [PATCH 5/6] service core: add unit tests
> 
> -----Original Message-----
> > Date: Fri, 23 Jun 2017 10:06:18 +0100
> > From: Harry van Haaren <harry.van.haaren at intel.com>
<snip>
> > Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
> > ---
> > +#define SERVICE_DELAY 1
> > +
> > +static int
> > +testsuite_setup(void)
> > +{
> > +	/* assuming lcore 1 is available for service-core testing */
> 
> We can check the number of available lcores is >= 2.

The service cores are now added at runtime (to allow checking if add/remove works as expected), so this is obsolete.


> > +	score_id = 1;
> 
> How about slcore_id?

Done.

> > +/* unregister all services */
> > +static int
> > +dummy_unregister(void)
> 
> How about unregister_all(void) as it is un registering all services.

Done.


> > +	/* expected failure cases */
> > +	TEST_ASSERT_EQUAL(-EINVAL, rte_service_enable_on_core(s, 100000),
> > +			"Enable on invalid core did not fail");
> > +	TEST_ASSERT_EQUAL(-EINVAL, rte_service_disable_on_core(s, 100000),
> > +			"Dispable on invalid core did not fail");
> 
> s/Dispable/Disable

Fixed


> IMO, Following functions/functionality coverage is missing. How about add that
> in unit test case.
> 1) rte_service_get_name()

Done

> 2) Verify RTE_SERVICE_CAP_MT_SAFE with available lcores

Done, using an atomic in a dummy multi-thread safe service callback to detect that two threads are allowed into the service callback concurrently.

> 3) rte_service_probe_capability()

Done

> 4) rte_service_dump()

Done


> 5) Moving service lcore to/from rte lcore back and forth.

Already covered in service_lcore_add_del(). The rte_service_lcore_add() assigns ROLE_SERVICE to the lcore in lcore_config, so EAL will be aware of it now being a service core. Similarly, _del() will return the lcore to ROLE_RTE.