From e30dd31847d212cd1b766612cbd980c7d8240baa Mon Sep 17 00:00:00 2001 From: Gage Eads Date: Thu, 21 Jun 2018 09:23:22 -0500 Subject: [PATCH] service: add mechanism for quiescing Existing service functions allow us to stop a service, but doing so doesn't guarantee that the service has finished running on a service core. This commit introduces rte_service_may_be_active(), which returns whether the service may be executing on one or more lcores currently, or definitely is not. The service core layer supports this function by setting a flag when a service core is going to execute a service, and unsetting the flag when the core is no longer able to run the service (its runstate becomes stopped or the lcore is no longer mapped). With this new function, applications can set a service's runstate to stopped, then poll rte_service_may_be_active() until it returns false. At that point, the service is quiesced. Signed-off-by: Gage Eads Acked-by: Harry van Haaren --- lib/librte_eal/common/include/rte_service.h | 20 ++++++++++ lib/librte_eal/common/rte_service.c | 32 +++++++++++++-- lib/librte_eal/rte_eal_version.map | 1 + test/test/test_service_cores.c | 43 +++++++++++++++++++++ 4 files changed, 92 insertions(+), 4 deletions(-) diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h index ba2c3cd3a2..34b41aff2a 100644 --- a/lib/librte_eal/common/include/rte_service.h +++ b/lib/librte_eal/common/include/rte_service.h @@ -161,6 +161,26 @@ int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate); */ int32_t rte_service_runstate_get(uint32_t id); +/** + * @warning + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice + * + * This function returns whether the service may be currently executing on + * at least one lcore, or definitely is not. This function can be used to + * determine if, after setting the service runstate to stopped, the service + * is still executing a service lcore. + * + * Care must be taken if calling this function when the service runstate is + * running, since the result of this function may be incorrect by the time the + * function returns due to service cores running in parallel. + * + * @retval 1 Service may be running on one or more lcores + * @retval 0 Service is not running on any lcore + * @retval -EINVAL Invalid service id + */ +int32_t __rte_experimental +rte_service_may_be_active(uint32_t id); + /** * Enable or disable the check for a service-core being mapped to the service. * An application can disable the check when takes the responsibility to run a diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index fa15f66440..8767c72250 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -52,6 +52,7 @@ struct rte_service_spec_impl { rte_atomic32_t num_mapped_cores; uint64_t calls; uint64_t cycles_spent; + uint8_t active_on_lcore[RTE_MAX_LCORE]; } __rte_cache_aligned; /* the internal values of a service core */ @@ -347,15 +348,19 @@ rte_service_runner_do_callback(struct rte_service_spec_impl *s, static inline int32_t -service_run(uint32_t i, struct core_state *cs, uint64_t service_mask) +service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask) { if (!service_valid(i)) return -EINVAL; struct rte_service_spec_impl *s = &rte_services[i]; if (s->comp_runstate != RUNSTATE_RUNNING || s->app_runstate != RUNSTATE_RUNNING || - !(service_mask & (UINT64_C(1) << i))) + !(service_mask & (UINT64_C(1) << i))) { + s->active_on_lcore[lcore] = 0; return -ENOEXEC; + } + + s->active_on_lcore[lcore] = 1; /* check do we need cmpset, if MT safe or <= 1 core * mapped, atomic ops are not required. @@ -374,6 +379,25 @@ service_run(uint32_t i, struct core_state *cs, uint64_t service_mask) return 0; } +int32_t __rte_experimental +rte_service_may_be_active(uint32_t id) +{ + uint32_t ids[RTE_MAX_LCORE] = {0}; + struct rte_service_spec_impl *s = &rte_services[id]; + int32_t lcore_count = rte_service_lcore_list(ids, RTE_MAX_LCORE); + int i; + + if (!service_valid(id)) + return -EINVAL; + + for (i = 0; i < lcore_count; i++) { + if (s->active_on_lcore[ids[i]]) + return 1; + } + + return 0; +} + int32_t rte_service_run_iter_on_app_lcore(uint32_t id, uint32_t serialize_mt_unsafe) { @@ -398,7 +422,7 @@ int32_t rte_service_run_iter_on_app_lcore(uint32_t id, return -EBUSY; } - int ret = service_run(id, cs, UINT64_MAX); + int ret = service_run(id, rte_lcore_id(), cs, UINT64_MAX); if (serialize_mt_unsafe) rte_atomic32_dec(&s->num_mapped_cores); @@ -419,7 +443,7 @@ rte_service_runner_func(void *arg) for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { /* return value ignored as no change to code flow */ - service_run(i, cs, service_mask); + service_run(i, lcore, cs, service_mask); } cs->loops++; diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map index 8b868cfc7c..5a02ac7409 100644 --- a/lib/librte_eal/rte_eal_version.map +++ b/lib/librte_eal/rte_eal_version.map @@ -296,6 +296,7 @@ EXPERIMENTAL { rte_mp_sendmsg; rte_service_lcore_attr_get; rte_service_lcore_attr_reset_all; + rte_service_may_be_active; rte_socket_count; rte_socket_id_by_idx; rte_vfio_dma_map; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index 9f21d594b2..ec31882d99 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -843,6 +843,48 @@ service_lcore_start_stop(void) return unregister_all(); } +/* stop a service and wait for it to become inactive */ +static int +service_may_be_active(void) +{ + const uint32_t sid = 0; + int i; + + /* expected failure cases */ + TEST_ASSERT_EQUAL(-EINVAL, rte_service_may_be_active(10000), + "Invalid service may be active check did not fail"); + + /* start the service */ + TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 1), + "Starting valid service failed"); + TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id), + "Add service core failed when not in use before"); + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_id, 1), + "Enabling valid service on valid core failed"); + TEST_ASSERT_EQUAL(0, rte_service_lcore_start(slcore_id), + "Service core start after add failed"); + + /* ensures core really is running the service function */ + TEST_ASSERT_EQUAL(1, service_lcore_running_check(), + "Service core expected to poll service but it didn't"); + + /* stop the service */ + TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0), + "Error: Service stop returned non-zero"); + + /* give the service 100ms to stop running */ + for (i = 0; i < 100; i++) { + if (!rte_service_may_be_active(sid)) + break; + rte_delay_ms(SERVICE_DELAY); + } + + TEST_ASSERT_EQUAL(0, rte_service_may_be_active(sid), + "Error: Service not stopped after 100ms"); + + return unregister_all(); +} + static struct unit_test_suite service_tests = { .suite_name = "service core test suite", .setup = testsuite_setup, @@ -863,6 +905,7 @@ static struct unit_test_suite service_tests = { TEST_CASE_ST(dummy_register, NULL, service_mt_safe_poll), TEST_CASE_ST(dummy_register, NULL, service_app_lcore_mt_safe), TEST_CASE_ST(dummy_register, NULL, service_app_lcore_mt_unsafe), + TEST_CASE_ST(dummy_register, NULL, service_may_be_active), TEST_CASES_END() /**< NULL terminate unit test array */ } }; -- 2.20.1