[dpdk-dev] [PATCH] eal: promote some service core functions to stable

[dpdk-dev] [PATCH] eal: promote some service core functions to stable

[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 eventdev and the sw PMD, and this
commit allows them 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 +++---
 lib/librte_eventdev/Makefile                |  1 -
 lib/librte_eventdev/meson.build             |  1 -
 7 files changed, 9 insertions(+), 22 deletions(-)

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;
diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index 53079f4c2..fdaec7d06 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -11,7 +11,6 @@ LIB = librte_eventdev.a
 LIBABIVER := 6
 
 # build flags
-CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUX),y)
diff --git a/lib/librte_eventdev/meson.build b/lib/librte_eventdev/meson.build
index 6cfe60e1f..f623d74f8 100644
--- a/lib/librte_eventdev/meson.build
+++ b/lib/librte_eventdev/meson.build
@@ -2,7 +2,6 @@
 # Copyright(c) 2017 Intel Corporation
 
 version = 6
-allow_experimental_apis = true
 
 if is_linux
 	cflags += '-DLINUX'
-- 
2.13.6

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Aaron Conole]

Gage Eads <gage.eads@intel.com> writes:

> 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 eventdev and the sw PMD, and this
> commit allows them 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 +++---
>  lib/librte_eventdev/Makefile                |  1 -
>  lib/librte_eventdev/meson.build             |  1 -
>  7 files changed, 9 insertions(+), 22 deletions(-)
>
> 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

I don't think you can remove these.  There are still some experimental
APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
marked that function as experimental and it will cause build breakage).

Maybe I'm mis understanding it?  It would be good to get verification
from Bruce whether that API should not be marked as experimental (it was
just a rename, so not sure...) - maybe that's a follow up for this
patch?

See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example

The odd thing is I only see it on the clang builds - perhaps it's a
missing definition for the clang compiler.

>  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;
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
> index 53079f4c2..fdaec7d06 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -11,7 +11,6 @@ LIB = librte_eventdev.a
>  LIBABIVER := 6
>  
>  # build flags
> -CFLAGS += -DALLOW_EXPERIMENTAL_API
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
>  ifeq ($(CONFIG_RTE_EXEC_ENV_LINUX),y)
> diff --git a/lib/librte_eventdev/meson.build b/lib/librte_eventdev/meson.build
> index 6cfe60e1f..f623d74f8 100644
> --- a/lib/librte_eventdev/meson.build
> +++ b/lib/librte_eventdev/meson.build
> @@ -2,7 +2,6 @@
>  # Copyright(c) 2017 Intel Corporation
>  
>  version = 6
> -allow_experimental_apis = true
>  
>  if is_linux
>  	cflags += '-DLINUX'

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Eads, Gage]

> Gage Eads <gage.eads@intel.com> writes:
> 
> > 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 eventdev and the sw PMD, and
> > this commit allows them 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 +++---
> >  lib/librte_eventdev/Makefile                |  1 -
> >  lib/librte_eventdev/meson.build             |  1 -
> >  7 files changed, 9 insertions(+), 22 deletions(-)
> >
> > 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
> 
> I don't think you can remove these.  There are still some experimental APIs
> (f.e. the rename for rte_cryptodev_sym_session_get_private_data
> marked that function as experimental and it will cause build breakage).
> 
> Maybe I'm mis understanding it?  It would be good to get verification from
> Bruce whether that API should not be marked as experimental (it was just a
> rename, so not sure...) - maybe that's a follow up for this patch?
> 
> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
> 
> The odd thing is I only see it on the clang builds - perhaps it's a missing
> definition for the clang compiler.
> 

You're right, eventdev still uses that experimental API (which this patch is unrelated to). I tested this change with GCC (5.4.0) and it built without errors, which I took to mean no more experimental APIs were in use. That's concerning that GCC didn't catch it.

At any rate, I'll correct this in v2.

Thanks,
Gage

[dpdk-dev] [PATCH v2] eal: promote some service core functions to stable

[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>
---
 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);
 
 #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

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Thu, Jun 20, 2019 at 8:26 PM Aaron Conole <aconole@redhat.com> wrote:

> Gage Eads <gage.eads@intel.com> writes:
>
> > 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 eventdev and the sw PMD, and this
> > commit allows them 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 +++---
> >  lib/librte_eventdev/Makefile                |  1 -
> >  lib/librte_eventdev/meson.build             |  1 -
> >  7 files changed, 9 insertions(+), 22 deletions(-)
> >
> > 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
>
> I don't think you can remove these.  There are still some experimental
> APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
> marked that function as experimental and it will cause build breakage).
>
> Maybe I'm mis understanding it?  It would be good to get verification
> from Bruce whether that API should not be marked as experimental (it was
> just a rename, so not sure...) - maybe that's a follow up for this
> patch?
>
> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
>
> The odd thing is I only see it on the clang builds - perhaps it's a
> missing definition for the clang compiler.
>

Erf, it looks like the __rte_experimental tag is affected by the order in
the declaration of the symbol.

--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -1245,7 +1245,7 @@ struct rte_cryptodev_asym_session * __rte_experimental
  *  - On success return pointer to user data.
  *  - On failure returns NULL.
  */
-void * __rte_experimental
+__rte_experimental void *
 rte_cryptodev_sym_session_get_user_data(
                                        struct rte_cryptodev_sym_session
*sess);

With this, I get the proper warning...



-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Thu, Jun 20, 2019 at 9:45 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Thu, Jun 20, 2019 at 8:26 PM Aaron Conole <aconole@redhat.com> wrote:
>
>> Gage Eads <gage.eads@intel.com> writes:
>>
>> > 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 eventdev and the sw PMD, and this
>> > commit allows them 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 +++---
>> >  lib/librte_eventdev/Makefile                |  1 -
>> >  lib/librte_eventdev/meson.build             |  1 -
>> >  7 files changed, 9 insertions(+), 22 deletions(-)
>> >
>> > 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
>>
>> I don't think you can remove these.  There are still some experimental
>> APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
>> marked that function as experimental and it will cause build breakage).
>>
>> Maybe I'm mis understanding it?  It would be good to get verification
>> from Bruce whether that API should not be marked as experimental (it was
>> just a rename, so not sure...) - maybe that's a follow up for this
>> patch?
>>
>> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
>>
>> The odd thing is I only see it on the clang builds - perhaps it's a
>> missing definition for the clang compiler.
>>
>
> Erf, it looks like the __rte_experimental tag is affected by the order in
> the declaration of the symbol.
>
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -1245,7 +1245,7 @@ struct rte_cryptodev_asym_session *
> __rte_experimental
>   *  - On success return pointer to user data.
>   *  - On failure returns NULL.
>   */
> -void * __rte_experimental
> +__rte_experimental void *
>  rte_cryptodev_sym_session_get_user_data(
>                                         struct rte_cryptodev_sym_session
> *sess);
>
>
https://hastebin.com/micomogoqi.cs

Does it mean that the "void *" type had been marked as deprecated with the
previous syntax?
Requesting compiler experts assistance :-)


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Thu, Jun 20, 2019 at 10:16 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Thu, Jun 20, 2019 at 9:45 PM David Marchand <david.marchand@redhat.com>
> wrote:
>
>>
>>
>> On Thu, Jun 20, 2019 at 8:26 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>>> Gage Eads <gage.eads@intel.com> writes:
>>>
>>> > 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 eventdev and the sw PMD, and
>>> this
>>> > commit allows them 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 +++---
>>> >  lib/librte_eventdev/Makefile                |  1 -
>>> >  lib/librte_eventdev/meson.build             |  1 -
>>> >  7 files changed, 9 insertions(+), 22 deletions(-)
>>> >
>>> > 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
>>>
>>> I don't think you can remove these.  There are still some experimental
>>> APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
>>> marked that function as experimental and it will cause build breakage).
>>>
>>> Maybe I'm mis understanding it?  It would be good to get verification
>>> from Bruce whether that API should not be marked as experimental (it was
>>> just a rename, so not sure...) - maybe that's a follow up for this
>>> patch?
>>>
>>> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
>>>
>>> The odd thing is I only see it on the clang builds - perhaps it's a
>>> missing definition for the clang compiler.
>>>
>>
>> Erf, it looks like the __rte_experimental tag is affected by the order in
>> the declaration of the symbol.
>>
>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>> @@ -1245,7 +1245,7 @@ struct rte_cryptodev_asym_session *
>> __rte_experimental
>>   *  - On success return pointer to user data.
>>   *  - On failure returns NULL.
>>   */
>> -void * __rte_experimental
>> +__rte_experimental void *
>>  rte_cryptodev_sym_session_get_user_data(
>>                                         struct rte_cryptodev_sym_session
>> *sess);
>>
>>
> https://hastebin.com/micomogoqi.cs
>
> Does it mean that the "void *" type had been marked as deprecated with the
> previous syntax?
> Requesting compiler experts assistance :-)
>


