[v2] eal: promote some service core functions to stable

Message ID 20190620190227.18206-1-gage.eads@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] eal: promote some service core functions to stable |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Eads, Gage June 20, 2019, 7:02 p.m. UTC
  The functions rte_service_may_be_active(), rte_service_lcore_attr_get(),
and rte_service_attr_reset_all() were introduced nearly a year ago in DPDK
18.08. They can be considered non-experimental for the 19.08 release.

rte_service_may_be_active() is used by the sw PMD, and this commit allows
it to not need any experimental API.

Signed-off-by: Gage Eads <gage.eads@intel.com>
---
 drivers/event/sw/Makefile                   |  1 -
 drivers/event/sw/meson.build                |  1 -
 lib/librte_eal/common/include/rte_service.h | 15 +++------------
 lib/librte_eal/common/rte_service.c         |  6 +++---
 lib/librte_eal/rte_eal_version.map          |  6 +++---
 5 files changed, 9 insertions(+), 20 deletions(-)

v2: add allow-experimental flag back to eventdev, which still uses an
    experimental cryptodev API
  

Comments

David Marchand June 27, 2019, 12:48 p.m. UTC | #1
On Thu, Jun 20, 2019 at 9:03 PM Gage Eads <gage.eads@intel.com> wrote:

> The functions rte_service_may_be_active(), rte_service_lcore_attr_get(),
> and rte_service_attr_reset_all() were introduced nearly a year ago in DPDK
> 18.08. They can be considered non-experimental for the 19.08 release.
>
> rte_service_may_be_active() is used by the sw PMD, and this commit allows
> it to not need any experimental API.
>
> Signed-off-by: Gage Eads <gage.eads@intel.com>
> ---
>  drivers/event/sw/Makefile                   |  1 -
>  drivers/event/sw/meson.build                |  1 -
>  lib/librte_eal/common/include/rte_service.h | 15 +++------------
>  lib/librte_eal/common/rte_service.c         |  6 +++---
>  lib/librte_eal/rte_eal_version.map          |  6 +++---
>  5 files changed, 9 insertions(+), 20 deletions(-)
>
> v2: add allow-experimental flag back to eventdev, which still uses an
>     experimental cryptodev API
>
> diff --git a/drivers/event/sw/Makefile b/drivers/event/sw/Makefile
> index 81236a392..c6600e836 100644
> --- a/drivers/event/sw/Makefile
> +++ b/drivers/event/sw/Makefile
> @@ -7,7 +7,6 @@ include $(RTE_SDK)/mk/rte.vars.mk
>  LIB = librte_pmd_sw_event.a
>
>  # build flags
> -CFLAGS += -DALLOW_EXPERIMENTAL_API
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
>  # for older GCC versions, allow us to initialize an event using
> diff --git a/drivers/event/sw/meson.build b/drivers/event/sw/meson.build
> index 30d221647..985012219 100644
> --- a/drivers/event/sw/meson.build
> +++ b/drivers/event/sw/meson.build
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2017 Intel Corporation
>
> -allow_experimental_apis = true
>  sources = files('sw_evdev_scheduler.c',
>         'sw_evdev_selftest.c',
>         'sw_evdev_worker.c',
> diff --git a/lib/librte_eal/common/include/rte_service.h
> b/lib/librte_eal/common/include/rte_service.h
> index bf25aec35..d8701dd4c 100644
> --- a/lib/librte_eal/common/include/rte_service.h
> +++ b/lib/librte_eal/common/include/rte_service.h
> @@ -162,9 +162,6 @@ int32_t rte_service_runstate_set(uint32_t id, uint32_t
> runstate);
>  int32_t rte_service_runstate_get(uint32_t id);
>
>  /**
> - * @warning
> - * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> - *
>   * This function returns whether the service may be currently executing on
>   * at least one lcore, or definitely is not. This function can be used to
>   * determine if, after setting the service runstate to stopped, the
> service
> @@ -178,7 +175,7 @@ int32_t rte_service_runstate_get(uint32_t id);
>   * @retval 0 Service is not running on any lcore
>   * @retval -EINVAL Invalid service id
>   */
> -int32_t __rte_experimental
> +int32_t
>  rte_service_may_be_active(uint32_t id);
>
>  /**
> @@ -389,9 +386,6 @@ int32_t rte_service_attr_reset_all(uint32_t id);
>  #define RTE_SERVICE_LCORE_ATTR_LOOPS 0
>
>  /**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
>   * Get an attribute from a service core.
>   *
>   * @param lcore Id of the service core.
> @@ -401,14 +395,11 @@ int32_t rte_service_attr_reset_all(uint32_t id);
>   *         -EINVAL Invalid lcore, attr_id or attr_value was NULL.
>   *         -ENOTSUP lcore is not a service core.
>   */
> -int32_t __rte_experimental
> +int32_t
>  rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
>                            uint64_t *attr_value);
>
>  /**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
>   * Reset all attribute values of a service core.
>   *
>   * @param lcore The service core to reset all the statistics of
> @@ -416,7 +407,7 @@ rte_service_lcore_attr_get(uint32_t lcore, uint32_t
> attr_id,
>   *         -EINVAL Invalid service id provided
>   *         -ENOTSUP lcore is not a service core.
>   */
> -int32_t __rte_experimental
> +int32_t
>  rte_service_lcore_attr_reset_all(uint32_t lcore);
>

