[dpdk-dev] [PATCH 4/8] service: rework service start stop to runstate

[Harry van Haaren <harry.van.haaren@intel.com>]

This commit reworks the API to move from two seperate start
and stop functions, to a "runstate" API which allows setting
the runstate. The is_running API is replaced with an function
to query the runstate. The runstate functions take a id value
for service. Unit tests and the eventdev sw pmd are updated.

Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 drivers/event/sw/sw_evdev.c                     |  3 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  5 ++--
 lib/librte_eal/common/include/rte_service.h     | 37 +++++++++---------------
 lib/librte_eal/common/rte_service.c             | 38 ++++++++++---------------
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  5 ++--
 test/test/test_service_cores.c                  | 19 ++++++-------
 6 files changed, 42 insertions(+), 65 deletions(-)

diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c
index 819d3ef..fd2fb72 100644
--- a/drivers/event/sw/sw_evdev.c
+++ b/drivers/event/sw/sw_evdev.c
@@ -617,8 +617,7 @@ sw_start(struct rte_eventdev *dev)
 	struct sw_evdev *sw = sw_pmd_priv(dev);
 
 	/* check a service core is mapped to this service */
-	struct rte_service_spec *s = rte_service_get_by_name(sw->service_name);
-	if (!rte_service_is_running(s))
+	if (!rte_service_runstate_get(sw->service_id))
 		SW_LOG_ERR("Warning: No Service core enabled on service %s\n",
 				sw->service_name);
 
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f626a6f..f76d1ea 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -216,7 +216,6 @@ EXPERIMENTAL {
 	rte_service_get_by_id;
 	rte_service_get_by_name;
 	rte_service_get_count;
-	rte_service_is_running;
 	rte_service_lcore_add;
 	rte_service_lcore_count;
 	rte_service_lcore_del;
@@ -229,10 +228,10 @@ EXPERIMENTAL {
 	rte_service_probe_capability;
 	rte_service_register;
 	rte_service_reset;
+	rte_service_runstate_get;
+	rte_service_runstate_set;
 	rte_service_set_stats_enable;
-	rte_service_start;
 	rte_service_start_with_defaults;
-	rte_service_stop;
 	rte_service_unregister;
 
 } DPDK_17.08;
diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index de69695..e133ca1 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -192,40 +192,31 @@ int32_t rte_service_map_lcore_get(uint32_t service_id, uint32_t lcore);
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
- * Enable *service* to run.
+ * Set the runstate of the service.
  *
- * This function switches on a service during runtime.
- * @retval 0 The service was successfully started
- */
-int32_t rte_service_start(struct rte_service_spec *service);
-
-/**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
+ * Each service is either running or stopped. Setting a non-zero runstate
+ * enables the service to run, while setting runstate zero disables it.
  *
- * Disable *service*.
+ * @param id The id of the service
+ * @param runstate The run state to apply to the service
  *
- * Switch off a service, so it is not run until it is *rte_service_start* is
- * called on it.
- * @retval 0 Service successfully switched off
+ * @retval 0 The service was successfully started
+ * @retval -EINVAL Invalid service id
  */
-int32_t rte_service_stop(struct rte_service_spec *service);
+int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate);
 
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
  *
- * Returns if *service* is currently running.
- *
- * This function returns true if the service has been started using
- * *rte_service_start*, AND a service core is mapped to the service. This
- * function can be used to ensure that the service will be run.
+ * Get the runstate for the service with *id*. See *rte_service_runstate_set*
+ * for details of runstates.
  *
- * @retval 1 Service is currently running, and has a service lcore mapped
- * @retval 0 Service is currently stopped, or no service lcore is mapped
- * @retval -EINVAL Invalid service pointer provided
+ * @retval 1 Service is running
+ * @retval 0 Service is stopped
+ * @retval -EINVAL Invalid service id
  */