Ok, did a new pass on the tree.. found quite some sites where we have
issues (and other discrepancies... I started a new patchset).
Looked at gcc documentation [1], and to me the safer approach would be to
enforce that __rte_experimental is the first thing of a symbol declaration.

Comments?


1: https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Neil Horman]

On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> On Thu, Jun 20, 2019 at 10:16 PM David Marchand <david.marchand@redhat.com>
> wrote:
> 
> >
> >
> > On Thu, Jun 20, 2019 at 9:45 PM David Marchand <david.marchand@redhat.com>
> > wrote:
> >
> >>
> >>
> >> On Thu, Jun 20, 2019 at 8:26 PM Aaron Conole <aconole@redhat.com> wrote:
> >>
> >>> Gage Eads <gage.eads@intel.com> writes:
> >>>
> >>> > 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 eventdev and the sw PMD, and
> >>> this
> >>> > commit allows them 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 +++---
> >>> >  lib/librte_eventdev/Makefile                |  1 -
> >>> >  lib/librte_eventdev/meson.build             |  1 -
> >>> >  7 files changed, 9 insertions(+), 22 deletions(-)
> >>> >
> >>> > 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
> >>>
> >>> I don't think you can remove these.  There are still some experimental
> >>> APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
> >>> marked that function as experimental and it will cause build breakage).
> >>>
> >>> Maybe I'm mis understanding it?  It would be good to get verification
> >>> from Bruce whether that API should not be marked as experimental (it was
> >>> just a rename, so not sure...) - maybe that's a follow up for this
> >>> patch?
> >>>
> >>> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
> >>>
> >>> The odd thing is I only see it on the clang builds - perhaps it's a
> >>> missing definition for the clang compiler.
> >>>
> >>
> >> Erf, it looks like the __rte_experimental tag is affected by the order in
> >> the declaration of the symbol.
> >>
> >> --- a/lib/librte_cryptodev/rte_cryptodev.h
> >> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> >> @@ -1245,7 +1245,7 @@ struct rte_cryptodev_asym_session *
> >> __rte_experimental
> >>   *  - On success return pointer to user data.
> >>   *  - On failure returns NULL.
> >>   */
> >> -void * __rte_experimental
> >> +__rte_experimental void *
> >>  rte_cryptodev_sym_session_get_user_data(
> >>                                         struct rte_cryptodev_sym_session
> >> *sess);
> >>
> >>
> > https://hastebin.com/micomogoqi.cs
> >
> > Does it mean that the "void *" type had been marked as deprecated with the
> > previous syntax?
> > Requesting compiler experts assistance :-)
> >
> 
> 
> Ok, did a new pass on the tree.. found quite some sites where we have
> issues (and other discrepancies... I started a new patchset).
> Looked at gcc documentation [1], and to me the safer approach would be to
> enforce that __rte_experimental is the first thing of a symbol declaration.
> 
> Comments?
> 
Yes, thats the only way it works, in fact I'm suprised gcc didn't throw an error
about expecting an asm statement if you put it anywhere else

Neil

> 
> 1: https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html
> 
> -- 
> David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > Ok, did a new pass on the tree.. found quite some sites where we have
> > issues (and other discrepancies... I started a new patchset).
> > Looked at gcc documentation [1], and to me the safer approach would be to
> > enforce that __rte_experimental is the first thing of a symbol
> declaration.
> >
> > Comments?
> >
> Yes, thats the only way it works, in fact I'm suprised gcc didn't throw an
> error
> about expecting an asm statement if you put it anywhere else
>

- I tried this, but then I hit issues with inlines.
Like for example:

static inline char * __rte_experimental
rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
{
  return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
}

I did not find a way to move the __rte_experimental tag without getting
warnings.
If I try to compile some sources which includes rte_mbuf.h but without
-DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
complaining that rte_mbuf_buf_addr() is deprecated, even if this inline is
not called.