It seems we want to return errors via the return code.
Error codes from errno.h are int so we are fine with int only.

What is the reason why this is returning a int32_t ?



>  #ifdef __cplusplus
> diff --git a/lib/librte_eal/common/rte_service.c
> b/lib/librte_eal/common/rte_service.c
> index 5f75e5a53..c3653ebae 100644
> --- a/lib/librte_eal/common/rte_service.c
> +++ b/lib/librte_eal/common/rte_service.c
> @@ -378,7 +378,7 @@ service_run(uint32_t i, int lcore, struct core_state
> *cs, uint64_t service_mask)
>         return 0;
>  }
>
> -int32_t __rte_experimental
> +int32_t
>  rte_service_may_be_active(uint32_t id)
>  {
>         uint32_t ids[RTE_MAX_LCORE] = {0};
> @@ -754,7 +754,7 @@ rte_service_attr_get(uint32_t id, uint32_t attr_id,
> uint64_t *attr_value)
>         }
>  }
>
> -int32_t __rte_experimental
> +int32_t
>  rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
>                            uint64_t *attr_value)
>  {
> @@ -814,7 +814,7 @@ rte_service_attr_reset_all(uint32_t id)
>         return 0;
>  }
>
> -int32_t __rte_experimental
> +int32_t
>  rte_service_lcore_attr_reset_all(uint32_t lcore)
>  {
>         struct core_state *cs;
> diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> index 824edf0ff..fc26b9503 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -292,6 +292,9 @@ DPDK_19.08 {
>
>         rte_lcore_index;
>         rte_lcore_to_socket_id;
> +       rte_service_lcore_attr_get;
> +       rte_service_lcore_attr_reset_all;
> +       rte_service_may_be_active;
>
>  } DPDK_19.05;
>
> @@ -383,9 +386,6 @@ EXPERIMENTAL {
>         rte_mp_sendmsg;
>         rte_option_register;
>         rte_realloc_socket;
> -       rte_service_lcore_attr_get;
> -       rte_service_lcore_attr_reset_all;
> -       rte_service_may_be_active;
>
>         # added in 19.08
>         rte_lcore_cpuset;
> --
> 2.13.6
>
  
Eads, Gage June 27, 2019, 4:25 p.m. UTC | #2
On Thu, Jun 20, 2019 at 9:03 PM Gage Eads <gage.eads@intel.com<mailto:gage.eads@intel.com>> wrote:
The functions rte_service_may_be_active(), rte_service_lcore_attr_get(),
and rte_service_attr_reset_all() were introduced nearly a year ago in DPDK
18.08. They can be considered non-experimental for the 19.08 release.

rte_service_may_be_active() is used by the sw PMD, and this commit allows
it to not need any experimental API.

Signed-off-by: Gage Eads <gage.eads@intel.com<mailto:gage.eads@intel.com>>
---
 drivers/event/sw/Makefile                   |  1 -
 drivers/event/sw/meson.build                |  1 -
 lib/librte_eal/common/include/rte_service.h | 15 +++------------
 lib/librte_eal/common/rte_service.c         |  6 +++---
 lib/librte_eal/rte_eal_version.map          |  6 +++---
 5 files changed, 9 insertions(+), 20 deletions(-)

v2: add allow-experimental flag back to eventdev, which still uses an
    experimental cryptodev API

diff --git a/drivers/event/sw/Makefile b/drivers/event/sw/Makefile
index 81236a392..c6600e836 100644
--- a/drivers/event/sw/Makefile
+++ b/drivers/event/sw/Makefile
@@ -7,7 +7,6 @@ include $(RTE_SDK)/mk/rte.vars.mk<http://rte.vars.mk>
 LIB = librte_pmd_sw_event.a

 # build flags
-CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 # for older GCC versions, allow us to initialize an event using
diff --git a/drivers/event/sw/meson.build b/drivers/event/sw/meson.build
index 30d221647..985012219 100644
--- a/drivers/event/sw/meson.build
+++ b/drivers/event/sw/meson.build
@@ -1,7 +1,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation

-allow_experimental_apis = true
 sources = files('sw_evdev_scheduler.c',
        'sw_evdev_selftest.c',
        'sw_evdev_worker.c',
diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index bf25aec35..d8701dd4c 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -162,9 +162,6 @@ int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate);
 int32_t rte_service_runstate_get(uint32_t id);

 /**
- * @warning
- * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
- *
  * This function returns whether the service may be currently executing on
  * at least one lcore, or definitely is not. This function can be used to
  * determine if, after setting the service runstate to stopped, the service
@@ -178,7 +175,7 @@ int32_t rte_service_runstate_get(uint32_t id);
  * @retval 0 Service is not running on any lcore
  * @retval -EINVAL Invalid service id
  */
-int32_t __rte_experimental
+int32_t
 rte_service_may_be_active(uint32_t id);

 /**
@@ -389,9 +386,6 @@ int32_t rte_service_attr_reset_all(uint32_t id);
 #define RTE_SERVICE_LCORE_ATTR_LOOPS 0

 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Get an attribute from a service core.
  *
  * @param lcore Id of the service core.
@@ -401,14 +395,11 @@ int32_t rte_service_attr_reset_all(uint32_t id);
  *         -EINVAL Invalid lcore, attr_id or attr_value was NULL.
  *         -ENOTSUP lcore is not a service core.
  */
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
                           uint64_t *attr_value);

 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Reset all attribute values of a service core.
  *
  * @param lcore The service core to reset all the statistics of
@@ -416,7 +407,7 @@ rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
  *         -EINVAL Invalid service id provided
  *         -ENOTSUP lcore is not a service core.
  */
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_reset_all(uint32_t lcore);

It seems we want to return errors via the return code.
Error codes from errno.h are int so we are fine with int only.

What is the reason why this is returning a int32_t ?

Simply to match the other (non-experimental) rte_service_* functions that return int32_t.
  
Thomas Monjalon July 8, 2019, 10:23 a.m. UTC | #3
20/06/2019 21:02, Gage Eads:
> The functions rte_service_may_be_active(), rte_service_lcore_attr_get(),
> and rte_service_attr_reset_all() were introduced nearly a year ago in DPDK
> 18.08. They can be considered non-experimental for the 19.08 release.
> 
> rte_service_may_be_active() is used by the sw PMD, and this commit allows
> it to not need any experimental API.
> 
> Signed-off-by: Gage Eads <gage.eads@intel.com>

Applied, thanks
  

Patch

diff --git a/drivers/event/sw/Makefile b/drivers/event/sw/Makefile
index 81236a392..c6600e836 100644
--- a/drivers/event/sw/Makefile
+++ b/drivers/event/sw/Makefile
@@ -7,7 +7,6 @@  include $(RTE_SDK)/mk/rte.vars.mk
 LIB = librte_pmd_sw_event.a
 
 # build flags
-CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 # for older GCC versions, allow us to initialize an event using
diff --git a/drivers/event/sw/meson.build b/drivers/event/sw/meson.build
index 30d221647..985012219 100644
--- a/drivers/event/sw/meson.build
+++ b/drivers/event/sw/meson.build
@@ -1,7 +1,6 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-allow_experimental_apis = true
 sources = files('sw_evdev_scheduler.c',
 	'sw_evdev_selftest.c',
 	'sw_evdev_worker.c',
diff --git a/lib/librte_eal/common/include/rte_service.h b/lib/librte_eal/common/include/rte_service.h
index bf25aec35..d8701dd4c 100644
--- a/lib/librte_eal/common/include/rte_service.h
+++ b/lib/librte_eal/common/include/rte_service.h
@@ -162,9 +162,6 @@  int32_t rte_service_runstate_set(uint32_t id, uint32_t runstate);
 int32_t rte_service_runstate_get(uint32_t id);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
- *
  * This function returns whether the service may be currently executing on
  * at least one lcore, or definitely is not. This function can be used to
  * determine if, after setting the service runstate to stopped, the service
@@ -178,7 +175,7 @@  int32_t rte_service_runstate_get(uint32_t id);
  * @retval 0 Service is not running on any lcore
  * @retval -EINVAL Invalid service id
  */
-int32_t __rte_experimental
+int32_t
 rte_service_may_be_active(uint32_t id);
 
 /**
@@ -389,9 +386,6 @@  int32_t rte_service_attr_reset_all(uint32_t id);
 #define RTE_SERVICE_LCORE_ATTR_LOOPS 0
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Get an attribute from a service core.
  *
  * @param lcore Id of the service core.
@@ -401,14 +395,11 @@  int32_t rte_service_attr_reset_all(uint32_t id);
  *         -EINVAL Invalid lcore, attr_id or attr_value was NULL.
  *         -ENOTSUP lcore is not a service core.
  */
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
 			   uint64_t *attr_value);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Reset all attribute values of a service core.
  *
  * @param lcore The service core to reset all the statistics of
@@ -416,7 +407,7 @@  rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
  *         -EINVAL Invalid service id provided
  *         -ENOTSUP lcore is not a service core.
  */
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_reset_all(uint32_t lcore);
 
 #ifdef __cplusplus
diff --git a/lib/librte_eal/common/rte_service.c b/lib/librte_eal/common/rte_service.c
index 5f75e5a53..c3653ebae 100644
--- a/lib/librte_eal/common/rte_service.c
+++ b/lib/librte_eal/common/rte_service.c
@@ -378,7 +378,7 @@  service_run(uint32_t i, int lcore, struct core_state *cs, uint64_t service_mask)
 	return 0;
 }
 
