From: "lihuisong (C)" <lihuisong@huawei.com>
To: Jie Hai <haijie1@huawei.com>
Cc: <fengchengwen@huawei.com>, <dev@dpdk.org>,
Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
Ferruh Yigit <ferruh.yigit@amd.com>
Subject: Re: [PATCH v5 2/7] ethdev: add telemetry cmd for registers
Date: Fri, 8 Mar 2024 16:48:15 +0800 [thread overview]
Message-ID: <d17e41ee-ffcf-65ee-96c3-a6d9cad17a50@huawei.com> (raw)
In-Reply-To: <20240307030247.599394-3-haijie1@huawei.com>
在 2024/3/7 11:02, Jie Hai 写道:
> This patch adds a telemetry command for registers dump,
> and supports get registers with specified names.
> The length of the string exported by telemetry is limited
> by MAX_OUTPUT_LEN. Therefore, the filter should be more
> precise.
>
> An example usage is shown below:
> --> /ethdev/regs,0,INTR
> {
> "/ethdev/regs": {
> "registers_length": 318,
> "registers_width": 4,
> "register_offset": "0x0",
> "version": "0x1140011",
> "group_0": {
> "HNS3_CMDQ_INTR_STS_REG": "0x0",
> "HNS3_CMDQ_INTR_EN_REG": "0x2",
> "HNS3_CMDQ_INTR_GEN_REG": "0x0",
> "queue_0_HNS3_TQP_INTR_CTRL_REG": "0x0",
> "queue_0_HNS3_TQP_INTR_GL0_REG": "0xa",
> "queue_0_HNS3_TQP_INTR_GL1_REG": "0xa",
> "queue_0_HNS3_TQP_INTR_GL2_REG": "0x0",
> ...
> },
> "group_1": {
> ...
> },
> ...
> }
>
> or as below if the number of registers not exceed the
> RTE_TEL_MAX_DICT_ENTRIES:
> --> /ethdev/regs,0,ppp
> {
> "/ethdev/regs": {
> "registers_length": 156,
> "registers_width": 4,
> "register_offset": "0x0",
> "version": "0x1140011",
> "ppp_key_drop_num": "0x0",
> "ppp_rlt_drop_num": "0x0",
> "ssu_ppp_mac_key_num_l": "0x1",
> "ssu_ppp_mac_key_num_h": "0x0",
> "ssu_ppp_host_key_num_l": "0x1",
> "ssu_ppp_host_key_num_h": "0x0",
> "ppp_ssu_mac_rlt_num_l": "0x1",
> ...
> }
> }
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
> lib/ethdev/rte_ethdev_telemetry.c | 158 ++++++++++++++++++++++++++++++
> 1 file changed, 158 insertions(+)
>
> diff --git a/lib/ethdev/rte_ethdev_telemetry.c b/lib/ethdev/rte_ethdev_telemetry.c
> index 6b873e7abe68..0c570792fb5f 100644
> --- a/lib/ethdev/rte_ethdev_telemetry.c
> +++ b/lib/ethdev/rte_ethdev_telemetry.c
> @@ -5,6 +5,7 @@
> #include <ctype.h>
> #include <stdlib.h>
>
> +#include <rte_malloc.h>
> #include <rte_kvargs.h>
> #include <rte_telemetry.h>
>
> @@ -1395,6 +1396,161 @@ eth_dev_handle_port_tm_node_caps(const char *cmd __rte_unused,
> return ret;
> }
>
> +static void
> +eth_dev_add_u32_hex(struct rte_tel_data *d, const char *name, uint32_t **data)
> +{
> + if (*data == NULL)
> + return;
> + rte_tel_data_add_dict_uint_hex(d, name, **data, 0);
> + (*data)++;
suggest that move "(*data)++" to the place after called this interface.
> +}
> +
> +static void
> +eth_dev_add_u64_hex(struct rte_tel_data *d, const char *name, uint64_t **data)
> +{
> + if (*data == NULL)
> + return;
> + rte_tel_data_add_dict_uint_hex(d, name, **data, 0);
> + (*data)++;
the same as above.
> +}
> +
> +static int
> +eth_dev_store_regs(struct rte_tel_data *d, struct rte_dev_reg_info *reg_info)
> +{
> + struct rte_tel_data *groups[RTE_TEL_MAX_DICT_ENTRIES];
> + char group_name[RTE_TEL_MAX_STRING_LEN] = {0};
> + struct rte_tel_data *group = NULL;
> + uint32_t *data0 = NULL;
> + uint64_t *data1 = NULL;
> + uint32_t grp_num = 0;
> + int ret = 0;
> + uint32_t i;
> +
> + rte_tel_data_start_dict(d);
> + rte_tel_data_add_dict_uint(d, "register_length", reg_info->length);
> + rte_tel_data_add_dict_uint(d, "register_width", reg_info->width);
> + rte_tel_data_add_dict_uint_hex(d, "register_offset", reg_info->offset, 0);
> + rte_tel_data_add_dict_uint_hex(d, "version", reg_info->version, 0);
> +
> + if (reg_info->width == sizeof(uint32_t))
> + data0 = reg_info->data;
> + else
> + data1 = reg_info->data;
> +
> + if (reg_info->length <= RTE_TEL_MAX_DICT_ENTRIES) {
> + for (i = 0; i < reg_info->length; i++) {
> + eth_dev_add_u32_hex(d, reg_info->names[i].name, &data0);
> + eth_dev_add_u64_hex(d, reg_info->names[i].name, &data1);
Here adding u32_hex or u64_hex should depend on the reg_info->width, right?
Even though it is ok now due to the validity check in
eth_dev_add_u32_hex and eth_dev_add_u64_hex.
Using the validity check to control the code flow is ok.
But u32 and u64 interface are parallel.
It's obvious that one of them will be selected for execution here.
So suggest that modify it as following logic:
"
if (reg_info->width == sizeof(uint32_t))
eth_dev_add_u32_hex(d, reg_info->names[i].name, &data0)
else
eth_dev_add_u64_hex(d, reg_info->names[i].name, &data1);
"
> + }
> + return 0;
> + }
> +
> + for (i = 0; i < reg_info->length; i++) {
> + if (i % RTE_TEL_MAX_DICT_ENTRIES == 0) {
> + group = rte_tel_data_alloc();
> + if (group == NULL) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + groups[grp_num++] = group;
> + rte_tel_data_start_dict(group);
> + }
> + eth_dev_add_u32_hex(group, reg_info->names[i].name, &data0);
> + eth_dev_add_u64_hex(group, reg_info->names[i].name, &data1);
> + }
> +
> + for (i = 0; i < grp_num; i++) {
> + snprintf(group_name, RTE_TEL_MAX_STRING_LEN,
> + "group_%u", i);
> + rte_tel_data_add_dict_container(d, group_name, groups[i], 0);
> + }
> + return 0;
> +out:
> + for (i = 0; i < grp_num; i++)
> + rte_tel_data_free(groups[i]);
> +
> + return ret;
> +}
> +
> +static int
> +eth_dev_get_port_regs(int port_id, struct rte_tel_data *d, char *filter)
> +{
> + struct rte_dev_reg_info reg_info;
> + uint32_t max_length;
> + int ret;
> +
> + memset(®_info, 0, sizeof(reg_info));
> + reg_info.filter = filter;
> +
> + ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
> + if (ret != 0) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Error getting device reg info: %d", ret);
> + return ret;
> + }
> +
> + /* 4 is space for other information of registers. */
> + max_length = RTE_TEL_MAX_DICT_ENTRIES *
> + (RTE_TEL_MAX_DICT_ENTRIES - 4);
please define a macro for this "4" and add more detail description.
> + if (reg_info.length > max_length) {
> + RTE_ETHDEV_LOG_LINE(WARNING,
> + "Reduced register number to be obtained from %u to %u due to limited memory",
> + reg_info.length, max_length);
> + reg_info.length = max_length;
> + }
> +
> + reg_info.data = calloc(reg_info.length, reg_info.width);
> + if (reg_info.data == NULL) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Fail to allocate memory for reg_info.data");
> + return -ENOMEM;
> + }
> +
> + reg_info.names = calloc(reg_info.length, sizeof(struct rte_eth_reg_name));
> + if (reg_info.names == NULL) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Fail to allocate memory for reg_info.names");
> + free(reg_info.data);
> + return -ENOMEM;
> + }
> +
> + ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
> + if (ret != 0) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Error getting regs from device: %d", ret);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + ret = eth_dev_store_regs(d, ®_info);
> +out:
> + free(reg_info.data);
> + free(reg_info.names);
> +
> + return ret;
> +}
> +
> +static int
> +eth_dev_handle_port_regs(const char *cmd __rte_unused,
> + const char *params,
> + struct rte_tel_data *d)
> +{
> + char *filter = NULL;
> + uint16_t port_id;
> + char *end_param;
> + int ret;
> +
> + ret = eth_dev_parse_port_params(params, &port_id, &end_param, true);
> + if (ret != 0)
> + return ret;
> +
> + filter = strtok(end_param, ",");
> + if (filter != NULL && strlen(filter) == 0)
> + filter = NULL;
> +
> + return eth_dev_get_port_regs(port_id, d, filter);
> +}
> +
> RTE_INIT(ethdev_init_telemetry)
> {
> rte_telemetry_register_cmd("/ethdev/list", eth_dev_handle_port_list,
> @@ -1436,4 +1592,6 @@ RTE_INIT(ethdev_init_telemetry)
> "Returns TM Level Capabilities info for a port. Parameters: int port_id, int level_id (see tm_capability for the max)");
> rte_telemetry_register_cmd("/ethdev/tm_node_capability", eth_dev_handle_port_tm_node_caps,
> "Returns TM Node Capabilities info for a port. Parameters: int port_id, int node_id (see tm_capability for the max)");
> + rte_telemetry_register_cmd("/ethdev/regs", eth_dev_handle_port_regs,
> + "Returns regs for a port. Parameters: int port_id, string filter");
"Returns all regs or specified type regs for a port. Parameters: int port_id, string filter"?
> }
next prev parent reply other threads:[~2024-03-08 8:48 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-14 1:56 [PATCH] ethdev: add dump regs for telemetry Jie Hai
2023-12-14 12:49 ` Ferruh Yigit
2024-01-09 2:19 ` Jie Hai
2024-01-09 2:41 ` Jie Hai
2024-01-09 18:06 ` Ferruh Yigit
2024-01-10 1:38 ` fengchengwen
2024-01-10 12:15 ` Ferruh Yigit
2024-01-10 14:09 ` Thomas Monjalon
2024-01-10 15:48 ` Ferruh Yigit
2024-01-11 1:55 ` fengchengwen
2024-01-11 11:11 ` Ferruh Yigit
2024-01-11 12:43 ` fengchengwen
2024-02-05 10:51 ` [PATCH v2 0/7] support dump reigser names and filter them Jie Hai
2024-02-05 10:51 ` [PATCH v2 1/7] ethdev: support report register names and filter Jie Hai
2024-02-07 17:00 ` Ferruh Yigit
2024-02-20 8:43 ` Jie Hai
2024-02-05 10:51 ` [PATCH v2 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-07 17:03 ` Ferruh Yigit
2024-02-22 9:01 ` Jie Hai
2024-02-05 10:51 ` [PATCH v2 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-05 10:51 ` [PATCH v2 4/7] net/hns3: remove dump format " Jie Hai
2024-02-05 10:51 ` [PATCH v2 5/7] net/hns3: add names for registers Jie Hai
2024-02-05 10:51 ` [PATCH v2 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-05 10:51 ` [PATCH v2 7/7] net/hns3: support filter dump of registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 0/7] support dump reigser names and filter them Jie Hai
2024-02-20 10:58 ` [PATCH v3 1/7] ethdev: support report register names and filter Jie Hai
2024-02-20 15:09 ` Stephen Hemminger
2024-02-26 2:33 ` Jie Hai
2024-02-20 15:13 ` Stephen Hemminger
2024-02-26 2:41 ` Jie Hai
2024-02-20 15:14 ` Stephen Hemminger
2024-02-26 2:57 ` Jie Hai
2024-02-20 15:14 ` Stephen Hemminger
2024-02-26 2:33 ` Jie Hai
2024-02-20 10:58 ` [PATCH v3 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 4/7] net/hns3: remove dump format " Jie Hai
2024-02-20 10:58 ` [PATCH v3 5/7] net/hns3: add names for registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-20 10:58 ` [PATCH v3 7/7] net/hns3: support filter dump of registers Jie Hai
2024-02-26 3:07 ` [PATCH v4 0/7] support dump reigser names and filter them Jie Hai
2024-02-26 3:07 ` [PATCH v4 1/7] ethdev: support report register names and filter Jie Hai
2024-02-26 8:01 ` fengchengwen
2024-03-06 7:22 ` Jie Hai
2024-02-29 9:52 ` Thomas Monjalon
2024-03-05 7:45 ` Jie Hai
2024-02-26 3:07 ` [PATCH v4 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-02-26 9:09 ` fengchengwen
2024-03-06 7:18 ` Jie Hai
2024-02-26 3:07 ` [PATCH v4 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-02-26 3:07 ` [PATCH v4 4/7] net/hns3: remove dump format " Jie Hai
2024-02-26 3:07 ` [PATCH v4 5/7] net/hns3: add names for registers Jie Hai
2024-02-26 3:07 ` [PATCH v4 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-02-26 3:07 ` [PATCH v4 7/7] net/hns3: support filter dump of registers Jie Hai
2024-03-07 3:02 ` [PATCH v5 0/7] support dump reigser names and filter them Jie Hai
2024-03-07 3:02 ` [PATCH v5 1/7] ethdev: support report register names and filter Jie Hai
2024-03-08 8:09 ` lihuisong (C)
2024-03-07 3:02 ` [PATCH v5 2/7] ethdev: add telemetry cmd for registers Jie Hai
2024-03-08 8:48 ` lihuisong (C) [this message]
2024-03-07 3:02 ` [PATCH v5 3/7] net/hns3: fix dump counter of registers Jie Hai
2024-03-08 8:49 ` lihuisong (C)
2024-03-07 3:02 ` [PATCH v5 4/7] net/hns3: remove dump format " Jie Hai
2024-03-08 9:17 ` lihuisong (C)
2024-03-07 3:02 ` [PATCH v5 5/7] net/hns3: add names for registers Jie Hai
2024-03-08 9:41 ` lihuisong (C)
2024-03-08 10:24 ` lihuisong (C)
2024-03-07 3:02 ` [PATCH v5 6/7] net/hns3: support filter directly accessed registers Jie Hai
2024-03-08 9:41 ` lihuisong (C)
2024-03-07 3:02 ` [PATCH v5 7/7] net/hns3: support filter dump of registers Jie Hai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d17e41ee-ffcf-65ee-96c3-a6d9cad17a50@huawei.com \
--to=lihuisong@huawei.com \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=haijie1@huawei.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).