For now, I hid all those inlines under the ALLOW_EXPERIMENTAL_API flag.


- I have accumulated other fixes and I dropped all the marking in the .c
files.
This ended up with a huge series...

 118 files changed, 867 insertions(+), 646 deletions(-)

https://github.com/david-marchand/dpdk/commits/experimental

I will let the week end pass on this.


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Neil Horman]

On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > Ok, did a new pass on the tree.. found quite some sites where we have
> > > issues (and other discrepancies... I started a new patchset).
> > > Looked at gcc documentation [1], and to me the safer approach would be to
> > > enforce that __rte_experimental is the first thing of a symbol
> > declaration.
> > >
> > > Comments?
> > >
> > Yes, thats the only way it works, in fact I'm suprised gcc didn't throw an
> > error
> > about expecting an asm statement if you put it anywhere else
> >
> 
> - I tried this, but then I hit issues with inlines.
> Like for example:
> 
> static inline char * __rte_experimental
> rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> {
>   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> }
> 
> I did not find a way to move the __rte_experimental tag without getting
> warnings.
Right, thats the way its supposed to work on gcc/icc/clang.  function attributes
must be declared between the return type and the function name, anything else
will generate compiler warnings/errors.  Because __rte_experimental expands to a
__attribute__(...), you have to place it there.

> If I try to compile some sources which includes rte_mbuf.h but without
> -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> complaining that rte_mbuf_buf_addr() is deprecated, even if this inline is
> not called.
> 
Thats...odd.  I wonder if thats an artifact of the function being marked as
inline.  The compiler is supposed to insert the warning for any remaining calls
after dead code eliminitaion.  If the function is inline, I wonder if the
compiler conservatively inserts the warning because it got expanded into another
function, when it can't tell if it will be entirely elimintated.  Can you
provide a code sample that demonstrates this?

> For now, I hid all those inlines under the ALLOW_EXPERIMENTAL_API flag.
> 
> 
> - I have accumulated other fixes and I dropped all the marking in the .c
> files.
> This ended up with a huge series...
> 
>  118 files changed, 867 insertions(+), 646 deletions(-)
> 
> https://github.com/david-marchand/dpdk/commits/experimental
> 
> I will let the week end pass on this.
> 
> 
> -- 
> David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Fri, Jun 21, 2019 at 7:41 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> > On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com>
> wrote:
> >
> > > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > > Ok, did a new pass on the tree.. found quite some sites where we have
> > > > issues (and other discrepancies... I started a new patchset).
> > > > Looked at gcc documentation [1], and to me the safer approach would
> be to
> > > > enforce that __rte_experimental is the first thing of a symbol
> > > declaration.
> > > >
> > > > Comments?
> > > >
> > > Yes, thats the only way it works, in fact I'm suprised gcc didn't
> throw an
> > > error
> > > about expecting an asm statement if you put it anywhere else
> > >
> >
> > - I tried this, but then I hit issues with inlines.
> > Like for example:
> >
> > static inline char * __rte_experimental
> > rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> > {
> >   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> > }
> >
> > I did not find a way to move the __rte_experimental tag without getting
> > warnings.
> Right, thats the way its supposed to work on gcc/icc/clang.  function
> attributes
> must be declared between the return type and the function name, anything
> else
> will generate compiler warnings/errors.  Because __rte_experimental
> expands to a
> __attribute__(...), you have to place it there.
>
> > If I try to compile some sources which includes rte_mbuf.h but without
> > -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> > complaining that rte_mbuf_buf_addr() is deprecated, even if this inline
> is
> > not called.
> >
> Thats...odd.  I wonder if thats an artifact of the function being marked as
> inline.  The compiler is supposed to insert the warning for any remaining
> calls
> after dead code eliminitaion.  If the function is inline, I wonder if the
> compiler conservatively inserts the warning because it got expanded into
> another
> function, when it can't tell if it will be entirely elimintated.  Can you
> provide a code sample that demonstrates this?
>
>
rte_mbuf_buf_addr() is called in rte_mbuf_data_addr_default(), both of them
are unused by the includers of rte_mbuf.h.


Reproduced it like this:

[dmarchan@dmarchan ~]$ cat deprecated.c
__attribute__((deprecated)) static inline void *plap(void)
{
return 0;
}