-int32_t __rte_experimental
+int32_t
 rte_service_may_be_active(uint32_t id)
 {
 	uint32_t ids[RTE_MAX_LCORE] = {0};
@@ -754,7 +754,7 @@  rte_service_attr_get(uint32_t id, uint32_t attr_id, uint64_t *attr_value)
 	}
 }
 
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_get(uint32_t lcore, uint32_t attr_id,
 			   uint64_t *attr_value)
 {
@@ -814,7 +814,7 @@  rte_service_attr_reset_all(uint32_t id)
 	return 0;
 }
 
-int32_t __rte_experimental
+int32_t
 rte_service_lcore_attr_reset_all(uint32_t lcore)
 {
 	struct core_state *cs;
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 824edf0ff..fc26b9503 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -292,6 +292,9 @@  DPDK_19.08 {
 
 	rte_lcore_index;
 	rte_lcore_to_socket_id;
+	rte_service_lcore_attr_get;
+	rte_service_lcore_attr_reset_all;
+	rte_service_may_be_active;
 
 } DPDK_19.05;
 
@@ -383,9 +386,6 @@  EXPERIMENTAL {
 	rte_mp_sendmsg;
 	rte_option_register;
 	rte_realloc_socket;
-	rte_service_lcore_attr_get;
-	rte_service_lcore_attr_reset_all;
-	rte_service_may_be_active;
 
 	# added in 19.08
 	rte_lcore_cpuset;