[PATCH 0/2] add ring telemetry cmds

[PATCH 0/2] add ring telemetry cmds

[Jie Hai]

This patch set supports telemetry list rings and dump info of a ring
by its name.

Jie Hai (2):
  ring: add ring list telemetry cmd
  ring: add ring info telemetry cmd

 lib/ring/meson.build |   1 +
 lib/ring/rte_ring.c  | 128 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)

-- 
2.33.0

[PATCH 1/2] ring: add ring list telemetry cmd

[Jie Hai]

This patch supports the list of rings with telemetry cmd.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index cddaf6b2876f..bb1dafd4d1ca 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+rte_ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *te;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(te, ring_list, next) {
+		(*func)((struct rte_ring *) te->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	rte_ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available ring. Takes no parameters");
+}
-- 
2.33.0

[PATCH 2/2] ring: add ring info telemetry cmd

[Jie Hai]

This patch supports dump of the info of ring by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": 0,
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": 262143,
    "capacity": 262143,
    "used_count": 147173,
    "consumer_tail": 8283,
    "consumer_head": 8283,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097920,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": 0
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/rte_ring.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index bb1dafd4d1ca..effdd1fcdba6 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -45,6 +45,9 @@ EAL_REGISTER_TAILQ(rte_ring_tailq)
 /* by default set head/tail distance as 1/8 of ring capacity */
 #define HTD_MAX_DEF	8
 
+/* size of name of producer/consumer synchronization modes */
+#define SYNC_MODE_NAME_SZ	16
+
 /* return the size of memory occupied by a ring */
 ssize_t
 rte_ring_get_memsize_elem(unsigned int esize, unsigned int count)
@@ -454,8 +457,93 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static void
+ring_get_sync_name_by_type(struct rte_ring *r, char *prod, char *cons)
+{
+	switch(r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		strcpy(prod, "MP");
+		break;
+	case RTE_RING_SYNC_ST:
+		strcpy(prod, "SP");
+		break;
+	case RTE_RING_SYNC_MT_RTS:
+		strcpy(prod, "MP_RTS");
+		break;
+	case RTE_RING_SYNC_MT_HTS:
+		strcpy(prod, "MP_HTS");
+		break;
+	default:
+		strcpy(prod, "Unknown");
+	}
+
+	switch(r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		strcpy(cons, "MC");
+		break;
+	case RTE_RING_SYNC_ST:
+		strcpy(cons, "SC");
+		break;
+	case RTE_RING_SYNC_MT_RTS:
+		strcpy(cons, "MC_RTS");
+		break;
+	case RTE_RING_SYNC_MT_HTS:
+		strcpy(cons, "MC_HTS");
+		break;
+	default:
+		strcpy(cons, "Unknown");
+	}
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char prod_type[SYNC_MODE_NAME_SZ];
+	char cons_type[SYNC_MODE_NAME_SZ];
+	const struct rte_memzone *mz;
+	char name[RTE_RING_NAMESIZE];
+	struct rte_ring *r;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_MEMZONE_NAMESIZE)
+		return -EINVAL;
+
+	strlcpy(name, params, RTE_MEMZONE_NAMESIZE);
+	r = rte_ring_lookup(name);
+	if (r == NULL)
+		return -EINVAL;
+
+	rte_tel_data_start_dict(d);
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_int(d, "flags", r->flags);
+	ring_get_sync_name_by_type(r, prod_type, cons_type);
+	rte_tel_data_add_dict_string(d, "producer_type", prod_type);
+	rte_tel_data_add_dict_string(d, "consumer_type", cons_type);
+	rte_tel_data_add_dict_u64(d, "size", r->size);
+	rte_tel_data_add_dict_u64(d, "mask", r->mask);
+	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available ring. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

[PATCH v2 0/2] add ring telemetry cmds

[Jie Hai]

This patch set supports telemetry list rings and dump ring info
by its name.

Jie Hai (2):
  ring: add ring list telemetry cmd
  ring: add ring info telemetry cmd

 lib/ring/meson.build |   1 +
 lib/ring/rte_ring.c  | 128 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)

-- 
2.33.0

[PATCH v2 1/2] ring: add ring list telemetry cmd

[Jie Hai]

This patch supports the list of rings with telemetry cmd.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index cddaf6b2876f..bb1dafd4d1ca 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+rte_ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *te;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(te, ring_list, next) {
+		(*func)((struct rte_ring *) te->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	rte_ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available ring. Takes no parameters");
+}
-- 
2.33.0

[PATCH v2 2/2] ring: add ring info telemetry cmd

[Jie Hai]

This patch supports dump of the info of ring by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": 0,
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": 262143,
    "capacity": 262143,
    "used_count": 147173,
    "consumer_tail": 8283,
    "consumer_head": 8283,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097920,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": 0
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/rte_ring.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index bb1dafd4d1ca..82f3d6a6cd60 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -45,6 +45,9 @@ EAL_REGISTER_TAILQ(rte_ring_tailq)
 /* by default set head/tail distance as 1/8 of ring capacity */
 #define HTD_MAX_DEF	8
 
+/* size of name of producer/consumer synchronization modes */
+#define SYNC_MODE_NAME_SZ	16
+
 /* return the size of memory occupied by a ring */
 ssize_t
 rte_ring_get_memsize_elem(unsigned int esize, unsigned int count)
@@ -454,8 +457,93 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static void
+ring_get_sync_name_by_type(struct rte_ring *r, char *prod, char *cons)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		strcpy(prod, "MP");
+		break;
+	case RTE_RING_SYNC_ST:
+		strcpy(prod, "SP");
+		break;
+	case RTE_RING_SYNC_MT_RTS:
+		strcpy(prod, "MP_RTS");
+		break;
+	case RTE_RING_SYNC_MT_HTS:
+		strcpy(prod, "MP_HTS");
+		break;
+	default:
+		strcpy(prod, "Unknown");
+	}
+
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		strcpy(cons, "MC");
+		break;
+	case RTE_RING_SYNC_ST:
+		strcpy(cons, "SC");
+		break;
+	case RTE_RING_SYNC_MT_RTS:
+		strcpy(cons, "MC_RTS");
+		break;
+	case RTE_RING_SYNC_MT_HTS:
+		strcpy(cons, "MC_HTS");
+		break;
+	default:
+		strcpy(cons, "Unknown");
+	}
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char prod_type[SYNC_MODE_NAME_SZ];
+	char cons_type[SYNC_MODE_NAME_SZ];
+	const struct rte_memzone *mz;
+	char name[RTE_RING_NAMESIZE];
+	struct rte_ring *r;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	strlcpy(name, params, RTE_RING_NAMESIZE);
+	r = rte_ring_lookup(name);
+	if (r == NULL)
+		return -EINVAL;
+
+	rte_tel_data_start_dict(d);
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_int(d, "flags", r->flags);
+	ring_get_sync_name_by_type(r, prod_type, cons_type);
+	rte_tel_data_add_dict_string(d, "producer_type", prod_type);
+	rte_tel_data_add_dict_string(d, "consumer_type", cons_type);
+	rte_tel_data_add_dict_u64(d, "size", r->size);
+	rte_tel_data_add_dict_u64(d, "mask", r->mask);
+	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available ring. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

Re: [PATCH v2 1/2] ring: add ring list telemetry cmd

[Konstantin Ananyev]

Hi Jie,

> This patch supports the list of rings with telemetry cmd.
> An example using this command is shown below:
> 
> --> /ring/list
> {
>    "/ring/list": [
>      "HT_0000:7d:00.2",
>      "MP_mb_pool_0"
>    ]
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>   lib/ring/meson.build |  1 +
>   lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
> 
> diff --git a/lib/ring/meson.build b/lib/ring/meson.build
> index c20685c689ac..7fca958ed7fa 100644
> --- a/lib/ring/meson.build
> +++ b/lib/ring/meson.build
> @@ -18,3 +18,4 @@ indirect_headers += files (
>           'rte_ring_rts.h',
>           'rte_ring_rts_elem_pvt.h',
>   )
> +deps += ['telemetry']
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index cddaf6b2876f..bb1dafd4d1ca 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -22,6 +22,7 @@
>   #include <rte_errno.h>
>   #include <rte_string_fns.h>
>   #include <rte_tailq.h>
> +#include <rte_telemetry.h>
>   
>   #include "rte_ring.h"
>   #include "rte_ring_elem.h"
> @@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
>   
>   	return r;
>   }
> +
> +static void
> +rte_ring_walk(void (*func)(struct rte_ring *, void *), void *arg)

As a nit: it is a static function, so I think we can skip 'rte_' prefix 
for it.
Apart from that:
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>

> +{
> +	struct rte_ring_list *ring_list;
> +	struct rte_tailq_entry *te;
> +
> +	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
> +	rte_mcfg_tailq_read_lock();
> +
> +	TAILQ_FOREACH(te, ring_list, next) {
> +		(*func)((struct rte_ring *) te->data, arg);
> +	}
> +
> +	rte_mcfg_tailq_read_unlock();
> +}
> +
> +static void
> +ring_list_cb(struct rte_ring *r, void *arg)
> +{
> +	struct rte_tel_data *d = (struct rte_tel_data *)arg;
> +
> +	rte_tel_data_add_array_string(d, r->name);
> +}
> +
> +static int
> +ring_handle_list(const char *cmd __rte_unused,
> +		const char *params __rte_unused, struct rte_tel_data *d)
> +{
> +	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
> +	rte_ring_walk(ring_list_cb, d);
> +	return 0;
> +}
> +
> +RTE_INIT(ring_init_telemetry)
> +{
> +	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
> +		"Returns list of available ring. Takes no parameters");
> +}

Re: [PATCH v2 2/2] ring: add ring info telemetry cmd

[Konstantin Ananyev]