__attribute__((deprecated)) static inline void *plep(void)
{
plap();
return 0;
}

int main(int argc, char *argv[])
{
return 0;
}
[dmarchan@dmarchan ~]$ gcc -o deprecated -Wall deprecated.c
deprecated.c: In function ‘plep’:
deprecated.c:8:2: warning: ‘plap’ is deprecated (declared at
deprecated.c:1) [-Wdeprecated-declarations]
  plap();
  ^

-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Neil Horman]

On Fri, Jun 21, 2019 at 09:58:41PM +0200, David Marchand wrote:
> On Fri, Jun 21, 2019 at 7:41 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> > > On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com>
> > wrote:
> > >
> > > > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > > > Ok, did a new pass on the tree.. found quite some sites where we have
> > > > > issues (and other discrepancies... I started a new patchset).
> > > > > Looked at gcc documentation [1], and to me the safer approach would
> > be to
> > > > > enforce that __rte_experimental is the first thing of a symbol
> > > > declaration.
> > > > >
> > > > > Comments?
> > > > >
> > > > Yes, thats the only way it works, in fact I'm suprised gcc didn't
> > throw an
> > > > error
> > > > about expecting an asm statement if you put it anywhere else
> > > >
> > >
> > > - I tried this, but then I hit issues with inlines.
> > > Like for example:
> > >
> > > static inline char * __rte_experimental
> > > rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> > > {
> > >   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> > > }
> > >
> > > I did not find a way to move the __rte_experimental tag without getting
> > > warnings.
> > Right, thats the way its supposed to work on gcc/icc/clang.  function
> > attributes
> > must be declared between the return type and the function name, anything
> > else
> > will generate compiler warnings/errors.  Because __rte_experimental
> > expands to a
> > __attribute__(...), you have to place it there.
> >
> > > If I try to compile some sources which includes rte_mbuf.h but without
> > > -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> > > complaining that rte_mbuf_buf_addr() is deprecated, even if this inline
> > is
> > > not called.
> > >
> > Thats...odd.  I wonder if thats an artifact of the function being marked as
> > inline.  The compiler is supposed to insert the warning for any remaining
> > calls
> > after dead code eliminitaion.  If the function is inline, I wonder if the
> > compiler conservatively inserts the warning because it got expanded into
> > another
> > function, when it can't tell if it will be entirely elimintated.  Can you
> > provide a code sample that demonstrates this?
> >
> >
> rte_mbuf_buf_addr() is called in rte_mbuf_data_addr_default(), both of them
> are unused by the includers of rte_mbuf.h.
> 
> 
> Reproduced it like this:
> 
> [dmarchan@dmarchan ~]$ cat deprecated.c
> __attribute__((deprecated)) static inline void *plap(void)
> {
> return 0;
> }
> 
> __attribute__((deprecated)) static inline void *plep(void)
> {
> plap();
> return 0;
> }
> 
> int main(int argc, char *argv[])
> {
> return 0;
> }
> [dmarchan@dmarchan ~]$ gcc -o deprecated -Wall deprecated.c
> deprecated.c: In function ‘plep’:
> deprecated.c:8:2: warning: ‘plap’ is deprecated (declared at
> deprecated.c:1) [-Wdeprecated-declarations]
>   plap();
>   ^
> 
Hmm, yes, that seems buggy to me.  I wonder if you are seeing this bug in
action:

https://gcc.gnu.org/bugzilla//show_bug.cgi?id=80680

Seem like the behavior fits.  It would be interesting to know if clang and icc
suffer from the same issue

Neil

> -- 
> David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[David Marchand]

