Re: [PATCH 5/6] service core: add unit tests

From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
Sent: Monday, June 26, 2017 2:06 PM
To: Van Haaren, Harry <redacted>
Cc: dev@dpdk.org; thomas@monjalon.net; Wiles, Keith <redacted>; Richardson,
Bruce [off-list ref]
Subject: Re: [PATCH 5/6] service core: add unit tests

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

quoted

Date: Fri, 23 Jun 2017 10:06:18 +0100
From: Harry van Haaren <redacted>

<snip>

quoted

Signed-off-by: Harry van Haaren <redacted>
---
+#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.

quoted

+	score_id = 1;

How about slcore_id?

Done.

quoted

+/* unregister all services */
+static int
+dummy_unregister(void)

How about unregister_all(void) as it is un registering all services.

Done.

quoted

+	/* 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.