-int32_t rte_service_is_running(const struct rte_service_spec *service);
+int32_t rte_service_runstate_get(uint32_t id);
 
 /**
  * @warning
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 0ee0c13..7e9bf4d 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -230,18 +230,6 @@ rte_service_probe_capability(uint32_t id, uint32_t capability)
 }
 
 int32_t
-rte_service_is_running(const struct rte_service_spec *spec)
-{
-	const struct rte_service_spec_impl *impl =
-		(const struct rte_service_spec_impl *)spec;
-	if (!impl)
-		return -EINVAL;
-
-	return (impl->runstate == RUNSTATE_RUNNING) &&
-		(impl->num_mapped_cores > 0);
-}
-
-int32_t
 rte_service_register(const struct rte_service_spec *spec, uint32_t *id_ptr)
 {
 	uint32_t i;
@@ -308,23 +296,27 @@ rte_service_unregister(struct rte_service_spec *spec)
 }
 
 int32_t
-rte_service_start(struct rte_service_spec *service)
+rte_service_runstate_set(uint32_t id, uint32_t runstate)
 {
-	struct rte_service_spec_impl *s =
-		(struct rte_service_spec_impl *)service;
-	s->runstate = RUNSTATE_RUNNING;
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	if (runstate)
+		s->runstate = RUNSTATE_RUNNING;
+	else
+		s->runstate = RUNSTATE_STOPPED;
+
 	rte_smp_wmb();
 	return 0;
 }
 
 int32_t
-rte_service_stop(struct rte_service_spec *service)
+rte_service_runstate_get(uint32_t id)
 {
-	struct rte_service_spec_impl *s =
-		(struct rte_service_spec_impl *)service;
-	s->runstate = RUNSTATE_STOPPED;
-	rte_smp_wmb();
-	return 0;
+	struct rte_service_spec_impl *s;
+	SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
+
+	return (s->runstate == RUNSTATE_RUNNING) && (s->num_mapped_cores > 0);
 }
 
 static int32_t
@@ -447,7 +439,7 @@ rte_service_start_with_defaults(void)
 		if (lcore_iter >= lcore_count)
 			lcore_iter = 0;
 
-		ret = rte_service_start(s);
+		ret = rte_service_runstate_set(i, 1);
 		if (ret)
 			return -ENOEXEC;
 	}
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 452ae1c..557b5e7 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -221,7 +221,6 @@ EXPERIMENTAL {
 	rte_service_get_by_id;
 	rte_service_get_by_name;
 	rte_service_get_count;
-	rte_service_is_running;
 	rte_service_lcore_add;
 	rte_service_lcore_count;
 	rte_service_lcore_del;
@@ -234,10 +233,10 @@ EXPERIMENTAL {
 	rte_service_probe_capability;
 	rte_service_register;
 	rte_service_reset;
+	rte_service_runstate_get;
+	rte_service_runstate_set;
 	rte_service_set_stats_enable;
-	rte_service_start;
 	rte_service_start_with_defaults;
-	rte_service_stop;
 	rte_service_unregister;
 
 } DPDK_17.08;
diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c
index 72d774d..ba252c5 100644
--- a/test/test/test_service_cores.c
+++ b/test/test/test_service_cores.c
@@ -274,7 +274,6 @@ static int
 service_start_stop(void)
 {
 	const uint32_t sid = 0;
-	struct rte_service_spec *s = rte_service_get_by_id(0);
 
 	/* runstate_get() returns if service is running and slcore is mapped */
 	TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id),
@@ -283,19 +282,19 @@ service_start_stop(void)
 	TEST_ASSERT_EQUAL(0, ret,
 			"Enabling service core, expected 0 got %d", ret);
 
-	TEST_ASSERT_EQUAL(0, rte_service_is_running(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_get(sid),
 			"Error: Service should be stopped");
 
-	TEST_ASSERT_EQUAL(0, rte_service_stop(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0),
 			"Error: Service stopped returned non-zero");
 
-	TEST_ASSERT_EQUAL(0, rte_service_is_running(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_get(sid),
 			"Error: Service is running - should be stopped");
 
-	TEST_ASSERT_EQUAL(0, rte_service_start(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 1),
 			"Error: Service start returned non-zero");
 
-	TEST_ASSERT_EQUAL(1, rte_service_is_running(s),
+	TEST_ASSERT_EQUAL(1, rte_service_runstate_get(sid),
 			"Error: Service is not running");
 
 	return unregister_all();
@@ -474,9 +473,8 @@ service_threaded_test(int mt_safe)
 	TEST_ASSERT_EQUAL(0, rte_service_register(&service, NULL),
 			"Register of MT SAFE service failed");
 
-	struct rte_service_spec *s = rte_service_get_by_id(0);
 	const uint32_t sid = 0;
-	TEST_ASSERT_EQUAL(0, rte_service_start(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 1),
 			"Starting valid service failed");
 	TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_1, 1),
 			"Failed to enable lcore 1 on mt safe service");
@@ -493,7 +491,7 @@ service_threaded_test(int mt_safe)
 	TEST_ASSERT_EQUAL(1, test_params[1],
 			"MT Safe service not run by two cores concurrently");
 
-	TEST_ASSERT_EQUAL(0, rte_service_stop(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0),
 			"Failed to stop MT Safe service");
 
 	unregister_all();
@@ -532,8 +530,7 @@ service_lcore_start_stop(void)
 {
 	/* start service core and service, create mapping so tick() runs */
 	const uint32_t sid = 0;
-	struct rte_service_spec *s = rte_service_get_by_id(0);
-	TEST_ASSERT_EQUAL(0, rte_service_start(s),
+	TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 1),
 			"Starting valid service failed");
 	TEST_ASSERT_EQUAL(-EINVAL, rte_service_map_lcore_set(sid, slcore_id, 1),
 			"Enabling valid service on non-service core must fail");
-- 
2.7.4