diff mbox series

[v5,3/3] app/procinfo: support descriptor dump

Message ID 20221006120514.28830-4-liudongdong3@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series support ethdev Rx/Tx descriptor dump | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Dongdong Liu Oct. 6, 2022, 12:05 p.m. UTC 
This patch support Rx/Tx descriptor dump

The command is like:
dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx --
--show-rx-descriptor queue_id:offset:num

dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx --
--show-tx-descriptor queue_id:offset:num

queue_id: A queue identifier on this port.
offset: The offset of the descriptor starting from tail.
num: The number of the descriptors to dump.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 app/proc-info/main.c | 80 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Pattan, Reshma Oct. 7, 2022, 2:43 p.m. UTC | #1 
> -----Original Message-----
> +/* Enable dump Rx/Tx descriptor. */
> +#define DESC_PARAM_NUM 3
> +
> +struct desc_param {
> +	uint16_t queue_id;  /* A queue identifier on this port. */
> +	uint16_t offset;    /* The offset of the descriptor starting from tail. */
> +	uint16_t num;       /* The number of the descriptors to dump. */
> +	bool valid;

You don't need to keep if the descriptor parameters are valid or not, as you are exiting the application when you see invalid parameters are entered by user.

> 
> +static int
> +parse_descriptor_param(char *list, struct desc_param *desc) {
> +	int ret;
> +
> +	ret = sscanf(list, "%hu:%hu:%hu", &desc->queue_id, &desc->offset,
> +		     &desc->num);
> +	if (ret != DESC_PARAM_NUM) {
> +		desc->valid = false;
> +		return -EINVAL;

On error return application is exiting , so no need to maintain desc->valid

>  main(int argc, char **argv)
>  {
> @@ -1564,6 +1638,12 @@ main(int argc, char **argv)
>  			metrics_display(i);
>  #endif
> 
> +		if (rx_desc_param.valid)

So if rx_desc dump is requested in command line you can set some global variable like "enable-show-rx-desc-dump" and display below info only if that variable is set. 
So we no need to use valid here.

 

> +			nic_descriptor_display(i, &rx_desc_param,
> +					       rte_eth_rx_descriptor_dump);
> +		if (tx_desc_param.valid)

Same here as above comment.
Dongdong Liu Oct. 8, 2022, 9:46 a.m. UTC | #2 
Hi Reshma

Many thanks for your review.
I have sent out the below patchset. this patchset have fixed the comments.

[v8,7/8] app/procinfo: support descriptor dump
https://patches.dpdk.org/project/dpdk/list/?series=25047

Thanks,
Dongdong.
On 2022/10/7 22:43, Pattan, Reshma wrote:
>
>
>> -----Original Message-----
>> +/* Enable dump Rx/Tx descriptor. */
>> +#define DESC_PARAM_NUM 3
>> +
>> +struct desc_param {
>> +	uint16_t queue_id;  /* A queue identifier on this port. */
>> +	uint16_t offset;    /* The offset of the descriptor starting from tail. */
>> +	uint16_t num;       /* The number of the descriptors to dump. */
>> +	bool valid;
>
> You don't need to keep if the descriptor parameters are valid or not, as you are exiting the application when you see invalid parameters are entered by user.
>
>>
>> +static int
>> +parse_descriptor_param(char *list, struct desc_param *desc) {
>> +	int ret;
>> +
>> +	ret = sscanf(list, "%hu:%hu:%hu", &desc->queue_id, &desc->offset,
>> +		     &desc->num);
>> +	if (ret != DESC_PARAM_NUM) {
>> +		desc->valid = false;
>> +		return -EINVAL;
>
> On error return application is exiting , so no need to maintain desc->valid
>
>>  main(int argc, char **argv)
>>  {
>> @@ -1564,6 +1638,12 @@ main(int argc, char **argv)
>>  			metrics_display(i);
>>  #endif
>>
>> +		if (rx_desc_param.valid)
>
> So if rx_desc dump is requested in command line you can set some global variable like "enable-show-rx-desc-dump" and display below info only if that variable is set.
> So we no need to use valid here.
>
>
>
>> +			nic_descriptor_display(i, &rx_desc_param,
>> +					       rte_eth_rx_descriptor_dump);
>> +		if (tx_desc_param.valid)
>
> Same here as above comment.
>
> .
>
diff mbox series

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index d52ac8a038..d8a9639ad2 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -48,6 +48,9 @@ 
 #define STATS_BDR_STR(w, s) printf("%.*s%s%.*s\n", w, \
 	STATS_BDR_FMT, s, w, STATS_BDR_FMT)
 
+typedef int (*desc_dump_t)(uint16_t port_id, uint16_t queue_id,
+			   uint16_t offset, uint16_t num, FILE *file);
+
 /**< mask of enabled ports */
 static unsigned long enabled_port_mask;
 /**< Enable stats. */
@@ -103,6 +106,19 @@  static char *mempool_iter_name;
 static uint32_t enable_dump_regs;
 static char *dump_regs_file_prefix;
 
+/* Enable dump Rx/Tx descriptor. */
+#define DESC_PARAM_NUM 3
+
+struct desc_param {
+	uint16_t queue_id;  /* A queue identifier on this port. */
+	uint16_t offset;    /* The offset of the descriptor starting from tail. */
+	uint16_t num;       /* The number of the descriptors to dump. */
+	bool valid;
+};
+
+static struct desc_param rx_desc_param;
+static struct desc_param tx_desc_param;
+
 /**< display usage */
 static void
 proc_info_usage(const char *prgname)
@@ -130,6 +146,14 @@  proc_info_usage(const char *prgname)
 		"  --show-crypto: to display crypto information\n"
 		"  --show-ring[=name]: to display ring information\n"
 		"  --show-mempool[=name]: to display mempool information\n"
+		"  --show-rx-descriptor queue_id:offset:num to display ports Rx descriptor information. "
+			"queue_id: A Rx queue identifier on this port. "
+			"offset: The offset of the descriptor starting from tail. "
+			"num: The number of the descriptors to dump.\n"
+		"  --show-tx-descriptor queue_id:offset:num to display ports Tx descriptor information. "
+			"queue_id: A Tx queue identifier on this port. "
+			"offset: The offset of the descriptor starting from tail. "
+			"num: The number of the descriptors to dump.\n"
 		"  --iter-mempool=name: iterate mempool elements to display content\n"
 		"  --dump-regs=file-prefix: dump registers to file with the file-prefix\n",
 		prgname);
@@ -182,6 +206,23 @@  parse_xstats_ids(char *list, uint64_t *ids, int limit) {
 	return length;
 }
 
+static int
+parse_descriptor_param(char *list, struct desc_param *desc)
+{
+	int ret;
+
+	ret = sscanf(list, "%hu:%hu:%hu", &desc->queue_id, &desc->offset,
+		     &desc->num);
+	if (ret != DESC_PARAM_NUM) {
+		desc->valid = false;
+		return -EINVAL;
+	}
+
+	desc->valid = true;
+
+	return 0;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -242,6 +283,8 @@  proc_info_parse_args(int argc, char **argv)
 		{"show-mempool", optional_argument, NULL, 0},
 		{"iter-mempool", required_argument, NULL, 0},
 		{"dump-regs", required_argument, NULL, 0},
+		{"show-rx-descriptor", required_argument, NULL, 1},
+		{"show-tx-descriptor", required_argument, NULL, 1},
 		{NULL, 0, 0, 0}
 	};
 
@@ -334,6 +377,22 @@  proc_info_parse_args(int argc, char **argv)
 					return -1;
 				}
 				nb_xstats_ids = ret;
+			} else if (!strncmp(long_option[option_index].name,
+				"show-rx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+							&rx_desc_param);
+				if (ret < 0) {
+					printf("Rx descriptor param parse error.\n");
+					return -1;
+				}
+			} else if (!strncmp(long_option[option_index].name,
+				"show-tx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+							&tx_desc_param);
+				if (ret < 0) {
+					printf("Tx descriptor param parse error.\n");
+					return -1;
+				}
 			}
 			break;
 		default:
@@ -1476,6 +1535,21 @@  dump_regs(char *file_prefix)
 	}
 }
 
+static void
+nic_descriptor_display(uint16_t port_id, struct desc_param *desc,
+		       desc_dump_t desc_dump)
+{
+	static const char *nic_desc_border = "###";
+	uint16_t queue_id = desc->queue_id;
+	uint16_t offset = desc->offset;
+	uint16_t num = desc->num;
+
+	printf("%s NIC descriptor for port %u %s\n",
+		   nic_desc_border, port_id, nic_desc_border);
+
+	desc_dump(port_id, queue_id, offset, num, stdout);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1564,6 +1638,12 @@  main(int argc, char **argv)
 			metrics_display(i);
 #endif
 
+		if (rx_desc_param.valid)
+			nic_descriptor_display(i, &rx_desc_param,
+					       rte_eth_rx_descriptor_dump);
+		if (tx_desc_param.valid)
+			nic_descriptor_display(i, &tx_desc_param,
+					       rte_eth_tx_descriptor_dump);
 	}
 
 #ifdef RTE_LIB_METRICS