> This patch supports dump of the info of ring by its name.
> An example using this command is shown below:
> 
> --> /ring/info,MP_mb_pool_0
> {
>    "/ring/info": {
>      "name": "MP_mb_pool_0",
>      "socket": 0,
>      "flags": 0,
>      "producer_type": "MP",
>      "consumer_type": "MC",
>      "size": 262144,
>      "mask": 262143,
>      "capacity": 262143,
>      "used_count": 147173,
>      "consumer_tail": 8283,
>      "consumer_head": 8283,
>      "producer_tail": 155456,
>      "producer_head": 155456,
>      "mz_name": "RG_MP_mb_pool_0",
>      "mz_len": 2097920,
>      "mz_hugepage_sz": 1073741824,
>      "mz_socket_id": 0,
>      "mz_flags": 0
>    }
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>   lib/ring/rte_ring.c | 88 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 88 insertions(+)
> 
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index bb1dafd4d1ca..82f3d6a6cd60 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -45,6 +45,9 @@ EAL_REGISTER_TAILQ(rte_ring_tailq)
>   /* by default set head/tail distance as 1/8 of ring capacity */
>   #define HTD_MAX_DEF	8
>   
> +/* size of name of producer/consumer synchronization modes */
> +#define SYNC_MODE_NAME_SZ	16
> +
>   /* return the size of memory occupied by a ring */
>   ssize_t
>   rte_ring_get_memsize_elem(unsigned int esize, unsigned int count)
> @@ -454,8 +457,93 @@ ring_handle_list(const char *cmd __rte_unused,
>   	return 0;
>   }
>   
> +static void
> +ring_get_sync_name_by_type(struct rte_ring *r, char *prod, char *cons)
> +{
> +	switch (r->prod.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		strcpy(prod, "MP");
> +		break;
> +	case RTE_RING_SYNC_ST:
> +		strcpy(prod, "SP");
> +		break;
> +	case RTE_RING_SYNC_MT_RTS:
> +		strcpy(prod, "MP_RTS");
> +		break;
> +	case RTE_RING_SYNC_MT_HTS:
> +		strcpy(prod, "MP_HTS");
> +		break;
> +	default:
> +		strcpy(prod, "Unknown");
> +	}

It is probably not the best option to blindly copy strings somewhere.
I think it would be better to introduce function like that:
static const char *
ring_prod_sync_type_to_name(enum rte_ring_sync_type type)
{
	switch(type) {
		case RTE_RING_SYNC_MT: return "MP";
		case RTE_RING_SYNC_ST: return "SP";
		...
	}
	return "Unknown";
}

Same for _cons_ type and use them accordingly.


> +
> +	switch (r->cons.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		strcpy(cons, "MC");
> +		break;
> +	case RTE_RING_SYNC_ST:
> +		strcpy(cons, "SC");
> +		break;
> +	case RTE_RING_SYNC_MT_RTS:
> +		strcpy(cons, "MC_RTS");
> +		break;
> +	case RTE_RING_SYNC_MT_HTS:
> +		strcpy(cons, "MC_HTS");
> +		break;
> +	default:
> +		strcpy(cons, "Unknown");
> +	}
> +}




> +
> +static int
> +ring_handle_info(const char *cmd __rte_unused, const char *params,
> +		struct rte_tel_data *d)
> +{
> +	char prod_type[SYNC_MODE_NAME_SZ];
> +	char cons_type[SYNC_MODE_NAME_SZ];
> +	const struct rte_memzone *mz;
> +	char name[RTE_RING_NAMESIZE];
> +	struct rte_ring *r;
> +
> +	if (params == NULL || strlen(params) == 0 ||
> +		strlen(params) >= RTE_RING_NAMESIZE)
> +		return -EINVAL;
> +
> +	strlcpy(name, params, RTE_RING_NAMESIZE);

That copy looks absolutely redundant, you can do just
rte_ring_lookup(params) instead.

> +	r = rte_ring_lookup(name);
> +	if (r == NULL)
> +		return -EINVAL;
> +
> +	rte_tel_data_start_dict(d);
> +	rte_tel_data_add_dict_string(d, "name", r->name);


Do I get it right that it could be executed from specific telemetry thread?
If so, we probably shouldn't release rte_mcfg_tailq_read_lock while
accessing ring data.


> +	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);

You do print it below, when printing memzone related data.

> +	rte_tel_data_add_dict_int(d, "flags", r->flags);
> +	ring_get_sync_name_by_type(r, prod_type, cons_type);
> +	rte_tel_data_add_dict_string(d, "producer_type", prod_type);
> +	rte_tel_data_add_dict_string(d, "consumer_type", cons_type);
> +	rte_tel_data_add_dict_u64(d, "size", r->size);
> +	rte_tel_data_add_dict_u64(d, "mask", r->mask);
> +	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
> +	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
> +	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
> +	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
> +	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
> +	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
> +
> +	mz = r->memzone;``

Would it make sense to check that mz != NULL here?
I know that it shouldn't be NULL for valid ring created by 
rte_ring_create(), but still probably no harm.

> +	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
> +	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
> +	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
> +	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
> +	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
> +
> +	return 0;
> +}
> +
>   RTE_INIT(ring_init_telemetry)
>   {
>   	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>   		"Returns list of available ring. Takes no parameters");
> +	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
> +		"Returns ring info. Parameters: ring_name.");
>   }

[PATCH v3 0/2] add ring telemetry cmds

[Jie Hai]

This patch set supports telemetry list rings and dump info of a ring
by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

Jie Hai (2):
  ring: add ring list telemetry cmd
  ring: add ring info telemetry cmd

 lib/ring/meson.build |   1 +
 lib/ring/rte_ring.c  | 123 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

-- 
2.33.0

[PATCH v3 1/2] ring: add ring list telemetry cmd

[Jie Hai]

This patch supports the list of rings with telemetry cmd.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index cddaf6b2876f..e6aac332d88f 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *te;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(te, ring_list, next) {
+		(*func)((struct rte_ring *) te->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available ring. Takes no parameters");
+}
-- 
2.33.0

[PATCH v3 2/2] ring: add ring info telemetry cmd

[Jie Hai]

This patch supports dump of the info of ring by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": 0,
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": 262143,
    "capacity": 262143,
    "used_count": 147173,
    "consumer_tail": 8283,
    "consumer_head": 8283,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097920,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": 0
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/rte_ring.c | 83 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index e6aac332d88f..2e57aa653339 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -454,8 +454,91 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	const struct rte_memzone *mz;
+	struct rte_ring *r;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	r = rte_ring_lookup(params);
+	if (r == NULL)
+		return -EINVAL;
+
+	rte_mcfg_tailq_read_lock();
+
+	rte_tel_data_start_dict(d);
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_int(d, "flags", r->flags);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_u64(d, "size", r->size);
+	rte_tel_data_add_dict_u64(d, "mask", r->mask);
+	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return 0;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
+
+	rte_mcfg_tailq_read_unlock();
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available ring. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

Re: [PATCH v2 1/2] ring: add ring list telemetry cmd

[Jie Hai]

On 2023/1/23 0:40, Konstantin Ananyev wrote:
> Hi Jie,
> 
>> This patch supports the list of rings with telemetry cmd.
>> An example using this command is shown below:
>>
>> --> /ring/list
>> {
>>    "/ring/list": [
>>      "HT_0000:7d:00.2",
>>      "MP_mb_pool_0"
>>    ]
>> }
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
>>   lib/ring/meson.build |  1 +
>>   lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 41 insertions(+)
>>
>> diff --git a/lib/ring/meson.build b/lib/ring/meson.build
>> index c20685c689ac..7fca958ed7fa 100644
>> --- a/lib/ring/meson.build
>> +++ b/lib/ring/meson.build
>> @@ -18,3 +18,4 @@ indirect_headers += files (
>>           'rte_ring_rts.h',
>>           'rte_ring_rts_elem_pvt.h',
>>   )
>> +deps += ['telemetry']
>> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
>> index cddaf6b2876f..bb1dafd4d1ca 100644
>> --- a/lib/ring/rte_ring.c
>> +++ b/lib/ring/rte_ring.c
>> @@ -22,6 +22,7 @@
>>   #include <rte_errno.h>
>>   #include <rte_string_fns.h>
>>   #include <rte_tailq.h>
>> +#include <rte_telemetry.h>
>>   #include "rte_ring.h"
>>   #include "rte_ring_elem.h"
>> @@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
>>       return r;
>>   }
>> +
>> +static void
>> +rte_ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
> 
> As a nit: it is a static function, so I think we can skip 'rte_' prefix 
> for it.
> Apart from that:
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> 
Hi, Konstantin,

Thanks for your review. Accepted and fixed in v3.

Jie Hai
>> +{
>> +    struct rte_ring_list *ring_list;
>> +    struct rte_tailq_entry *te;
>> +
>> +    ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
>> +    rte_mcfg_tailq_read_lock();
>> +
>> +    TAILQ_FOREACH(te, ring_list, next) {
>> +        (*func)((struct rte_ring *) te->data, arg);
>> +    }
>> +
>> +    rte_mcfg_tailq_read_unlock();
>> +}
>> +
>> +static void
>> +ring_list_cb(struct rte_ring *r, void *arg)
>> +{
>> +    struct rte_tel_data *d = (struct rte_tel_data *)arg;
>> +
>> +    rte_tel_data_add_array_string(d, r->name);
>> +}
>> +
>> +static int
>> +ring_handle_list(const char *cmd __rte_unused,
>> +        const char *params __rte_unused, struct rte_tel_data *d)
>> +{
>> +    rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
>> +    rte_ring_walk(ring_list_cb, d);
>> +    return 0;
>> +}
>> +
>> +RTE_INIT(ring_init_telemetry)
>> +{
>> +    rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>> +        "Returns list of available ring. Takes no parameters");
>> +}
> 
> .

Re: [PATCH v2 2/2] ring: add ring info telemetry cmd

[Jie Hai]

Hi, Konstantin,

Thanks for your review. All accepted and fixed in v3.

Jie Hai

On 2023/1/23 1:49, Konstantin Ananyev wrote:
> 
>> This patch supports dump of the info of ring by its name.
>> An example using this command is shown below:
>>
>> --> /ring/info,MP_mb_pool_0
>> {
>>    "/ring/info": {
>>      "name": "MP_mb_pool_0",
>>      "socket": 0,
>>      "flags": 0,
>>      "producer_type": "MP",
>>      "consumer_type": "MC",
>>      "size": 262144,
>>      "mask": 262143,
>>      "capacity": 262143,
>>      "used_count": 147173,
>>      "consumer_tail": 8283,
>>      "consumer_head": 8283,
>>      "producer_tail": 155456,
>>      "producer_head": 155456,
>>      "mz_name": "RG_MP_mb_pool_0",
>>      "mz_len": 2097920,
>>      "mz_hugepage_sz": 1073741824,
>>      "mz_socket_id": 0,
>>      "mz_flags": 0
>>    }
>> }
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>

RE: [PATCH v3 2/2] ring: add ring info telemetry cmd

[Honnappa Nagarahalli]

Few minor nits. Otherwise,

Reviewed-by: Honnappa Nagarahalli <Honnappa.nagarahalli@arm.com>

> -----Original Message-----
> From: Jie Hai <haijie1@huawei.com>
> Sent: Monday, January 30, 2023 8:29 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> konstantin.v.ananyev@yandex.ru; dev@dpdk.org
> Cc: liudongdong3@huawei.com; haijie1@huawei.com
> Subject: [PATCH v3 2/2] ring: add ring info telemetry cmd
"ring: add telemetry cmd for ring info"
> 
> This patch supports dump of the info of ring by its name.
This patch supports dump of ring information by its name.

