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

-----Original Message-----

Date: Sun, 2 Jul 2017 22:35:11 +0100
From: Harry van Haaren <redacted>
To: dev@dpdk.org
CC: jerin.jacob@caviumnetworks.com, thomas@monjalon.net,
 keith.wiles@intel.com, bruce.richardson@intel.com, Harry van Haaren
 [off-list ref]
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 <redacted>

---

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 <redacted>

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