From patchwork Mon Aug 21 12:58:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27684 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 2DA297D17; Mon, 21 Aug 2017 14:58:30 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 6AF067CFC for ; Mon, 21 Aug 2017 14:58:27 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383259" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:25 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:02 +0100 Message-Id: <1503320296-51122-2-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 01/15] service: rework probe and get name to use ids X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit adds a macro to easily validate a service ID, and then lookup the service pointer, or return a user-specified error code. This marco will be heavily used in the following patches as it will be ID based instead of pointer-based. The probe_capability function is reworked to use an integer ID instead of a pointer. Rework the service_get_name() function is updated to use IDs. Unit tests are updated to keep things compiling after each commit. Signed-off-by: Harry van Haaren Acked-by: Pavan Nikhilesh --- lib/librte_eal/common/include/rte_service.h | 5 ++--- lib/librte_eal/common/rte_service.c | 20 +++++++++++++++----- test/test/test_service_cores.c | 7 +++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h index 7c6f738..bed1a61 100644 --- a/lib/librte_eal/common/include/rte_service.h +++ b/lib/librte_eal/common/include/rte_service.h @@ -133,7 +133,7 @@ struct rte_service_spec *rte_service_get_by_name(const char *name); * @return A pointer to the name of the service. The returned pointer remains * in ownership of the service, and the application must not free it. */ -const char *rte_service_get_name(const struct rte_service_spec *service); +const char *rte_service_get_name(uint32_t id); /** * @warning @@ -146,8 +146,7 @@ const char *rte_service_get_name(const struct rte_service_spec *service); * @retval 1 Capability supported by this service instance * @retval 0 Capability not supported by this service instance */ -int32_t rte_service_probe_capability(const struct rte_service_spec *service, - uint32_t capability); +int32_t rte_service_probe_capability(uint32_t id, uint32_t capability); /** * @warning diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 7efb76d..c969406 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -144,6 +144,13 @@ service_valid(uint32_t id) return !!(rte_services[id].internal_flags & SERVICE_F_REGISTERED); } +/* validate ID and retrieve service pointer, or return error value */ +#define SERVICE_VALID_GET_OR_ERR_RET(id, service, retval) do { \ + if (id >= RTE_SERVICE_NUM_MAX || !service_valid(id)) \ + return retval; \ + service = &rte_services[id]; \ +} while (0) + /* returns 1 if statistics should be colleced for service * Returns 0 if statistics should not be collected for service */ @@ -207,16 +214,19 @@ struct rte_service_spec *rte_service_get_by_name(const char *name) } const char * -rte_service_get_name(const struct rte_service_spec *service) +rte_service_get_name(uint32_t id) { - return service->name; + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, 0); + return s->spec.name; } int32_t -rte_service_probe_capability(const struct rte_service_spec *service, - uint32_t capability) +rte_service_probe_capability(uint32_t id, uint32_t capability) { - return service->capabilities & capability; + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + return s->spec.capabilities & capability; } int32_t diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index 88fac8f..940bc62 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -225,8 +225,8 @@ service_probe_capability(void) "Register of MT SAFE service failed"); /* verify flag is enabled */ - struct rte_service_spec *s = rte_service_get_by_id(0); - int32_t mt = rte_service_probe_capability(s, RTE_SERVICE_CAP_MT_SAFE); + const uint32_t sid = 0; + int32_t mt = rte_service_probe_capability(sid, RTE_SERVICE_CAP_MT_SAFE); TEST_ASSERT_EQUAL(1, mt, "MT SAFE capability flag not set."); @@ -239,8 +239,7 @@ service_probe_capability(void) "Register of non-MT safe service failed"); /* verify flag is enabled */ - s = rte_service_get_by_id(0); - mt = rte_service_probe_capability(s, RTE_SERVICE_CAP_MT_SAFE); + mt = rte_service_probe_capability(sid, RTE_SERVICE_CAP_MT_SAFE); TEST_ASSERT_EQUAL(0, mt, "MT SAFE cap flag set on non MT SAFE service"); return unregister_all(); From patchwork Mon Aug 21 12:58:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27685 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 636017D22; Mon, 21 Aug 2017 14:58:31 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id A7F2D7D0F for ; Mon, 21 Aug 2017 14:58:29 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383268" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:26 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:03 +0100 Message-Id: <1503320296-51122-3-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 02/15] service: rework lcore to service map functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit updates the APIs exposed to map service cores and services. The previous APIs required a pointer to a service, and used two separate functions for enable and disable. The new API uses an integer ID for the service and has a parameter for map or unmap. Unit tests are updated and passing, and the map file is updated to the new function names. Signed-off-by: Harry van Haaren --- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 5 ++- lib/librte_eal/common/include/rte_service.h | 43 +++++++++---------------- lib/librte_eal/common/rte_service.c | 31 ++++++++---------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 5 ++- test/test/test_service_cores.c | 27 +++++++++------- 5 files changed, 48 insertions(+), 63 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index aac6fd7..f626a6f 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -212,13 +212,10 @@ EXPERIMENTAL { rte_eal_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; - rte_service_disable_on_lcore; rte_service_dump; - rte_service_enable_on_lcore; rte_service_get_by_id; rte_service_get_by_name; rte_service_get_count; - rte_service_get_enabled_on_lcore; rte_service_is_running; rte_service_lcore_add; rte_service_lcore_count; @@ -227,6 +224,8 @@ EXPERIMENTAL { rte_service_lcore_reset_all; rte_service_lcore_start; rte_service_lcore_stop; + rte_service_map_lcore_get; + rte_service_map_lcore_set; rte_service_probe_capability; rte_service_register; rte_service_reset; diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h index bed1a61..de69695 100644 --- a/lib/librte_eal/common/include/rte_service.h +++ b/lib/librte_eal/common/include/rte_service.h @@ -152,10 +152,10 @@ int32_t rte_service_probe_capability(uint32_t id, uint32_t capability); * @warning * @b EXPERIMENTAL: this API may change without prior notice * - * Enable a core to run a service. + * Map or unmap a lcore to a service. * - * Each core can be added or removed from running specific services. This - * functions adds *lcore* to the set of cores that will run *service*. + * Each core can be added or removed from running a specific service. This + * function enables or disables *lcore* to run *service_id*. * * If multiple cores are enabled on a service, an atomic is used to ensure that * only one cores runs the service at a time. The exception to this is when @@ -163,43 +163,30 @@ int32_t rte_service_probe_capability(uint32_t id, uint32_t capability); * called RTE_SERVICE_CAP_MT_SAFE. With the multi-thread safe capability set, * the service function can be run on multiple threads at the same time. * - * @retval 0 lcore added successfully - * @retval -EINVAL An invalid service or lcore was provided. - */ -int32_t rte_service_enable_on_lcore(struct rte_service_spec *service, - uint32_t lcore); - -/** - * @warning - * @b EXPERIMENTAL: this API may change without prior notice - * - * Disable a core to run a service. + * @param service_id the service to apply the lcore to + * @param lcore The lcore that will be mapped to service + * @param enable Zero to unmap or disable the core, non-zero to enable * - * Each core can be added or removed from running specific services. This - * functions removes *lcore* to the set of cores that will run *service*. - * - * @retval 0 Lcore removed successfully + * @retval 0 lcore map updated successfully * @retval -EINVAL An invalid service or lcore was provided. */ -int32_t rte_service_disable_on_lcore(struct rte_service_spec *service, - uint32_t lcore); +int32_t rte_service_map_lcore_set(uint32_t service_id, uint32_t lcore, + uint32_t enable); /** * @warning * @b EXPERIMENTAL: this API may change without prior notice * - * Return if an lcore is enabled for the service. + * Retrieve the mapping of an lcore to a service. * - * This function allows the application to query if *lcore* is currently set to - * run *service*. + * @param service_id the service to apply the lcore to + * @param lcore The lcore that will be mapped to service * - * @retval 1 Lcore enabled on this lcore - * @retval 0 Lcore disabled on this lcore + * @retval 1 lcore is mapped to service + * @retval 0 lcore is not mapped to service * @retval -EINVAL An invalid service or lcore was provided. */ -int32_t rte_service_get_enabled_on_lcore(struct rte_service_spec *service, - uint32_t lcore); - +int32_t rte_service_map_lcore_get(uint32_t service_id, uint32_t lcore); /** * @warning diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index c969406..c5a8d0d 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -436,7 +436,7 @@ rte_service_start_with_defaults(void) * should multiplex to a single core, or 1:1 if there are the * same amount of services as service-cores */ - ret = rte_service_enable_on_lcore(s, ids[lcore_iter]); + ret = rte_service_map_lcore_set(i, ids[lcore_iter], 1); if (ret) return -ENODEV; @@ -492,28 +492,25 @@ service_update(struct rte_service_spec *service, uint32_t lcore, return 0; } -int32_t rte_service_get_enabled_on_lcore(struct rte_service_spec *service, - uint32_t lcore) -{ - uint32_t enabled; - int ret = service_update(service, lcore, 0, &enabled); - if (ret == 0) - return enabled; - return -EINVAL; -} - int32_t -rte_service_enable_on_lcore(struct rte_service_spec *service, uint32_t lcore) +rte_service_map_lcore_set(uint32_t id, uint32_t lcore, uint32_t enabled) { - uint32_t on = 1; - return service_update(service, lcore, &on, 0); + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + uint32_t on = enabled > 0; + return service_update(&s->spec, lcore, &on, 0); } int32_t -rte_service_disable_on_lcore(struct rte_service_spec *service, uint32_t lcore) +rte_service_map_lcore_get(uint32_t id, uint32_t lcore) { - uint32_t off = 0; - return service_update(service, lcore, &off, 0); + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + uint32_t enabled; + int ret = service_update(&s->spec, lcore, 0, &enabled); + if (ret == 0) + return enabled; + return ret; } int32_t rte_service_lcore_reset_all(void) diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index 3a8f154..452ae1c 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -217,13 +217,10 @@ EXPERIMENTAL { rte_eal_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; - rte_service_disable_on_lcore; rte_service_dump; - rte_service_enable_on_lcore; rte_service_get_by_id; rte_service_get_by_name; rte_service_get_count; - rte_service_get_enabled_on_lcore; rte_service_is_running; rte_service_lcore_add; rte_service_lcore_count; @@ -232,6 +229,8 @@ EXPERIMENTAL { rte_service_lcore_reset_all; rte_service_lcore_start; rte_service_lcore_stop; + rte_service_map_lcore_get; + rte_service_map_lcore_set; rte_service_probe_capability; rte_service_register; rte_service_reset; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index 940bc62..fd63efd 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -273,12 +273,13 @@ service_dump(void) static int service_start_stop(void) { + const uint32_t sid = 0; struct rte_service_spec *service = rte_service_get_by_id(0); /* is_running() returns if service is running and slcore is mapped */ TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id), "Service core add did not return zero"); - int ret = rte_service_enable_on_lcore(service, slcore_id); + int ret = rte_service_map_lcore_set(sid, slcore_id, 1); TEST_ASSERT_EQUAL(0, ret, "Enabling service core, expected 0 got %d", ret); @@ -313,12 +314,12 @@ service_remote_launch_func(void *arg) static int service_lcore_en_dis_able(void) { - struct rte_service_spec *s = rte_service_get_by_id(0); + const uint32_t sid = 0; /* expected failure cases */ - TEST_ASSERT_EQUAL(-EINVAL, rte_service_enable_on_lcore(s, 100000), + TEST_ASSERT_EQUAL(-EINVAL, rte_service_map_lcore_set(sid, 100000, 1), "Enable on invalid core did not fail"); - TEST_ASSERT_EQUAL(-EINVAL, rte_service_disable_on_lcore(s, 100000), + TEST_ASSERT_EQUAL(-EINVAL, rte_service_map_lcore_set(sid, 100000, 0), "Disable on invalid core did not fail"); /* add service core to allow enabling */ @@ -326,15 +327,15 @@ service_lcore_en_dis_able(void) "Add service core failed when not in use before"); /* valid enable */ - TEST_ASSERT_EQUAL(0, rte_service_enable_on_lcore(s, slcore_id), + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_id, 1), "Enabling valid service and core failed"); - TEST_ASSERT_EQUAL(1, rte_service_get_enabled_on_lcore(s, slcore_id), + TEST_ASSERT_EQUAL(1, rte_service_map_lcore_get(sid, slcore_id), "Enabled core returned not-enabled"); /* valid disable */ - TEST_ASSERT_EQUAL(0, rte_service_disable_on_lcore(s, slcore_id), + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_id, 0), "Disabling valid service and lcore failed"); - TEST_ASSERT_EQUAL(0, rte_service_get_enabled_on_lcore(s, slcore_id), + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_get(sid, slcore_id), "Disabled core returned enabled"); /* call remote_launch to verify that app can launch ex-service lcore */ @@ -474,11 +475,12 @@ service_threaded_test(int mt_safe) "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), "Starting valid service failed"); - TEST_ASSERT_EQUAL(0, rte_service_enable_on_lcore(s, slcore_1), + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_1, 1), "Failed to enable lcore 1 on mt safe service"); - TEST_ASSERT_EQUAL(0, rte_service_enable_on_lcore(s, slcore_2), + TEST_ASSERT_EQUAL(0, rte_service_map_lcore_set(sid, slcore_2, 1), "Failed to enable lcore 2 on mt safe service"); rte_service_lcore_start(slcore_1); rte_service_lcore_start(slcore_2); @@ -529,10 +531,11 @@ static int 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), "Starting valid service failed"); - TEST_ASSERT_EQUAL(-EINVAL, rte_service_enable_on_lcore(s, slcore_id), + TEST_ASSERT_EQUAL(-EINVAL, rte_service_map_lcore_set(sid, slcore_id, 1), "Enabling valid service on non-service core must fail"); /* core start */ @@ -540,7 +543,7 @@ service_lcore_start_stop(void) "Service core start without add should return EINVAL"); TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id), "Service core add did not return zero"); - TEST_ASSERT_EQUAL(0, rte_service_enable_on_lcore(s, slcore_id), + 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"); From patchwork Mon Aug 21 12:58:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27686 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 588B97D3B; Mon, 21 Aug 2017 14:58:32 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id D93617D12 for ; Mon, 21 Aug 2017 14:58:29 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383273" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:28 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:04 +0100 Message-Id: <1503320296-51122-4-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 03/15] service: rework register to return service id X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reworks the service register function to accept an extra parameter. The parameter is a uint32_t *, which when provided will be set to the integer service_id that the newly registered service is represented by. This is useful for services that wish to validate settings at a later point in time - they need to know their own service id. This commit updates the eventdev sw pmd, as well as unit tests to use the new register API. Signed-off-by: Harry van Haaren --- v2: - Rename rte_service_register to rte_service_component_register This makes it clearer which APIs apply to components vs services. --- drivers/event/sw/sw_evdev.c | 4 +-- drivers/event/sw/sw_evdev.h | 1 + lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- .../common/include/rte_service_component.h | 6 +++- lib/librte_eal/common/rte_service.c | 6 +++- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- test/test/test_service_cores.c | 36 ++++++++++++---------- 7 files changed, 35 insertions(+), 22 deletions(-) diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c index 9c534b7..9cc63a0 100644 --- a/drivers/event/sw/sw_evdev.c +++ b/drivers/event/sw/sw_evdev.c @@ -620,7 +620,7 @@ sw_start(struct rte_eventdev *dev) struct rte_service_spec *s = rte_service_get_by_name(sw->service_name); if (!rte_service_is_running(s)) SW_LOG_ERR("Warning: No Service core enabled on service %s\n", - s->name); + sw->service_name); /* check all ports are set up */ for (i = 0; i < sw->port_count; i++) @@ -855,7 +855,7 @@ sw_probe(struct rte_vdev_device *vdev) service.callback = sw_sched_service_func; service.callback_userdata = (void *)dev; - int32_t ret = rte_service_register(&service); + int32_t ret = rte_service_component_register(&service, &sw->service_id); if (ret) { SW_LOG_ERR("service register() failed"); return -ENOEXEC; diff --git a/drivers/event/sw/sw_evdev.h b/drivers/event/sw/sw_evdev.h index 71de3c1..e0dec91 100644 --- a/drivers/event/sw/sw_evdev.h +++ b/drivers/event/sw/sw_evdev.h @@ -278,6 +278,7 @@ struct sw_evdev { uint16_t xstats_count_per_qid[RTE_EVENT_MAX_QUEUES_PER_DEV]; uint16_t xstats_offset_for_qid[RTE_EVENT_MAX_QUEUES_PER_DEV]; + uint32_t service_id; char service_name[SW_PMD_NAME_MAX]; }; diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index f626a6f..2b1089d 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -212,6 +212,7 @@ EXPERIMENTAL { rte_eal_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; + rte_service_component_register; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; @@ -227,7 +228,6 @@ EXPERIMENTAL { rte_service_map_lcore_get; rte_service_map_lcore_set; rte_service_probe_capability; - rte_service_register; rte_service_reset; rte_service_set_stats_enable; rte_service_start; diff --git a/lib/librte_eal/common/include/rte_service_component.h b/lib/librte_eal/common/include/rte_service_component.h index 7a946a1..6ef3ee4 100644 --- a/lib/librte_eal/common/include/rte_service_component.h +++ b/lib/librte_eal/common/include/rte_service_component.h @@ -89,11 +89,15 @@ struct rte_service_spec { * *rte_service_set_coremask*. * * @param spec The specification of the service to register + * @param[out] service_id A pointer to a uint32_t, which will be filled in + * during registration of the service. It is set to the integers + * service number given to the service. This parameter may be NULL. * @retval 0 Successfully registered the service. * -EINVAL Attempted to register an invalid service (eg, no callback * set) */ -int32_t rte_service_register(const struct rte_service_spec *spec); +int32_t rte_service_component_register(const struct rte_service_spec *spec, + uint32_t *service_id); /** * @warning diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index c5a8d0d..152eafe 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -242,7 +242,8 @@ rte_service_is_running(const struct rte_service_spec *spec) } int32_t -rte_service_register(const struct rte_service_spec *spec) +rte_service_component_register(const struct rte_service_spec *spec, + uint32_t *id_ptr) { uint32_t i; int32_t free_slot = -1; @@ -267,6 +268,9 @@ rte_service_register(const struct rte_service_spec *spec) rte_smp_wmb(); rte_service_count++; + if (id_ptr) + *id_ptr = free_slot; + return 0; } diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index 452ae1c..336e40e 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -217,6 +217,7 @@ EXPERIMENTAL { rte_eal_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; + rte_service_component_register; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; @@ -232,7 +233,6 @@ EXPERIMENTAL { rte_service_map_lcore_get; rte_service_map_lcore_set; rte_service_probe_capability; - rte_service_register; rte_service_reset; rte_service_set_stats_enable; rte_service_start; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index fd63efd..7767fa2 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -160,15 +160,17 @@ dummy_register(void) struct rte_service_spec service; memset(&service, 0, sizeof(struct rte_service_spec)); - TEST_ASSERT_EQUAL(-EINVAL, rte_service_register(&service), + TEST_ASSERT_EQUAL(-EINVAL, + rte_service_component_register(&service, NULL), "Invalid callback"); service.callback = dummy_cb; - TEST_ASSERT_EQUAL(-EINVAL, rte_service_register(&service), + TEST_ASSERT_EQUAL(-EINVAL, + rte_service_component_register(&service, NULL), "Invalid name"); snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME); - TEST_ASSERT_EQUAL(0, rte_service_register(&service), + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Failed to register valid service"); return TEST_SUCCESS; @@ -187,13 +189,15 @@ service_get_by_name(void) /* register service */ struct rte_service_spec service; memset(&service, 0, sizeof(struct rte_service_spec)); - TEST_ASSERT_EQUAL(-EINVAL, rte_service_register(&service), + TEST_ASSERT_EQUAL(-EINVAL, + rte_service_component_register(&service, NULL), "Invalid callback"); service.callback = dummy_cb; - TEST_ASSERT_EQUAL(-EINVAL, rte_service_register(&service), + TEST_ASSERT_EQUAL(-EINVAL, + rte_service_component_register(&service, NULL), "Invalid name"); snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME); - TEST_ASSERT_EQUAL(0, rte_service_register(&service), + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Failed to register valid service"); /* ensure with dummy services registered returns same ptr as ID */ @@ -221,7 +225,7 @@ service_probe_capability(void) service.callback = dummy_cb; snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME); service.capabilities |= RTE_SERVICE_CAP_MT_SAFE; - TEST_ASSERT_EQUAL(0, rte_service_register(&service), + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Register of MT SAFE service failed"); /* verify flag is enabled */ @@ -235,7 +239,7 @@ service_probe_capability(void) memset(&service, 0, sizeof(struct rte_service_spec)); service.callback = dummy_cb; snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME); - TEST_ASSERT_EQUAL(0, rte_service_register(&service), + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Register of non-MT safe service failed"); /* verify flag is enabled */ @@ -274,28 +278,28 @@ static int service_start_stop(void) { const uint32_t sid = 0; - struct rte_service_spec *service = rte_service_get_by_id(0); + struct rte_service_spec *s = rte_service_get_by_id(0); - /* is_running() returns if service is running and slcore is mapped */ + /* runstate_get() returns if service is running and slcore is mapped */ TEST_ASSERT_EQUAL(0, rte_service_lcore_add(slcore_id), "Service core add did not return zero"); int ret = rte_service_map_lcore_set(sid, slcore_id, 1); TEST_ASSERT_EQUAL(0, ret, "Enabling service core, expected 0 got %d", ret); - TEST_ASSERT_EQUAL(0, rte_service_is_running(service), + TEST_ASSERT_EQUAL(0, rte_service_is_running(s), "Error: Service should be stopped"); - TEST_ASSERT_EQUAL(0, rte_service_stop(service), + TEST_ASSERT_EQUAL(0, rte_service_stop(s), "Error: Service stopped returned non-zero"); - TEST_ASSERT_EQUAL(0, rte_service_is_running(service), + TEST_ASSERT_EQUAL(0, rte_service_is_running(s), "Error: Service is running - should be stopped"); - TEST_ASSERT_EQUAL(0, rte_service_start(service), + TEST_ASSERT_EQUAL(0, rte_service_start(s), "Error: Service start returned non-zero"); - TEST_ASSERT_EQUAL(1, rte_service_is_running(service), + TEST_ASSERT_EQUAL(1, rte_service_is_running(s), "Error: Service is not running"); return unregister_all(); @@ -471,7 +475,7 @@ service_threaded_test(int mt_safe) service.callback = dummy_mt_unsafe_cb; } - TEST_ASSERT_EQUAL(0, rte_service_register(&service), + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Register of MT SAFE service failed"); struct rte_service_spec *s = rte_service_get_by_id(0); From patchwork Mon Aug 21 12:58:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27687 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id EA9517D43; Mon, 21 Aug 2017 14:58:33 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 8687C7D0F for ; Mon, 21 Aug 2017 14:58:30 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383280" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:29 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:05 +0100 Message-Id: <1503320296-51122-5-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 04/15] service: rework service start stop to runstate X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reworks the API to move from two separate 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 --- 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 9cc63a0..da6ac30 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 2b1089d..b336f54 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -217,7 +217,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_map_lcore_set; rte_service_probe_capability; 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 152eafe..2ff1ab0 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_component_register(const struct rte_service_spec *spec, uint32_t *id_ptr) { @@ -309,23 +297,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 @@ -448,7 +440,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 336e40e..b2b2235 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -222,7 +222,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_map_lcore_set; rte_service_probe_capability; 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 7767fa2..196277b 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -278,7 +278,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), @@ -287,19 +286,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(); @@ -478,9 +477,8 @@ service_threaded_test(int mt_safe) TEST_ASSERT_EQUAL(0, rte_service_component_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"); @@ -497,7 +495,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(); @@ -536,8 +534,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"); From patchwork Mon Aug 21 12:58:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27688 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 53D537D5F; Mon, 21 Aug 2017 14:58:37 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 036A97D33 for ; Mon, 21 Aug 2017 14:58:31 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383288" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:30 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:06 +0100 Message-Id: <1503320296-51122-6-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 05/15] service: rework service stats functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reworks the statistics functions to use integer ids for services instead of pointers. Passing UINT32_MAX to the dump function prints all info, similar to passing NULL previously. Signed-off-by: Harry van Haaren --- lib/librte_eal/common/include/rte_service.h | 14 ++++++++------ lib/librte_eal/common/rte_service.c | 24 ++++++++++++------------ test/test/test_service_cores.c | 10 +++++----- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h index e133ca1..afa6c0bf 100644 --- a/lib/librte_eal/common/include/rte_service.h +++ b/lib/librte_eal/common/include/rte_service.h @@ -318,13 +318,12 @@ int32_t rte_service_lcore_reset_all(void); * Enable or disable statistics collection for *service*. * * This function enables per core, per-service cycle count collection. - * @param service The service to enable statistics gathering on. + * @param id The service to enable statistics gathering on. * @param enable Zero to disable statistics, non-zero to enable. * @retval 0 Success * @retval -EINVAL Invalid service pointer passed */ -int32_t rte_service_set_stats_enable(struct rte_service_spec *service, - int32_t enable); +int32_t rte_service_set_stats_enable(uint32_t id, int32_t enable); /** * @warning @@ -351,10 +350,13 @@ int32_t rte_service_lcore_list(uint32_t array[], uint32_t n); * @warning * @b EXPERIMENTAL: this API may change without prior notice * - * Dumps any information available about the service. If service is NULL, - * dumps info for all services. + * Dumps any information available about the service. When id is UINT32_MAX, + * this function dumps info for all services. + * + * @retval 0 Statistics have been successfully dumped + * @retval -EINVAL Invalid service id provided */ -int32_t rte_service_dump(FILE *f, struct rte_service_spec *service); +int32_t rte_service_dump(FILE *f, uint32_t id); #ifdef __cplusplus } diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 2ff1ab0..ac1c90c 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -166,18 +166,15 @@ service_mt_safe(struct rte_service_spec_impl *s) return s->spec.capabilities & RTE_SERVICE_CAP_MT_SAFE; } -int32_t rte_service_set_stats_enable(struct rte_service_spec *service, - int32_t enabled) +int32_t rte_service_set_stats_enable(uint32_t id, int32_t enabled) { - struct rte_service_spec_impl *impl = - (struct rte_service_spec_impl *)service; - if (!impl) - return -EINVAL; + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, 0); if (enabled) - impl->internal_flags |= SERVICE_F_STATS_ENABLED; + s->internal_flags |= SERVICE_F_STATS_ENABLED; else - impl->internal_flags &= ~(SERVICE_F_STATS_ENABLED); + s->internal_flags &= ~(SERVICE_F_STATS_ENABLED); return 0; } @@ -670,9 +667,10 @@ service_dump_calls_per_lcore(FILE *f, uint32_t lcore, uint32_t reset) fprintf(f, "\n"); } -int32_t rte_service_dump(FILE *f, struct rte_service_spec *service) +int32_t rte_service_dump(FILE *f, uint32_t id) { uint32_t i; + int print_one = (id != UINT32_MAX); uint64_t total_cycles = 0; for (i = 0; i < rte_service_count; i++) { @@ -681,15 +679,17 @@ int32_t rte_service_dump(FILE *f, struct rte_service_spec *service) total_cycles += rte_services[i].cycles_spent; } - if (service) { - struct rte_service_spec_impl *s = - (struct rte_service_spec_impl *)service; + /* print only the specified service */ + if (print_one) { + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); fprintf(f, "Service %s Summary\n", s->spec.name); uint32_t reset = 0; rte_service_dump_one(f, s, total_cycles, reset); return 0; } + /* print all services, as UINT32_MAX was passed as id */ fprintf(f, "Services Summary\n"); for (i = 0; i < rte_service_count; i++) { uint32_t reset = 1; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index 196277b..d52fe49 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -265,11 +265,11 @@ service_name(void) static int service_dump(void) { - struct rte_service_spec *service = rte_service_get_by_id(0); - rte_service_set_stats_enable(service, 1); - rte_service_dump(stdout, service); - rte_service_set_stats_enable(service, 0); - rte_service_dump(stdout, service); + const uint32_t sid = 0; + rte_service_set_stats_enable(sid, 1); + rte_service_dump(stdout, 0); + rte_service_set_stats_enable(sid, 0); + rte_service_dump(stdout, 0); return unregister_all(); } From patchwork Mon Aug 21 12:58:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27689 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 7947E7D6B; Mon, 21 Aug 2017 14:58:38 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id B34EC7D42 for ; Mon, 21 Aug 2017 14:58:32 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383294" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:31 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:07 +0100 Message-Id: <1503320296-51122-7-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 06/15] service: rework unregister api to use integers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reworks the unregister API to accept an integer. Signed-off-by: Harry van Haaren --- v2: - Renamed unregister to rte_service_component_unregister for consistency in APIs for components. --- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- .../common/include/rte_service_component.h | 4 ++-- lib/librte_eal/common/rte_service.c | 25 ++++++---------------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- test/test/test_service_cores.c | 10 +++------ 5 files changed, 13 insertions(+), 30 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index b336f54..507a38e 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -213,6 +213,7 @@ EXPERIMENTAL { rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_service_component_register; + rte_service_component_unregister; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; @@ -232,6 +233,5 @@ EXPERIMENTAL { rte_service_runstate_set; rte_service_set_stats_enable; rte_service_start_with_defaults; - rte_service_unregister; } DPDK_17.08; diff --git a/lib/librte_eal/common/include/rte_service_component.h b/lib/librte_eal/common/include/rte_service_component.h index 6ef3ee4..af632c6 100644 --- a/lib/librte_eal/common/include/rte_service_component.h +++ b/lib/librte_eal/common/include/rte_service_component.h @@ -103,7 +103,7 @@ int32_t rte_service_component_register(const struct rte_service_spec *spec, * @warning * @b EXPERIMENTAL: this API may change without prior notice * - * Unregister a service. + * Unregister a service component. * * The service being removed must be stopped before calling this function. * @@ -111,7 +111,7 @@ int32_t rte_service_component_register(const struct rte_service_spec *spec, * @retval -EBUSY The service is currently running, stop the service before * calling unregister. No action has been taken. */ -int32_t rte_service_unregister(struct rte_service_spec *service); +int32_t rte_service_component_unregister(uint32_t id); /** * @warning diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index ac1c90c..adcd860 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -260,35 +260,22 @@ rte_service_component_register(const struct rte_service_spec *spec, } int32_t -rte_service_unregister(struct rte_service_spec *spec) +rte_service_component_unregister(uint32_t id) { - struct rte_service_spec_impl *s = NULL; - struct rte_service_spec_impl *spec_impl = - (struct rte_service_spec_impl *)spec; - uint32_t i; - uint32_t service_id; - for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { - if (&rte_services[i] == spec_impl) { - s = spec_impl; - service_id = i; - break; - } - } - - if (!s) - return -EINVAL; + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); rte_service_count--; rte_smp_wmb(); s->internal_flags &= ~(SERVICE_F_REGISTERED); + /* clear the run-bit in all cores */ for (i = 0; i < RTE_MAX_LCORE; i++) - lcore_states[i].service_mask &= ~(UINT64_C(1) << service_id); + lcore_states[i].service_mask &= ~(UINT64_C(1) << id); - memset(&rte_services[service_id], 0, - sizeof(struct rte_service_spec_impl)); + memset(&rte_services[id], 0, sizeof(struct rte_service_spec_impl)); return 0; } diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index b2b2235..a7431da 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -218,6 +218,7 @@ EXPERIMENTAL { rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_service_component_register; + rte_service_component_unregister; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; @@ -237,6 +238,5 @@ EXPERIMENTAL { rte_service_runstate_set; rte_service_set_stats_enable; rte_service_start_with_defaults; - rte_service_unregister; } DPDK_17.08; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index d52fe49..e650b20 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -131,17 +131,13 @@ static int unregister_all(void) { uint32_t i; - struct rte_service_spec *dead = (struct rte_service_spec *)0xdead; - TEST_ASSERT_EQUAL(-EINVAL, rte_service_unregister(0), - "Unregistered NULL pointer"); - TEST_ASSERT_EQUAL(-EINVAL, rte_service_unregister(dead), - "Unregistered invalid pointer"); + TEST_ASSERT_EQUAL(-EINVAL, rte_service_component_unregister(1000), + "Unregistered invalid service id"); uint32_t c = rte_service_get_count(); for (i = 0; i < c; i++) { - struct rte_service_spec *s = rte_service_get_by_id(i); - TEST_ASSERT_EQUAL(0, rte_service_unregister(s), + TEST_ASSERT_EQUAL(0, rte_service_component_unregister(i), "Error unregistering a valid service"); } From patchwork Mon Aug 21 12:58:08 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27690 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id D20B07D7A; Mon, 21 Aug 2017 14:58:39 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 30E7F7D0F for ; Mon, 21 Aug 2017 14:58:34 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383304" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:32 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:08 +0100 Message-Id: <1503320296-51122-8-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 07/15] service: rework get by name function to use id X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit reworks the service_get_by_name() function to accept an integer, and removes the service_get_by_id() function. All functions now accept an integer argument representing the service, so it is no longer required to expose the service_spec pointers to the application. Signed-off-by: Harry van Haaren --- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 + lib/librte_eal/common/include/rte_service.h | 45 ++++++++++--------------- lib/librte_eal/common/rte_service.c | 24 ++++--------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + test/test/test_service_cores.c | 30 ++++++++++------- 5 files changed, 43 insertions(+), 58 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index 507a38e..4b2c36b 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -218,6 +218,7 @@ EXPERIMENTAL { rte_service_get_by_id; rte_service_get_by_name; rte_service_get_count; + rte_service_get_name; rte_service_lcore_add; rte_service_lcore_count; rte_service_lcore_del; diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h index afa6c0bf..e06b075 100644 --- a/lib/librte_eal/common/include/rte_service.h +++ b/lib/librte_eal/common/include/rte_service.h @@ -61,9 +61,6 @@ extern "C" { #include -/* forward declaration only. Definition in rte_service_private.h */ -struct rte_service_spec; - #define RTE_SERVICE_NAME_MAX 32 /* Capabilities of a service. @@ -89,40 +86,32 @@ struct rte_service_spec; */ uint32_t rte_service_get_count(void); - /** * @warning * @b EXPERIMENTAL: this API may change without prior notice * - * Return the specification of a service by integer id. - * - * This function provides the specification of a service. This can be used by - * the application to understand what the service represents. The service - * must not be modified by the application directly, only passed to the various - * rte_service_* functions. - * - * @param id The integer id of the service to retrieve - * @retval non-zero A valid pointer to the service_spec - * @retval NULL Invalid *id* provided. - */ -struct rte_service_spec *rte_service_get_by_id(uint32_t id); - -/** - * @warning - * @b EXPERIMENTAL: this API may change without prior notice + * Return the id of a service by name. * - * Return the specification of a service by name. + * This function provides the id of the service using the service name as + * lookup key. The service id is to be passed to other functions in the + * rte_service_* API. * - * This function provides the specification of a service using the service name - * as lookup key. This can be used by the application to understand what the - * service represents. The service must not be modified by the application - * directly, only passed to the various rte_service_* functions. + * Example usage: + * @code + * uint32_t service_id; + * int32_t ret = rte_service_get_by_name("service_X", &service_id); + * if (ret) { + * // handle error + * } + * @endcode * * @param name The name of the service to retrieve - * @retval non-zero A valid pointer to the service_spec - * @retval NULL Invalid *name* provided. + * @param[out] service_id A pointer to a uint32_t, to be filled in with the id. + * @retval 0 Success. The service id is provided in *service_id*. + * @retval -EINVAL Null *service_id* pointer provided + * @retval -ENODEV No such service registered */ -struct rte_service_spec *rte_service_get_by_name(const char *name); +int32_t rte_service_get_by_name(const char *name, uint32_t *service_id); /** * @warning diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index adcd860..1bfd149 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -185,29 +185,21 @@ rte_service_get_count(void) return rte_service_count; } -struct rte_service_spec * -rte_service_get_by_id(uint32_t id) +int32_t rte_service_get_by_name(const char *name, uint32_t *service_id) { - struct rte_service_spec *service = NULL; - if (id < rte_service_count) - service = (struct rte_service_spec *)&rte_services[id]; - - return service; -} + if (!service_id) + return -EINVAL; -struct rte_service_spec *rte_service_get_by_name(const char *name) -{ - struct rte_service_spec *service = NULL; int i; for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { if (service_valid(i) && strcmp(name, rte_services[i].spec.name) == 0) { - service = (struct rte_service_spec *)&rte_services[i]; - break; + *service_id = i; + return 0; } } - return service; + return -ENODEV; } const char * @@ -407,10 +399,6 @@ rte_service_start_with_defaults(void) rte_service_lcore_start(ids[i]); for (i = 0; i < count; i++) { - struct rte_service_spec *s = rte_service_get_by_id(i); - if (!s) - return -EINVAL; - /* do 1:1 core mapping here, with each service getting * assigned a single core by default. Adding multiple services * should multiplex to a single core, or 1:1 if there are the diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index a7431da..f7a7352 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -223,6 +223,7 @@ EXPERIMENTAL { rte_service_get_by_id; rte_service_get_by_name; rte_service_get_count; + rte_service_get_name; rte_service_lcore_add; rte_service_lcore_count; rte_service_lcore_del; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index e650b20..5ae7b20 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -178,9 +178,13 @@ service_get_by_name(void) { unregister_all(); - /* ensure with no services registered returns NULL */ - TEST_ASSERT_EQUAL(0, rte_service_get_by_name(DUMMY_SERVICE_NAME), - "Service get by name should return NULL"); + uint32_t sid; + TEST_ASSERT_EQUAL(-ENODEV, + rte_service_get_by_name(DUMMY_SERVICE_NAME, &sid), + "get by name with invalid name should return -ENODEV"); + TEST_ASSERT_EQUAL(-EINVAL, + rte_service_get_by_name(DUMMY_SERVICE_NAME, 0x0), + "get by name with NULL ptr should return -ENODEV"); /* register service */ struct rte_service_spec service; @@ -196,16 +200,19 @@ service_get_by_name(void) TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), "Failed to register valid service"); - /* ensure with dummy services registered returns same ptr as ID */ - struct rte_service_spec *s_by_id = rte_service_get_by_id(0); - TEST_ASSERT_EQUAL(s_by_id, rte_service_get_by_name(DUMMY_SERVICE_NAME), - "Service get_by_name should equal get_by_id()"); + /* we unregistered all service, now registering 1, should be id 0 */ + uint32_t service_id_as_expected = 0; + TEST_ASSERT_EQUAL(0, rte_service_get_by_name(DUMMY_SERVICE_NAME, &sid), + "Service get_by_name should return 0 on valid inputs"); + TEST_ASSERT_EQUAL(service_id_as_expected, sid, + "Service get_by_name should equal expected id"); unregister_all(); /* ensure after unregister, get_by_name returns NULL */ - TEST_ASSERT_EQUAL(0, rte_service_get_by_name(DUMMY_SERVICE_NAME), - "get by name should return NULL after unregister"); + TEST_ASSERT_EQUAL(-ENODEV, + rte_service_get_by_name(DUMMY_SERVICE_NAME, &sid), + "get by name should return -ENODEV after unregister"); return TEST_SUCCESS; } @@ -249,9 +256,8 @@ service_probe_capability(void) static int service_name(void) { - struct rte_service_spec *service = rte_service_get_by_id(0); - - int equal = strcmp(service->name, DUMMY_SERVICE_NAME); + const char *name = rte_service_get_name(0); + int equal = strcmp(name, DUMMY_SERVICE_NAME); TEST_ASSERT_EQUAL(0, equal, "Error: Service name not correct"); return unregister_all(); From patchwork Mon Aug 21 12:58:09 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27691 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 07E587D86; Mon, 21 Aug 2017 14:58:41 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id E289B7D0F for ; Mon, 21 Aug 2017 14:58:34 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383314" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:33 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:09 +0100 Message-Id: <1503320296-51122-9-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 08/15] service: fix and refactor atomic service accesses X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit fixes an issue in the service runner function, where the atomic value was not cleared on exiting the service function. This resulted in future attempts to run the service to appear like the function was running, however it was in reality deadlocked. This commit refactors the atomic handling to be more readable, by splitting the implementation code into a new static inline function. The remaining flow control of atomics in the existing function is refactored for readability. Fixes: 21698354c832 ("service: introduce service cores concept") Signed-off-by: Harry van Haaren --- doc/guides/rel_notes/release_17_11.rst | 7 ++++++ lib/librte_eal/common/rte_service.c | 45 ++++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 170f4f9..6e6ba1c 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -65,6 +65,13 @@ Resolved Issues EAL ~~~ +* **Service core fails to call service callback due to atomic lock** + + In a specific configuration of multi-thread unsafe services and service + cores, a service core previously did not correctly release the atomic lock + on the service. This would result in the cores polling the service, but it + looked like another thread was executing the service callback. The logic for + atomic locking of the services has been fixed and refactored for readability. Drivers ~~~~~~~ diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 1bfd149..6291c0c 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -296,6 +296,23 @@ rte_service_runstate_get(uint32_t id) return (s->runstate == RUNSTATE_RUNNING) && (s->num_mapped_cores > 0); } +static inline void +rte_service_runner_do_callback(struct rte_service_spec_impl *s, + struct core_state *cs, uint32_t service_idx) +{ + void *userdata = s->spec.callback_userdata; + + if (service_stats_enabled(s)) { + uint64_t start = rte_rdtsc(); + s->spec.callback(userdata); + uint64_t end = rte_rdtsc(); + s->cycles_spent += end - start; + cs->calls_per_service[service_idx]++; + s->calls++; + } else + s->spec.callback(userdata); +} + static int32_t rte_service_runner_func(void *arg) { @@ -315,26 +332,16 @@ rte_service_runner_func(void *arg) /* check do we need cmpset, if MT safe or <= 1 core * mapped, atomic ops are not required. */ - const int need_cmpset = !((service_mt_safe(s) == 0) && - (s->num_mapped_cores > 1)); - uint32_t *lock = (uint32_t *)&s->execute_lock; - - if (need_cmpset || rte_atomic32_cmpset(lock, 0, 1)) { - void *userdata = s->spec.callback_userdata; - - if (service_stats_enabled(s)) { - uint64_t start = rte_rdtsc(); - s->spec.callback(userdata); - uint64_t end = rte_rdtsc(); - s->cycles_spent += end - start; - cs->calls_per_service[i]++; - s->calls++; - } else - s->spec.callback(userdata); - - if (need_cmpset) + const int use_atomics = (service_mt_safe(s) == 0) && + (s->num_mapped_cores > 1); + if (use_atomics) { + uint32_t *lock = (uint32_t *)&s->execute_lock; + if (rte_atomic32_cmpset(lock, 0, 1)) { + rte_service_runner_do_callback(s, cs, i); rte_atomic32_clear(&s->execute_lock); - } + } + } else + rte_service_runner_do_callback(s, cs, i); } rte_smp_rmb(); From patchwork Mon Aug 21 12:58:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27692 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 4DA7D7D8E; Mon, 21 Aug 2017 14:58:42 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 5C4D27D23 for ; Mon, 21 Aug 2017 14:58:36 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383323" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:34 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:10 +0100 Message-Id: <1503320296-51122-10-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 09/15] service: fix loops to always scan all services X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Services can be registered and unregistered, and "holes" can appear in the contiguous array of services if a service is unregistered. As a result, we must never iterate to the number of services (as counted by rte_service_count), instead scanning the service array and checking if the service is valid. After this commit, the rte_service_count variable is only used for its intended purpose; tracking the number of services that are present. Fixes: 21698354c832 ("service: introduce service cores concept") Signed-off-by: Harry van Haaren --- lib/librte_eal/common/rte_service.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 6291c0c..a5fbcf9 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -323,7 +323,10 @@ rte_service_runner_func(void *arg) while (lcore_states[lcore].runstate == RUNSTATE_RUNNING) { const uint64_t service_mask = cs->service_mask; - for (i = 0; i < rte_service_count; i++) { + + for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { + if (!service_valid(i)) + continue; struct rte_service_spec_impl *s = &rte_services[i]; if (s->runstate != RUNSTATE_RUNNING || !(service_mask & (UINT64_C(1) << i))) @@ -655,7 +658,8 @@ int32_t rte_service_dump(FILE *f, uint32_t id) int print_one = (id != UINT32_MAX); uint64_t total_cycles = 0; - for (i = 0; i < rte_service_count; i++) { + + for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { if (!service_valid(i)) continue; total_cycles += rte_services[i].cycles_spent; @@ -673,7 +677,9 @@ int32_t rte_service_dump(FILE *f, uint32_t id) /* print all services, as UINT32_MAX was passed as id */ fprintf(f, "Services Summary\n"); - for (i = 0; i < rte_service_count; i++) { + for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { + if (!service_valid(i)) + continue; uint32_t reset = 1; rte_service_dump_one(f, &rte_services[i], total_cycles, reset); } From patchwork Mon Aug 21 12:58:11 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27693 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id C66B18C7E; Mon, 21 Aug 2017 14:58:44 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 070C07D0F for ; Mon, 21 Aug 2017 14:58:36 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:36 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383330" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:35 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:11 +0100 Message-Id: <1503320296-51122-11-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 10/15] service: fix return values of functions to 0 or 1 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Previously to this commit, the return value of the following functions was the mask value, instead of a neat 0 or 1. This commit fixes this using the "!!" trick, to force the number to 1 instead of the bitmask value itself, bringing the return value in line with the function documentation. Fixes: 21698354c832 ("service: introduce service cores concept") Signed-off-by: Harry van Haaren --- lib/librte_eal/common/rte_service.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index a5fbcf9..4034130 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -163,7 +163,7 @@ service_stats_enabled(struct rte_service_spec_impl *impl) static inline int service_mt_safe(struct rte_service_spec_impl *s) { - return s->spec.capabilities & RTE_SERVICE_CAP_MT_SAFE; + return !!(s->spec.capabilities & RTE_SERVICE_CAP_MT_SAFE); } int32_t rte_service_set_stats_enable(uint32_t id, int32_t enabled) @@ -215,7 +215,7 @@ rte_service_probe_capability(uint32_t id, uint32_t capability) { struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); - return s->spec.capabilities & capability; + return !!(s->spec.capabilities & capability); } int32_t @@ -463,7 +463,7 @@ service_update(struct rte_service_spec *service, uint32_t lcore, } if (enabled) - *enabled = (lcore_states[lcore].service_mask & (sid_mask)); + *enabled = !!(lcore_states[lcore].service_mask & (sid_mask)); rte_smp_wmb(); From patchwork Mon Aug 21 12:58:12 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27694 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id C2BB490F2; Mon, 21 Aug 2017 14:58:45 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 220FF7D42 for ; Mon, 21 Aug 2017 14:58:37 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383343" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:36 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:12 +0100 Message-Id: <1503320296-51122-12-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 11/15] service: fix lcore in wait state in lcore add X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit ensures that after an lcore is added, that it is in the WAIT state. Previously, adding an lcore did not ensure that the core was ready for being relaunch, which would cause errors during lcore_start(). Now that the lcore is ensured to be in WAIT state by the lcore_add() function, this is no longer an issue. Fixes: 21698354c832 ("service: introduce service cores concept") Signed-off-by: Harry van Haaren --- lib/librte_eal/common/rte_service.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 4034130..789f63b 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -537,7 +537,8 @@ rte_service_lcore_add(uint32_t lcore) lcore_states[lcore].runstate = RUNSTATE_STOPPED; rte_smp_wmb(); - return 0; + + return rte_eal_wait_lcore(lcore); } int32_t From patchwork Mon Aug 21 12:58:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27695 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 51EC27D18; Mon, 21 Aug 2017 14:58:47 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 3553B7D42 for ; Mon, 21 Aug 2017 14:58:39 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383363" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:37 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:13 +0100 Message-Id: <1503320296-51122-13-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 12/15] service: reset core call stats on dump X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This aligns with the service stats, which are currently also reset on read. A generic statistics API would be helpful for the service library in future. Signed-off-by: Harry van Haaren --- lib/librte_eal/common/rte_service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 789f63b..58b53b1 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -690,7 +690,7 @@ int32_t rte_service_dump(FILE *f, uint32_t id) if (lcore_config[i].core_role != ROLE_SERVICE) continue; - uint32_t reset = 0; + uint32_t reset = 1; service_dump_calls_per_lcore(f, i, reset); } From patchwork Mon Aug 21 12:58:14 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27696 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 801E590FA; Mon, 21 Aug 2017 14:58:48 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 80BD07D7F for ; Mon, 21 Aug 2017 14:58:40 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383374" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:39 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:14 +0100 Message-Id: <1503320296-51122-14-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 13/15] service: add component runstate X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit adds a new flag that the component (or "backend") can use to indicate readyness. The service function callback will not be called until the component sets itself as ready. The use-case behind adding this feature is eg: a service that requires configuration before it can start. Any service that emulates an ethdev will have rte_eth_dev_configure() called, and only after that the service will know how many queues/etc to allocate. Once that configuration is complete, the service marks itself as ready using rte_service_component_runstate_set(). This feature request results from prototyping services, and requiring a flag in each service to note "internal" readyness. Instead that logic is now lifted to the service library. The unit tests have been updated to test the component runstate. Signed-off-by: Harry van Haaren --- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 + .../common/include/rte_service_component.h | 17 +++++++++++ lib/librte_eal/common/rte_service.c | 34 +++++++++++++++++----- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + test/test/test_service_cores.c | 32 +++++++++++++++----- 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index 4b2c36b..05a3de1 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -214,6 +214,7 @@ EXPERIMENTAL { rte_eal_hotplug_remove; rte_service_component_register; rte_service_component_unregister; + rte_service_component_runstate_set; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; diff --git a/lib/librte_eal/common/include/rte_service_component.h b/lib/librte_eal/common/include/rte_service_component.h index af632c6..5e4573b 100644 --- a/lib/librte_eal/common/include/rte_service_component.h +++ b/lib/librte_eal/common/include/rte_service_component.h @@ -135,6 +135,23 @@ int32_t rte_service_start_with_defaults(void); * @warning * @b EXPERIMENTAL: this API may change without prior notice * + * Set the backend runstate of a component. + * + * This function allows services to be registered at startup, but not yet + * enabled to run by default. When the service has been configured (via the + * usual method; eg rte_eventdev_configure, the service can mark itself as + * ready to run. The differentiation between backend runstate and + * service_runstate is that the backend runstate is set by the service + * component while the service runstate is reserved for application usage. + * + * @retval 0 Success + */ +int32_t rte_service_component_runstate_set(uint32_t id, uint32_t runstate); + +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice + * * Initialize the service library. * * In order to use the service library, it must be initialized. EAL initializes diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c index 58b53b1..e7ed597 100644 --- a/lib/librte_eal/common/rte_service.c +++ b/lib/librte_eal/common/rte_service.c @@ -71,7 +71,8 @@ struct rte_service_spec_impl { rte_atomic32_t execute_lock; /* API set/get-able variables */ - int32_t runstate; + int8_t app_runstate; + int8_t comp_runstate; uint8_t internal_flags; /* per service statistics */ @@ -273,15 +274,30 @@ rte_service_component_unregister(uint32_t id) } int32_t +rte_service_component_runstate_set(uint32_t id, uint32_t runstate) +{ + struct rte_service_spec_impl *s; + SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); + + if (runstate) + s->comp_runstate = RUNSTATE_RUNNING; + else + s->comp_runstate = RUNSTATE_STOPPED; + + rte_smp_wmb(); + return 0; +} + +int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate) { struct rte_service_spec_impl *s; SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); if (runstate) - s->runstate = RUNSTATE_RUNNING; + s->app_runstate = RUNSTATE_RUNNING; else - s->runstate = RUNSTATE_STOPPED; + s->app_runstate = RUNSTATE_STOPPED; rte_smp_wmb(); return 0; @@ -292,8 +308,10 @@ rte_service_runstate_get(uint32_t id) { 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); + rte_smp_rmb(); + return (s->app_runstate == RUNSTATE_RUNNING) && + (s->comp_runstate == RUNSTATE_RUNNING) && + (s->num_mapped_cores > 0); } static inline void @@ -328,7 +346,8 @@ rte_service_runner_func(void *arg) if (!service_valid(i)) continue; struct rte_service_spec_impl *s = &rte_services[i]; - if (s->runstate != RUNSTATE_RUNNING || + if (s->comp_runstate != RUNSTATE_RUNNING || + s->app_runstate != RUNSTATE_RUNNING || !(service_mask & (UINT64_C(1) << i))) continue; @@ -596,8 +615,7 @@ rte_service_lcore_stop(uint32_t lcore) for (i = 0; i < RTE_SERVICE_NUM_MAX; i++) { int32_t enabled = lcore_states[i].service_mask & (UINT64_C(1) << i); - int32_t service_running = rte_services[i].runstate != - RUNSTATE_STOPPED; + int32_t service_running = rte_service_runstate_get(i); int32_t only_core = rte_services[i].num_mapped_cores == 1; /* if the core is mapped, and the service is running, and this diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index f7a7352..0e1d09b 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -219,6 +219,7 @@ EXPERIMENTAL { rte_eal_hotplug_remove; rte_service_component_register; rte_service_component_unregister; + rte_service_component_runstate_set; rte_service_dump; rte_service_get_by_id; rte_service_get_by_name; diff --git a/test/test/test_service_cores.c b/test/test/test_service_cores.c index 5ae7b20..ddea7f0 100644 --- a/test/test/test_service_cores.c +++ b/test/test/test_service_cores.c @@ -166,9 +166,12 @@ dummy_register(void) "Invalid name"); snprintf(service.name, sizeof(service.name), DUMMY_SERVICE_NAME); - TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), + uint32_t id; + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, &id), "Failed to register valid service"); + rte_service_component_runstate_set(id, 1); + return TEST_SUCCESS; } @@ -470,13 +473,11 @@ service_threaded_test(int mt_safe) if (mt_safe) { service.callback = dummy_mt_safe_cb; service.capabilities |= RTE_SERVICE_CAP_MT_SAFE; - } else { - /* initialize to pass, see callback comment for details */ - test_params[1] = 1; + } else service.callback = dummy_mt_unsafe_cb; - } - TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, NULL), + uint32_t id; + TEST_ASSERT_EQUAL(0, rte_service_component_register(&service, &id), "Register of MT SAFE service failed"); const uint32_t sid = 0; @@ -494,9 +495,26 @@ service_threaded_test(int mt_safe) rte_service_lcore_stop(slcore_1); rte_service_lcore_stop(slcore_2); + TEST_ASSERT_EQUAL(0, test_params[1], + "Service run with component runstate = 0"); + + /* enable backend runstate: the service should run after this */ + rte_service_component_runstate_set(id, 1); + + /* initialize to pass, see callback comment for details */ + if (!mt_safe) + test_params[1] = 1; + + rte_service_lcore_start(slcore_1); + rte_service_lcore_start(slcore_2); + + /* wait for the worker threads to run */ + rte_delay_ms(500); + rte_service_lcore_stop(slcore_1); + rte_service_lcore_stop(slcore_2); + TEST_ASSERT_EQUAL(1, test_params[1], "MT Safe service not run by two cores concurrently"); - TEST_ASSERT_EQUAL(0, rte_service_runstate_set(sid, 0), "Failed to stop MT Safe service"); From patchwork Mon Aug 21 12:58:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27697 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 55C439105; Mon, 21 Aug 2017 14:58:50 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 6A9C07D3A for ; Mon, 21 Aug 2017 14:58:41 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383381" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:40 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:15 +0100 Message-Id: <1503320296-51122-15-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 14/15] service: clarify documentation for register X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" This commit adds a section to the service register function to make it clear that registering a service, must not configure service-cores (eg: adding lcores or changing mappings). Signed-off-by: Harry van Haaren --- lib/librte_eal/common/include/rte_service_component.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/include/rte_service_component.h b/lib/librte_eal/common/include/rte_service_component.h index 5e4573b..ac965cb 100644 --- a/lib/librte_eal/common/include/rte_service_component.h +++ b/lib/librte_eal/common/include/rte_service_component.h @@ -85,8 +85,13 @@ struct rte_service_spec { * * For example the eventdev SW PMD requires CPU cycles to perform its * scheduling. This can be achieved by registering it as a service, and the - * application can then assign CPU resources to it using - * *rte_service_set_coremask*. + * application can then assign CPU resources to that service. + * + * Note that when a service component registers itself, it is not permitted to + * add or remove service-core threads, or modify lcore-to-service mappings. The + * only API that may be called by the service-component is + * *rte_service_component_runstate_set*, which indicates that the service + * component is ready to be executed. * * @param spec The specification of the service to register * @param[out] service_id A pointer to a uint32_t, which will be filled in From patchwork Mon Aug 21 12:58:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Van Haaren, Harry" X-Patchwork-Id: 27698 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 6A193910B; Mon, 21 Aug 2017 14:58:52 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id AE2127D96 for ; Mon, 21 Aug 2017 14:58:42 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 21 Aug 2017 05:58:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.41,409,1498546800"; d="scan'208"; a="1208383389" Received: from silpixa00398672.ir.intel.com ([10.237.223.128]) by fmsmga002.fm.intel.com with ESMTP; 21 Aug 2017 05:58:41 -0700 From: Harry van Haaren To: dev@dpdk.org Cc: Harry van Haaren Date: Mon, 21 Aug 2017 13:58:16 +0100 Message-Id: <1503320296-51122-16-git-send-email-harry.van.haaren@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> References: <1502800360-15782-1-git-send-email-harry.van.haaren@intel.com> <1503320296-51122-1-git-send-email-harry.van.haaren@intel.com> Subject: [dpdk-dev] [PATCH v2 15/15] docs: add notes on service cores API updates X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Add a section on the service cores API changes to 17.11 release notes. Suggested-by: Neil Horman Signed-off-by: Harry van Haaren --- doc/guides/rel_notes/release_17_11.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/guides/rel_notes/release_17_11.rst b/doc/guides/rel_notes/release_17_11.rst index 6e6ba1c..8bf91bd 100644 --- a/doc/guides/rel_notes/release_17_11.rst +++ b/doc/guides/rel_notes/release_17_11.rst @@ -117,6 +117,19 @@ API Changes Also, make sure to start the actual text at the margin. ========================================================= +* **Service cores API updated for usability** + + The service cores API has been changed, removing pointers from the API + where possible, instead using integer IDs to identify each service. This + simplifed application code, aids debugging, and provides better + encapsulation. A summary of the main changes made is as follows: + + * Services identified by ID not by ``rte_service_spec`` pointer + * Reduced API surface by using ``set`` functions instead of enable/disable + * Reworked ``rte_service_register`` to provide the service ID to registrar + * Rework start and stop APIs into ``rte_service_runstate_set`` + * Added API to set runstate of service implementation to indicate readyness + ABI Changes -----------