[RFC,2/7] telemetry: add uint type as alias for u64

Message ID 20221213182730.97065-3-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Standardize telemetry int types |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Bruce Richardson Dec. 13, 2022, 6:27 p.m. UTC
  For future standardization on the "uint" name for unsigned values rather
than the existing "u64" one, we can for now:
* rename all internal values to use uint rather than u64
* add new function names to alias the existing u64 ones

Suggested-by: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
 lib/telemetry/telemetry.c      | 16 +++++++--------
 lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
 lib/telemetry/telemetry_data.h |  4 ++--
 lib/telemetry/version.map      |  7 +++++++
 5 files changed, 73 insertions(+), 18 deletions(-)
  

Comments

Tyler Retzlaff Dec. 14, 2022, 5:38 p.m. UTC | #1
On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> For future standardization on the "uint" name for unsigned values rather
> than the existing "u64" one, we can for now:
> * rename all internal values to use uint rather than u64
> * add new function names to alias the existing u64 ones
> 
> Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
>  lib/telemetry/telemetry.c      | 16 +++++++--------
>  lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
>  lib/telemetry/telemetry_data.h |  4 ++--
>  lib/telemetry/version.map      |  7 +++++++
>  5 files changed, 73 insertions(+), 18 deletions(-)
> 
> diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> index c2ad65effe..60877dae0a 100644
> --- a/lib/telemetry/rte_telemetry.h
> +++ b/lib/telemetry/rte_telemetry.h
> @@ -8,6 +8,8 @@
>  #ifndef _RTE_TELEMETRY_H_
>  #define _RTE_TELEMETRY_H_
>  
> +#include <rte_compat.h>
> +
>  #ifdef __cplusplus
>  extern "C" {
>  #endif
> @@ -121,6 +123,22 @@ int
>  rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
>  
>  /**

when adding __rte_experimental api i have been asked to add the
following boilerplate documentation block. i'm not pushing it, just
recalling it is what i get asked for, so in case it's something we do?
see lib/eal/include/rte_thread.h as an example


```
 * @warning
 * @b EXPERIMENTAL: this API may change without prior notice.
```

> + * Add an unsigned value to an array.
> + * The array must have been started by rte_tel_data_start_array() with
> + * RTE_TEL_UINT_VAL as the type parameter.
> + *
> + * @param d
> + *   The data structure passed to the callback
> + * @param x
> + *   The number to be returned in the array
> + * @return
> + *   0 on success, negative errno on error
> + */
> +__rte_experimental
> +int
> +rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x);
> +
  
lihuisong (C) Dec. 15, 2022, 1:49 a.m. UTC | #2
在 2022/12/14 2:27, Bruce Richardson 写道:
> For future standardization on the "uint" name for unsigned values rather
> than the existing "u64" one, we can for now:
> * rename all internal values to use uint rather than u64
> * add new function names to alias the existing u64 ones
>
> Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
>   lib/telemetry/telemetry.c      | 16 +++++++--------
>   lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
>   lib/telemetry/telemetry_data.h |  4 ++--
>   lib/telemetry/version.map      |  7 +++++++
>   5 files changed, 73 insertions(+), 18 deletions(-)
>
> diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> index c2ad65effe..60877dae0a 100644
> --- a/lib/telemetry/rte_telemetry.h
> +++ b/lib/telemetry/rte_telemetry.h
> @@ -8,6 +8,8 @@
>   #ifndef _RTE_TELEMETRY_H_
>   #define _RTE_TELEMETRY_H_
>   
> +#include <rte_compat.h>
> +
>   #ifdef __cplusplus
>   extern "C" {
>   #endif
> @@ -121,6 +123,22 @@ int
>   rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
>   
>   /**
> + * Add an unsigned value to an array.
> + * The array must have been started by rte_tel_data_start_array() with
> + * RTE_TEL_UINT_VAL as the type parameter.
> + *
> + * @param d
> + *   The data structure passed to the callback
> + * @param x
> + *   The number to be returned in the array
> + * @return
> + *   0 on success, negative errno on error
> + */
> +__rte_experimental
> +int
> +rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x);
> +
> + /**
>    * Add a uint64_t to an array.
>    * The array must have been started by rte_tel_data_start_array() with
>    * RTE_TEL_UINT_VAL as the type parameter.
> @@ -193,6 +211,24 @@ int
>   rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val);
>   
>   /**
> + * Add an unsigned value to a dictionary.
> + * The dict must have been started by rte_tel_data_start_dict().
> + *
> + * @param d
> + *   The data structure passed to the callback
> + * @param name
> + *   The name the value is to be stored under in the dict
> + *   Must contain only alphanumeric characters or the symbols: '_' or '/'
> + * @param val
> + *   The number to be stored in the dict
> + * @return
> + *   0 on success, negative errno on error, E2BIG on string truncation of name.
> + */
> +int __rte_experimental
__rte_experimental
int

