Re: [dpdk-dev] [PATCH v3 4/7] service cores: add unit tests

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

-----Original Message-----
> Date: Sun, 2 Jul 2017 22:35:11 +0100
> From: Harry van Haaren <harry.van.haaren@intel.com>
> To: dev@dpdk.org
> CC: jerin.jacob@caviumnetworks.com, thomas@monjalon.net,
>  keith.wiles@intel.com, bruce.richardson@intel.com, Harry van Haaren
>  <harry.van.haaren@intel.com>
> Subject: [PATCH v3 4/7] service cores: add unit tests
> X-Mailer: git-send-email 2.7.4
> 
> Add a bunch of unit tests, to ensure that the service
> core functions are operating as expected.
> 
> As part of these tests a dummy service is registered which
> allows identifying if a service callback has been invoked
> by using the CPU tick counter. This allows identifying if
> functions to start and stop service lcores are actually having
> effect.
> 
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> 
> ---
> 
> v2 changes;
> - Rename variable to slcore_id (Jerin)
> - Rename function to unregister_all() (Jerin)
> - Fix typos (Jerin)
> - Add unit test for get_by_name()
> - Add unit tests (all suggestions by Jerin)
> -- get_name()
> -- Verify probe_capability API
> -- Verify MT_SAFE capability (see code for details)
> -- Verify rte_service_dump() API

I think now UT is addressing all the APIS.
A few comments below.

> ---
> +testsuite_setup(void)
> +{
> +	/* assuming lcore 1 is available for service-core testing */
> +	slcore_id = 1;

Rather than assuming, How about taking enabled lcore for testing using 
rte_get_next_lcore()?

dummy_mt_safe_cb() tests using the second lcore now. So I think, you can
the get the enabled lcore for MT_SAFE using rte_get_next_lcore() and exit
if two lcores not available.

> +	return TEST_SUCCESS;
> +}
> +
> +
> +static struct unit_test_suite service_tests  = {
> +	.suite_name = "service core test suite",
> +	.setup = testsuite_setup,
> +	.teardown = testsuite_teardown,
> +	.unit_test_cases = {
> +		TEST_CASE_ST(dummy_register, NULL, unregister_all),
> +		TEST_CASE_ST(dummy_register, NULL, service_name),
> +		TEST_CASE_ST(dummy_register, NULL, service_get_by_name),
> +		TEST_CASE_ST(dummy_register, NULL, service_dump),
> +		TEST_CASE_ST(dummy_register, NULL, service_probe_capability),
> +		TEST_CASE_ST(dummy_register, NULL, service_start_stop),
> +		TEST_CASE_ST(dummy_register, NULL, service_lcore_add_del),
> +		TEST_CASE_ST(dummy_register, NULL, service_lcore_start_stop),
> +		TEST_CASE_ST(dummy_register, NULL, service_lcore_en_dis_able),
> +		TEST_CASE_ST(dummy_register, NULL, service_mt_safe_poll),
> +		TEST_CASES_END() /**< NULL terminate unit test array */

Regarding "Moving service lcore to/from rte lcore back and forth" test
reply in
http://dpdk.org/dev/patchwork/patch/25655/

I was thinking to enable the test by, calling service_lcore_del() and
then call remote launch on that lcore to check it is properly in
ROLE_RTE state. How about adding a test for the same to make sure the
"Moving service lcore to/from rte lcore back and forth" is OK.

With above comments:

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> +	}
> +};
> +
> +static int
> +test_service_common(void)
> +{
> +	return unit_test_suite_runner(&service_tests);
> +}
> +
> +REGISTER_TEST_COMMAND(service_autotest, test_service_common);
> -- 
> 2.7.4
>