On Sat, Jun 22, 2019 at 6:17 PM Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Jun 21, 2019 at 09:58:41PM +0200, David Marchand wrote:
> > On Fri, Jun 21, 2019 at 7:41 PM Neil Horman <nhorman@tuxdriver.com>
> wrote:
> >
> > > On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> > > > On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com>
> > > wrote:
> > > >
> > > > > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > > > > Ok, did a new pass on the tree.. found quite some sites where we
> have
> > > > > > issues (and other discrepancies... I started a new patchset).
> > > > > > Looked at gcc documentation [1], and to me the safer approach
> would
> > > be to
> > > > > > enforce that __rte_experimental is the first thing of a symbol
> > > > > declaration.
> > > > > >
> > > > > > Comments?
> > > > > >
> > > > > Yes, thats the only way it works, in fact I'm suprised gcc didn't
> > > throw an
> > > > > error
> > > > > about expecting an asm statement if you put it anywhere else
> > > > >
> > > >
> > > > - I tried this, but then I hit issues with inlines.
> > > > Like for example:
> > > >
> > > > static inline char * __rte_experimental
> > > > rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> > > > {
> > > >   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> > > > }
> > > >
> > > > I did not find a way to move the __rte_experimental tag without
> getting
> > > > warnings.
> > > Right, thats the way its supposed to work on gcc/icc/clang.  function
> > > attributes
> > > must be declared between the return type and the function name,
> anything
> > > else
> > > will generate compiler warnings/errors.  Because __rte_experimental
> > > expands to a
> > > __attribute__(...), you have to place it there.
> > >
> > > > If I try to compile some sources which includes rte_mbuf.h but
> without
> > > > -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> > > > complaining that rte_mbuf_buf_addr() is deprecated, even if this
> inline
> > > is
> > > > not called.
> > > >
> > > Thats...odd.  I wonder if thats an artifact of the function being
> marked as
> > > inline.  The compiler is supposed to insert the warning for any
> remaining
> > > calls
> > > after dead code eliminitaion.  If the function is inline, I wonder if
> the
> > > compiler conservatively inserts the warning because it got expanded
> into
> > > another
> > > function, when it can't tell if it will be entirely elimintated.  Can
> you
> > > provide a code sample that demonstrates this?
> > >
> > >
> > rte_mbuf_buf_addr() is called in rte_mbuf_data_addr_default(), both of
> them
> > are unused by the includers of rte_mbuf.h.
> >
> >
> > Reproduced it like this:
> >
> > [dmarchan@dmarchan ~]$ cat deprecated.c
> > __attribute__((deprecated)) static inline void *plap(void)
> > {
> > return 0;
> > }
> >
> > __attribute__((deprecated)) static inline void *plep(void)
> > {
> > plap();
> > return 0;
> > }
> >
> > int main(int argc, char *argv[])
> > {
> > return 0;
> > }
> > [dmarchan@dmarchan ~]$ gcc -o deprecated -Wall deprecated.c
> > deprecated.c: In function ‘plep’:
> > deprecated.c:8:2: warning: ‘plap’ is deprecated (declared at
> > deprecated.c:1) [-Wdeprecated-declarations]
> >   plap();
> >   ^
> >
> Hmm, yes, that seems buggy to me.  I wonder if you are seeing this bug in
> action:
>
> https://gcc.gnu.org/bugzilla//show_bug.cgi?id=80680


It has the same flavor yes.
Currently using gcc version 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC)



>
> Seem like the behavior fits.  It would be interesting to know if clang and
> icc
> suffer from the same issue
>

Just tried, clang is fine.
clang version 3.4.2 (tags/RELEASE_34/dot2-final)


Actually, I went and protected this call to rte_mbuf_buf_addr().
And with just this, it builds fine.
I think I am going to take this approach, just a little comment :-).


-- 
David Marchand

Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable

[Neil Horman]