Right?
> +rte_tel_data_add_dict_uint(struct rte_tel_data *d,
> +		const char *name, uint64_t val);
> +
> + /**
>    * Add a uint64_t value to a dictionary.
>    * The dict must have been started by rte_tel_data_start_dict().
>    *
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index 75957fe0b1..fd8186383f 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -167,16 +167,16 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len)
>   	size_t used = 0;
>   	unsigned int i;
>   
> -	if (d->type != RTE_TEL_DICT && d->type != RTE_TEL_ARRAY_U64 &&
> +	if (d->type != RTE_TEL_DICT && d->type != RTE_TEL_ARRAY_UINT &&
>   		d->type != RTE_TEL_ARRAY_INT && d->type != RTE_TEL_ARRAY_STRING)
>   		return snprintf(out_buf, buf_len, "null");
>   
>   	used = rte_tel_json_empty_array(out_buf, buf_len, 0);
> -	if (d->type == RTE_TEL_ARRAY_U64)
> +	if (d->type == RTE_TEL_ARRAY_UINT)
>   		for (i = 0; i < d->data_len; i++)
>   			used = rte_tel_json_add_array_u64(out_buf,
>   				buf_len, used,
> -				d->data.array[i].u64val);
> +				d->data.array[i].uval);
>   	if (d->type == RTE_TEL_ARRAY_INT)
>   		for (i = 0; i < d->data_len; i++)
>   			used = rte_tel_json_add_array_int(out_buf,
> @@ -204,7 +204,7 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len)
>   			case RTE_TEL_UINT_VAL:
>   				used = rte_tel_json_add_obj_u64(out_buf,
>   						buf_len, used,
> -						v->name, v->value.u64val);
> +						v->name, v->value.uval);
>   				break;
>   			case RTE_TEL_CONTAINER:
>   			{
> @@ -271,7 +271,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>   			case RTE_TEL_UINT_VAL:
>   				used = rte_tel_json_add_obj_u64(cb_data_buf,
>   						buf_len, used,
> -						v->name, v->value.u64val);
> +						v->name, v->value.uval);
>   				break;
>   			case RTE_TEL_CONTAINER:
>   			{
> @@ -293,7 +293,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>   
>   	case RTE_TEL_ARRAY_STRING:
>   	case RTE_TEL_ARRAY_INT:
> -	case RTE_TEL_ARRAY_U64:
> +	case RTE_TEL_ARRAY_UINT:
>   	case RTE_TEL_ARRAY_CONTAINER:
>   		used = rte_tel_json_empty_array(cb_data_buf, buf_len, 0);
>   		for (i = 0; i < d->data_len; i++)
> @@ -306,10 +306,10 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>   				used = rte_tel_json_add_array_int(cb_data_buf,
>   						buf_len, used,
>   						d->data.array[i].ival);
> -			else if (d->type == RTE_TEL_ARRAY_U64)
> +			else if (d->type == RTE_TEL_ARRAY_UINT)
>   				used = rte_tel_json_add_array_u64(cb_data_buf,
>   						buf_len, used,
> -						d->data.array[i].u64val);
> +						d->data.array[i].uval);
>   			else if (d->type == RTE_TEL_ARRAY_CONTAINER) {
>   				char temp[buf_len];
>   				const struct container *rec_data =
> diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
> index 3c996484ec..077b0c4a6f 100644
> --- a/lib/telemetry/telemetry_data.c
> +++ b/lib/telemetry/telemetry_data.c
> @@ -18,7 +18,7 @@ rte_tel_data_start_array(struct rte_tel_data *d, enum rte_tel_value_type type)
>   	enum tel_container_types array_types[] = {
>   			RTE_TEL_ARRAY_STRING, /* RTE_TEL_STRING_VAL = 0 */
>   			RTE_TEL_ARRAY_INT,    /* RTE_TEL_INT_VAL = 1 */
> -			RTE_TEL_ARRAY_U64,    /* RTE_TEL_UINT_VAL = 2 */
> +			RTE_TEL_ARRAY_UINT,    /* RTE_TEL_UINT_VAL = 2 */
>   			RTE_TEL_ARRAY_CONTAINER, /* RTE_TEL_CONTAINER = 3 */
>   	};
>   	d->type = array_types[type];
> @@ -70,22 +70,28 @@ rte_tel_data_add_array_int(struct rte_tel_data *d, int x)
>   }
>   
>   int
> -rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x)
> +rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
>   {
> -	if (d->type != RTE_TEL_ARRAY_U64)
> +	if (d->type != RTE_TEL_ARRAY_UINT)
>   		return -EINVAL;
>   	if (d->data_len >= RTE_TEL_MAX_ARRAY_ENTRIES)
>   		return -ENOSPC;
> -	d->data.array[d->data_len++].u64val = x;
> +	d->data.array[d->data_len++].uval = x;
>   	return 0;
>   }
>   
> +int
> +rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x)
> +{
> +	return rte_tel_data_add_array_uint(d, x);
> +}
> +
>   int
>   rte_tel_data_add_array_container(struct rte_tel_data *d,
>   		struct rte_tel_data *val, int keep)
>   {
>   	if (d->type != RTE_TEL_ARRAY_CONTAINER ||
> -			(val->type != RTE_TEL_ARRAY_U64
> +			(val->type != RTE_TEL_ARRAY_UINT
>   			&& val->type != RTE_TEL_ARRAY_INT
>   			&& val->type != RTE_TEL_ARRAY_STRING))
>   		return -EINVAL;
> @@ -160,7 +166,7 @@ rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val)
>   }
>   
>   int
> -rte_tel_data_add_dict_u64(struct rte_tel_data *d,
> +rte_tel_data_add_dict_uint(struct rte_tel_data *d,
>   		const char *name, uint64_t val)
>   {
>   	struct tel_dict_entry *e = &d->data.dict[d->data_len];
> @@ -174,18 +180,24 @@ rte_tel_data_add_dict_u64(struct rte_tel_data *d,
>   
>   	d->data_len++;
>   	e->type = RTE_TEL_UINT_VAL;
> -	e->value.u64val = val;
> +	e->value.uval = val;
>   	const size_t bytes = strlcpy(e->name, name, RTE_TEL_MAX_STRING_LEN);
>   	return bytes < RTE_TEL_MAX_STRING_LEN ? 0 : E2BIG;
>   }
>   
> +int
> +rte_tel_data_add_dict_u64(struct rte_tel_data *d, const char *name, uint64_t val)
> +{
> +	return rte_tel_data_add_dict_uint(d, name, val);
> +}
> +
>   int
>   rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name,
>   		struct rte_tel_data *val, int keep)
>   {
>   	struct tel_dict_entry *e = &d->data.dict[d->data_len];
>   
> -	if (d->type != RTE_TEL_DICT || (val->type != RTE_TEL_ARRAY_U64
> +	if (d->type != RTE_TEL_DICT || (val->type != RTE_TEL_ARRAY_UINT
>   			&& val->type != RTE_TEL_ARRAY_INT
>   			&& val->type != RTE_TEL_ARRAY_STRING
>   			&& val->type != RTE_TEL_DICT))
> diff --git a/lib/telemetry/telemetry_data.h b/lib/telemetry/telemetry_data.h
> index 26aa28e72c..939deaa618 100644
> --- a/lib/telemetry/telemetry_data.h
> +++ b/lib/telemetry/telemetry_data.h
> @@ -13,7 +13,7 @@ enum tel_container_types {
>   	RTE_TEL_DICT,	      /** name-value pairs, of individual value type */
>   	RTE_TEL_ARRAY_STRING, /** array of string values only */
>   	RTE_TEL_ARRAY_INT,    /** array of signed, 32-bit int values */
> -	RTE_TEL_ARRAY_U64,    /** array of unsigned 64-bit int values */
> +	RTE_TEL_ARRAY_UINT,   /** array of unsigned 64-bit int values */
>   	RTE_TEL_ARRAY_CONTAINER, /** array of container structs */
>   };
>   
> @@ -29,7 +29,7 @@ struct container {
>   union tel_value {
>   	char sval[RTE_TEL_MAX_STRING_LEN];
>   	int ival;
> -	uint64_t u64val;
> +	uint64_t uval;
>   	struct container container;
>   };
>   
> diff --git a/lib/telemetry/version.map b/lib/telemetry/version.map
> index 9794f9ea20..0f70d82dfc 100644
> --- a/lib/telemetry/version.map
> +++ b/lib/telemetry/version.map
> @@ -19,6 +19,13 @@ DPDK_23 {
>   	local: *;
>   };
>   
> +EXPERIMENTAL {
> +	global:
> +
> +	rte_tel_data_add_array_uint;
> +	rte_tel_data_add_dict_uint;
> +};
> +
>   INTERNAL {
>   	rte_telemetry_legacy_register;
>   	rte_telemetry_init;
  
Bruce Richardson Dec. 15, 2022, 9:42 a.m. UTC | #3
On Thu, Dec 15, 2022 at 09:49:06AM +0800, lihuisong (C) wrote:
> 
> 在 2022/12/14 2:27, Bruce Richardson 写道:
> > For future standardization on the "uint" name for unsigned values rather
> > than the existing "u64" one, we can for now:
> > * rename all internal values to use uint rather than u64
> > * add new function names to alias the existing u64 ones
> > 
> > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >   lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> >   lib/telemetry/telemetry.c      | 16 +++++++--------
> >   lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> >   lib/telemetry/telemetry_data.h |  4 ++--
> >   lib/telemetry/version.map      |  7 +++++++
> >   5 files changed, 73 insertions(+), 18 deletions(-)
> > 
> > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > index c2ad65effe..60877dae0a 100644
> > --- a/lib/telemetry/rte_telemetry.h
> > +++ b/lib/telemetry/rte_telemetry.h
> > @@ -8,6 +8,8 @@
> >   #ifndef _RTE_TELEMETRY_H_
> >   #define _RTE_TELEMETRY_H_
> > +#include <rte_compat.h>
> > +
> >   #ifdef __cplusplus
> >   extern "C" {
> >   #endif
> > @@ -121,6 +123,22 @@ int
> >   rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> >   /**
> > + * Add an unsigned value to an array.
> > + * The array must have been started by rte_tel_data_start_array() with
> > + * RTE_TEL_UINT_VAL as the type parameter.
> > + *
> > + * @param d
> > + *   The data structure passed to the callback
> > + * @param x
> > + *   The number to be returned in the array
> > + * @return
> > + *   0 on success, negative errno on error
> > + */
> > +__rte_experimental
> > +int
> > +rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x);
> > +
> > + /**
> >    * Add a uint64_t to an array.
> >    * The array must have been started by rte_tel_data_start_array() with
> >    * RTE_TEL_UINT_VAL as the type parameter.
> > @@ -193,6 +211,24 @@ int
> >   rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val);
> >   /**
> > + * Add an unsigned value to a dictionary.
> > + * The dict must have been started by rte_tel_data_start_dict().
> > + *
> > + * @param d
> > + *   The data structure passed to the callback
> > + * @param name
> > + *   The name the value is to be stored under in the dict
> > + *   Must contain only alphanumeric characters or the symbols: '_' or '/'
> > + * @param val
> > + *   The number to be stored in the dict
> > + * @return
> > + *   0 on success, negative errno on error, E2BIG on string truncation of name.
> > + */
> > +int __rte_experimental
> __rte_experimental
> int
> 
> Right?

Yes, that is right.
  
Bruce Richardson Dec. 15, 2022, 9:44 a.m. UTC | #4
On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > For future standardization on the "uint" name for unsigned values rather
> > than the existing "u64" one, we can for now:
> > * rename all internal values to use uint rather than u64
> > * add new function names to alias the existing u64 ones
> > 
> > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> >  lib/telemetry/telemetry.c      | 16 +++++++--------
> >  lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> >  lib/telemetry/telemetry_data.h |  4 ++--
> >  lib/telemetry/version.map      |  7 +++++++
> >  5 files changed, 73 insertions(+), 18 deletions(-)
> > 
> > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > index c2ad65effe..60877dae0a 100644
> > --- a/lib/telemetry/rte_telemetry.h
> > +++ b/lib/telemetry/rte_telemetry.h
> > @@ -8,6 +8,8 @@
> >  #ifndef _RTE_TELEMETRY_H_
> >  #define _RTE_TELEMETRY_H_
> >  
> > +#include <rte_compat.h>
> > +
> >  #ifdef __cplusplus
> >  extern "C" {
> >  #endif
> > @@ -121,6 +123,22 @@ int
> >  rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> >  
> >  /**
> 
> when adding __rte_experimental api i have been asked to add the
> following boilerplate documentation block. i'm not pushing it, just
> recalling it is what i get asked for, so in case it's something we do?
> see lib/eal/include/rte_thread.h as an example
> 
> 
> ```
>  * @warning
>  * @b EXPERIMENTAL: this API may change without prior notice.
> ```
>

Ok, thanks for the notice.

Actually, related to this, since we are adding these functions as aliases
for existing stable functions, I would like to see these being added not as
experimental. The reason for that, is that while they are experimental, we
cannot feasibly mark the old function names as deprecated. :-(

Adding Thomas and David on CC for their thoughts.

/Bruce
  
Thomas Monjalon Dec. 15, 2022, 1:36 p.m. UTC | #5
15/12/2022 10:44, Bruce Richardson:
> On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> > On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > > For future standardization on the "uint" name for unsigned values rather
> > > than the existing "u64" one, we can for now:
> > > * rename all internal values to use uint rather than u64
> > > * add new function names to alias the existing u64 ones
> > > 
> > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> > >  lib/telemetry/telemetry.c      | 16 +++++++--------
> > >  lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> > >  lib/telemetry/telemetry_data.h |  4 ++--
> > >  lib/telemetry/version.map      |  7 +++++++
> > >  5 files changed, 73 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > > index c2ad65effe..60877dae0a 100644
> > > --- a/lib/telemetry/rte_telemetry.h
> > > +++ b/lib/telemetry/rte_telemetry.h
> > > @@ -8,6 +8,8 @@
> > >  #ifndef _RTE_TELEMETRY_H_
> > >  #define _RTE_TELEMETRY_H_
> > >  
> > > +#include <rte_compat.h>
> > > +
> > >  #ifdef __cplusplus
> > >  extern "C" {
> > >  #endif
> > > @@ -121,6 +123,22 @@ int
> > >  rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> > >  
> > >  /**
> > 
> > when adding __rte_experimental api i have been asked to add the
> > following boilerplate documentation block. i'm not pushing it, just
> > recalling it is what i get asked for, so in case it's something we do?
> > see lib/eal/include/rte_thread.h as an example
> > 
> > 
> > ```
> >  * @warning
> >  * @b EXPERIMENTAL: this API may change without prior notice.
> > ```
> >
> 
> Ok, thanks for the notice.
> 
> Actually, related to this, since we are adding these functions as aliases
> for existing stable functions, I would like to see these being added not as
> experimental. The reason for that, is that while they are experimental, we
> cannot feasibly mark the old function names as deprecated. :-(
> 
> Adding Thomas and David on CC for their thoughts.

Is it related to telemetry?

In general, yes we cannot deprecate something if there is no stable replacement.
The recommended step is to introduce a new experimental API
and deprecate the old one when the new API is stable.
  
Bruce Richardson Dec. 15, 2022, 1:58 p.m. UTC | #6
On Thu, Dec 15, 2022 at 02:36:51PM +0100, Thomas Monjalon wrote:
> 15/12/2022 10:44, Bruce Richardson:
> > On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> > > On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > > > For future standardization on the "uint" name for unsigned values rather
> > > > than the existing "u64" one, we can for now:
> > > > * rename all internal values to use uint rather than u64
> > > > * add new function names to alias the existing u64 ones
> > > > 
> > > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > ---
> > > >  lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> > > >  lib/telemetry/telemetry.c      | 16 +++++++--------
> > > >  lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> > > >  lib/telemetry/telemetry_data.h |  4 ++--
> > > >  lib/telemetry/version.map      |  7 +++++++
> > > >  5 files changed, 73 insertions(+), 18 deletions(-)
> > > > 
> > > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > > > index c2ad65effe..60877dae0a 100644
> > > > --- a/lib/telemetry/rte_telemetry.h
> > > > +++ b/lib/telemetry/rte_telemetry.h
> > > > @@ -8,6 +8,8 @@
> > > >  #ifndef _RTE_TELEMETRY_H_
> > > >  #define _RTE_TELEMETRY_H_
> > > >  
> > > > +#include <rte_compat.h>
> > > > +
> > > >  #ifdef __cplusplus
> > > >  extern "C" {
> > > >  #endif
> > > > @@ -121,6 +123,22 @@ int
> > > >  rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> > > >  
> > > >  /**
> > > 
> > > when adding __rte_experimental api i have been asked to add the
> > > following boilerplate documentation block. i'm not pushing it, just
> > > recalling it is what i get asked for, so in case it's something we do?
> > > see lib/eal/include/rte_thread.h as an example
> > > 
> > > 
> > > ```
> > >  * @warning
> > >  * @b EXPERIMENTAL: this API may change without prior notice.
> > > ```
> > >
> > 
> > Ok, thanks for the notice.
> > 
> > Actually, related to this, since we are adding these functions as aliases
> > for existing stable functions, I would like to see these being added not as
> > experimental. The reason for that, is that while they are experimental, we
> > cannot feasibly mark the old function names as deprecated. :-(
> > 
> > Adding Thomas and David on CC for their thoughts.
> 
> Is it related to telemetry?
> 
> In general, yes we cannot deprecate something if there is no stable replacement.
> The recommended step is to introduce a new experimental API
> and deprecate the old one when the new API is stable.
> 
Yes, understood.
What we are really trying to do here is to rename an API, by process of
adding the new API and then marking the old one as deprecated. The small
issue is that adding the new one it is by default experimental, meaning we
need to wait for deprecating old one. Ideally, as soon as the new API is
added, we would like to point people to use that, but can't really do so
while it is experimental.

---

By way of explicit detail, Morten pointed out the inconsistency in the
telemetry APIs and types:

* we have add_*_int, which takes a 32-bit signed value
* we have add_*_u64 which takes 64-bit unsigned (as name suggests).

The ideal end-state is to always use 64-bit values (since there is no space
saving from 32-bit as a union is used), and just name everything as "int"
or "uint" for signed/unsigned. The two big steps here are:

* expanding type of the "int" functions to take 64-bit parameters - this is
  ABI change but not API one, since existing code will happily promote
  values on compile. Therefore, we just use ABI versioning to have a 32-bit
  version for older linked binaries.
* the rename of the rte_tel_data_add_array_u64 and
  rte_tel_data_add_dict_u64 to *_uint variants. Though keeping
  compatibility is easier, as we can just add new functions, the overall
  process is slower since the new functions technically should be added as
  experimental - hence the email. For the case of function renaming, do we
  still need to have the "renamed" versions as experimental initially?

Given where we are in the overall DPDK releases cycle, it's not a major
issue either way, I just would like some clarity. My main concern with
having it spread over a couple of releases, is that it's more likely a step
will be missed/forgotten somewhere along the way! 

/Bruce
  
Tyler Retzlaff Dec. 15, 2022, 5:58 p.m. UTC | #7
On Thu, Dec 15, 2022 at 09:44:49AM +0000, Bruce Richardson wrote:
> On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> > On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > > For future standardization on the "uint" name for unsigned values rather
> > > than the existing "u64" one, we can for now:
> > > * rename all internal values to use uint rather than u64
> > > * add new function names to alias the existing u64 ones
> > > 
> > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >  lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> > >  lib/telemetry/telemetry.c      | 16 +++++++--------
> > >  lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> > >  lib/telemetry/telemetry_data.h |  4 ++--
> > >  lib/telemetry/version.map      |  7 +++++++
> > >  5 files changed, 73 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > > index c2ad65effe..60877dae0a 100644
> > > --- a/lib/telemetry/rte_telemetry.h
> > > +++ b/lib/telemetry/rte_telemetry.h
> > > @@ -8,6 +8,8 @@
> > >  #ifndef _RTE_TELEMETRY_H_
> > >  #define _RTE_TELEMETRY_H_
> > >  
> > > +#include <rte_compat.h>
> > > +
> > >  #ifdef __cplusplus
> > >  extern "C" {
> > >  #endif
> > > @@ -121,6 +123,22 @@ int
> > >  rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> > >  
> > >  /**
> > 
> > when adding __rte_experimental api i have been asked to add the
> > following boilerplate documentation block. i'm not pushing it, just
> > recalling it is what i get asked for, so in case it's something we do?
> > see lib/eal/include/rte_thread.h as an example
> > 
> > 
> > ```
> >  * @warning
> >  * @b EXPERIMENTAL: this API may change without prior notice.
> > ```
> >
> 
> Ok, thanks for the notice.
> 
> Actually, related to this, since we are adding these functions as aliases
> for existing stable functions, I would like to see these being added not as
> experimental. The reason for that, is that while they are experimental, we
> cannot feasibly mark the old function names as deprecated. :-(

seems reasonable to me, if they're just aliases and they haven't churned
then i don't see a reason why they need to spend time being
experimental.

> 
> Adding Thomas and David on CC for their thoughts.
> 
> /Bruce
  
Tyler Retzlaff Dec. 15, 2022, 6:02 p.m. UTC | #8
On Thu, Dec 15, 2022 at 09:42:40AM +0000, Bruce Richardson wrote:
> On Thu, Dec 15, 2022 at 09:49:06AM +0800, lihuisong (C) wrote:
> > 
> > 在 2022/12/14 2:27, Bruce Richardson 写道:
> > > For future standardization on the "uint" name for unsigned values rather
> > > than the existing "u64" one, we can for now:
> > > * rename all internal values to use uint rather than u64
> > > * add new function names to alias the existing u64 ones
> > > 
> > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > >   lib/telemetry/rte_telemetry.h  | 36 ++++++++++++++++++++++++++++++++++
> > >   lib/telemetry/telemetry.c      | 16 +++++++--------
> > >   lib/telemetry/telemetry_data.c | 28 ++++++++++++++++++--------
> > >   lib/telemetry/telemetry_data.h |  4 ++--
> > >   lib/telemetry/version.map      |  7 +++++++
> > >   5 files changed, 73 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
> > > index c2ad65effe..60877dae0a 100644
> > > --- a/lib/telemetry/rte_telemetry.h
> > > +++ b/lib/telemetry/rte_telemetry.h
> > > @@ -8,6 +8,8 @@
> > >   #ifndef _RTE_TELEMETRY_H_
> > >   #define _RTE_TELEMETRY_H_
> > > +#include <rte_compat.h>
> > > +
> > >   #ifdef __cplusplus
> > >   extern "C" {
> > >   #endif
> > > @@ -121,6 +123,22 @@ int
> > >   rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
> > >   /**
> > > + * Add an unsigned value to an array.
> > > + * The array must have been started by rte_tel_data_start_array() with
> > > + * RTE_TEL_UINT_VAL as the type parameter.
> > > + *
> > > + * @param d
> > > + *   The data structure passed to the callback
> > > + * @param x
> > > + *   The number to be returned in the array
> > > + * @return
> > > + *   0 on success, negative errno on error
> > > + */
> > > +__rte_experimental
> > > +int
> > > +rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x);
> > > +
> > > + /**
> > >    * Add a uint64_t to an array.
> > >    * The array must have been started by rte_tel_data_start_array() with
> > >    * RTE_TEL_UINT_VAL as the type parameter.
> > > @@ -193,6 +211,24 @@ int
> > >   rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val);
> > >   /**
> > > + * Add an unsigned value to a dictionary.
> > > + * The dict must have been started by rte_tel_data_start_dict().
> > > + *
> > > + * @param d
> > > + *   The data structure passed to the callback
> > > + * @param name
> > > + *   The name the value is to be stored under in the dict
> > > + *   Must contain only alphanumeric characters or the symbols: '_' or '/'
> > > + * @param val
> > > + *   The number to be stored in the dict
> > > + * @return
> > > + *   0 on success, negative errno on error, E2BIG on string truncation of name.
> > > + */
> > > +int __rte_experimental
> > __rte_experimental
> > int
> > 
> > Right?
> 
> Yes, that is right.

actually, it is consistent with how it is used in the rest of dpdk
and most compilers accept it before the return type. but actually it
applies to the symbol name.

this is not an argument to change the existing pattern, just pointing it
out the details.
  
Thomas Monjalon Dec. 19, 2022, 10:37 a.m. UTC | #9
15/12/2022 14:58, Bruce Richardson:
> On Thu, Dec 15, 2022 at 02:36:51PM +0100, Thomas Monjalon wrote:
> > 15/12/2022 10:44, Bruce Richardson:
> > > On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> > > > On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > > > > For future standardization on the "uint" name for unsigned values rather
> > > > > than the existing "u64" one, we can for now:
> > > > > * rename all internal values to use uint rather than u64
> > > > > * add new function names to alias the existing u64 ones
> > > > > 
> > > > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > 
> > > > when adding __rte_experimental api i have been asked to add the
> > > > following boilerplate documentation block. i'm not pushing it, just
> > > > recalling it is what i get asked for, so in case it's something we do?
> > > > see lib/eal/include/rte_thread.h as an example
> > > > 
> > > > 
> > > > ```
> > > >  * @warning
> > > >  * @b EXPERIMENTAL: this API may change without prior notice.
> > > > ```
> > > >
> > > 
> > > Ok, thanks for the notice.
> > > 
> > > Actually, related to this, since we are adding these functions as aliases
> > > for existing stable functions, I would like to see these being added not as
> > > experimental. The reason for that, is that while they are experimental, we
> > > cannot feasibly mark the old function names as deprecated. :-(
> > > 
> > > Adding Thomas and David on CC for their thoughts.
> > 
> > Is it related to telemetry?
> > 
> > In general, yes we cannot deprecate something if there is no stable replacement.
> > The recommended step is to introduce a new experimental API
> > and deprecate the old one when the new API is stable.
> > 
> Yes, understood.
> What we are really trying to do here is to rename an API, by process of
> adding the new API and then marking the old one as deprecated. The small
> issue is that adding the new one it is by default experimental, meaning we
> need to wait for deprecating old one. Ideally, as soon as the new API is
> added, we would like to point people to use that, but can't really do so
> while it is experimental.
> 
> ---
> 
> By way of explicit detail, Morten pointed out the inconsistency in the
> telemetry APIs and types:
> 
> * we have add_*_int, which takes a 32-bit signed value
> * we have add_*_u64 which takes 64-bit unsigned (as name suggests).
> 
> The ideal end-state is to always use 64-bit values (since there is no space
> saving from 32-bit as a union is used), and just name everything as "int"
> or "uint" for signed/unsigned. The two big steps here are:
> 
> * expanding type of the "int" functions to take 64-bit parameters - this is
>   ABI change but not API one, since existing code will happily promote
>   values on compile. Therefore, we just use ABI versioning to have a 32-bit
>   version for older linked binaries.
> * the rename of the rte_tel_data_add_array_u64 and
>   rte_tel_data_add_dict_u64 to *_uint variants. Though keeping
>   compatibility is easier, as we can just add new functions, the overall
>   process is slower since the new functions technically should be added as
>   experimental - hence the email. For the case of function renaming, do we
>   still need to have the "renamed" versions as experimental initially?

If a function is simply renamed, I think there is no need for the experimental step.
Would we keep an alias with the old name for some time?
  
Bruce Richardson Dec. 19, 2022, 1:22 p.m. UTC | #10
On Mon, Dec 19, 2022 at 11:37:19AM +0100, Thomas Monjalon wrote:
> 15/12/2022 14:58, Bruce Richardson:
> > On Thu, Dec 15, 2022 at 02:36:51PM +0100, Thomas Monjalon wrote:
> > > 15/12/2022 10:44, Bruce Richardson:
> > > > On Wed, Dec 14, 2022 at 09:38:45AM -0800, Tyler Retzlaff wrote:
> > > > > On Tue, Dec 13, 2022 at 06:27:25PM +0000, Bruce Richardson wrote:
> > > > > > For future standardization on the "uint" name for unsigned values rather
> > > > > > than the existing "u64" one, we can for now:
> > > > > > * rename all internal values to use uint rather than u64
> > > > > > * add new function names to alias the existing u64 ones
> > > > > > 
> > > > > > Suggested-by: Morten Brørup <mb@smartsharesystems.com>
> > > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > 
> > > > > when adding __rte_experimental api i have been asked to add the
> > > > > following boilerplate documentation block. i'm not pushing it, just
> > > > > recalling it is what i get asked for, so in case it's something we do?
> > > > > see lib/eal/include/rte_thread.h as an example
> > > > > 
> > > > > 
> > > > > ```
> > > > >  * @warning
> > > > >  * @b EXPERIMENTAL: this API may change without prior notice.
> > > > > ```
> > > > >
> > > > 
> > > > Ok, thanks for the notice.
> > > > 
> > > > Actually, related to this, since we are adding these functions as aliases
> > > > for existing stable functions, I would like to see these being added not as
> > > > experimental. The reason for that, is that while they are experimental, we
> > > > cannot feasibly mark the old function names as deprecated. :-(
> > > > 
> > > > Adding Thomas and David on CC for their thoughts.
> > > 
> > > Is it related to telemetry?
> > > 
> > > In general, yes we cannot deprecate something if there is no stable replacement.
> > > The recommended step is to introduce a new experimental API
> > > and deprecate the old one when the new API is stable.
> > > 
> > Yes, understood.
> > What we are really trying to do here is to rename an API, by process of
> > adding the new API and then marking the old one as deprecated. The small
> > issue is that adding the new one it is by default experimental, meaning we
> > need to wait for deprecating old one. Ideally, as soon as the new API is
> > added, we would like to point people to use that, but can't really do so
> > while it is experimental.
> > 
> > ---
> > 
> > By way of explicit detail, Morten pointed out the inconsistency in the
> > telemetry APIs and types:
> > 
> > * we have add_*_int, which takes a 32-bit signed value
> > * we have add_*_u64 which takes 64-bit unsigned (as name suggests).
> > 
> > The ideal end-state is to always use 64-bit values (since there is no space
> > saving from 32-bit as a union is used), and just name everything as "int"
> > or "uint" for signed/unsigned. The two big steps here are:
> > 
> > * expanding type of the "int" functions to take 64-bit parameters - this is
> >   ABI change but not API one, since existing code will happily promote
> >   values on compile. Therefore, we just use ABI versioning to have a 32-bit
> >   version for older linked binaries.
> > * the rename of the rte_tel_data_add_array_u64 and
> >   rte_tel_data_add_dict_u64 to *_uint variants. Though keeping
> >   compatibility is easier, as we can just add new functions, the overall
> >   process is slower since the new functions technically should be added as
> >   experimental - hence the email. For the case of function renaming, do we
> >   still need to have the "renamed" versions as experimental initially?
> 
> If a function is simply renamed, I think there is no need for the experimental step.
> Would we keep an alias with the old name for some time?
> 
Yes, the old name should go through the deprecation process. No 
hurry with removal.

/Bruce
  

Patch

diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h
index c2ad65effe..60877dae0a 100644
--- a/lib/telemetry/rte_telemetry.h
+++ b/lib/telemetry/rte_telemetry.h
@@ -8,6 +8,8 @@ 
 #ifndef _RTE_TELEMETRY_H_
 #define _RTE_TELEMETRY_H_
 
+#include <rte_compat.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -121,6 +123,22 @@  int
 rte_tel_data_add_array_int(struct rte_tel_data *d, int x);
 
 /**
+ * Add an unsigned value to an array.
+ * The array must have been started by rte_tel_data_start_array() with
+ * RTE_TEL_UINT_VAL as the type parameter.
+ *
+ * @param d
+ *   The data structure passed to the callback
+ * @param x
+ *   The number to be returned in the array
+ * @return
+ *   0 on success, negative errno on error
+ */
+__rte_experimental
+int
+rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x);
+
+ /**
  * Add a uint64_t to an array.
  * The array must have been started by rte_tel_data_start_array() with
  * RTE_TEL_UINT_VAL as the type parameter.
@@ -193,6 +211,24 @@  int
 rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val);
 
 /**
+ * Add an unsigned value to a dictionary.
+ * The dict must have been started by rte_tel_data_start_dict().
+ *
+ * @param d
+ *   The data structure passed to the callback
+ * @param name
+ *   The name the value is to be stored under in the dict
+ *   Must contain only alphanumeric characters or the symbols: '_' or '/'
+ * @param val
+ *   The number to be stored in the dict
+ * @return
+ *   0 on success, negative errno on error, E2BIG on string truncation of name.
+ */
+int __rte_experimental
+rte_tel_data_add_dict_uint(struct rte_tel_data *d,
+		const char *name, uint64_t val);
+
+ /**
  * Add a uint64_t value to a dictionary.
  * The dict must have been started by rte_tel_data_start_dict().
  *
diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 75957fe0b1..fd8186383f 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -167,16 +167,16 @@  container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len)
 	size_t used = 0;
 	unsigned int i;
 
-	if (d->type != RTE_TEL_DICT && d->type != RTE_TEL_ARRAY_U64 &&
+	if (d->type != RTE_TEL_DICT && d->type != RTE_TEL_ARRAY_UINT &&
 		d->type != RTE_TEL_ARRAY_INT && d->type != RTE_TEL_ARRAY_STRING)
 		return snprintf(out_buf, buf_len, "null");
 
 	used = rte_tel_json_empty_array(out_buf, buf_len, 0);
-	if (d->type == RTE_TEL_ARRAY_U64)
+	if (d->type == RTE_TEL_ARRAY_UINT)
 		for (i = 0; i < d->data_len; i++)
 			used = rte_tel_json_add_array_u64(out_buf,
 				buf_len, used,
-				d->data.array[i].u64val);
+				d->data.array[i].uval);
 	if (d->type == RTE_TEL_ARRAY_INT)
 		for (i = 0; i < d->data_len; i++)
 			used = rte_tel_json_add_array_int(out_buf,
@@ -204,7 +204,7 @@  container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len)
 			case RTE_TEL_UINT_VAL:
 				used = rte_tel_json_add_obj_u64(out_buf,
 						buf_len, used,
-						v->name, v->value.u64val);
+						v->name, v->value.uval);
 				break;
 			case RTE_TEL_CONTAINER:
 			{
@@ -271,7 +271,7 @@  output_json(const char *cmd, const struct rte_tel_data *d, int s)
 			case RTE_TEL_UINT_VAL:
 				used = rte_tel_json_add_obj_u64(cb_data_buf,
 						buf_len, used,
-						v->name, v->value.u64val);
+						v->name, v->value.uval);
 				break;
 			case RTE_TEL_CONTAINER:
 			{
@@ -293,7 +293,7 @@  output_json(const char *cmd, const struct rte_tel_data *d, int s)
 
 	case RTE_TEL_ARRAY_STRING:
 	case RTE_TEL_ARRAY_INT:
-	case RTE_TEL_ARRAY_U64:
+	case RTE_TEL_ARRAY_UINT:
 	case RTE_TEL_ARRAY_CONTAINER:
 		used = rte_tel_json_empty_array(cb_data_buf, buf_len, 0);
 		for (i = 0; i < d->data_len; i++)
@@ -306,10 +306,10 @@  output_json(const char *cmd, const struct rte_tel_data *d, int s)
 				used = rte_tel_json_add_array_int(cb_data_buf,
 						buf_len, used,
 						d->data.array[i].ival);
-			else if (d->type == RTE_TEL_ARRAY_U64)
+			else if (d->type == RTE_TEL_ARRAY_UINT)
 				used = rte_tel_json_add_array_u64(cb_data_buf,
 						buf_len, used,
-						d->data.array[i].u64val);
+						d->data.array[i].uval);
 			else if (d->type == RTE_TEL_ARRAY_CONTAINER) {
 				char temp[buf_len];
 				const struct container *rec_data =
diff --git a/lib/telemetry/telemetry_data.c b/lib/telemetry/telemetry_data.c
index 3c996484ec..077b0c4a6f 100644
--- a/lib/telemetry/telemetry_data.c
+++ b/lib/telemetry/telemetry_data.c
@@ -18,7 +18,7 @@  rte_tel_data_start_array(struct rte_tel_data *d, enum rte_tel_value_type type)
 	enum tel_container_types array_types[] = {
 			RTE_TEL_ARRAY_STRING, /* RTE_TEL_STRING_VAL = 0 */
 			RTE_TEL_ARRAY_INT,    /* RTE_TEL_INT_VAL = 1 */
-			RTE_TEL_ARRAY_U64,    /* RTE_TEL_UINT_VAL = 2 */
+			RTE_TEL_ARRAY_UINT,    /* RTE_TEL_UINT_VAL = 2 */
 			RTE_TEL_ARRAY_CONTAINER, /* RTE_TEL_CONTAINER = 3 */
 	};
 	d->type = array_types[type];
@@ -70,22 +70,28 @@  rte_tel_data_add_array_int(struct rte_tel_data *d, int x)
 }
 
 int
-rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x)
+rte_tel_data_add_array_uint(struct rte_tel_data *d, uint64_t x)
 {
-	if (d->type != RTE_TEL_ARRAY_U64)
+	if (d->type != RTE_TEL_ARRAY_UINT)
 		return -EINVAL;
 	if (d->data_len >= RTE_TEL_MAX_ARRAY_ENTRIES)
 		return -ENOSPC;
-	d->data.array[d->data_len++].u64val = x;
+	d->data.array[d->data_len++].uval = x;
 	return 0;
 }
 
+int
+rte_tel_data_add_array_u64(struct rte_tel_data *d, uint64_t x)
+{
+	return rte_tel_data_add_array_uint(d, x);
+}
+
 int
 rte_tel_data_add_array_container(struct rte_tel_data *d,
 		struct rte_tel_data *val, int keep)
 {
 	if (d->type != RTE_TEL_ARRAY_CONTAINER ||
-			(val->type != RTE_TEL_ARRAY_U64
+			(val->type != RTE_TEL_ARRAY_UINT
 			&& val->type != RTE_TEL_ARRAY_INT
 			&& val->type != RTE_TEL_ARRAY_STRING))
 		return -EINVAL;
@@ -160,7 +166,7 @@  rte_tel_data_add_dict_int(struct rte_tel_data *d, const char *name, int val)
 }
 
 int
-rte_tel_data_add_dict_u64(struct rte_tel_data *d,
+rte_tel_data_add_dict_uint(struct rte_tel_data *d,
 		const char *name, uint64_t val)
 {
 	struct tel_dict_entry *e = &d->data.dict[d->data_len];
@@ -174,18 +180,24 @@  rte_tel_data_add_dict_u64(struct rte_tel_data *d,
 
 	d->data_len++;
 	e->type = RTE_TEL_UINT_VAL;
-	e->value.u64val = val;
+	e->value.uval = val;
 	const size_t bytes = strlcpy(e->name, name, RTE_TEL_MAX_STRING_LEN);
 	return bytes < RTE_TEL_MAX_STRING_LEN ? 0 : E2BIG;
 }
 
+int
+rte_tel_data_add_dict_u64(struct rte_tel_data *d, const char *name, uint64_t val)
+{
+	return rte_tel_data_add_dict_uint(d, name, val);
+}
+
 int
 rte_tel_data_add_dict_container(struct rte_tel_data *d, const char *name,
 		struct rte_tel_data *val, int keep)
 {
 	struct tel_dict_entry *e = &d->data.dict[d->data_len];
 
-	if (d->type != RTE_TEL_DICT || (val->type != RTE_TEL_ARRAY_U64
+	if (d->type != RTE_TEL_DICT || (val->type != RTE_TEL_ARRAY_UINT
 			&& val->type != RTE_TEL_ARRAY_INT
 			&& val->type != RTE_TEL_ARRAY_STRING
 			&& val->type != RTE_TEL_DICT))
diff --git a/lib/telemetry/telemetry_data.h b/lib/telemetry/telemetry_data.h
index 26aa28e72c..939deaa618 100644
--- a/lib/telemetry/telemetry_data.h
+++ b/lib/telemetry/telemetry_data.h
@@ -13,7 +13,7 @@  enum tel_container_types {
 	RTE_TEL_DICT,	      /** name-value pairs, of individual value type */
 	RTE_TEL_ARRAY_STRING, /** array of string values only */
 	RTE_TEL_ARRAY_INT,    /** array of signed, 32-bit int values */
-	RTE_TEL_ARRAY_U64,    /** array of unsigned 64-bit int values */
+	RTE_TEL_ARRAY_UINT,   /** array of unsigned 64-bit int values */
 	RTE_TEL_ARRAY_CONTAINER, /** array of container structs */
 };
 
@@ -29,7 +29,7 @@  struct container {
 union tel_value {
 	char sval[RTE_TEL_MAX_STRING_LEN];
 	int ival;
-	uint64_t u64val;
+	uint64_t uval;
 	struct container container;
 };
 
diff --git a/lib/telemetry/version.map b/lib/telemetry/version.map
index 9794f9ea20..0f70d82dfc 100644
--- a/lib/telemetry/version.map
+++ b/lib/telemetry/version.map
@@ -19,6 +19,13 @@  DPDK_23 {
 	local: *;
 };
 
+EXPERIMENTAL {
+	global:
+
+	rte_tel_data_add_array_uint;
+	rte_tel_data_add_dict_uint;
+};
+
 INTERNAL {
 	rte_telemetry_legacy_register;
 	rte_telemetry_init;