> An example using this command is shown below:
> 
> --> /ring/info,MP_mb_pool_0
> {
>   "/ring/info": {
>     "name": "MP_mb_pool_0",
>     "socket": 0,
>     "flags": 0,
>     "producer_type": "MP",
>     "consumer_type": "MC",
>     "size": 262144,
>     "mask": 262143,
>     "capacity": 262143,
>     "used_count": 147173,
>     "consumer_tail": 8283,
>     "consumer_head": 8283,
>     "producer_tail": 155456,
>     "producer_head": 155456,
Sometimes it is much easier to understand these numbers if they are in hexadecimal. Is it possible to add the hexadecimal format in brackets? Something like:
"size": 262144 (0x40000)

>     "mz_name": "RG_MP_mb_pool_0",
>     "mz_len": 2097920,
>     "mz_hugepage_sz": 1073741824,
>     "mz_socket_id": 0,
>     "mz_flags": 0
>   }
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>  lib/ring/rte_ring.c | 83 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c index
> e6aac332d88f..2e57aa653339 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -454,8 +454,91 @@ ring_handle_list(const char *cmd __rte_unused,
>  	return 0;
>  }
> 
> +static const char *
> +ring_prod_sync_type_to_name(struct rte_ring *r) {
> +	switch (r->prod.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MP";
> +	case RTE_RING_SYNC_ST:
> +		return "SP";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MP_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MP_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static const char *
> +ring_cons_sync_type_to_name(struct rte_ring *r) {
> +	switch (r->cons.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MC";
> +	case RTE_RING_SYNC_ST:
> +		return "SC";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MC_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MC_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static int
> +ring_handle_info(const char *cmd __rte_unused, const char *params,
> +		struct rte_tel_data *d)
> +{
> +	const struct rte_memzone *mz;
> +	struct rte_ring *r;
> +
> +	if (params == NULL || strlen(params) == 0 ||
> +		strlen(params) >= RTE_RING_NAMESIZE)
> +		return -EINVAL;
> +
> +	r = rte_ring_lookup(params);
> +	if (r == NULL)
> +		return -EINVAL;
> +
> +	rte_mcfg_tailq_read_lock();
> +
> +	rte_tel_data_start_dict(d);
> +	rte_tel_data_add_dict_string(d, "name", r->name);
> +	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
> +	rte_tel_data_add_dict_int(d, "flags", r->flags);
> +	rte_tel_data_add_dict_string(d, "producer_type",
> +		ring_prod_sync_type_to_name(r));
> +	rte_tel_data_add_dict_string(d, "consumer_type",
> +		ring_cons_sync_type_to_name(r));
> +	rte_tel_data_add_dict_u64(d, "size", r->size);
> +	rte_tel_data_add_dict_u64(d, "mask", r->mask);
> +	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
> +	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
> +	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
> +	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
> +	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
> +	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
> +
> +	mz = r->memzone;
> +	if (mz == NULL)
> +		return 0;
> +	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
> +	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
> +	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
> +	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
> +	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
> +
> +	rte_mcfg_tailq_read_unlock();
> +	return 0;
> +}
> +
>  RTE_INIT(ring_init_telemetry)
>  {
>  	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>  		"Returns list of available ring. Takes no parameters");
> +	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
> +		"Returns ring info. Parameters: ring_name.");
>  }
> --
> 2.33.0

RE: [PATCH v3 1/2] ring: add ring list telemetry cmd

[Honnappa Nagarahalli]

Few minor comments inline. Otherwise,

Reviewed-by: Honnappa Nagarahalli <Honnappa.nagarahalli@arm.com>

> -----Original Message-----
> From: Jie Hai <haijie1@huawei.com>
> Sent: Monday, January 30, 2023 8:29 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> konstantin.v.ananyev@yandex.ru; dev@dpdk.org
> Cc: liudongdong3@huawei.com; haijie1@huawei.com
> Subject: [PATCH v3 1/2] ring: add ring list telemetry cmd
How about "ring: add telemetry cmd to list rings"

> 
> This patch supports the list of rings with telemetry cmd.
Add a telemetry command to list the rings used in the system.

> An example using this command is shown below:
> 
> --> /ring/list
> {
>   "/ring/list": [
>     "HT_0000:7d:00.2",
>     "MP_mb_pool_0"
>   ]
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> ---
>  lib/ring/meson.build |  1 +
>  lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/lib/ring/meson.build b/lib/ring/meson.build index
> c20685c689ac..7fca958ed7fa 100644
> --- a/lib/ring/meson.build
> +++ b/lib/ring/meson.build
> @@ -18,3 +18,4 @@ indirect_headers += files (
>          'rte_ring_rts.h',
>          'rte_ring_rts_elem_pvt.h',
>  )
> +deps += ['telemetry']
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c index
> cddaf6b2876f..e6aac332d88f 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -22,6 +22,7 @@
>  #include <rte_errno.h>
>  #include <rte_string_fns.h>
>  #include <rte_tailq.h>
> +#include <rte_telemetry.h>
> 
>  #include "rte_ring.h"
>  #include "rte_ring_elem.h"
> @@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
> 
>  	return r;
>  }
> +
> +static void
> +ring_walk(void (*func)(struct rte_ring *, void *), void *arg) {
> +	struct rte_ring_list *ring_list;
> +	struct rte_tailq_entry *te;
> +
> +	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
> +	rte_mcfg_tailq_read_lock();
> +
> +	TAILQ_FOREACH(te, ring_list, next) {
> +		(*func)((struct rte_ring *) te->data, arg);
> +	}
> +
> +	rte_mcfg_tailq_read_unlock();
> +}
> +
> +static void
> +ring_list_cb(struct rte_ring *r, void *arg) {
> +	struct rte_tel_data *d = (struct rte_tel_data *)arg;
> +
> +	rte_tel_data_add_array_string(d, r->name); }
> +
> +static int
> +ring_handle_list(const char *cmd __rte_unused,
> +		const char *params __rte_unused, struct rte_tel_data *d) {
> +	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
> +	ring_walk(ring_list_cb, d);
> +	return 0;
> +}
> +
> +RTE_INIT(ring_init_telemetry)
> +{
> +	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
> +		"Returns list of available ring. Takes no parameters"); }
                                                                         ^^^^ rings
> --
> 2.33.0

Re: [PATCH v3 2/2] ring: add ring info telemetry cmd

[Konstantin Ananyev]

31/01/2023 02:28, Jie Hai пишет:
> This patch supports dump of the info of ring by its name.
> An example using this command is shown below:
> 
> --> /ring/info,MP_mb_pool_0
> {
>    "/ring/info": {
>      "name": "MP_mb_pool_0",
>      "socket": 0,
>      "flags": 0,
>      "producer_type": "MP",
>      "consumer_type": "MC",
>      "size": 262144,
>      "mask": 262143,
>      "capacity": 262143,
>      "used_count": 147173,
>      "consumer_tail": 8283,
>      "consumer_head": 8283,
>      "producer_tail": 155456,
>      "producer_head": 155456,
>      "mz_name": "RG_MP_mb_pool_0",
>      "mz_len": 2097920,
>      "mz_hugepage_sz": 1073741824,
>      "mz_socket_id": 0,
>      "mz_flags": 0
>    }
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>   lib/ring/rte_ring.c | 83 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
> 
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index e6aac332d88f..2e57aa653339 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -454,8 +454,91 @@ ring_handle_list(const char *cmd __rte_unused,
>   	return 0;
>   }
>   
> +static const char *
> +ring_prod_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->prod.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MP";
> +	case RTE_RING_SYNC_ST:
> +		return "SP";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MP_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MP_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static const char *
> +ring_cons_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->cons.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MC";
> +	case RTE_RING_SYNC_ST:
> +		return "SC";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MC_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MC_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static int
> +ring_handle_info(const char *cmd __rte_unused, const char *params,
> +		struct rte_tel_data *d)
> +{
> +	const struct rte_memzone *mz;
> +	struct rte_ring *r;
> +
> +	if (params == NULL || strlen(params) == 0 ||
> +		strlen(params) >= RTE_RING_NAMESIZE)
> +		return -EINVAL;
> +
> +	r = rte_ring_lookup(params);
> +	if (r == NULL)
> +		return -EINVAL;

thanks for the update, but I think there still a potential problem here:
as we release tailq_lock inside ring_lookup() and then grab it after again.
Between these two points we have sort of race condition.
We need a way not to release it in between.
Probably the simplest way - make this function to use ring_walk()
that you introduced in previous patch, instead of ring_lookup().
Similar to what mempool_handle_info() is doing.


> +
> +	rte_mcfg_tailq_read_lock();
> +
> +	rte_tel_data_start_dict(d);
> +	rte_tel_data_add_dict_string(d, "name", r->name);
> +	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
> +	rte_tel_data_add_dict_int(d, "flags", r->flags);
> +	rte_tel_data_add_dict_string(d, "producer_type",
> +		ring_prod_sync_type_to_name(r));
> +	rte_tel_data_add_dict_string(d, "consumer_type",
> +		ring_cons_sync_type_to_name(r));
> +	rte_tel_data_add_dict_u64(d, "size", r->size);
> +	rte_tel_data_add_dict_u64(d, "mask", r->mask);
> +	rte_tel_data_add_dict_u64(d, "capacity", r->capacity);
> +	rte_tel_data_add_dict_u64(d, "used_count", rte_ring_count(r));
> +	rte_tel_data_add_dict_u64(d, "consumer_tail", r->cons.tail);
> +	rte_tel_data_add_dict_u64(d, "consumer_head", r->cons.head);
> +	rte_tel_data_add_dict_u64(d, "producer_tail", r->prod.tail);
> +	rte_tel_data_add_dict_u64(d, "producer_head", r->prod.head);
> +
> +	mz = r->memzone;
> +	if (mz == NULL)
> +		return 0;
> +	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
> +	rte_tel_data_add_dict_int(d, "mz_len", mz->len);
> +	rte_tel_data_add_dict_int(d, "mz_hugepage_sz", mz->hugepage_sz);
> +	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
> +	rte_tel_data_add_dict_int(d, "mz_flags", mz->flags);
> +
> +	rte_mcfg_tailq_read_unlock();
> +	return 0;
> +}
> +
>   RTE_INIT(ring_init_telemetry)
>   {
>   	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>   		"Returns list of available ring. Takes no parameters");
> +	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
> +		"Returns ring info. Parameters: ring_name.");
>   }

Re: [PATCH v3 2/2] ring: add ring info telemetry cmd

[Jie Hai]

On 2023/2/1 0:44, Honnappa Nagarahalli wrote:
> Few minor nits. Otherwise,
> 
> Reviewed-by: Honnappa Nagarahalli <Honnappa.nagarahalli@arm.com>
> 
Thanks for your review.
>> -----Original Message-----
>> From: Jie Hai <haijie1@huawei.com>
>> Sent: Monday, January 30, 2023 8:29 PM
>> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
>> konstantin.v.ananyev@yandex.ru; dev@dpdk.org
>> Cc: liudongdong3@huawei.com; haijie1@huawei.com
>> Subject: [PATCH v3 2/2] ring: add ring info telemetry cmd
> "ring: add telemetry cmd for ring info"
>>
>> This patch supports dump of the info of ring by its name.
> This patch supports dump of ring information by its name.
> 
Accepted and will change it in v4.
>> An example using this command is shown below:
>>
>> --> /ring/info,MP_mb_pool_0
>> {
>>    "/ring/info": {
>>      "name": "MP_mb_pool_0",
>>      "socket": 0,
>>      "flags": 0,
>>      "producer_type": "MP",
>>      "consumer_type": "MC",
>>      "size": 262144,
>>      "mask": 262143,
>>      "capacity": 262143,
>>      "used_count": 147173,
>>      "consumer_tail": 8283,
>>      "consumer_head": 8283,
>>      "producer_tail": 155456,
>>      "producer_head": 155456,
> Sometimes it is much easier to understand these numbers if they are in hexadecimal. Is it possible to add the hexadecimal format in brackets? Something like:
> "size": 262144 (0x40000)
>
Huisong Li <lihuisong@huawei.com> makes it possible in 
http://patches.dpdk.org/project/dpdk/patch/20221219070648.33817-7-lihuisong@huawei.com/.
We can change rte_tel_data_add_dict_u64 to 
rte_tel_data_add_dict_uint_hex after his patch accepted.

>> --
>> 2.33.0
> 
> .

Re: [PATCH v3 1/2] ring: add ring list telemetry cmd

[Jie Hai]

On 2023/2/1 0:44, Honnappa Nagarahalli wrote:
> Few minor comments inline. Otherwise,
> 
> Reviewed-by: Honnappa Nagarahalli <Honnappa.nagarahalli@arm.com>
> 
Thanks for your review.
>> -----Original Message-----
>> From: Jie Hai <haijie1@huawei.com>
>> Sent: Monday, January 30, 2023 8:29 PM
>> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
>> konstantin.v.ananyev@yandex.ru; dev@dpdk.org
>> Cc: liudongdong3@huawei.com; haijie1@huawei.com
>> Subject: [PATCH v3 1/2] ring: add ring list telemetry cmd
> How about "ring: add telemetry cmd to list rings"
> 
>>
>> This patch supports the list of rings with telemetry cmd.
> Add a telemetry command to list the rings used in the system.
> 
That sounds easier to understand. Accepted and will change it in v4.
>> An example using this command is shown below:
>>
>> --> /ring/list
>> {
>>    "/ring/list": [
>>      "HT_0000:7d:00.2",
>>      "MP_mb_pool_0"
>>    ]
>> }
>> +RTE_INIT(ring_init_telemetry)
>> +{
>> +	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>> +		"Returns list of available ring. Takes no parameters"); }
>                                                                           ^^^^ rings
Thank you very much for your comments. I will change it to "Returns list 
of available rings" in v4.
>> --
>> 2.33.0
> 
> .

Re: [PATCH v3 2/2] ring: add ring info telemetry cmd

[Jie Hai]

On 2023/2/2 21:07, Konstantin Ananyev wrote:
> 31/01/2023 02:28, Jie Hai пишет:
>> This patch supports dump of the info of ring by its name.
>> An example using this command is shown below:
>>
>> --> /ring/info,MP_mb_pool_0
>> {
>>    "/ring/info": {
>>      "name": "MP_mb_pool_0",
>>      "socket": 0,
>>      "flags": 0,
>>      "producer_type": "MP",
>>      "consumer_type": "MC",
>>      "size": 262144,
>>      "mask": 262143,
>>      "capacity": 262143,
>>      "used_count": 147173,
>>      "consumer_tail": 8283,
>>      "consumer_head": 8283,
>>      "producer_tail": 155456,
>>      "producer_head": 155456,
>>      "mz_name": "RG_MP_mb_pool_0",
>>      "mz_len": 2097920,
>>      "mz_hugepage_sz": 1073741824,
>>      "mz_socket_id": 0,
>>      "mz_flags": 0
>>    }
>> }
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>

>> +static int
>> +ring_handle_info(const char *cmd __rte_unused, const char *params,
>> +        struct rte_tel_data *d)
>> +{
>> +    const struct rte_memzone *mz;
>> +    struct rte_ring *r;
>> +
>> +    if (params == NULL || strlen(params) == 0 ||
>> +        strlen(params) >= RTE_RING_NAMESIZE)
>> +        return -EINVAL;
>> +
>> +    r = rte_ring_lookup(params);
>> +    if (r == NULL)
>> +        return -EINVAL;
> 
> thanks for the update, but I think there still a potential problem here:
> as we release tailq_lock inside ring_lookup() and then grab it after again.
> Between these two points we have sort of race condition.
> We need a way not to release it in between.
> Probably the simplest way - make this function to use ring_walk()
> that you introduced in previous patch, instead of ring_lookup().
> Similar to what mempool_handle_info() is doing.
> 
> 
Thanks for your comments, and I have learned a lot from it. That will be 
corrected in v4.

> .

[PATCH v4 0/3] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

Jie Hai (3):
  ring: fix unmatched type definition and usage
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build     |   1 +
 lib/ring/rte_ring.c      | 139 +++++++++++++++++++++++++++++++++++++++
 lib/ring/rte_ring_core.h |   2 +-
 3 files changed, 141 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v4 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

Field flags of struct rte_ring is defined as int type. However,
it is used as unsigned int. To ensure consistency, change the
type of flags to unsigned int.

Fixes: cc4b218790f6 ("ring: support configurable element size")

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 lib/ring/rte_ring_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index 82b237091b71..1c809abeb531 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {
 struct rte_ring {
 	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
 	/**< Name of the ring. */
-	int flags;               /**< Flags supplied at creation. */
+	uint32_t flags;               /**< Flags supplied at creation. */
 	const struct rte_memzone *memzone;
 			/**< Memzone, if any, containing the rte_ring */
 	uint32_t size;           /**< Size of ring. */
-- 
2.33.0

[PATCH v4 2/3] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index cddaf6b2876f..148defe22aa5 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -419,3 +420,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.33.0

[PATCH v4 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": "0x0",
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "consumer_tail": 2259,
    "consumer_head": 2259,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/ring/rte_ring.c | 99 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 148defe22aa5..9de51ddf492d 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -454,8 +454,107 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_uint(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_uint(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_uint(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_uint(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Konstantin Ananyev]

10/02/2023 02:48, Jie Hai пишет:
> This patch supports dump of ring information by its name.
> An example using this command is shown below:
> 
> --> /ring/info,MP_mb_pool_0
> {
>    "/ring/info": {
>      "name": "MP_mb_pool_0",
>      "socket": 0,
>      "flags": "0x0",
>      "producer_type": "MP",
>      "consumer_type": "MC",
>      "size": 262144,
>      "mask": "0x3ffff",
>      "capacity": 262143,
>      "used_count": 153197,
>      "consumer_tail": 2259,
>      "consumer_head": 2259,
>      "producer_tail": 155456,
>      "producer_head": 155456,
>      "mz_name": "RG_MP_mb_pool_0",
>      "mz_len": 2097536,
>      "mz_hugepage_sz": 1073741824,
>      "mz_socket_id": 0,
>      "mz_flags": "0x0"
>    }
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---

>   }

Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>

Re: [PATCH v4 0/3] add telemetry cmds for ring

[fengchengwen]

LGTM
Series-acked-by: Chengwen Feng <fengchengwen@huawei.com>

On 2023/2/10 10:48, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>    for mask and flags.
> 
> Jie Hai (3):
>   ring: fix unmatched type definition and usage
>   ring: add telemetry cmd to list rings
>   ring: add telemetry cmd for ring info
> 
>  lib/ring/meson.build     |   1 +
>  lib/ring/rte_ring.c      | 139 +++++++++++++++++++++++++++++++++++++++
>  lib/ring/rte_ring_core.h |   2 +-
>  3 files changed, 141 insertions(+), 1 deletion(-)
> 

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[lihuisong (C)]

在 2023/2/10 10:48, Jie Hai 写道:
> This patch supports dump of ring information by its name.
> An example using this command is shown below:
>
> --> /ring/info,MP_mb_pool_0
> {
>    "/ring/info": {
>      "name": "MP_mb_pool_0",
>      "socket": 0,
>      "flags": "0x0",
>      "producer_type": "MP",
>      "consumer_type": "MC",
>      "size": 262144,
>      "mask": "0x3ffff",
>      "capacity": 262143,
>      "used_count": 153197,
>      "consumer_tail": 2259,
>      "consumer_head": 2259,
>      "producer_tail": 155456,
>      "producer_head": 155456,
>      "mz_name": "RG_MP_mb_pool_0",
>      "mz_len": 2097536,
>      "mz_hugepage_sz": 1073741824,
>      "mz_socket_id": 0,
>      "mz_flags": "0x0"
>    }
> }
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
lgtm,
Acked-by: Huisong Li <lihuisong@huawei.com>

Re: [PATCH v4 2/3] ring: add telemetry cmd to list rings

[lihuisong (C)]

在 2023/2/10 10:48, Jie Hai 写道:
> Add a telemetry command to list the rings used in the system.
> An example using this command is shown below:
>
> --> /ring/list
> {
>    "/ring/list": [
>      "HT_0000:7d:00.2",
>      "MP_mb_pool_0"
>    ]
> }
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
lgtm,
Acked-by: Huisong Li <lihuisong@huawei.com>

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[David Marchand]

On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
>
> This patch supports dump of ring information by its name.
> An example using this command is shown below:
>
> --> /ring/info,MP_mb_pool_0
> {
>   "/ring/info": {
>     "name": "MP_mb_pool_0",
>     "socket": 0,
>     "flags": "0x0",
>     "producer_type": "MP",
>     "consumer_type": "MC",
>     "size": 262144,
>     "mask": "0x3ffff",
>     "capacity": 262143,
>     "used_count": 153197,
>     "consumer_tail": 2259,
>     "consumer_head": 2259,
>     "producer_tail": 155456,
>     "producer_head": 155456,

What would an external user make of such an information?

I'd like to have a better idea what your usecase is.
If it is for debugging, well, gdb is probably a better candidate.


>     "mz_name": "RG_MP_mb_pool_0",
>     "mz_len": 2097536,
>     "mz_hugepage_sz": 1073741824,
>     "mz_socket_id": 0,
>     "mz_flags": "0x0"
>   }
> }


-- 
David Marchand

[PATCH v5 0/3] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

v4-v5:
1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
2. Add ABI change explanation for commit message of patch 1/3.

Jie Hai (3):
  ring: fix unmatched type definition and usage
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build     |   1 +
 lib/ring/rte_ring.c      | 139 +++++++++++++++++++++++++++++++++++++++
 lib/ring/rte_ring_core.h |   2 +-
 3 files changed, 141 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v5 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

Field 'flags' of struct rte_ring is defined as int type. However,
it is used as unsigned int. To ensure consistency, change the
type of flags to unsigned int. Since these two types has the
same byte size, this change is not an ABI change.

Fixes: cc4b218790f6 ("ring: support configurable element size")

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ring/rte_ring_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index 82b237091b71..1c809abeb531 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {
 struct rte_ring {
 	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
 	/**< Name of the ring. */
-	int flags;               /**< Flags supplied at creation. */
+	uint32_t flags;               /**< Flags supplied at creation. */
 	const struct rte_memzone *memzone;
 			/**< Memzone, if any, containing the rte_ring */
 	uint32_t size;           /**< Size of ring. */
-- 
2.33.0

[PATCH v5 2/3] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 8ed455043dee..0e83d0099363 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -420,3 +421,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.33.0

[PATCH v5 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": "0x0",
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "consumer_tail": 2259,
    "consumer_head": 2259,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ring/rte_ring.c | 99 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 0e83d0099363..26c8f2a2e6a2 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -455,8 +455,107 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_uint(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_uint(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_uint(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_uint(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

RE: [PATCH v5 1/3] ring: fix unmatched type definition and usage

[Ruifeng Wang]

> -----Original Message-----
> From: Jie Hai <haijie1@huawei.com>
> Sent: Tuesday, May 9, 2023 9:29 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin Ananyev
> <konstantin.v.ananyev@yandex.ru>; Ruifeng Wang <Ruifeng.Wang@arm.com>; Gavin Hu
> <Gavin.Hu@arm.com>; Olivier Matz <olivier.matz@6wind.com>; Dharmik Jayesh Thakkar
> <DharmikJayesh.Thakkar@arm.com>
> Cc: dev@dpdk.org; liudongdong3@huawei.com
> Subject: [PATCH v5 1/3] ring: fix unmatched type definition and usage
> 
> Field 'flags' of struct rte_ring is defined as int type. However, it is used as unsigned
> int. To ensure consistency, change the type of flags to unsigned int. Since these two
> types has the same byte size, this change is not an ABI change.
> 
> Fixes: cc4b218790f6 ("ring: support configurable element size")

The change looks good.
However, I think the fix line is not accurate. 
I suppose it fixes af75078fece3 ("first public release").

> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  lib/ring/rte_ring_core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h index
> 82b237091b71..1c809abeb531 100644
> --- a/lib/ring/rte_ring_core.h
> +++ b/lib/ring/rte_ring_core.h
> @@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {  struct rte_ring {
>  	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
>  	/**< Name of the ring. */
> -	int flags;               /**< Flags supplied at creation. */
> +	uint32_t flags;               /**< Flags supplied at creation. */
>  	const struct rte_memzone *memzone;
>  			/**< Memzone, if any, containing the rte_ring */
>  	uint32_t size;           /**< Size of ring. */
> --
> 2.33.0

RE: [PATCH v5 3/3] ring: add telemetry cmd for ring info

[Morten Brørup]

> From: Jie Hai [mailto:haijie1@huawei.com]
> Sent: Tuesday, 9 May 2023 03.29
> 
> This patch supports dump of ring information by its name.
> An example using this command is shown below:
> 
> --> /ring/info,MP_mb_pool_0
> {
>   "/ring/info": {
>     "name": "MP_mb_pool_0",
>     "socket": 0,
>     "flags": "0x0",
>     "producer_type": "MP",
>     "consumer_type": "MC",
>     "size": 262144,
>     "mask": "0x3ffff",
>     "capacity": 262143,
>     "used_count": 153197,
>     "consumer_tail": 2259,
>     "consumer_head": 2259,
>     "producer_tail": 155456,
>     "producer_head": 155456,
>     "mz_name": "RG_MP_mb_pool_0",
>     "mz_len": 2097536,
>     "mz_hugepage_sz": 1073741824,
>     "mz_socket_id": 0,
>     "mz_flags": "0x0"
>   }
> }
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  lib/ring/rte_ring.c | 99 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index 0e83d0099363..26c8f2a2e6a2 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -455,8 +455,107 @@ ring_handle_list(const char *cmd __rte_unused,
>  	return 0;
>  }
> 
> +static const char *
> +ring_prod_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->prod.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MP";
> +	case RTE_RING_SYNC_ST:
> +		return "SP";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MP_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MP_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static const char *
> +ring_cons_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->cons.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MC";
> +	case RTE_RING_SYNC_ST:
> +		return "SC";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MC_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MC_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}

I considered if these two functions should be replaced by one, returning e.g. "MT" instead of "MP"/"MC"; however, the ring flags describe the sync types as e.g. "MP" and  "MP RTS", so I eventually came to agree with the approach in the patch.

The structure documentation requires that the sync_type field must be in the same position, regardless which of the union's types are being used. So this is also correct.

Series-Acked-by: Morten Brørup <mb@smartsharesystems.com>

Re: [PATCH v5 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

On 2023/5/9 14:23, Ruifeng Wang wrote:
>> -----Original Message-----
>> From: Jie Hai <haijie1@huawei.com>
>> Sent: Tuesday, May 9, 2023 9:29 AM
>> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin Ananyev
>> <konstantin.v.ananyev@yandex.ru>; Ruifeng Wang <Ruifeng.Wang@arm.com>; Gavin Hu
>> <Gavin.Hu@arm.com>; Olivier Matz <olivier.matz@6wind.com>; Dharmik Jayesh Thakkar
>> <DharmikJayesh.Thakkar@arm.com>
>> Cc: dev@dpdk.org; liudongdong3@huawei.com
>> Subject: [PATCH v5 1/3] ring: fix unmatched type definition and usage
>>
>> Field 'flags' of struct rte_ring is defined as int type. However, it is used as unsigned
>> int. To ensure consistency, change the type of flags to unsigned int. Since these two
>> types has the same byte size, this change is not an ABI change.
>>
>> Fixes: cc4b218790f6 ("ring: support configurable element size")
> 
> The change looks good.
> However, I think the fix line is not accurate.
> I suppose it fixes af75078fece3 ("first public release").
> 
Thanks for your review. Sorry for quoting the wrong commit.
This issue was indeed introduced by commit af75078fece3 ("first public 
release").
I will fix this in the next version.
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>   lib/ring/rte_ring_core.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h index
>> 82b237091b71..1c809abeb531 100644
>> --- a/lib/ring/rte_ring_core.h
>> +++ b/lib/ring/rte_ring_core.h
>> @@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {  struct rte_ring {
>>   	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
>>   	/**< Name of the ring. */
>> -	int flags;               /**< Flags supplied at creation. */
>> +	uint32_t flags;               /**< Flags supplied at creation. */
>>   	const struct rte_memzone *memzone;
>>   			/**< Memzone, if any, containing the rte_ring */
>>   	uint32_t size;           /**< Size of ring. */
>> --
>> 2.33.0
> 
> .

[PATCH v6 0/3] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

v4->v5:
1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
2. Add ABI change explanation for commit message of patch 1/3.

v5->v6:
1. Add Acked-by Morten Brørup.
2. Fix incorrect reference of commit.

Jie Hai (3):
  ring: fix unmatched type definition and usage
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build     |   1 +
 lib/ring/rte_ring.c      | 139 +++++++++++++++++++++++++++++++++++++++
 lib/ring/rte_ring_core.h |   2 +-
 3 files changed, 141 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v6 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

Field 'flags' of struct rte_ring is defined as int type. However,
it is used as unsigned int. To ensure consistency, change the
type of flags to unsigned int. Since these two types has the
same byte size, this change is not an ABI change.

Fixes: af75078fece3 ("first public release")

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index 82b237091b71..1c809abeb531 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {
 struct rte_ring {
 	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
 	/**< Name of the ring. */
-	int flags;               /**< Flags supplied at creation. */
+	uint32_t flags;               /**< Flags supplied at creation. */
 	const struct rte_memzone *memzone;
 			/**< Memzone, if any, containing the rte_ring */
 	uint32_t size;           /**< Size of ring. */
-- 
2.33.0

[PATCH v6 2/3] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 8ed455043dee..0e83d0099363 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -420,3 +421,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.33.0

[PATCH v6 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": "0x0",
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "consumer_tail": 2259,
    "consumer_head": 2259,
    "producer_tail": 155456,
    "producer_head": 155456,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring.c | 99 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 0e83d0099363..26c8f2a2e6a2 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -455,8 +455,107 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+	rte_tel_data_add_dict_uint(d, "consumer_tail", r->cons.tail);
+	rte_tel_data_add_dict_uint(d, "consumer_head", r->cons.head);
+	rte_tel_data_add_dict_uint(d, "producer_tail", r->prod.tail);
+	rte_tel_data_add_dict_uint(d, "producer_head", r->prod.head);
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

Re: [PATCH v6 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas and all maintainers,
Kindly ping for comments, thanks.

On 2023/5/9 17:24, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 139 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 141 insertions(+), 1 deletion(-)
> 

Re: [PATCH v6 0/3] add telemetry cmds for ring

[Thomas Monjalon]

30/05/2023 11:27, Jie Hai:
> Hi, Thomas and all maintainers,
> Kindly ping for comments, thanks.

You didn't reply to the question from David,
suggesting that GDB would be better used for debugging
than telemetry.

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

On 2023/2/20 20:55, David Marchand wrote:
> On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
>>
>> This patch supports dump of ring information by its name.
>> An example using this command is shown below:
>>
>> --> /ring/info,MP_mb_pool_0
>> {
>>    "/ring/info": {
>>      "name": "MP_mb_pool_0",
>>      "socket": 0,
>>      "flags": "0x0",
>>      "producer_type": "MP",
>>      "consumer_type": "MC",
>>      "size": 262144,
>>      "mask": "0x3ffff",
>>      "capacity": 262143,
>>      "used_count": 153197,
>>      "consumer_tail": 2259,
>>      "consumer_head": 2259,
>>      "producer_tail": 155456,
>>      "producer_head": 155456,
> 
> What would an external user make of such an information?
> 
> I'd like to have a better idea what your usecase is.
> If it is for debugging, well, gdb is probably a better candidate.
> 
> 
Hi David,
Thanks for your question and I'm sorry for getting back to you so late.
There was a problem with my mailbox and I lost all my mails.

The ring information exported by telemetry can be used to check the ring
status periodically during normal use. When an error occurs, the fault 
cause can be deduced based on the information.
GDB is more suitable for locating errors only when they are sure that
errors will occur.

>>      "mz_name": "RG_MP_mb_pool_0",
>>      "mz_len": 2097536,
>>      "mz_hugepage_sz": 1073741824,
>>      "mz_socket_id": 0,
>>      "mz_flags": "0x0"
>>    }
>> }
> 
> 

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Thomas Monjalon]

20/06/2023 10:14, Jie Hai:
> On 2023/2/20 20:55, David Marchand wrote:
> > On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
> >>
> >> This patch supports dump of ring information by its name.
> >> An example using this command is shown below:
> >>
> >> --> /ring/info,MP_mb_pool_0
> >> {
> >>    "/ring/info": {
> >>      "name": "MP_mb_pool_0",
> >>      "socket": 0,
> >>      "flags": "0x0",
> >>      "producer_type": "MP",
> >>      "consumer_type": "MC",
> >>      "size": 262144,
> >>      "mask": "0x3ffff",
> >>      "capacity": 262143,
> >>      "used_count": 153197,
> >>      "consumer_tail": 2259,
> >>      "consumer_head": 2259,
> >>      "producer_tail": 155456,
> >>      "producer_head": 155456,
> > 
> > What would an external user make of such an information?
> > 
> > I'd like to have a better idea what your usecase is.
> > If it is for debugging, well, gdb is probably a better candidate.
> > 
> > 
> Hi David,
> Thanks for your question and I'm sorry for getting back to you so late.
> There was a problem with my mailbox and I lost all my mails.
> 
> The ring information exported by telemetry can be used to check the ring
> status periodically during normal use. When an error occurs, the fault 
> cause can be deduced based on the information.
> GDB is more suitable for locating errors only when they are sure that
> errors will occur.

Yes, when an error occurs, you can use GDB,
and you don't need all these internal values in telemetry.

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

On 2023/6/20 22:34, Thomas Monjalon wrote:
> 20/06/2023 10:14, Jie Hai:
>> On 2023/2/20 20:55, David Marchand wrote:
>>> On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
>>>>
>>>> This patch supports dump of ring information by its name.
>>>> An example using this command is shown below:
>>>>
>>>> --> /ring/info,MP_mb_pool_0
>>>> {
>>>>     "/ring/info": {
>>>>       "name": "MP_mb_pool_0",
>>>>       "socket": 0,
>>>>       "flags": "0x0",
>>>>       "producer_type": "MP",
>>>>       "consumer_type": "MC",
>>>>       "size": 262144,
>>>>       "mask": "0x3ffff",
>>>>       "capacity": 262143,
>>>>       "used_count": 153197,
>>>>       "consumer_tail": 2259,
>>>>       "consumer_head": 2259,
>>>>       "producer_tail": 155456,
>>>>       "producer_head": 155456,
>>>
>>> What would an external user make of such an information?
>>>
>>> I'd like to have a better idea what your usecase is.
>>> If it is for debugging, well, gdb is probably a better candidate.
>>>
>>>
>> Hi David,
>> Thanks for your question and I'm sorry for getting back to you so late.
>> There was a problem with my mailbox and I lost all my mails.
>>
>> The ring information exported by telemetry can be used to check the ring
>> status periodically during normal use. When an error occurs, the fault
>> cause can be deduced based on the information.
>> GDB is more suitable for locating errors only when they are sure that
>> errors will occur.
> 
> Yes, when an error occurs, you can use GDB,
> and you don't need all these internal values in telemetry.
> 
> 
Hi, David, Thomas,

Would it be better to delete the last four items?
        "consumer_tail": 2259,
        "consumer_head": 2259,
        "producer_tail": 155456,
        "producer_head": 155456,

Thanks, Jie Hai

> 
> .

[PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

v4->v5:
1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
2. Add ABI change explanation for commit message of patch 1/3.

v5->v6:
1. Add Acked-by Morten Brørup.
2. Fix incorrect reference of commit.

v6->v7:
1. Remove prod/consumer head/tail info.

Jie Hai (3):
  ring: fix unmatched type definition and usage
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build     |   1 +
 lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
 lib/ring/rte_ring_core.h |   2 +-
 3 files changed, 137 insertions(+), 1 deletion(-)

-- 
2.33.0

[PATCH v7 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

Field 'flags' of struct rte_ring is defined as int type. However,
it is used as unsigned int. To ensure consistency, change the
type of flags to unsigned int. Since these two types has the
same byte size, this change is not an ABI change.

Fixes: af75078fece3 ("first public release")

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index 82b237091b71..1c809abeb531 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -120,7 +120,7 @@ struct rte_ring_hts_headtail {
 struct rte_ring {
 	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
 	/**< Name of the ring. */
-	int flags;               /**< Flags supplied at creation. */
+	uint32_t flags;               /**< Flags supplied at creation. */
 	const struct rte_memzone *memzone;
 			/**< Memzone, if any, containing the rte_ring */
 	uint32_t size;           /**< Size of ring. */
-- 
2.33.0

[PATCH v7 2/3] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 057d25ff6f2f..6a10280fb093 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -418,3 +419,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.33.0

[PATCH v7 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": "0x0",
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 6a10280fb093..5ee15037548e 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -453,8 +453,103 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.33.0

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Thomas Monjalon]

04/07/2023 10:04, Jie Hai:
> On 2023/6/20 22:34, Thomas Monjalon wrote:
> > 20/06/2023 10:14, Jie Hai:
> >> On 2023/2/20 20:55, David Marchand wrote:
> >>> On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
> >>>>
> >>>> This patch supports dump of ring information by its name.
> >>>> An example using this command is shown below:
> >>>>
> >>>> --> /ring/info,MP_mb_pool_0
> >>>> {
> >>>>     "/ring/info": {
> >>>>       "name": "MP_mb_pool_0",
> >>>>       "socket": 0,
> >>>>       "flags": "0x0",
> >>>>       "producer_type": "MP",
> >>>>       "consumer_type": "MC",
> >>>>       "size": 262144,
> >>>>       "mask": "0x3ffff",
> >>>>       "capacity": 262143,
> >>>>       "used_count": 153197,
> >>>>       "consumer_tail": 2259,
> >>>>       "consumer_head": 2259,
> >>>>       "producer_tail": 155456,
> >>>>       "producer_head": 155456,
> >>>
> >>> What would an external user make of such an information?
> >>>
> >>> I'd like to have a better idea what your usecase is.
> >>> If it is for debugging, well, gdb is probably a better candidate.
> >>>
> >>>
> >> Hi David,
> >> Thanks for your question and I'm sorry for getting back to you so late.
> >> There was a problem with my mailbox and I lost all my mails.
> >>
> >> The ring information exported by telemetry can be used to check the ring
> >> status periodically during normal use. When an error occurs, the fault
> >> cause can be deduced based on the information.
> >> GDB is more suitable for locating errors only when they are sure that
> >> errors will occur.
> > 
> > Yes, when an error occurs, you can use GDB,
> > and you don't need all these internal values in telemetry.
> > 
> > 
> Hi, David, Thomas,
> 
> Would it be better to delete the last four items?
>         "consumer_tail": 2259,
>         "consumer_head": 2259,
>         "producer_tail": 155456,
>         "producer_head": 155456,

Yes it would be better.
David, other maintainers, would it make the telemetry command a good idea?

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[David Marchand]

On Tue, Jul 4, 2023 at 4:11 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 04/07/2023 10:04, Jie Hai:
> > On 2023/6/20 22:34, Thomas Monjalon wrote:
> > > 20/06/2023 10:14, Jie Hai:
> > >> On 2023/2/20 20:55, David Marchand wrote:
> > >>> On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
> > >>>>
> > >>>> This patch supports dump of ring information by its name.
> > >>>> An example using this command is shown below:
> > >>>>
> > >>>> --> /ring/info,MP_mb_pool_0
> > >>>> {
> > >>>>     "/ring/info": {
> > >>>>       "name": "MP_mb_pool_0",
> > >>>>       "socket": 0,
> > >>>>       "flags": "0x0",
> > >>>>       "producer_type": "MP",
> > >>>>       "consumer_type": "MC",
> > >>>>       "size": 262144,
> > >>>>       "mask": "0x3ffff",
> > >>>>       "capacity": 262143,
> > >>>>       "used_count": 153197,
> > >>>>       "consumer_tail": 2259,
> > >>>>       "consumer_head": 2259,
> > >>>>       "producer_tail": 155456,
> > >>>>       "producer_head": 155456,
> > >>>
> > >>> What would an external user make of such an information?
> > >>>
> > >>> I'd like to have a better idea what your usecase is.
> > >>> If it is for debugging, well, gdb is probably a better candidate.
> > >>>
> > >>>
> > >> Hi David,
> > >> Thanks for your question and I'm sorry for getting back to you so late.
> > >> There was a problem with my mailbox and I lost all my mails.
> > >>
> > >> The ring information exported by telemetry can be used to check the ring
> > >> status periodically during normal use. When an error occurs, the fault
> > >> cause can be deduced based on the information.
> > >> GDB is more suitable for locating errors only when they are sure that
> > >> errors will occur.
> > >
> > > Yes, when an error occurs, you can use GDB,
> > > and you don't need all these internal values in telemetry.
> > >
> > >
> > Hi, David, Thomas,
> >
> > Would it be better to delete the last four items?
> >         "consumer_tail": 2259,
> >         "consumer_head": 2259,
> >         "producer_tail": 155456,
> >         "producer_head": 155456,
>
> Yes it would be better.
> David, other maintainers, would it make the telemetry command a good idea?
>
>

Without the ring head/tail exposed, it seems ok.
It still exposes the ring flags which are kind of internal things, but
those are parts of the API/ABI, iiuc, so it should not be an issue.


-- 
David Marchand

Re: [PATCH v4 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

On 2023/7/6 16:52, David Marchand wrote:
> On Tue, Jul 4, 2023 at 4:11 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 04/07/2023 10:04, Jie Hai:
>>> On 2023/6/20 22:34, Thomas Monjalon wrote:
>>>> 20/06/2023 10:14, Jie Hai:
>>>>> On 2023/2/20 20:55, David Marchand wrote:
>>>>>> On Fri, Feb 10, 2023 at 3:50 AM Jie Hai <haijie1@huawei.com> wrote:
>>>>>>>
>>>>>>> This patch supports dump of ring information by its name.
>>>>>>> An example using this command is shown below:
>>>>>>>
>>>>>>> --> /ring/info,MP_mb_pool_0
>>>>>>> {
>>>>>>>      "/ring/info": {
>>>>>>>        "name": "MP_mb_pool_0",
>>>>>>>        "socket": 0,
>>>>>>>        "flags": "0x0",
>>>>>>>        "producer_type": "MP",
>>>>>>>        "consumer_type": "MC",
>>>>>>>        "size": 262144,
>>>>>>>        "mask": "0x3ffff",
>>>>>>>        "capacity": 262143,
>>>>>>>        "used_count": 153197,
>>>>>>>        "consumer_tail": 2259,
>>>>>>>        "consumer_head": 2259,
>>>>>>>        "producer_tail": 155456,
>>>>>>>        "producer_head": 155456,
>>>>>>
>>>>>> What would an external user make of such an information?
>>>>>>
>>>>>> I'd like to have a better idea what your usecase is.
>>>>>> If it is for debugging, well, gdb is probably a better candidate.
>>>>>>
>>>>>>
>>>>> Hi David,
>>>>> Thanks for your question and I'm sorry for getting back to you so late.
>>>>> There was a problem with my mailbox and I lost all my mails.
>>>>>
>>>>> The ring information exported by telemetry can be used to check the ring
>>>>> status periodically during normal use. When an error occurs, the fault
>>>>> cause can be deduced based on the information.
>>>>> GDB is more suitable for locating errors only when they are sure that
>>>>> errors will occur.
>>>>
>>>> Yes, when an error occurs, you can use GDB,
>>>> and you don't need all these internal values in telemetry.
>>>>
>>>>
>>> Hi, David, Thomas,
>>>
>>> Would it be better to delete the last four items?
>>>          "consumer_tail": 2259,
>>>          "consumer_head": 2259,
>>>          "producer_tail": 155456,
>>>          "producer_head": 155456,
>>
>> Yes it would be better.
>> David, other maintainers, would it make the telemetry command a good idea?
>>
>>
> 
> Without the ring head/tail exposed, it seems ok.
> It still exposes the ring flags which are kind of internal things, but
> those are parts of the API/ABI, iiuc, so it should not be an issue.
> 
> 
Similar to "name" and "size" of ring, "flags" of ring is also determined 
by user input. I think it's ok to expose it, users can use it to check 
if the configuration is as they want.
And the proc-info also exposes this flag.

Re: [PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas,

Kindly ping for review.

Thanks, Jie Hai

On 2023/7/4 17:04, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas,

Kindly ping for review.

Thanks,
Jie Hai

On 2023/7/4 17:04, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas,

Kindly ping for review.

Thanks,
Jie Hai

On 2023/7/4 17:04, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas,

Kindly ping for review.

Thanks,
Jie Hai
On 2023/7/4 17:04, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [PATCH v6 0/3] add telemetry cmds for ring

[Jie Hai]

On 2023/6/12 22:54, Thomas Monjalon wrote:
> 30/05/2023 11:27, Jie Hai:
>> Hi, Thomas and all maintainers,
>> Kindly ping for comments, thanks.
> 
> You didn't reply to the question from David,
> suggesting that GDB would be better used for debugging
> than telemetry.
> 
Hi, Thomas Monjalon,
The question has been solved by removing the display of the related field.

Thanks,
Jie Hai
> 
> 
> .

Re: [PATCH v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, all maintainers,

Is there any other problem with this patchset?
Please let me know if there is any, thank you.

Thanks,
Jie Hai

On 2023/7/4 17:04, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [PATCH v7 0/3] add telemetry cmds for ring

[lihuisong (C)]

lgtm, for series,
Acked-by: Huisong Li <lihuisong@huawei.com>


在 2023/7/4 17:04, Jie Hai 写道:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
>
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
>
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
>
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
>
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
>
> v5->v6:
> 1. Add Acked-by Morten Brørup.
> 2. Fix incorrect reference of commit.
>
> v6->v7:
> 1. Remove prod/consumer head/tail info.
>
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
>
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
>

[RESEND v7 0/3] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

v4->v5:
1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
2. Add ABI change explanation for commit message of patch 1/3.

v5->v6:
1. Add Acked-by Morten Br?rup.
2. Fix incorrect reference of commit.

v6->v7:
1. Remove prod/consumer head/tail info.

Jie Hai (3):
  ring: fix unmatched type definition and usage
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build     |   1 +
 lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
 lib/ring/rte_ring_core.h |   2 +-
 3 files changed, 137 insertions(+), 1 deletion(-)

-- 
2.30.0

[RESEND v7 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

Field 'flags' of struct rte_ring is defined as int type. However,
it is used as unsigned int. To ensure consistency, change the
type of flags to unsigned int. Since these two types has the
same byte size, this change is not an ABI change.

Fixes: af75078fece3 ("first public release")

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring_core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
index b7708730658a..14dac6495d83 100644
--- a/lib/ring/rte_ring_core.h
+++ b/lib/ring/rte_ring_core.h
@@ -119,7 +119,7 @@ struct rte_ring_hts_headtail {
 struct rte_ring {
 	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
 	/**< Name of the ring. */
-	int flags;               /**< Flags supplied at creation. */
+	uint32_t flags;               /**< Flags supplied at creation. */
 	const struct rte_memzone *memzone;
 			/**< Memzone, if any, containing the rte_ring */
 	uint32_t size;           /**< Size of ring. */
-- 
2.30.0

[RESEND v7 2/3] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 057d25ff6f2f..6a10280fb093 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -418,3 +419,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.30.0

[RESEND v7 3/3] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": "0x0",
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 6a10280fb093..5ee15037548e 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -453,8 +453,103 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.30.0

Re: [RESEND v7 1/3] ring: fix unmatched type definition and usage

[lihuisong (C)]

Acked-by: Huisong Li <lihuisong@huawei.com>

在 2023/11/9 18:20, Jie Hai 写道:
> Field 'flags' of struct rte_ring is defined as int type. However,
> it is used as unsigned int. To ensure consistency, change the
> type of flags to unsigned int. Since these two types has the
> same byte size, this change is not an ABI change.
>
> Fixes: af75078fece3 ("first public release")
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>   lib/ring/rte_ring_core.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
> index b7708730658a..14dac6495d83 100644
> --- a/lib/ring/rte_ring_core.h
> +++ b/lib/ring/rte_ring_core.h
> @@ -119,7 +119,7 @@ struct rte_ring_hts_headtail {
>   struct rte_ring {
>   	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
>   	/**< Name of the ring. */
> -	int flags;               /**< Flags supplied at creation. */
> +	uint32_t flags;               /**< Flags supplied at creation. */
>   	const struct rte_memzone *memzone;
>   			/**< Memzone, if any, containing the rte_ring */
>   	uint32_t size;           /**< Size of ring. */

Re: [RESEND v7 2/3] ring: add telemetry cmd to list rings

[lihuisong (C)]

Acked-by: Huisong Li <lihuisong@huawei.com>

在 2023/11/9 18:20, Jie Hai 写道:
> Add a telemetry command to list the rings used in the system.
> An example using this command is shown below:
>
> --> /ring/list
> {
>    "/ring/list": [
>      "HT_0000:7d:00.2",
>      "MP_mb_pool_0"
>    ]
> }
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>   lib/ring/meson.build |  1 +
>   lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 41 insertions(+)
>
> diff --git a/lib/ring/meson.build b/lib/ring/meson.build
> index c20685c689ac..7fca958ed7fa 100644
> --- a/lib/ring/meson.build
> +++ b/lib/ring/meson.build
> @@ -18,3 +18,4 @@ indirect_headers += files (
>           'rte_ring_rts.h',
>           'rte_ring_rts_elem_pvt.h',
>   )
> +deps += ['telemetry']
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index 057d25ff6f2f..6a10280fb093 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -22,6 +22,7 @@
>   #include <rte_errno.h>
>   #include <rte_string_fns.h>
>   #include <rte_tailq.h>
> +#include <rte_telemetry.h>
>   
>   #include "rte_ring.h"
>   #include "rte_ring_elem.h"
> @@ -418,3 +419,42 @@ rte_ring_lookup(const char *name)
>   
>   	return r;
>   }
> +
> +static void
> +ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
> +{
> +	struct rte_ring_list *ring_list;
> +	struct rte_tailq_entry *tailq_entry;
> +
> +	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
> +	rte_mcfg_tailq_read_lock();
> +
> +	TAILQ_FOREACH(tailq_entry, ring_list, next) {
> +		(*func)((struct rte_ring *) tailq_entry->data, arg);
> +	}
> +
> +	rte_mcfg_tailq_read_unlock();
> +}
> +
> +static void
> +ring_list_cb(struct rte_ring *r, void *arg)
> +{
> +	struct rte_tel_data *d = (struct rte_tel_data *)arg;
> +
> +	rte_tel_data_add_array_string(d, r->name);
> +}
> +
> +static int
> +ring_handle_list(const char *cmd __rte_unused,
> +		const char *params __rte_unused, struct rte_tel_data *d)
> +{
> +	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
> +	ring_walk(ring_list_cb, d);
> +	return 0;
> +}
> +
> +RTE_INIT(ring_init_telemetry)
> +{
> +	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
> +		"Returns list of available rings. Takes no parameters");
> +}

Re: [RESEND v7 3/3] ring: add telemetry cmd for ring info

[lihuisong (C)]

Acked-by: Huisong Li <lihuisong@huawei.com>

在 2023/11/9 18:20, Jie Hai 写道:
> This patch supports dump of ring information by its name.
> An example using this command is shown below:
>
> --> /ring/info,MP_mb_pool_0
> {
>    "/ring/info": {
>      "name": "MP_mb_pool_0",
>      "socket": 0,
>      "flags": "0x0",
>      "producer_type": "MP",
>      "consumer_type": "MC",
>      "size": 262144,
>      "mask": "0x3ffff",
>      "capacity": 262143,
>      "used_count": 153197,
>      "mz_name": "RG_MP_mb_pool_0",
>      "mz_len": 2097536,
>      "mz_hugepage_sz": 1073741824,
>      "mz_socket_id": 0,
>      "mz_flags": "0x0"
>    }
> }
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Huisong Li <lihuisong@huawei.com>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>   lib/ring/rte_ring.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 95 insertions(+)
>
> diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
> index 6a10280fb093..5ee15037548e 100644
> --- a/lib/ring/rte_ring.c
> +++ b/lib/ring/rte_ring.c
> @@ -453,8 +453,103 @@ ring_handle_list(const char *cmd __rte_unused,
>   	return 0;
>   }
>   
> +static const char *
> +ring_prod_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->prod.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MP";
> +	case RTE_RING_SYNC_ST:
> +		return "SP";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MP_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MP_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +static const char *
> +ring_cons_sync_type_to_name(struct rte_ring *r)
> +{
> +	switch (r->cons.sync_type) {
> +	case RTE_RING_SYNC_MT:
> +		return "MC";
> +	case RTE_RING_SYNC_ST:
> +		return "SC";
> +	case RTE_RING_SYNC_MT_RTS:
> +		return "MC_RTS";
> +	case RTE_RING_SYNC_MT_HTS:
> +		return "MC_HTS";
> +	default:
> +		return "Unknown";
> +	}
> +}
> +
> +struct ring_info_cb_arg {
> +	char *ring_name;
> +	struct rte_tel_data *d;
> +};
> +
> +static void
> +ring_info_cb(struct rte_ring *r, void *arg)
> +{
> +	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
> +	struct rte_tel_data *d = ring_arg->d;
> +	const struct rte_memzone *mz;
> +
> +	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
> +		return;
> +
> +	rte_tel_data_add_dict_string(d, "name", r->name);
> +	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
> +	rte_tel_data_add_dict_uint_hex(d, "flags", r->flags, 0);
> +	rte_tel_data_add_dict_string(d, "producer_type",
> +		ring_prod_sync_type_to_name(r));
> +	rte_tel_data_add_dict_string(d, "consumer_type",
> +		ring_cons_sync_type_to_name(r));
> +	rte_tel_data_add_dict_uint(d, "size", r->size);
> +	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
> +	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
> +	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
> +
> +	mz = r->memzone;
> +	if (mz == NULL)
> +		return;
> +	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
> +	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
> +	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
> +	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
> +	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
> +}
> +
> +static int
> +ring_handle_info(const char *cmd __rte_unused, const char *params,
> +		struct rte_tel_data *d)
> +{
> +	char name[RTE_RING_NAMESIZE] = {0};
> +	struct ring_info_cb_arg ring_arg;
> +
> +	if (params == NULL || strlen(params) == 0 ||
> +		strlen(params) >= RTE_RING_NAMESIZE)
> +		return -EINVAL;
> +
> +	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
> +
> +	ring_arg.ring_name = name;
> +	ring_arg.d = d;
> +
> +	rte_tel_data_start_dict(d);
> +	ring_walk(ring_info_cb, &ring_arg);
> +
> +	return 0;
> +}
> +
>   RTE_INIT(ring_init_telemetry)
>   {
>   	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
>   		"Returns list of available rings. Takes no parameters");
> +	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
> +		"Returns ring info. Parameters: ring_name.");
>   }

Re: [RESEND v7 0/3] add telemetry cmds for ring

[Jie Hai]

Hi, Thomas,

Kindly ping for review.

Thanks,
Jie Hai


On 2023/11/9 18:20, Jie Hai wrote:
> This patch set supports telemetry cmd to list rings and dump information
> of a ring by its name.
> 
> v1->v2:
> 1. Add space after "switch".
> 2. Fix wrong strlen parameter.
> 
> v2->v3:
> 1. Remove prefix "rte_" for static function.
> 2. Add Acked-by Konstantin Ananyev for PATCH 1.
> 3. Introduce functions to return strings instead copy strings.
> 4. Check pointer to memzone of ring.
> 5. Remove redundant variable.
> 6. Hold lock when access ring data.
> 
> v3->v4:
> 1. Update changelog according to reviews of Honnappa Nagarahalli.
> 2. Add Reviewed-by Honnappa Nagarahalli.
> 3. Correct grammar in help information.
> 4. Correct spell warning on "te" reported by checkpatch.pl.
> 5. Use ring_walk() to query ring info instead of rte_ring_lookup().
> 6. Fix that type definition the flag field of rte_ring does not match the usage.
> 7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
>     for mask and flags.
> 
> v4->v5:
> 1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
> 2. Add ABI change explanation for commit message of patch 1/3.
> 
> v5->v6:
> 1. Add Acked-by Morten Br?rup.
> 2. Fix incorrect reference of commit.
> 
> v6->v7:
> 1. Remove prod/consumer head/tail info.
> 
> Jie Hai (3):
>    ring: fix unmatched type definition and usage
>    ring: add telemetry cmd to list rings
>    ring: add telemetry cmd for ring info
> 
>   lib/ring/meson.build     |   1 +
>   lib/ring/rte_ring.c      | 135 +++++++++++++++++++++++++++++++++++++++
>   lib/ring/rte_ring_core.h |   2 +-
>   3 files changed, 137 insertions(+), 1 deletion(-)
> 

Re: [RESEND v7 1/3] ring: fix unmatched type definition and usage

[Thomas Monjalon]

09/11/2023 11:20, Jie Hai:
> Field 'flags' of struct rte_ring is defined as int type. However,
> it is used as unsigned int. To ensure consistency, change the
> type of flags to unsigned int. Since these two types has the
> same byte size, this change is not an ABI change.
> 
> Fixes: af75078fece3 ("first public release")
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
>  lib/ring/rte_ring_core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
> index b7708730658a..14dac6495d83 100644
> --- a/lib/ring/rte_ring_core.h
> +++ b/lib/ring/rte_ring_core.h
> @@ -119,7 +119,7 @@ struct rte_ring_hts_headtail {
>  struct rte_ring {
>  	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
>  	/**< Name of the ring. */
> -	int flags;               /**< Flags supplied at creation. */
> +	uint32_t flags;               /**< Flags supplied at creation. */

This triggers a warning in our ABI checker:

      in pointed to type 'struct rte_ring' at rte_ring_core.h:119:1:
        type size hasn't changed
        1 data member change:
          type of 'int flags' changed:
            entity changed from 'int' to compatible type 'typedef uint32_t' at stdint-uintn.h:26:1
              type name changed from 'int' to 'unsigned int'
              type size hasn't changed

I guess we were supposed to merge this in 23.11, sorry about this.

How can we proceed?

Re: [RESEND v7 1/3] ring: fix unmatched type definition and usage

[Jie Hai]

On 2024/2/19 2:11, Thomas Monjalon wrote:
> 09/11/2023 11:20, Jie Hai:
>> Field 'flags' of struct rte_ring is defined as int type. However,
>> it is used as unsigned int. To ensure consistency, change the
>> type of flags to unsigned int. Since these two types has the
>> same byte size, this change is not an ABI change.
>>
>> Fixes: af75078fece3 ("first public release")
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
>> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> ---
>>   lib/ring/rte_ring_core.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/ring/rte_ring_core.h b/lib/ring/rte_ring_core.h
>> index b7708730658a..14dac6495d83 100644
>> --- a/lib/ring/rte_ring_core.h
>> +++ b/lib/ring/rte_ring_core.h
>> @@ -119,7 +119,7 @@ struct rte_ring_hts_headtail {
>>   struct rte_ring {
>>   	char name[RTE_RING_NAMESIZE] __rte_cache_aligned;
>>   	/**< Name of the ring. */
>> -	int flags;               /**< Flags supplied at creation. */
>> +	uint32_t flags;               /**< Flags supplied at creation. */
> 
> This triggers a warning in our ABI checker:
> 
>        in pointed to type 'struct rte_ring' at rte_ring_core.h:119:1:
>          type size hasn't changed
>          1 data member change:
>            type of 'int flags' changed:
>              entity changed from 'int' to compatible type 'typedef uint32_t' at stdint-uintn.h:26:1
>                type name changed from 'int' to 'unsigned int'
>                type size hasn't changed
> 
> I guess we were supposed to merge this in 23.11, sorry about this.
> 
> How can we proceed?
> 
How about we drop this amendment (patch 1/3) for now?
> 
> .

[PATCH v8 0/2] add telemetry cmds for ring

[Jie Hai]

This patch set supports telemetry cmd to list rings and dump information
of a ring by its name.

v1->v2:
1. Add space after "switch".
2. Fix wrong strlen parameter.

v2->v3:
1. Remove prefix "rte_" for static function.
2. Add Acked-by Konstantin Ananyev for PATCH 1.
3. Introduce functions to return strings instead copy strings.
4. Check pointer to memzone of ring.
5. Remove redundant variable.
6. Hold lock when access ring data.

v3->v4:
1. Update changelog according to reviews of Honnappa Nagarahalli.
2. Add Reviewed-by Honnappa Nagarahalli.
3. Correct grammar in help information.
4. Correct spell warning on "te" reported by checkpatch.pl.
5. Use ring_walk() to query ring info instead of rte_ring_lookup().
6. Fix that type definition the flag field of rte_ring does not match the usage.
7. Use rte_tel_data_add_dict_uint_hex instead of rte_tel_data_add_dict_u64
   for mask and flags.

v4->v5:
1. Add Acked-by Konstantin Ananyev and Chengwen Feng.
2. Add ABI change explanation for commit message of patch 1/3.

v5->v6:
1. Add Acked-by Morten Br?rup.
2. Fix incorrect reference of commit.

v6->v7:
1. Remove prod/consumer head/tail info.

v7->v8:
1. Drop patch 1/3 and related codes.

Jie Hai (2):
  ring: add telemetry cmd to list rings
  ring: add telemetry cmd for ring info

 lib/ring/meson.build |   1 +
 lib/ring/rte_ring.c  | 135 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+)

-- 
2.30.0

[PATCH v8 1/2] ring: add telemetry cmd to list rings

[Jie Hai]

Add a telemetry command to list the rings used in the system.
An example using this command is shown below:

--> /ring/list
{
  "/ring/list": [
    "HT_0000:7d:00.2",
    "MP_mb_pool_0"
  ]
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/meson.build |  1 +
 lib/ring/rte_ring.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/lib/ring/meson.build b/lib/ring/meson.build
index c20685c689ac..7fca958ed7fa 100644
--- a/lib/ring/meson.build
+++ b/lib/ring/meson.build
@@ -18,3 +18,4 @@ indirect_headers += files (
         'rte_ring_rts.h',
         'rte_ring_rts_elem_pvt.h',
 )
+deps += ['telemetry']
diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index c59f62626383..75f53c723186 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -22,6 +22,7 @@
 #include <rte_errno.h>
 #include <rte_string_fns.h>
 #include <rte_tailq.h>
+#include <rte_telemetry.h>
 
 #include "rte_ring.h"
 #include "rte_ring_elem.h"
@@ -423,3 +424,42 @@ rte_ring_lookup(const char *name)
 
 	return r;
 }
+
+static void
+ring_walk(void (*func)(struct rte_ring *, void *), void *arg)
+{
+	struct rte_ring_list *ring_list;
+	struct rte_tailq_entry *tailq_entry;
+
+	ring_list = RTE_TAILQ_CAST(rte_ring_tailq.head, rte_ring_list);
+	rte_mcfg_tailq_read_lock();
+
+	TAILQ_FOREACH(tailq_entry, ring_list, next) {
+		(*func)((struct rte_ring *) tailq_entry->data, arg);
+	}
+
+	rte_mcfg_tailq_read_unlock();
+}
+
+static void
+ring_list_cb(struct rte_ring *r, void *arg)
+{
+	struct rte_tel_data *d = (struct rte_tel_data *)arg;
+
+	rte_tel_data_add_array_string(d, r->name);
+}
+
+static int
+ring_handle_list(const char *cmd __rte_unused,
+		const char *params __rte_unused, struct rte_tel_data *d)
+{
+	rte_tel_data_start_array(d, RTE_TEL_STRING_VAL);
+	ring_walk(ring_list_cb, d);
+	return 0;
+}
+
+RTE_INIT(ring_init_telemetry)
+{
+	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
+		"Returns list of available rings. Takes no parameters");
+}
-- 
2.30.0

[PATCH v8 2/2] ring: add telemetry cmd for ring info

[Jie Hai]

This patch supports dump of ring information by its name.
An example using this command is shown below:

--> /ring/info,MP_mb_pool_0
{
  "/ring/info": {
    "name": "MP_mb_pool_0",
    "socket": 0,
    "flags": 0,
    "producer_type": "MP",
    "consumer_type": "MC",
    "size": 262144,
    "mask": "0x3ffff",
    "capacity": 262143,
    "used_count": 153197,
    "mz_name": "RG_MP_mb_pool_0",
    "mz_len": 2097536,
    "mz_hugepage_sz": 1073741824,
    "mz_socket_id": 0,
    "mz_flags": "0x0"
  }
}

Signed-off-by: Jie Hai <haijie1@huawei.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
 lib/ring/rte_ring.c | 95 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/lib/ring/rte_ring.c b/lib/ring/rte_ring.c
index 75f53c723186..e6c746cce1f1 100644
--- a/lib/ring/rte_ring.c
+++ b/lib/ring/rte_ring.c
@@ -458,8 +458,103 @@ ring_handle_list(const char *cmd __rte_unused,
 	return 0;
 }
 
+static const char *
+ring_prod_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->prod.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MP";
+	case RTE_RING_SYNC_ST:
+		return "SP";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MP_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MP_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+static const char *
+ring_cons_sync_type_to_name(struct rte_ring *r)
+{
+	switch (r->cons.sync_type) {
+	case RTE_RING_SYNC_MT:
+		return "MC";
+	case RTE_RING_SYNC_ST:
+		return "SC";
+	case RTE_RING_SYNC_MT_RTS:
+		return "MC_RTS";
+	case RTE_RING_SYNC_MT_HTS:
+		return "MC_HTS";
+	default:
+		return "Unknown";
+	}
+}
+
+struct ring_info_cb_arg {
+	char *ring_name;
+	struct rte_tel_data *d;
+};
+
+static void
+ring_info_cb(struct rte_ring *r, void *arg)
+{
+	struct ring_info_cb_arg *ring_arg = (struct ring_info_cb_arg *)arg;
+	struct rte_tel_data *d = ring_arg->d;
+	const struct rte_memzone *mz;
+
+	if (strncmp(r->name, ring_arg->ring_name, RTE_RING_NAMESIZE))
+		return;
+
+	rte_tel_data_add_dict_string(d, "name", r->name);
+	rte_tel_data_add_dict_int(d, "socket", r->memzone->socket_id);
+	rte_tel_data_add_dict_int(d, "flags", r->flags);
+	rte_tel_data_add_dict_string(d, "producer_type",
+		ring_prod_sync_type_to_name(r));
+	rte_tel_data_add_dict_string(d, "consumer_type",
+		ring_cons_sync_type_to_name(r));
+	rte_tel_data_add_dict_uint(d, "size", r->size);
+	rte_tel_data_add_dict_uint_hex(d, "mask", r->mask, 0);
+	rte_tel_data_add_dict_uint(d, "capacity", r->capacity);
+	rte_tel_data_add_dict_uint(d, "used_count", rte_ring_count(r));
+
+	mz = r->memzone;
+	if (mz == NULL)
+		return;
+	rte_tel_data_add_dict_string(d, "mz_name", mz->name);
+	rte_tel_data_add_dict_uint(d, "mz_len", mz->len);
+	rte_tel_data_add_dict_uint(d, "mz_hugepage_sz", mz->hugepage_sz);
+	rte_tel_data_add_dict_int(d, "mz_socket_id", mz->socket_id);
+	rte_tel_data_add_dict_uint_hex(d, "mz_flags", mz->flags, 0);
+}
+
+static int
+ring_handle_info(const char *cmd __rte_unused, const char *params,
+		struct rte_tel_data *d)
+{
+	char name[RTE_RING_NAMESIZE] = {0};
+	struct ring_info_cb_arg ring_arg;
+
+	if (params == NULL || strlen(params) == 0 ||
+		strlen(params) >= RTE_RING_NAMESIZE)
+		return -EINVAL;
+
+	rte_strlcpy(name, params, RTE_RING_NAMESIZE);
+
+	ring_arg.ring_name = name;
+	ring_arg.d = d;
+
+	rte_tel_data_start_dict(d);
+	ring_walk(ring_info_cb, &ring_arg);
+
+	return 0;
+}
+
 RTE_INIT(ring_init_telemetry)
 {
 	rte_telemetry_register_cmd("/ring/list", ring_handle_list,
 		"Returns list of available rings. Takes no parameters");
+	rte_telemetry_register_cmd("/ring/info", ring_handle_info,
+		"Returns ring info. Parameters: ring_name.");
 }
-- 
2.30.0

Re: [PATCH v8 0/2] add telemetry cmds for ring

[Thomas Monjalon]

> Jie Hai (2):
>   ring: add telemetry cmd to list rings
>   ring: add telemetry cmd for ring info

Applied, thanks.