On Sat, Jun 22, 2019 at 07:51:10PM +0200, David Marchand wrote:
> On Sat, Jun 22, 2019 at 6:17 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Fri, Jun 21, 2019 at 09:58:41PM +0200, David Marchand wrote:
> > > On Fri, Jun 21, 2019 at 7:41 PM Neil Horman <nhorman@tuxdriver.com>
> > wrote:
> > >
> > > > On Fri, Jun 21, 2019 at 06:47:31PM +0200, David Marchand wrote:
> > > > > On Fri, Jun 21, 2019 at 6:28 PM Neil Horman <nhorman@tuxdriver.com>
> > > > wrote:
> > > > >
> > > > > > On Fri, Jun 21, 2019 at 02:45:45PM +0200, David Marchand wrote:
> > > > > > > Ok, did a new pass on the tree.. found quite some sites where we
> > have
> > > > > > > issues (and other discrepancies... I started a new patchset).
> > > > > > > Looked at gcc documentation [1], and to me the safer approach
> > would
> > > > be to
> > > > > > > enforce that __rte_experimental is the first thing of a symbol
> > > > > > declaration.
> > > > > > >
> > > > > > > Comments?
> > > > > > >
> > > > > > Yes, thats the only way it works, in fact I'm suprised gcc didn't
> > > > throw an
> > > > > > error
> > > > > > about expecting an asm statement if you put it anywhere else
> > > > > >
> > > > >
> > > > > - I tried this, but then I hit issues with inlines.
> > > > > Like for example:
> > > > >
> > > > > static inline char * __rte_experimental
> > > > > rte_mbuf_buf_addr(struct rte_mbuf *mb, struct rte_mempool *mp)
> > > > > {
> > > > >   return (char *)mb + sizeof(*mb) + rte_pktmbuf_priv_size(mp);
> > > > > }
> > > > >
> > > > > I did not find a way to move the __rte_experimental tag without
> > getting
> > > > > warnings.
> > > > Right, thats the way its supposed to work on gcc/icc/clang.  function
> > > > attributes
> > > > must be declared between the return type and the function name,
> > anything
> > > > else
> > > > will generate compiler warnings/errors.  Because __rte_experimental
> > > > expands to a
> > > > __attribute__(...), you have to place it there.
> > > >
> > > > > If I try to compile some sources which includes rte_mbuf.h but
> > without
> > > > > -DALLOW_EXPERIMENTAL_API, then gcc errors at including the header,
> > > > > complaining that rte_mbuf_buf_addr() is deprecated, even if this
> > inline
> > > > is
> > > > > not called.
> > > > >
> > > > Thats...odd.  I wonder if thats an artifact of the function being
> > marked as
> > > > inline.  The compiler is supposed to insert the warning for any
> > remaining
> > > > calls
> > > > after dead code eliminitaion.  If the function is inline, I wonder if
> > the
> > > > compiler conservatively inserts the warning because it got expanded
> > into
> > > > another
> > > > function, when it can't tell if it will be entirely elimintated.  Can
> > you
> > > > provide a code sample that demonstrates this?
> > > >
> > > >
> > > rte_mbuf_buf_addr() is called in rte_mbuf_data_addr_default(), both of
> > them
> > > are unused by the includers of rte_mbuf.h.
> > >
> > >
> > > Reproduced it like this:
> > >
> > > [dmarchan@dmarchan ~]$ cat deprecated.c
> > > __attribute__((deprecated)) static inline void *plap(void)
> > > {
> > > return 0;
> > > }
> > >
> > > __attribute__((deprecated)) static inline void *plep(void)
> > > {
> > > plap();
> > > return 0;
> > > }
> > >
> > > int main(int argc, char *argv[])
> > > {
> > > return 0;
> > > }
> > > [dmarchan@dmarchan ~]$ gcc -o deprecated -Wall deprecated.c
> > > deprecated.c: In function ‘plep’:
> > > deprecated.c:8:2: warning: ‘plap’ is deprecated (declared at
> > > deprecated.c:1) [-Wdeprecated-declarations]
> > >   plap();
> > >   ^
> > >
> > Hmm, yes, that seems buggy to me.  I wonder if you are seeing this bug in
> > action:
> >
> > https://gcc.gnu.org/bugzilla//show_bug.cgi?id=80680
> 
> 
> It has the same flavor yes.
> Currently using gcc version 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC)
> 
> 
> 
> >
> > Seem like the behavior fits.  It would be interesting to know if clang and
> > icc
> > suffer from the same issue
> >
> 
> Just tried, clang is fine.
> clang version 3.4.2 (tags/RELEASE_34/dot2-final)
> 
> 
> Actually, I went and protected this call to rte_mbuf_buf_addr().
> And with just this, it builds fine.
> I think I am going to take this approach, just a little comment :-).
> 
Thats probably the best workaround for this at the moment, I agree.  I'll add a
comment to the gcc bug and see if we can't get some movement on it

thanks
Neil

> 
> -- 
> David Marchand

Re: [dpdk-dev] [PATCH v2] eal: promote some service core functions to stable

[David Marchand]

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
>



-- 
David Marchand

Re: [dpdk-dev] [PATCH v2] eal: promote some service core functions to stable

[Eads, Gage]

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.

Re: [dpdk-dev] [PATCH v2] eal: promote some service core functions to stable

[Thomas Monjalon]

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