[PATCH 0/4] support HW Rx/Tx descriptor dump

[PATCH 0/4] support HW Rx/Tx descriptor dump

[Min Hu (Connor)]

This patch set support HW Rx/Tx descriptor dump by using procinfo tool.

Min Hu (Connor) (4):
  ethdev: introduce ethdev HW desc dump PI
  net/hns3: rename hns3 dump files
  net/hns3: support Rx/Tx bd dump
  app/procinfo: support descriptor dump

 app/proc-info/main.c                          | 81 +++++++++++++++++++
 doc/guides/rel_notes/release_22_07.rst        |  6 ++
 .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
 drivers/net/hns3/hns3_dump.h                  | 17 ++++
 drivers/net/hns3/hns3_ethdev.c                |  3 +
 drivers/net/hns3/hns3_ethdev.h                |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
 drivers/net/hns3/meson.build                  |  2 +-
 lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
 lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
 lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
 lib/ethdev/version.map                        |  2 +
 12 files changed, 308 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
 create mode 100644 drivers/net/hns3/hns3_dump.h

-- 
2.33.0

[PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Min Hu (Connor)]

Added the ethdev HW Rx desc dump API which provides functions for query
HW descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/rel_notes/release_22_07.rst |  6 ++++
 lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 5 files changed, 138 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..cfb9117dd3 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,12 @@ New Features
   * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
   * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
 
+* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
+
+  Added the ethdev HW Rx desc dump API which provides functions for query
+  HW descriptor from device. HW descriptor info differs in different NICs.
+  The information demonstrates I/O process which is important for debug.
+  As the information is different between NICs, the new API is introduced.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 69d9dc21d8..89512ab360 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
  */
 typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
 
+/**
+ * @internal
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
+				       uint16_t queue_id, uint16_t desc_id);
+
+/**
+ * @internal
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
+				       uint16_t queue_id, uint16_t desc_id);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1283,6 +1319,12 @@ struct eth_dev_ops {
 
 	/** Dump private info from device */
 	eth_dev_priv_dump_t eth_dev_priv_dump;
+
+	/** Dump ethdev Rx descriptor info */
+	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
+
+	/** Dump ethdev Tx descriptor info */
+	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a175867651..09abee6345 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5874,6 +5874,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
 	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
 }
 
+int
+rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
+int
+rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 02df65d923..56ae630209 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5456,6 +5456,50 @@ typedef struct {
 __rte_experimental
 int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index daca7851f2..109f4ea818 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,8 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+	rte_eth_rx_hw_desc_dump;
+	rte_eth_tx_hw_desc_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Stephen Hemminger]

On Fri, 27 May 2022 10:33:48 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Added the ethdev HW Rx desc dump API which provides functions for query
> HW descriptor from device. HW descriptor info differs in different NICs.
> The information demonstrates I/O process which is important for debug.
> As the information is different between NICs, the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  doc/guides/rel_notes/release_22_07.rst |  6 ++++
>  lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>  lib/ethdev/version.map                 |  2 ++
>  5 files changed, 138 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 0ed4f92820..cfb9117dd3 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -95,6 +95,12 @@ New Features
>    * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
>    * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
>  
> +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
> +
> +  Added the ethdev HW Rx desc dump API which provides functions for query
> +  HW descriptor from device. HW descriptor info differs in different NICs.
> +  The information demonstrates I/O process which is important for debug.
> +  As the information is different between NICs, the new API is introduced.
>  
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..89512ab360 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>   */
>  typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>  
> +/**
> + * @internal
> + * Dump ethdev Rx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
> +				       uint16_t queue_id, uint16_t desc_id);
> +
> +/**
> + * @internal
> + * Dump ethdev Tx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
> +				       uint16_t queue_id, uint16_t desc_id);

Should be 'const struct rte_eth_dev *dev' to remind developers that dump
should not modify device?

Re: [PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Min Hu (Connor)]

Thanks Stephen, v2 has been sent.

在 2022/5/27 23:34, Stephen Hemminger 写道:
> On Fri, 27 May 2022 10:33:48 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>> Added the ethdev HW Rx desc dump API which provides functions for query
>> HW descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_22_07.rst |  6 ++++
>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/version.map                 |  2 ++
>>   5 files changed, 138 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
>> index 0ed4f92820..cfb9117dd3 100644
>> --- a/doc/guides/rel_notes/release_22_07.rst
>> +++ b/doc/guides/rel_notes/release_22_07.rst
>> @@ -95,6 +95,12 @@ New Features
>>     * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
>>     * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
>>   
>> +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
>> +
>> +  Added the ethdev HW Rx desc dump API which provides functions for query
>> +  HW descriptor from device. HW descriptor info differs in different NICs.
>> +  The information demonstrates I/O process which is important for debug.
>> +  As the information is different between NICs, the new API is introduced.
>>   
>>   Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 69d9dc21d8..89512ab360 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>>    */
>>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>>   
>> +/**
>> + * @internal
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
>> +				       uint16_t queue_id, uint16_t desc_id);
>> +
>> +/**
>> + * @internal
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
>> +				       uint16_t queue_id, uint16_t desc_id);
> 
> Should be 'const struct rte_eth_dev *dev' to remind developers that dump
> should not modify device?
> .
> 

Re: [PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Ray Kinsella]

"Min Hu (Connor)" <humin29@huawei.com> writes:

> Added the ethdev HW Rx desc dump API which provides functions for query
> HW descriptor from device. HW descriptor info differs in different NICs.
> The information demonstrates I/O process which is important for debug.
> As the information is different between NICs, the new API is introduced.
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  doc/guides/rel_notes/release_22_07.rst |  6 ++++
>  lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>  lib/ethdev/version.map                 |  2 ++
>  5 files changed, 138 insertions(+)
>
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 0ed4f92820..cfb9117dd3 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -95,6 +95,12 @@ New Features
>    * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
>    * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
>  
> +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
> +
> +  Added the ethdev HW Rx desc dump API which provides functions for query
> +  HW descriptor from device. HW descriptor info differs in different NICs.
> +  The information demonstrates I/O process which is important for debug.
> +  As the information is different between NICs, the new API is introduced.
>  
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..89512ab360 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>   */
>  typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>  
> +/**
> + * @internal
> + * Dump ethdev Rx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
> +				       uint16_t queue_id, uint16_t desc_id);
> +
> +/**
> + * @internal
> + * Dump ethdev Tx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
> +				       uint16_t queue_id, uint16_t desc_id);
> +
>  /**
>   * @internal A structure containing the functions exported by an Ethernet driver.
>   */
> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>  
>  	/** Dump private info from device */
>  	eth_dev_priv_dump_t eth_dev_priv_dump;
> +
> +	/** Dump ethdev Rx descriptor info */
> +	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
> +
> +	/** Dump ethdev Tx descriptor info */
> +	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>  };
>  
>  /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index a175867651..09abee6345 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5874,6 +5874,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
>  	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
>  }
>  
> +int
> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +			uint16_t desc_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (queue_id >= dev->data->nb_rx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
> +		return -EINVAL;
> +	}
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
> +	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
> +						   desc_id);
> +
> +	return ret;
> +}
> +
> +int
> +rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +			uint16_t desc_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (queue_id >= dev->data->nb_tx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
> +		return -EINVAL;
> +	}
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
> +	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id,
> +						   desc_id);
> +
> +	return ret;
> +}
> +
>  RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
>  
>  RTE_INIT(ethdev_init_telemetry)
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 02df65d923..56ae630209 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5456,6 +5456,50 @@ typedef struct {
>  __rte_experimental
>  int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>  
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump ethdev Rx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +__rte_experimental
> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +			    uint16_t desc_id);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump ethdev Tx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +__rte_experimental
> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +			    uint16_t desc_id);
> +
>  #include <rte_ethdev_core.h>
>  
>  /**
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index daca7851f2..109f4ea818 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>  	rte_mtr_color_in_protocol_priority_get;
>  	rte_mtr_color_in_protocol_set;
>  	rte_mtr_meter_vlan_table_update;

Please annotate with a comment indicating in which release the symbols
have been added. 

> +	rte_eth_rx_hw_desc_dump;
> +	rte_eth_tx_hw_desc_dump;
>  };
>  
>  INTERNAL {


-- 
Regards, Ray K

Re: [PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Min Hu (Connor)]

Hi, Ray,

在 2022/5/30 17:17, Ray Kinsella 写道:
> 
> "Min Hu (Connor)" <humin29@huawei.com> writes:
> 
>> Added the ethdev HW Rx desc dump API which provides functions for query
>> HW descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_22_07.rst |  6 ++++
>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/version.map                 |  2 ++
>>   5 files changed, 138 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
>> index 0ed4f92820..cfb9117dd3 100644
>> --- a/doc/guides/rel_notes/release_22_07.rst
>> +++ b/doc/guides/rel_notes/release_22_07.rst
>> @@ -95,6 +95,12 @@ New Features
>>     * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
>>     * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
>>   
>> +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
>> +
>> +  Added the ethdev HW Rx desc dump API which provides functions for query
>> +  HW descriptor from device. HW descriptor info differs in different NICs.
>> +  The information demonstrates I/O process which is important for debug.
>> +  As the information is different between NICs, the new API is introduced.
>>   
>>   Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 69d9dc21d8..89512ab360 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>>    */
>>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>>   
>> +/**
>> + * @internal
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
>> +				       uint16_t queue_id, uint16_t desc_id);
>> +
>> +/**
>> + * @internal
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, struct rte_eth_dev *dev,
>> +				       uint16_t queue_id, uint16_t desc_id);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an Ethernet driver.
>>    */
>> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>>   
>>   	/** Dump private info from device */
>>   	eth_dev_priv_dump_t eth_dev_priv_dump;
>> +
>> +	/** Dump ethdev Rx descriptor info */
>> +	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
>> +
>> +	/** Dump ethdev Tx descriptor info */
>> +	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>>   };
>>   
>>   /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index a175867651..09abee6345 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -5874,6 +5874,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
>>   	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
>>   }
>>   
>> +int
>> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
>> +			uint16_t desc_id)
>> +{
>> +	struct rte_eth_dev *dev;
>> +	int ret;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	if (queue_id >= dev->data->nb_rx_queues) {
>> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
>> +	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
>> +						   desc_id);
>> +
>> +	return ret;
>> +}
>> +
>> +int
>> +rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
>> +			uint16_t desc_id)
>> +{
>> +	struct rte_eth_dev *dev;
>> +	int ret;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	if (queue_id >= dev->data->nb_tx_queues) {
>> +		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
>> +	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id,
>> +						   desc_id);
>> +
>> +	return ret;
>> +}
>> +
>>   RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
>>   
>>   RTE_INIT(ethdev_init_telemetry)
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 02df65d923..56ae630209 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -5456,6 +5456,50 @@ typedef struct {
>>   __rte_experimental
>>   int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>   
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
>> +			    uint16_t desc_id);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
>> +			    uint16_t desc_id);
>> +
>>   #include <rte_ethdev_core.h>
>>   
>>   /**
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index daca7851f2..109f4ea818 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>>   	rte_mtr_color_in_protocol_priority_get;
>>   	rte_mtr_color_in_protocol_set;
>>   	rte_mtr_meter_vlan_table_update;
> 
> Please annotate with a comment indicating in which release the symbols
> have been added.
The patch doesn't show the whole picture.
Actually, The code is:

         # added in 22.07
         rte_mtr_color_in_protocol_get;
         rte_mtr_color_in_protocol_priority_get;
         rte_mtr_color_in_protocol_set;
         rte_mtr_meter_vlan_table_update;
         rte_eth_rx_hw_desc_dump;
         rte_eth_tx_hw_desc_dump;

Thanks.

> 
>> +	rte_eth_rx_hw_desc_dump;
>> +	rte_eth_tx_hw_desc_dump;
>>   };
>>   
>>   INTERNAL {
> 
> 

Re: [PATCH 1/4] ethdev: introduce ethdev HW desc dump PI

[Ray Kinsella]

"Min Hu (Connor)" <humin29@huawei.com> writes:

> Hi, Ray,
>
> 在 2022/5/30 17:17, Ray Kinsella 写道:
>> "Min Hu (Connor)" <humin29@huawei.com> writes:
>> 
>>> Added the ethdev HW Rx desc dump API which provides functions for query
>>> HW descriptor from device. HW descriptor info differs in different NICs.
>>> The information demonstrates I/O process which is important for debug.
>>> As the information is different between NICs, the new API is introduced.
>>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> ---
>>>   doc/guides/rel_notes/release_22_07.rst |  6 ++++
>>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>>>   lib/ethdev/version.map                 |  2 ++
>>>   5 files changed, 138 insertions(+)
>>>

Acked-by: Ray Kinsella <mdr@ashroe.eu>

[PATCH 2/4] net/hns3: rename hns3 dump files

[Min Hu (Connor)]

This patch rename hns3 dump files and abstract a head file for dump.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 .../net/hns3/{hns3_ethdev_dump.c => hns3_dump.c}    |  2 +-
 drivers/net/hns3/hns3_dump.h                        | 13 +++++++++++++
 drivers/net/hns3/hns3_ethdev.c                      |  1 +
 drivers/net/hns3/hns3_ethdev.h                      |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c                   |  1 +
 drivers/net/hns3/meson.build                        |  2 +-
 6 files changed, 17 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (99%)
 create mode 100644 drivers/net/hns3/hns3_dump.h

diff --git a/drivers/net/hns3/hns3_ethdev_dump.c b/drivers/net/hns3/hns3_dump.c
similarity index 99%
rename from drivers/net/hns3/hns3_ethdev_dump.c
rename to drivers/net/hns3/hns3_dump.c
index 1bb2ab7556..2cfab429af 100644
--- a/drivers/net/hns3/hns3_ethdev_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -6,7 +6,7 @@
 #include "hns3_logs.h"
 #include "hns3_regs.h"
 #include "hns3_rxtx.h"
-#include "hns3_ethdev.h"
+#include "hns3_dump.h"
 
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
new file mode 100644
index 0000000000..b0fe37ee21
--- /dev/null
+++ b/drivers/net/hns3/hns3_dump.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2022 HiSilicon Limited
+ */
+
+#ifndef _HNS3_DUMP_H_
+#define _HNS3_DUMP_H_
+
+#include <stdio.h>
+
+#include <ethdev_driver.h>
+
+int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+#endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 0b565a5614..6fa07c4c94 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -8,6 +8,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_intr.h"
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 5e8a746514..8de5a712f4 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -1062,7 +1062,6 @@ int hns3_timesync_read_time(struct rte_eth_dev *dev, struct timespec *ts);
 int hns3_timesync_write_time(struct rte_eth_dev *dev,
 			const struct timespec *ts);
 int hns3_timesync_adjust_time(struct rte_eth_dev *dev, int64_t delta);
-int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
 
 static inline bool
 is_reset_pending(struct hns3_adapter *hns)
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 589de0ab3a..5fc6515de9 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -10,6 +10,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_regs.h"
diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index f2aede94ed..dc99cb0209 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -30,7 +30,7 @@ sources = files(
         'hns3_tm.c',
         'hns3_ptp.c',
         'hns3_common.c',
-        'hns3_ethdev_dump.c',
+        'hns3_dump.c',
 )
 
 deps += ['hash']
-- 
2.33.0

[PATCH 3/4] net/hns3: support Rx/Tx bd dump

[Min Hu (Connor)]

This patch support query HW descriptor from hns3 device. HW descriptor
is also called BD(buffer description) which is shared memory between
software and hardware.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_dump.c      | 64 +++++++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h      |  4 ++
 drivers/net/hns3/hns3_ethdev.c    |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c |  2 +
 4 files changed, 72 insertions(+)

diff --git a/drivers/net/hns3/hns3_dump.c b/drivers/net/hns3/hns3_dump.c
index 2cfab429af..f5de5be4eb 100644
--- a/drivers/net/hns3/hns3_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -8,6 +8,9 @@
 #include "hns3_rxtx.h"
 #include "hns3_dump.h"
 
+#define HNS3_BD_DW_NUM 8
+#define HNS3_BD_ADDRESS_LAST_DW 2
+
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
 {
@@ -911,3 +914,64 @@ hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file)
 
 	return 0;
 }
+
+int hns3_rx_hw_desc_dump(FILE *file, struct rte_eth_dev *dev, uint16_t queue_id,
+			 uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_rx_queue *rxq = dev->data->rx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= rxq->nb_rx_desc) {
+		hns3_err(hw, "Invalid Rx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)rxq->rx_ring;
+	fprintf(file, "Rx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+		fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
+
+int hns3_tx_hw_desc_dump(FILE *file, struct rte_eth_dev *dev, uint16_t queue_id,
+			 uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_tx_queue *txq = dev->data->tx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= txq->nb_tx_desc) {
+		hns3_err(hw, "Invalid Tx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)txq->tx_ring;
+	fprintf(file, "Tx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+
+		fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
index b0fe37ee21..7cc0b36834 100644
--- a/drivers/net/hns3/hns3_dump.h
+++ b/drivers/net/hns3/hns3_dump.h
@@ -10,4 +10,8 @@
 #include <ethdev_driver.h>
 
 int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+int hns3_rx_hw_desc_dump(FILE *file, struct rte_eth_dev *dev, uint16_t queue_id,
+			 uint16_t desc_id);
+int hns3_tx_hw_desc_dump(FILE *file, struct rte_eth_dev *dev, uint16_t queue_id,
+			 uint16_t desc_id);
 #endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 6fa07c4c94..ad5018f8a1 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6562,6 +6562,8 @@ static const struct eth_dev_ops hns3_eth_dev_ops = {
 	.timesync_read_time         = hns3_timesync_read_time,
 	.timesync_write_time        = hns3_timesync_write_time,
 	.eth_dev_priv_dump          = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump        = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump        = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3_reset_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 5fc6515de9..26173442b2 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2288,6 +2288,8 @@ static const struct eth_dev_ops hns3vf_eth_dev_ops = {
 	.dev_supported_ptypes_get = hns3_dev_supported_ptypes_get,
 	.tx_done_cleanup    = hns3_tx_done_cleanup,
 	.eth_dev_priv_dump  = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3vf_reset_ops = {
-- 
2.33.0

Re: [PATCH 3/4] net/hns3: support Rx/Tx bd dump

[Stephen Hemminger]

On Fri, 27 May 2022 10:33:50 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> +	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
> +		/*
> +		 * For the sake of security, first 8 bytes of BD which stands
> +		 * for physical address of packet should not be shown.
> +		 */
> +		if (i < HNS3_BD_ADDRESS_LAST_DW) {
> +			fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
> +			continue;
> +		}
> +
> +		fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i, *(bd_data + i));

Use hex dump that exists?
Also this is not the kernel so leaking physical address values is not a real
security concern.

Re: [PATCH 3/4] net/hns3: support Rx/Tx bd dump

[Min Hu (Connor)]

Hi, Stephen,

在 2022/5/27 23:36, Stephen Hemminger 写道:
> On Fri, 27 May 2022 10:33:50 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>> +	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
>> +		/*
>> +		 * For the sake of security, first 8 bytes of BD which stands
>> +		 * for physical address of packet should not be shown.
>> +		 */
>> +		if (i < HNS3_BD_ADDRESS_LAST_DW) {
>> +			fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
>> +			continue;
>> +		}
>> +
>> +		fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
> 
> Use hex dump that exists?
I redefine the dump style because it'd better be consistent with the
layout of decription for HW buffer. Like this :
### NIC descriptor for port 0 ###
Rx queue id:0 BD id:1
RX BD WORD[0]:0x00000000
RX BD WORD[1]:0x00000000
RX BD WORD[2]:0x00000000
RX BD WORD[3]:0x00000000
RX BD WORD[4]:0x00000000
RX BD WORD[5]:0x00000000
RX BD WORD[6]:0x00000000
RX BD WORD[7]:0x00000000

The style is more readable.


> Also this is not the kernel so leaking physical address values is not a real
> security concern.
This address is physical address of packet. It will be used illegal like
tamper with packets. This is unsafe to do so.

> .
> 

[PATCH 4/4] app/procinfo: support descriptor dump

[Min Hu (Connor)]

This patch support HW Rx/Tx descriptor dump

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

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

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

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b7e70b4bf5..dd481e26b8 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -49,6 +49,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)(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 /**< mask of enabled ports */
 static unsigned long enabled_port_mask;
 /**< Enable stats. */
@@ -107,6 +110,12 @@ static uint32_t enable_shw_version;
 static uint32_t enable_shw_rss_reta;
 /**< Enable show module info. */
 static uint32_t enable_shw_module_info;
+/**< Enable dump buffer descriptor. */
+#define MAX_NB_ITEM 2
+static uint16_t rx_nb_item;
+static uint16_t tx_nb_item;
+static uint16_t rx_item_opt[MAX_NB_ITEM];
+static uint16_t tx_item_opt[MAX_NB_ITEM];
 
 /**< display usage */
 static void
@@ -137,6 +146,8 @@ proc_info_usage(const char *prgname)
 		"  --show-version: to display DPDK version and firmware version\n"
 		"  --show-rss-reta: to display ports redirection table\n"
 		"  --show-module-info: to display ports module info\n"
+		"  --show-rx-descriptor queue_id:descriptor_id: to display ports Rx buffer description by queue id and descriptor id\n"
+		"  --show-tx-descriptor queue_id:descriptor_id: to display ports Tx buffer description by queue id and descriptor id\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);
@@ -189,6 +200,34 @@ parse_xstats_ids(char *list, uint64_t *ids, int limit) {
 	return length;
 }
 
+/*
+ * Parse ids value list into array
+ */
+static int
+parse_descriptor_param(char *list, uint16_t *item_opt, int limit)
+{
+	int length;
+	char *token;
+	char *ctx = NULL;
+	char *endptr;
+
+	length = 0;
+	token = strtok_r(list, ":", &ctx);
+	while (token != NULL) {
+		item_opt[length] = strtoul(token, &endptr, 10);
+		if (*endptr != '\0')
+			return -EINVAL;
+
+		length++;
+		if (length > limit)
+			return -E2BIG;
+
+		token = strtok_r(NULL, ":", &ctx);
+	}
+
+	return length;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -251,6 +290,8 @@ proc_info_parse_args(int argc, char **argv)
 		{"show-version", 0, NULL, 0},
 		{"show-rss-reta", 0, NULL, 0},
 		{"show-module-info", 0, NULL, 0},
+		{"show-rx-descriptor", required_argument, NULL, 1},
+		{"show-tx-descriptor", required_argument, NULL, 1},
 		{NULL, 0, 0, 0}
 	};
 
@@ -348,6 +389,26 @@ 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_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Rx descriptor param parse error.\n");
+					return -1;
+				}
+				rx_nb_item = ret;
+			} else if (!strncmp(long_option[option_index].name,
+				"show-tx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+								 tx_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Tx descriptor param parse error.\n");
+					return -1;
+				}
+				tx_nb_item = ret;
 			}
 			break;
 		default:
@@ -1597,6 +1658,20 @@ static void show_module_eeprom_info(void)
 	}
 }
 
+static void
+nic_descriptor_display(uint16_t port_id, uint16_t *item_opt,
+		       desc_dump_t desc_dump)
+{
+	static const char *nic_desc_border = "###";
+	uint16_t queue_id = item_opt[0];
+	uint16_t desc_id = item_opt[1];
+
+	printf("%s NIC descriptor for port %u %s\n",
+		   nic_desc_border, port_id, nic_desc_border);
+
+	desc_dump(stdout, port_id, queue_id, desc_id);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1680,6 +1755,12 @@ main(int argc, char **argv)
 		else if (nb_xstats_ids > 0)
 			nic_xstats_by_ids_display(i, xstats_ids,
 						  nb_xstats_ids);
+		else if (rx_nb_item > 0)
+			nic_descriptor_display(i, rx_item_opt,
+					       rte_eth_rx_hw_desc_dump);
+		else if (tx_nb_item > 0)
+			nic_descriptor_display(i, tx_item_opt,
+					       rte_eth_tx_hw_desc_dump);
 #ifdef RTE_LIB_METRICS
 		else if (enable_metrics)
 			metrics_display(i);
-- 
2.33.0

[PATCH v2 0/4] support HW Rx/Tx descriptor dump

[Min Hu (Connor)]

This patch set support HW Rx/Tx descriptor dump by using procinfo tool.

Min Hu (Connor) (4):
  ethdev: introduce ethdev HW desc dump PI
  net/hns3: rename hns3 dump files
  net/hns3: support Rx/Tx bd dump
  app/procinfo: support descriptor dump

 app/proc-info/main.c                          | 81 +++++++++++++++++++
 doc/guides/rel_notes/release_22_07.rst        |  6 ++
 .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
 drivers/net/hns3/hns3_dump.h                  | 17 ++++
 drivers/net/hns3/hns3_ethdev.c                |  3 +
 drivers/net/hns3/hns3_ethdev.h                |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
 drivers/net/hns3/meson.build                  |  2 +-
 lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
 lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
 lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
 lib/ethdev/version.map                        |  2 +
 12 files changed, 308 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
 create mode 100644 drivers/net/hns3/hns3_dump.h
---
v2:
* add 'const' for ethdev.

-- 
2.33.0

[PATCH v2 1/4] ethdev: introduce ethdev HW desc dump PI

[Min Hu (Connor)]

Added the ethdev HW Rx desc dump API which provides functions for query
HW descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/rel_notes/release_22_07.rst |  6 ++++
 lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 5 files changed, 138 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 0ed4f92820..cfb9117dd3 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -95,6 +95,12 @@ New Features
   * Added AH mode support in lookaside protocol (IPsec) for CN9K & CN10K.
   * Added AES-GMAC support in lookaside protocol (IPsec) for CN9K & CN10K.
 
+* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
+
+  Added the ethdev HW Rx desc dump API which provides functions for query
+  HW descriptor from device. HW descriptor info differs in different NICs.
+  The information demonstrates I/O process which is important for debug.
+  As the information is different between NICs, the new API is introduced.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 69d9dc21d8..9c1726eb2d 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
  */
 typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
 
+/**
+ * @internal
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t desc_id);
+
+/**
+ * @internal
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t desc_id);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1283,6 +1319,12 @@ struct eth_dev_ops {
 
 	/** Dump private info from device */
 	eth_dev_priv_dump_t eth_dev_priv_dump;
+
+	/** Dump ethdev Rx descriptor info */
+	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
+
+	/** Dump ethdev Tx descriptor info */
+	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a175867651..09abee6345 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5874,6 +5874,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
 	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
 }
 
+int
+rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
+int
+rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 02df65d923..56ae630209 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5456,6 +5456,50 @@ typedef struct {
 __rte_experimental
 int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index daca7851f2..109f4ea818 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,8 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+	rte_eth_rx_hw_desc_dump;
+	rte_eth_tx_hw_desc_dump;
 };
 
 INTERNAL {
-- 
2.33.0

[PATCH v2 2/4] net/hns3: rename hns3 dump files

[Min Hu (Connor)]

This patch rename hns3 dump files and abstract a head file for dump.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 .../net/hns3/{hns3_ethdev_dump.c => hns3_dump.c}    |  2 +-
 drivers/net/hns3/hns3_dump.h                        | 13 +++++++++++++
 drivers/net/hns3/hns3_ethdev.c                      |  1 +
 drivers/net/hns3/hns3_ethdev.h                      |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c                   |  1 +
 drivers/net/hns3/meson.build                        |  2 +-
 6 files changed, 17 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (99%)
 create mode 100644 drivers/net/hns3/hns3_dump.h

diff --git a/drivers/net/hns3/hns3_ethdev_dump.c b/drivers/net/hns3/hns3_dump.c
similarity index 99%
rename from drivers/net/hns3/hns3_ethdev_dump.c
rename to drivers/net/hns3/hns3_dump.c
index 1bb2ab7556..2cfab429af 100644
--- a/drivers/net/hns3/hns3_ethdev_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -6,7 +6,7 @@
 #include "hns3_logs.h"
 #include "hns3_regs.h"
 #include "hns3_rxtx.h"
-#include "hns3_ethdev.h"
+#include "hns3_dump.h"
 
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
new file mode 100644
index 0000000000..b0fe37ee21
--- /dev/null
+++ b/drivers/net/hns3/hns3_dump.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2022 HiSilicon Limited
+ */
+
+#ifndef _HNS3_DUMP_H_
+#define _HNS3_DUMP_H_
+
+#include <stdio.h>
+
+#include <ethdev_driver.h>
+
+int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+#endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 0b565a5614..6fa07c4c94 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -8,6 +8,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_intr.h"
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 5e8a746514..8de5a712f4 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -1062,7 +1062,6 @@ int hns3_timesync_read_time(struct rte_eth_dev *dev, struct timespec *ts);
 int hns3_timesync_write_time(struct rte_eth_dev *dev,
 			const struct timespec *ts);
 int hns3_timesync_adjust_time(struct rte_eth_dev *dev, int64_t delta);
-int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
 
 static inline bool
 is_reset_pending(struct hns3_adapter *hns)
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 589de0ab3a..5fc6515de9 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -10,6 +10,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_regs.h"
diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index f2aede94ed..dc99cb0209 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -30,7 +30,7 @@ sources = files(
         'hns3_tm.c',
         'hns3_ptp.c',
         'hns3_common.c',
-        'hns3_ethdev_dump.c',
+        'hns3_dump.c',
 )
 
 deps += ['hash']
-- 
2.33.0

[PATCH v2 3/4] net/hns3: support Rx/Tx bd dump

[Min Hu (Connor)]

This patch support query HW descriptor from hns3 device. HW descriptor
is also called BD(buffer description) which is shared memory between
software and hardware.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_dump.c      | 64 +++++++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h      |  4 ++
 drivers/net/hns3/hns3_ethdev.c    |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c |  2 +
 4 files changed, 72 insertions(+)

diff --git a/drivers/net/hns3/hns3_dump.c b/drivers/net/hns3/hns3_dump.c
index 2cfab429af..8db2f26144 100644
--- a/drivers/net/hns3/hns3_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -8,6 +8,9 @@
 #include "hns3_rxtx.h"
 #include "hns3_dump.h"
 
+#define HNS3_BD_DW_NUM 8
+#define HNS3_BD_ADDRESS_LAST_DW 2
+
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
 {
@@ -911,3 +914,64 @@ hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file)
 
 	return 0;
 }
+
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_rx_queue *rxq = dev->data->rx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= rxq->nb_rx_desc) {
+		hns3_err(hw, "Invalid Rx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)rxq->rx_ring;
+	fprintf(file, "Rx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+		fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
+
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_tx_queue *txq = dev->data->tx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= txq->nb_tx_desc) {
+		hns3_err(hw, "Invalid Tx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)txq->tx_ring;
+	fprintf(file, "Tx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+
+		fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
index b0fe37ee21..3dcd9a0466 100644
--- a/drivers/net/hns3/hns3_dump.h
+++ b/drivers/net/hns3/hns3_dump.h
@@ -10,4 +10,8 @@
 #include <ethdev_driver.h>
 
 int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id);
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id);
 #endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 6fa07c4c94..ad5018f8a1 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6562,6 +6562,8 @@ static const struct eth_dev_ops hns3_eth_dev_ops = {
 	.timesync_read_time         = hns3_timesync_read_time,
 	.timesync_write_time        = hns3_timesync_write_time,
 	.eth_dev_priv_dump          = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump        = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump        = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3_reset_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 5fc6515de9..26173442b2 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2288,6 +2288,8 @@ static const struct eth_dev_ops hns3vf_eth_dev_ops = {
 	.dev_supported_ptypes_get = hns3_dev_supported_ptypes_get,
 	.tx_done_cleanup    = hns3_tx_done_cleanup,
 	.eth_dev_priv_dump  = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3vf_reset_ops = {
-- 
2.33.0

[PATCH v2 4/4] app/procinfo: support descriptor dump

[Min Hu (Connor)]

This patch support HW Rx/Tx descriptor dump

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

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

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

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 56070a3317..eab1b546d1 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)(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 /**< mask of enabled ports */
 static unsigned long enabled_port_mask;
 /**< Enable stats. */
@@ -100,6 +103,12 @@ static char *mempool_iter_name;
 /**< Enable dump regs. */
 static uint32_t enable_dump_regs;
 static char *dump_regs_file_prefix;
+/**< Enable dump buffer descriptor. */
+#define MAX_NB_ITEM 2
+static uint16_t rx_nb_item;
+static uint16_t tx_nb_item;
+static uint16_t rx_item_opt[MAX_NB_ITEM];
+static uint16_t tx_item_opt[MAX_NB_ITEM];
 
 /**< display usage */
 static void
@@ -127,6 +136,8 @@ 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:descriptor_id: to display ports Rx buffer description by queue id and descriptor id\n"
+		"  --show-tx-descriptor queue_id:descriptor_id: to display ports Tx buffer description by queue id and descriptor id\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);
@@ -179,6 +190,34 @@ parse_xstats_ids(char *list, uint64_t *ids, int limit) {
 	return length;
 }
 
+/*
+ * Parse ids value list into array
+ */
+static int
+parse_descriptor_param(char *list, uint16_t *item_opt, int limit)
+{
+	int length;
+	char *token;
+	char *ctx = NULL;
+	char *endptr;
+
+	length = 0;
+	token = strtok_r(list, ":", &ctx);
+	while (token != NULL) {
+		item_opt[length] = strtoul(token, &endptr, 10);
+		if (*endptr != '\0')
+			return -EINVAL;
+
+		length++;
+		if (length > limit)
+			return -E2BIG;
+
+		token = strtok_r(NULL, ":", &ctx);
+	}
+
+	return length;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -238,6 +277,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}
 	};
 
@@ -327,6 +368,26 @@ 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_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Rx descriptor param parse error.\n");
+					return -1;
+				}
+				rx_nb_item = ret;
+			} else if (!strncmp(long_option[option_index].name,
+				"show-tx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+								 tx_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Tx descriptor param parse error.\n");
+					return -1;
+				}
+				tx_nb_item = ret;
 			}
 			break;
 		default:
@@ -1450,6 +1511,20 @@ dump_regs(char *file_prefix)
 	}
 }
 
+static void
+nic_descriptor_display(uint16_t port_id, uint16_t *item_opt,
+		       desc_dump_t desc_dump)
+{
+	static const char *nic_desc_border = "###";
+	uint16_t queue_id = item_opt[0];
+	uint16_t desc_id = item_opt[1];
+
+	printf("%s NIC descriptor for port %u %s\n",
+		   nic_desc_border, port_id, nic_desc_border);
+
+	desc_dump(stdout, port_id, queue_id, desc_id);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1533,6 +1608,12 @@ main(int argc, char **argv)
 		else if (nb_xstats_ids > 0)
 			nic_xstats_by_ids_display(i, xstats_ids,
 						  nb_xstats_ids);
+		else if (rx_nb_item > 0)
+			nic_descriptor_display(i, rx_item_opt,
+					       rte_eth_rx_hw_desc_dump);
+		else if (tx_nb_item > 0)
+			nic_descriptor_display(i, tx_item_opt,
+					       rte_eth_tx_hw_desc_dump);
 #ifdef RTE_LIB_METRICS
 		else if (enable_metrics)
 			metrics_display(i);
-- 
2.33.0

[PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Min Hu (Connor)]

This patch set support HW Rx/Tx descriptor dump by using procinfo tool.

Min Hu (Connor) (4):
  ethdev: introduce ethdev HW desc dump PI
  net/hns3: rename hns3 dump files
  net/hns3: support Rx/Tx bd dump
  app/procinfo: support descriptor dump

 app/proc-info/main.c                          | 81 +++++++++++++++++++
 doc/guides/rel_notes/release_22_07.rst        |  7 ++
 .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
 drivers/net/hns3/hns3_dump.h                  | 17 ++++
 drivers/net/hns3/hns3_ethdev.c                |  3 +
 drivers/net/hns3/hns3_ethdev.h                |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
 drivers/net/hns3/meson.build                  |  2 +-
 lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
 lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
 lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
 lib/ethdev/version.map                        |  2 +
 12 files changed, 309 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
 create mode 100644 drivers/net/hns3/hns3_dump.h
---
v3:
* fix desc id handling.

v2:
* add 'const' for ethdev.
-- 
2.33.0

[PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI

[Min Hu (Connor)]

Added the ethdev HW Rx desc dump API which provides functions for query
HW descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/rel_notes/release_22_07.rst |  7 ++++
 lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 ++
 5 files changed, 139 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
index 8932a1d478..56c675121a 100644
--- a/doc/guides/rel_notes/release_22_07.rst
+++ b/doc/guides/rel_notes/release_22_07.rst
@@ -137,6 +137,13 @@ New Features
   * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
   * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
 
+* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
+
+  Added the ethdev HW Rx desc dump API which provides functions for query
+  HW descriptor from device. HW descriptor info differs in different NICs.
+  The information demonstrates I/O process which is important for debug.
+  As the information is different between NICs, the new API is introduced.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 69d9dc21d8..9c1726eb2d 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
  */
 typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
 
+/**
+ * @internal
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t desc_id);
+
+/**
+ * @internal
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t desc_id);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1283,6 +1319,12 @@ struct eth_dev_ops {
 
 	/** Dump private info from device */
 	eth_dev_priv_dump_t eth_dev_priv_dump;
+
+	/** Dump ethdev Rx descriptor info */
+	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
+
+	/** Dump ethdev Tx descriptor info */
+	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 46c088dc88..bbd8439fa0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
 	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
 }
 
+int
+rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
+int
+rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t desc_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id,
+						   desc_id);
+
+	return ret;
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 02df65d923..56ae630209 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5456,6 +5456,50 @@ typedef struct {
 __rte_experimental
 int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param desc_id
+ *   The selected descriptor.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index daca7851f2..109f4ea818 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,8 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+	rte_eth_rx_hw_desc_dump;
+	rte_eth_tx_hw_desc_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI

[Andrew Rybchenko]

On 6/1/22 10:49, Min Hu (Connor) wrote:
> Added the ethdev HW Rx desc dump API which provides functions for query
> HW descriptor from device. HW descriptor info differs in different NICs.
> The information demonstrates I/O process which is important for debug.
> As the information is different between NICs, the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>   doc/guides/rel_notes/release_22_07.rst |  7 ++++
>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>   lib/ethdev/version.map                 |  2 ++
>   5 files changed, 139 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_07.rst b/doc/guides/rel_notes/release_22_07.rst
> index 8932a1d478..56c675121a 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -137,6 +137,13 @@ New Features
>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>   
> +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from device.**
> +
> +  Added the ethdev HW Rx desc dump API which provides functions for query
> +  HW descriptor from device. HW descriptor info differs in different NICs.
> +  The information demonstrates I/O process which is important for debug.
> +  As the information is different between NICs, the new API is introduced.
> +
>   
>   Removed Items
>   -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 69d9dc21d8..9c1726eb2d 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -1073,6 +1073,42 @@ typedef int (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>    */
>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>   
> +/**
> + * @internal
> + * Dump ethdev Rx descriptor info to a file.

ethdev is not required in the description above. It is an ethdev
operation type.

Is there any limitations on caller context? Should be it the same
CPU core (since typically it is the case for per-queue API) which
polls the queue? The answer must be in the API function description.

> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.

Is it an ID in the ring regardless of the current position or
is it offset relative to current position in the ring?
It should be clarified in any case.
I'd say that it should be an offset to be consistent with
descriptor status API.

It would be useful to be able to dump many descriptor at once.

> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
> +				     uint16_t queue_id, uint16_t desc_id);
> +
> +/**
> + * @internal
> + * Dump ethdev Tx descriptor info to a file.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param desc_id
> + *   The selected descriptor.
> + * @return
> + *   Negative errno value on error, zero on success.
> + */
> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
> +				     uint16_t queue_id, uint16_t desc_id);
> +
>   /**
>    * @internal A structure containing the functions exported by an Ethernet driver.
>    */
> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>   
>   	/** Dump private info from device */
>   	eth_dev_priv_dump_t eth_dev_priv_dump;
> +
> +	/** Dump ethdev Rx descriptor info */\\

It is an ethdev operations. So, 'ethdev' is not necessary in the
description.

> +	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
> +
> +	/** Dump ethdev Tx descriptor info */
> +	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>   };
>   
>   /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 46c088dc88..bbd8439fa0 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
>   	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
>   }
>   
> +int
> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +			uint16_t desc_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];

Shouldn't we check file vs null?

> +
> +	if (queue_id >= dev->data->nb_rx_queues) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
> +		return -EINVAL;
> +	}

Don't we need a check vs queue size?

> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
> +	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
> +						   desc_id);
> +
> +	return ret;
> +}

Re: [PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI

[Dongdong Liu]

Hi Andrew

Many thanks for your review.

On 2022/6/2 3:53, Andrew Rybchenko wrote:
> On 6/1/22 10:49, Min Hu (Connor) wrote:
>> Added the ethdev HW Rx desc dump API which provides functions for query
>> HW descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_22_07.rst |  7 ++++
>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>>   lib/ethdev/version.map                 |  2 ++
>>   5 files changed, 139 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_07.rst
>> b/doc/guides/rel_notes/release_22_07.rst
>> index 8932a1d478..56c675121a 100644
>> --- a/doc/guides/rel_notes/release_22_07.rst
>> +++ b/doc/guides/rel_notes/release_22_07.rst
>> @@ -137,6 +137,13 @@ New Features
>>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>>   +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from
>> device.**
>> +
>> +  Added the ethdev HW Rx desc dump API which provides functions for
>> query
>> +  HW descriptor from device. HW descriptor info differs in different
>> NICs.
>> +  The information demonstrates I/O process which is important for debug.
>> +  As the information is different between NICs, the new API is
>> introduced.
>> +
>>     Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 69d9dc21d8..9c1726eb2d 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -1073,6 +1073,42 @@ typedef int
>> (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>>    */
>>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE
>> *file);
>>   +/**
>> + * @internal
>> + * Dump ethdev Rx descriptor info to a file.
>
> ethdev is not required in the description above. It is an ethdev
> operation type.

Will remove it.
>
> Is there any limitations on caller context? Should be it the same
> CPU core (since typically it is the case for per-queue API) which
> polls the queue? The answer must be in the API function description.

"[PATCH v3 4/4] app/procinfo: support descriptor dump" shows how to use
the API. It is used for debug, not a dataplane API,  no special
limitations on caller context.

>
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>
> Is it an ID in the ring regardless of the current position or
> is it offset relative to current position in the ring?
> It should be clarified in any case.
> I'd say that it should be an offset to be consistent with
> descriptor status API.

It is an ID in the ring regardless of the current position.
>
> It would be useful to be able to dump many descriptor at once.
This can be done by the appliacation.
>
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct
>> rte_eth_dev *dev,
>> +                     uint16_t queue_id, uint16_t desc_id);
>> +
>> +/**
>> + * @internal
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param desc_id
>> + *   The selected descriptor.
>> + * @return
>> + *   Negative errno value on error, zero on success.
>> + */
>> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct
>> rte_eth_dev *dev,
>> +                     uint16_t queue_id, uint16_t desc_id);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an
>> Ethernet driver.
>>    */
>> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>>         /** Dump private info from device */
>>       eth_dev_priv_dump_t eth_dev_priv_dump;
>> +
>> +    /** Dump ethdev Rx descriptor info */\\
>
> It is an ethdev operations. So, 'ethdev' is not necessary in the
> description.
Will fix.
>
>> +    eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
>> +
>> +    /** Dump ethdev Tx descriptor info */
>> +    eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>>   };
>>     /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 46c088dc88..bbd8439fa0 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE
>> *file)
>>       return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev,
>> file));
>>   }
>>   +int
>> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
>> +            uint16_t desc_id)
>> +{
>> +    struct rte_eth_dev *dev;
>> +    int ret;
>> +
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +    dev = &rte_eth_devices[port_id];
>
> Shouldn't we check file vs null?
Yes, will do.
>
>> +
>> +    if (queue_id >= dev->data->nb_rx_queues) {
>> +        RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
>> +        return -EINVAL;
>> +    }
>
> Don't we need a check vs queue size?

This need to be done in pmd as "[PATCH v3 3/4] net/hns3: support Rx/Tx 
bd dump" shows.

Thanks,
Dongdong
>
>> +
>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump,
>> -ENOTSUP);
>> +    ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id,
>> +                           desc_id);
>> +
>> +    return ret;
>> +}
>
> .
>

Re: [PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI

[Andrew Rybchenko]

Cc various driver maintainers looking for opinion about API discussion
below

On 6/7/22 16:59, Dongdong Liu wrote:
> Hi Andrew
> 
> Many thanks for your review.
> 
> On 2022/6/2 3:53, Andrew Rybchenko wrote:
>> On 6/1/22 10:49, Min Hu (Connor) wrote:
>>> Added the ethdev HW Rx desc dump API which provides functions for query
>>> HW descriptor from device. HW descriptor info differs in different NICs.
>>> The information demonstrates I/O process which is important for debug.
>>> As the information is different between NICs, the new API is introduced.
>>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> ---
>>>   doc/guides/rel_notes/release_22_07.rst |  7 ++++
>>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.c                | 44 ++++++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 44 ++++++++++++++++++++++++++
>>>   lib/ethdev/version.map                 |  2 ++
>>>   5 files changed, 139 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/release_22_07.rst
>>> b/doc/guides/rel_notes/release_22_07.rst
>>> index 8932a1d478..56c675121a 100644
>>> --- a/doc/guides/rel_notes/release_22_07.rst
>>> +++ b/doc/guides/rel_notes/release_22_07.rst
>>> @@ -137,6 +137,13 @@ New Features
>>>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>>>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>>>   +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from
>>> device.**
>>> +
>>> +  Added the ethdev HW Rx desc dump API which provides functions for
>>> query
>>> +  HW descriptor from device. HW descriptor info differs in different
>>> NICs.
>>> +  The information demonstrates I/O process which is important for 
>>> debug.
>>> +  As the information is different between NICs, the new API is
>>> introduced.
>>> +
>>>     Removed Items
>>>   -------------
>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>>> index 69d9dc21d8..9c1726eb2d 100644
>>> --- a/lib/ethdev/ethdev_driver.h
>>> +++ b/lib/ethdev/ethdev_driver.h
>>> @@ -1073,6 +1073,42 @@ typedef int
>>> (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>>>    */
>>>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE
>>> *file);
>>>   +/**
>>> + * @internal
>>> + * Dump ethdev Rx descriptor info to a file.
>>
>> ethdev is not required in the description above. It is an ethdev
>> operation type.
> 
> Will remove it.
>>
>> Is there any limitations on caller context? Should be it the same
>> CPU core (since typically it is the case for per-queue API) which
>> polls the queue? The answer must be in the API function description.
> 
> "[PATCH v3 4/4] app/procinfo: support descriptor dump" shows how to use
> the API. It is used for debug, not a dataplane API,  no special
> limitations on caller context.

Please, be explicit in the ethdev API documentation.

>>
>>> + *
>>> + * @param file
>>> + *   A pointer to a file for output.
>>> + * @param dev
>>> + *   Port (ethdev) handle.
>>> + * @param queue_id
>>> + *   The selected queue.
>>> + * @param desc_id
>>> + *   The selected descriptor.
>>
>> Is it an ID in the ring regardless of the current position or
>> is it offset relative to current position in the ring?
>> It should be clarified in any case.
>> I'd say that it should be an offset to be consistent with
>> descriptor status API.
> 
> It is an ID in the ring regardless of the current position.

IMHO, it is inconvenient since typically it is interesting
what happens nearby current position.

>>
>> It would be useful to be able to dump many descriptor at once.
> This can be done by the appliacation.

Yes, that's true, but easy to do in the driver as well. It would
be especially important if ID semantics changes.

>>
>>> + * @return
>>> + *   Negative errno value on error, zero on success.
>>> + */
>>> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct
>>> rte_eth_dev *dev,
>>> +                     uint16_t queue_id, uint16_t desc_id);
>>> +
>>> +/**
>>> + * @internal
>>> + * Dump ethdev Tx descriptor info to a file.
>>> + *
>>> + * @param file
>>> + *   A pointer to a file for output.
>>> + * @param dev
>>> + *   Port (ethdev) handle.
>>> + * @param queue_id
>>> + *   The selected queue.
>>> + * @param desc_id
>>> + *   The selected descriptor.
>>> + * @return
>>> + *   Negative errno value on error, zero on success.
>>> + */
>>> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct
>>> rte_eth_dev *dev,
>>> +                     uint16_t queue_id, uint16_t desc_id);
>>> +
>>>   /**
>>>    * @internal A structure containing the functions exported by an
>>> Ethernet driver.
>>>    */
>>> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>>>         /** Dump private info from device */
>>>       eth_dev_priv_dump_t eth_dev_priv_dump;
>>> +
>>> +    /** Dump ethdev Rx descriptor info */\\
>>
>> It is an ethdev operations. So, 'ethdev' is not necessary in the
>> description.
> Will fix.
>>
>>> +    eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
>>> +
>>> +    /** Dump ethdev Tx descriptor info */
>>> +    eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>>>   };
>>>     /**
>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index 46c088dc88..bbd8439fa0 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE
>>> *file)
>>>       return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev,
>>> file));
>>>   }
>>>   +int
>>> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t 
>>> queue_id,
>>> +            uint16_t desc_id)
>>> +{
>>> +    struct rte_eth_dev *dev;
>>> +    int ret;
>>> +
>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> +    dev = &rte_eth_devices[port_id];
>>
>> Shouldn't we check file vs null?
> Yes, will do.
>>
>>> +
>>> +    if (queue_id >= dev->data->nb_rx_queues) {
>>> +        RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
>>> +        return -EINVAL;
>>> +    }
>>
>> Don't we need a check vs queue size?
> 
> This need to be done in pmd as "[PATCH v3 3/4] net/hns3: support Rx/Tx 
> bd dump" shows.

All applicable generic checks must be done on ethdev level to avoid code
duplication and missing checks in PMDs.

Andrew.

Re: [PATCH v3 1/4] ethdev: introduce ethdev HW desc dump PI

[Dongdong Liu]

Hi Andrew

Many thanks for your review.

On 2022/6/8 18:09, Andrew Rybchenko wrote:
> Cc various driver maintainers looking for opinion about API discussion
> below
>
> On 6/7/22 16:59, Dongdong Liu wrote:
>> Hi Andrew
>>
>> Many thanks for your review.
>>
>> On 2022/6/2 3:53, Andrew Rybchenko wrote:
>>> On 6/1/22 10:49, Min Hu (Connor) wrote:
>>>> Added the ethdev HW Rx desc dump API which provides functions for query
>>>> HW descriptor from device. HW descriptor info differs in different
>>>> NICs.
>>>> The information demonstrates I/O process which is important for debug.
>>>> As the information is different between NICs, the new API is
>>>> introduced.
>>>>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> ---
>>>>   doc/guides/rel_notes/release_22_07.rst |  7 ++++
>>>>   lib/ethdev/ethdev_driver.h             | 42 ++++++++++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.c                | 44
>>>> ++++++++++++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.h                | 44
>>>> ++++++++++++++++++++++++++
>>>>   lib/ethdev/version.map                 |  2 ++
>>>>   5 files changed, 139 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/release_22_07.rst
>>>> b/doc/guides/rel_notes/release_22_07.rst
>>>> index 8932a1d478..56c675121a 100644
>>>> --- a/doc/guides/rel_notes/release_22_07.rst
>>>> +++ b/doc/guides/rel_notes/release_22_07.rst
>>>> @@ -137,6 +137,13 @@ New Features
>>>>     * ``RTE_EVENT_QUEUE_ATTR_WEIGHT``
>>>>     * ``RTE_EVENT_QUEUE_ATTR_AFFINITY``
>>>>   +* **Added ethdev HW desc dump API, to dump Rx/Tx HW desc info from
>>>> device.**
>>>> +
>>>> +  Added the ethdev HW Rx desc dump API which provides functions for
>>>> query
>>>> +  HW descriptor from device. HW descriptor info differs in different
>>>> NICs.
>>>> +  The information demonstrates I/O process which is important for
>>>> debug.
>>>> +  As the information is different between NICs, the new API is
>>>> introduced.
>>>> +
>>>>     Removed Items
>>>>   -------------
>>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>>>> index 69d9dc21d8..9c1726eb2d 100644
>>>> --- a/lib/ethdev/ethdev_driver.h
>>>> +++ b/lib/ethdev/ethdev_driver.h
>>>> @@ -1073,6 +1073,42 @@ typedef int
>>>> (*eth_ip_reassembly_conf_set_t)(struct rte_eth_dev *dev,
>>>>    */
>>>>   typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE
>>>> *file);
>>>>   +/**
>>>> + * @internal
>>>> + * Dump ethdev Rx descriptor info to a file.
>>>
>>> ethdev is not required in the description above. It is an ethdev
>>> operation type.
>>
>> Will remove it.
>>>
>>> Is there any limitations on caller context? Should be it the same
>>> CPU core (since typically it is the case for per-queue API) which
>>> polls the queue? The answer must be in the API function description.
>>
>> "[PATCH v3 4/4] app/procinfo: support descriptor dump" shows how to use
>> the API. It is used for debug, not a dataplane API,  no special
>> limitations on caller context.
>
> Please, be explicit in the ethdev API documentation.
OK, will do
>
>>>
>>>> + *
>>>> + * @param file
>>>> + *   A pointer to a file for output.
>>>> + * @param dev
>>>> + *   Port (ethdev) handle.
>>>> + * @param queue_id
>>>> + *   The selected queue.
>>>> + * @param desc_id
>>>> + *   The selected descriptor.
>>>
>>> Is it an ID in the ring regardless of the current position or
>>> is it offset relative to current position in the ring?
>>> It should be clarified in any case.
>>> I'd say that it should be an offset to be consistent with
>>> descriptor status API.
>>
>> It is an ID in the ring regardless of the current position.
>
> IMHO, it is inconvenient since typically it is interesting
> what happens nearby current position.
>
OK, will fix.
>>>
>>> It would be useful to be able to dump many descriptor at once.
>> This can be done by the appliacation.
>
> Yes, that's true, but easy to do in the driver as well. It would
> be especially important if ID semantics changes.
How about changing parameter 'desc_id' to 'num' to dump
[current position, current position + num - 1] descriptors.

/**
  * @internal
  * Dump Rx descriptor info to a file.
  *
  * @param file
  *   A pointer to a file for output.
  * @param dev
  *   Port (ethdev) handle.
  * @param queue_id
  *   The selected queue.
  * @param num
  *   The number of the descriptors to dump.
  * @return
  *   Negative errno value on error, zero on success. 
 

  */
typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct 
rte_eth_dev *dev,
									 uint16_t queue_id, uint16_t num);
>
>>>
>>>> + * @return
>>>> + *   Negative errno value on error, zero on success.
>>>> + */
>>>> +typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct
>>>> rte_eth_dev *dev,
>>>> +                     uint16_t queue_id, uint16_t desc_id);
>>>> +
>>>> +/**
>>>> + * @internal
>>>> + * Dump ethdev Tx descriptor info to a file.
>>>> + *
>>>> + * @param file
>>>> + *   A pointer to a file for output.
>>>> + * @param dev
>>>> + *   Port (ethdev) handle.
>>>> + * @param queue_id
>>>> + *   The selected queue.
>>>> + * @param desc_id
>>>> + *   The selected descriptor.
>>>> + * @return
>>>> + *   Negative errno value on error, zero on success.
>>>> + */
>>>> +typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct
>>>> rte_eth_dev *dev,
>>>> +                     uint16_t queue_id, uint16_t desc_id);
>>>> +
>>>>   /**
>>>>    * @internal A structure containing the functions exported by an
>>>> Ethernet driver.
>>>>    */
>>>> @@ -1283,6 +1319,12 @@ struct eth_dev_ops {
>>>>         /** Dump private info from device */
>>>>       eth_dev_priv_dump_t eth_dev_priv_dump;
>>>> +
>>>> +    /** Dump ethdev Rx descriptor info */\\
>>>
>>> It is an ethdev operations. So, 'ethdev' is not necessary in the
>>> description.
>> Will fix.
>>>
>>>> +    eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
>>>> +
>>>> +    /** Dump ethdev Tx descriptor info */
>>>> +    eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
>>>>   };
>>>>     /**
>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>>> index 46c088dc88..bbd8439fa0 100644
>>>> --- a/lib/ethdev/rte_ethdev.c
>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>> @@ -5876,6 +5876,50 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE
>>>> *file)
>>>>       return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev,
>>>> file));
>>>>   }
>>>>   +int
>>>> +rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>>>> queue_id,
>>>> +            uint16_t desc_id)
>>>> +{
>>>> +    struct rte_eth_dev *dev;
>>>> +    int ret;
>>>> +
>>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>> +    dev = &rte_eth_devices[port_id];
>>>
>>> Shouldn't we check file vs null?
>> Yes, will do.
>>>
>>>> +
>>>> +    if (queue_id >= dev->data->nb_rx_queues) {
>>>> +        RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
>>>> +        return -EINVAL;
>>>> +    }
>>>
>>> Don't we need a check vs queue size?
>>
>> This need to be done in pmd as "[PATCH v3 3/4] net/hns3: support Rx/Tx
>> bd dump" shows.
>
> All applicable generic checks must be done on ethdev level to avoid code
> duplication and missing checks in PMDs.
The ethdev level does not know the queue size.
This is similar to rte_eth_rx_descriptor_status(),
queue size check is also done in PMDs.

Thanks,
Dongdong.
>
> Andrew.
> .
>

[PATCH v3 2/4] net/hns3: rename hns3 dump files

[Min Hu (Connor)]

This patch rename hns3 dump files and abstract a head file for dump.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 .../net/hns3/{hns3_ethdev_dump.c => hns3_dump.c}    |  2 +-
 drivers/net/hns3/hns3_dump.h                        | 13 +++++++++++++
 drivers/net/hns3/hns3_ethdev.c                      |  1 +
 drivers/net/hns3/hns3_ethdev.h                      |  1 -
 drivers/net/hns3/hns3_ethdev_vf.c                   |  1 +
 drivers/net/hns3/meson.build                        |  2 +-
 6 files changed, 17 insertions(+), 3 deletions(-)
 rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (99%)
 create mode 100644 drivers/net/hns3/hns3_dump.h

diff --git a/drivers/net/hns3/hns3_ethdev_dump.c b/drivers/net/hns3/hns3_dump.c
similarity index 99%
rename from drivers/net/hns3/hns3_ethdev_dump.c
rename to drivers/net/hns3/hns3_dump.c
index 1bb2ab7556..2cfab429af 100644
--- a/drivers/net/hns3/hns3_ethdev_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -6,7 +6,7 @@
 #include "hns3_logs.h"
 #include "hns3_regs.h"
 #include "hns3_rxtx.h"
-#include "hns3_ethdev.h"
+#include "hns3_dump.h"
 
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
new file mode 100644
index 0000000000..b0fe37ee21
--- /dev/null
+++ b/drivers/net/hns3/hns3_dump.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(C) 2022 HiSilicon Limited
+ */
+
+#ifndef _HNS3_DUMP_H_
+#define _HNS3_DUMP_H_
+
+#include <stdio.h>
+
+#include <ethdev_driver.h>
+
+int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+#endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 0b565a5614..6fa07c4c94 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -8,6 +8,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_intr.h"
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 5e8a746514..8de5a712f4 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -1062,7 +1062,6 @@ int hns3_timesync_read_time(struct rte_eth_dev *dev, struct timespec *ts);
 int hns3_timesync_write_time(struct rte_eth_dev *dev,
 			const struct timespec *ts);
 int hns3_timesync_adjust_time(struct rte_eth_dev *dev, int64_t delta);
-int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
 
 static inline bool
 is_reset_pending(struct hns3_adapter *hns)
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 589de0ab3a..5fc6515de9 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -10,6 +10,7 @@
 
 #include "hns3_ethdev.h"
 #include "hns3_common.h"
+#include "hns3_dump.h"
 #include "hns3_logs.h"
 #include "hns3_rxtx.h"
 #include "hns3_regs.h"
diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index f2aede94ed..dc99cb0209 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -30,7 +30,7 @@ sources = files(
         'hns3_tm.c',
         'hns3_ptp.c',
         'hns3_common.c',
-        'hns3_ethdev_dump.c',
+        'hns3_dump.c',
 )
 
 deps += ['hash']
-- 
2.33.0

[PATCH v3 3/4] net/hns3: support Rx/Tx bd dump

[Min Hu (Connor)]

This patch support query HW descriptor from hns3 device. HW descriptor
is also called BD(buffer description) which is shared memory between
software and hardware.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_dump.c      | 64 +++++++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h      |  4 ++
 drivers/net/hns3/hns3_ethdev.c    |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c |  2 +
 4 files changed, 72 insertions(+)

diff --git a/drivers/net/hns3/hns3_dump.c b/drivers/net/hns3/hns3_dump.c
index 2cfab429af..74f687f0d7 100644
--- a/drivers/net/hns3/hns3_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -8,6 +8,9 @@
 #include "hns3_rxtx.h"
 #include "hns3_dump.h"
 
+#define HNS3_BD_DW_NUM 8
+#define HNS3_BD_ADDRESS_LAST_DW 2
+
 static const char *
 get_adapter_state_name(enum hns3_adapter_state state)
 {
@@ -911,3 +914,64 @@ hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file)
 
 	return 0;
 }
+
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_rx_queue *rxq = dev->data->rx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= rxq->nb_rx_desc) {
+		hns3_err(hw, "Invalid Rx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)(&rxq->rx_ring[desc_id]);
+	fprintf(file, "Rx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+		fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
+
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_tx_queue *txq = dev->data->tx_queues[queue_id];
+	uint32_t *bd_data;
+	int i;
+
+	if (desc_id >= txq->nb_tx_desc) {
+		hns3_err(hw, "Invalid Tx BD id=%u\n", desc_id);
+		return -EINVAL;
+	}
+
+	bd_data = (uint32_t *)(&txq->tx_ring[desc_id]);
+	fprintf(file, "Tx queue id:%u BD id:%u\n", queue_id, desc_id);
+	for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+		/*
+		 * For the sake of security, first 8 bytes of BD which stands
+		 * for physical address of packet should not be shown.
+		 */
+		if (i < HNS3_BD_ADDRESS_LAST_DW) {
+			fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
+			continue;
+		}
+
+		fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i, *(bd_data + i));
+	}
+
+	return 0;
+}
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
index b0fe37ee21..3dcd9a0466 100644
--- a/drivers/net/hns3/hns3_dump.h
+++ b/drivers/net/hns3/hns3_dump.h
@@ -10,4 +10,8 @@
 #include <ethdev_driver.h>
 
 int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id);
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t desc_id);
 #endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 6fa07c4c94..ad5018f8a1 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6562,6 +6562,8 @@ static const struct eth_dev_ops hns3_eth_dev_ops = {
 	.timesync_read_time         = hns3_timesync_read_time,
 	.timesync_write_time        = hns3_timesync_write_time,
 	.eth_dev_priv_dump          = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump        = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump        = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3_reset_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 5fc6515de9..26173442b2 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2288,6 +2288,8 @@ static const struct eth_dev_ops hns3vf_eth_dev_ops = {
 	.dev_supported_ptypes_get = hns3_dev_supported_ptypes_get,
 	.tx_done_cleanup    = hns3_tx_done_cleanup,
 	.eth_dev_priv_dump  = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3vf_reset_ops = {
-- 
2.33.0

[PATCH v3 4/4] app/procinfo: support descriptor dump

[Min Hu (Connor)]

This patch support HW Rx/Tx descriptor dump

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

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

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

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 56070a3317..eab1b546d1 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)(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t desc_id);
+
 /**< mask of enabled ports */
 static unsigned long enabled_port_mask;
 /**< Enable stats. */
@@ -100,6 +103,12 @@ static char *mempool_iter_name;
 /**< Enable dump regs. */
 static uint32_t enable_dump_regs;
 static char *dump_regs_file_prefix;
+/**< Enable dump buffer descriptor. */
+#define MAX_NB_ITEM 2
+static uint16_t rx_nb_item;
+static uint16_t tx_nb_item;
+static uint16_t rx_item_opt[MAX_NB_ITEM];
+static uint16_t tx_item_opt[MAX_NB_ITEM];
 
 /**< display usage */
 static void
@@ -127,6 +136,8 @@ 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:descriptor_id: to display ports Rx buffer description by queue id and descriptor id\n"
+		"  --show-tx-descriptor queue_id:descriptor_id: to display ports Tx buffer description by queue id and descriptor id\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);
@@ -179,6 +190,34 @@ parse_xstats_ids(char *list, uint64_t *ids, int limit) {
 	return length;
 }
 
+/*
+ * Parse ids value list into array
+ */
+static int
+parse_descriptor_param(char *list, uint16_t *item_opt, int limit)
+{
+	int length;
+	char *token;
+	char *ctx = NULL;
+	char *endptr;
+
+	length = 0;
+	token = strtok_r(list, ":", &ctx);
+	while (token != NULL) {
+		item_opt[length] = strtoul(token, &endptr, 10);
+		if (*endptr != '\0')
+			return -EINVAL;
+
+		length++;
+		if (length > limit)
+			return -E2BIG;
+
+		token = strtok_r(NULL, ":", &ctx);
+	}
+
+	return length;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -238,6 +277,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}
 	};
 
@@ -327,6 +368,26 @@ 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_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Rx descriptor param parse error.\n");
+					return -1;
+				}
+				rx_nb_item = ret;
+			} else if (!strncmp(long_option[option_index].name,
+				"show-tx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+								 tx_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Tx descriptor param parse error.\n");
+					return -1;
+				}
+				tx_nb_item = ret;
 			}
 			break;
 		default:
@@ -1450,6 +1511,20 @@ dump_regs(char *file_prefix)
 	}
 }
 
+static void
+nic_descriptor_display(uint16_t port_id, uint16_t *item_opt,
+		       desc_dump_t desc_dump)
+{
+	static const char *nic_desc_border = "###";
+	uint16_t queue_id = item_opt[0];
+	uint16_t desc_id = item_opt[1];
+
+	printf("%s NIC descriptor for port %u %s\n",
+		   nic_desc_border, port_id, nic_desc_border);
+
+	desc_dump(stdout, port_id, queue_id, desc_id);
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1533,6 +1608,12 @@ main(int argc, char **argv)
 		else if (nb_xstats_ids > 0)
 			nic_xstats_by_ids_display(i, xstats_ids,
 						  nb_xstats_ids);
+		else if (rx_nb_item > 0)
+			nic_descriptor_display(i, rx_item_opt,
+					       rte_eth_rx_hw_desc_dump);
+		else if (tx_nb_item > 0)
+			nic_descriptor_display(i, tx_item_opt,
+					       rte_eth_tx_hw_desc_dump);
 #ifdef RTE_LIB_METRICS
 		else if (enable_metrics)
 			metrics_display(i);
-- 
2.33.0

Re: [PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Andrew Rybchenko]

Again, please, add maintainers using --cc-cmd or --to-cmd to patches.
Also, don't forget to keep in Cc participants of the discussion.

On 6/1/22 10:49, Min Hu (Connor) wrote:
> This patch set support HW Rx/Tx descriptor dump by using procinfo tool.
> 
> Min Hu (Connor) (4):
>    ethdev: introduce ethdev HW desc dump PI
>    net/hns3: rename hns3 dump files
>    net/hns3: support Rx/Tx bd dump
>    app/procinfo: support descriptor dump
> 
>   app/proc-info/main.c                          | 81 +++++++++++++++++++
>   doc/guides/rel_notes/release_22_07.rst        |  7 ++
>   .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
>   drivers/net/hns3/hns3_dump.h                  | 17 ++++
>   drivers/net/hns3/hns3_ethdev.c                |  3 +
>   drivers/net/hns3/hns3_ethdev.h                |  1 -
>   drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
>   drivers/net/hns3/meson.build                  |  2 +-
>   lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
>   lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
>   lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
>   lib/ethdev/version.map                        |  2 +
>   12 files changed, 309 insertions(+), 3 deletions(-)
>   rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
>   create mode 100644 drivers/net/hns3/hns3_dump.h
> ---
> v3:
> * fix desc id handling.
> 
> v2:
> * add 'const' for ethdev.

Re: [PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Ray Kinsella]

Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> writes:

> Again, please, add maintainers using --cc-cmd or --to-cmd to patches.
> Also, don't forget to keep in Cc participants of the discussion.

And you can add my Acked-by to 1/4.

> On 6/1/22 10:49, Min Hu (Connor) wrote:
>> This patch set support HW Rx/Tx descriptor dump by using procinfo tool.
>> Min Hu (Connor) (4):
>>    ethdev: introduce ethdev HW desc dump PI
>>    net/hns3: rename hns3 dump files
>>    net/hns3: support Rx/Tx bd dump
>>    app/procinfo: support descriptor dump
>>   app/proc-info/main.c                          | 81 +++++++++++++++++++
>>   doc/guides/rel_notes/release_22_07.rst        |  7 ++
>>   .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
>>   drivers/net/hns3/hns3_dump.h                  | 17 ++++
>>   drivers/net/hns3/hns3_ethdev.c                |  3 +
>>   drivers/net/hns3/hns3_ethdev.h                |  1 -
>>   drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
>>   drivers/net/hns3/meson.build                  |  2 +-
>>   lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
>>   lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
>>   lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
>>   lib/ethdev/version.map                        |  2 +
>>   12 files changed, 309 insertions(+), 3 deletions(-)
>>   rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
>>   create mode 100644 drivers/net/hns3/hns3_dump.h
>> ---
>> v3:
>> * fix desc id handling.
>> v2:
>> * add 'const' for ethdev.


-- 
Regards, Ray K

Re: [PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Dongdong Liu]

On 2022/6/2 2:48, Ray Kinsella wrote:
>
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> writes:
>
>> Again, please, add maintainers using --cc-cmd or --to-cmd to patches.
>> Also, don't forget to keep in Cc participants of the discussion.
>
> And you can add my Acked-by to 1/4.
will do,

Thanks,
Dongdong
>
>> On 6/1/22 10:49, Min Hu (Connor) wrote:
>>> This patch set support HW Rx/Tx descriptor dump by using procinfo tool.
>>> Min Hu (Connor) (4):
>>>    ethdev: introduce ethdev HW desc dump PI
>>>    net/hns3: rename hns3 dump files
>>>    net/hns3: support Rx/Tx bd dump
>>>    app/procinfo: support descriptor dump
>>>   app/proc-info/main.c                          | 81 +++++++++++++++++++
>>>   doc/guides/rel_notes/release_22_07.rst        |  7 ++
>>>   .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
>>>   drivers/net/hns3/hns3_dump.h                  | 17 ++++
>>>   drivers/net/hns3/hns3_ethdev.c                |  3 +
>>>   drivers/net/hns3/hns3_ethdev.h                |  1 -
>>>   drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
>>>   drivers/net/hns3/meson.build                  |  2 +-
>>>   lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
>>>   lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
>>>   lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
>>>   lib/ethdev/version.map                        |  2 +
>>>   12 files changed, 309 insertions(+), 3 deletions(-)
>>>   rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
>>>   create mode 100644 drivers/net/hns3/hns3_dump.h
>>> ---
>>> v3:
>>> * fix desc id handling.
>>> v2:
>>> * add 'const' for ethdev.
>
>

Re: [PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Andrew Rybchenko]

On 6/1/22 21:26, Andrew Rybchenko wrote:
> Again, please, add maintainers using --cc-cmd or --to-cmd to patches.
> Also, don't forget to keep in Cc participants of the discussion.
> 
> On 6/1/22 10:49, Min Hu (Connor) wrote:
>> This patch set support HW Rx/Tx descriptor dump by using procinfo tool.

One more thing. I'd remove HW from the description.
Format is undefined and driver can log any kind of information
related to particular descriptor.
Just highlight that the dupmp format is vendor-specific.

>>
>> Min Hu (Connor) (4):
>>    ethdev: introduce ethdev HW desc dump PI
>>    net/hns3: rename hns3 dump files
>>    net/hns3: support Rx/Tx bd dump
>>    app/procinfo: support descriptor dump
>>
>>   app/proc-info/main.c                          | 81 +++++++++++++++++++
>>   doc/guides/rel_notes/release_22_07.rst        |  7 ++
>>   .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
>>   drivers/net/hns3/hns3_dump.h                  | 17 ++++
>>   drivers/net/hns3/hns3_ethdev.c                |  3 +
>>   drivers/net/hns3/hns3_ethdev.h                |  1 -
>>   drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
>>   drivers/net/hns3/meson.build                  |  2 +-
>>   lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
>>   lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
>>   lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
>>   lib/ethdev/version.map                        |  2 +
>>   12 files changed, 309 insertions(+), 3 deletions(-)
>>   rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
>>   create mode 100644 drivers/net/hns3/hns3_dump.h
>> ---
>> v3:
>> * fix desc id handling.
>>
>> v2:
>> * add 'const' for ethdev.
> 

Re: [PATCH v3 0/4] support HW Rx/Tx descriptor dump

[Dongdong Liu]

Hi Andrew
On 2022/6/2 2:26, Andrew Rybchenko wrote:
> Again, please, add maintainers using --cc-cmd or --to-cmd to patches.
> Also, don't forget to keep in Cc participants of the discussion.

Thanks for remind this, will do.

Thanks,
Dongdong
>
> On 6/1/22 10:49, Min Hu (Connor) wrote:
>> This patch set support HW Rx/Tx descriptor dump by using procinfo tool.
>>
>> Min Hu (Connor) (4):
>>    ethdev: introduce ethdev HW desc dump PI
>>    net/hns3: rename hns3 dump files
>>    net/hns3: support Rx/Tx bd dump
>>    app/procinfo: support descriptor dump
>>
>>   app/proc-info/main.c                          | 81 +++++++++++++++++++
>>   doc/guides/rel_notes/release_22_07.rst        |  7 ++
>>   .../hns3/{hns3_ethdev_dump.c => hns3_dump.c}  | 66 ++++++++++++++-
>>   drivers/net/hns3/hns3_dump.h                  | 17 ++++
>>   drivers/net/hns3/hns3_ethdev.c                |  3 +
>>   drivers/net/hns3/hns3_ethdev.h                |  1 -
>>   drivers/net/hns3/hns3_ethdev_vf.c             |  3 +
>>   drivers/net/hns3/meson.build                  |  2 +-
>>   lib/ethdev/ethdev_driver.h                    | 42 ++++++++++
>>   lib/ethdev/rte_ethdev.c                       | 44 ++++++++++
>>   lib/ethdev/rte_ethdev.h                       | 44 ++++++++++
>>   lib/ethdev/version.map                        |  2 +
>>   12 files changed, 309 insertions(+), 3 deletions(-)
>>   rename drivers/net/hns3/{hns3_ethdev_dump.c => hns3_dump.c} (93%)
>>   create mode 100644 drivers/net/hns3/hns3_dump.h
>> ---
>> v3:
>> * fix desc id handling.
>>
>> v2:
>> * add 'const' for ethdev.
>
> .
>

[PATCH v4 0/3] support ethdev Rx/Tx descriptor dump

[Dongdong Liu]

Support ethdev Rx/Tx descriptor dump by using procinfo tool.

Note:
'[PATCH v4 1/3] net/hns3: support Rx/Tx bd dump' depend on [1] applied
first. Hope patchset [1] can be applied soon.

[1]: [PATCH RESEND 00/13] some bugfixes and clean code for hns3
https://patches.dpdk.org/project/dpdk/list/?series=24533

v3->v4:
1. Modify the desc dump API to dump specified number of descriptors.
2. Modify the hn3 pmd implementation and procinfo part.

Min Hu (Connor) (3):
  ethdev: introduce ethdev desc dump API
  net/hns3: support Rx/Tx bd dump
  app/procinfo: support descriptor dump

 app/proc-info/main.c                   | 80 ++++++++++++++++++++++++++
 doc/guides/rel_notes/release_22_11.rst |  7 +++
 drivers/net/hns3/hns3_dump.c           | 80 ++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h           |  5 ++
 drivers/net/hns3/hns3_ethdev.c         |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c      |  2 +
 lib/ethdev/ethdev_driver.h             | 46 +++++++++++++++
 lib/ethdev/rte_ethdev.c                | 52 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 49 ++++++++++++++++
 lib/ethdev/version.map                 |  2 +
 10 files changed, 325 insertions(+)

--
2.22.0

[PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Dongdong Liu]

From: "Min Hu (Connor)" <humin29@huawei.com>

Added the ethdev Rx/Tx desc dump API which provides functions for query
descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
 doc/guides/rel_notes/release_22_11.rst |  7 ++++
 lib/ethdev/ethdev_driver.h             | 46 +++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 52 ++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 49 ++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 +
 5 files changed, 156 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index f60161765b..d3f3f2e50c 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -55,6 +55,13 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added ethdev desc dump API, to dump Rx/Tx desc info from device.**
+
+Added the ethdev Rx/Tx desc dump API which provides functions for query
+descriptor from device. The descriptor info differs in different NICs.
+The information demonstrates I/O process which is important for debug.
+As the information is different between NICs, the new API is introduced.
+The dump format is vendor-specific.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index a0e0b2ae88..76808dae89 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1093,6 +1093,47 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
 					uint16_t *rx_queue_id,
 					uint8_t *avail_thresh);
 
+
+/**
+ * @internal
+ * Dump Rx descriptor info to a file.
+ *
+ * It is used for debugging, not a dataplane API.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param num
+ *   The number of the descriptors to dump.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t num);
+
+/**
+ * @internal
+ * Dump Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param num
+ *   The number of the descriptors to dump.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+				     uint16_t queue_id, uint16_t num);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1308,6 +1349,11 @@ struct eth_dev_ops {
 	eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
 	/** Query Rx queue available descriptors threshold event */
 	eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
+
+	/** Dump Rx descriptor info */
+	eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
+	/** Dump Tx descriptor info */
+	eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 1979dc0850..2093275d87 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5917,6 +5917,58 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
 	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
 }
 
+int
+rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t num)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id, num);
+
+	return ret;
+}
+
+int
+rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			uint16_t num)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id, num);
+
+	return ret;
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index b62ac5bb6f..4671e6b28e 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5221,6 +5221,55 @@ typedef struct {
 __rte_experimental
 int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param num
+ *   The number of the descriptors to dump.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t num);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   The selected queue.
+ * @param num
+ *   The number of the descriptors to dump.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+			    uint16_t num);
+
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 03f52fee91..3c7c75b582 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -285,6 +285,8 @@ EXPERIMENTAL {
 	rte_mtr_color_in_protocol_priority_get;
 	rte_mtr_color_in_protocol_set;
 	rte_mtr_meter_vlan_table_update;
+	rte_eth_rx_hw_desc_dump;
+	rte_eth_tx_hw_desc_dump;
 };
 
 INTERNAL {
-- 
2.22.0

Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Ferruh Yigit]

On 9/23/2022 8:43 AM, Dongdong Liu wrote:

> 
> From: "Min Hu (Connor)" <humin29@huawei.com>
> 
> Added the ethdev Rx/Tx desc dump API which provides functions for query
> descriptor from device. HW descriptor info differs in different NICs.
> The information demonstrates I/O process which is important for debug.
> As the information is different

Overall OK to have these new APIs, please find comments below.

Do you think does it worth to list this as one of the PMD future in 
future list, in 'doc/guides/nics/features.rst' ?

  between NICs, the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>

<...>

>   int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump ethdev Rx descriptor info to a file.
> + *
> + * This API is used for debugging, not a dataplane API.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param num
> + *   The number of the descriptors to dump.
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +__rte_experimental
> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +                           uint16_t num);

There are other HW desc related APIs, like 'rte_eth_rx_descriptor_status()'.
Should this APIs follow same naming convention:
'rte_eth_rx_descriptor_dump()'
'rte_eth_tx_descriptor_dump()'

> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump ethdev Tx descriptor info to a file.
> + *
> + * This API is used for debugging, not a dataplane API.
> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param queue_id
> + *   The selected queue.
> + * @param num
> + *   The number of the descriptors to dump.
> + * @return
> + *   - On success, zero.
> + *   - On failure, a negative value.
> + */
> +__rte_experimental
> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
> +                           uint16_t num);

'num' is provided, does it assume it starts from offset 0, what do you 
think to provide 'offset' as parameter?
It may be a use case to start from where tail/head pointer is.

> +
> +
>   #include <rte_ethdev_core.h>
> 
>   /**
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 03f52fee91..3c7c75b582 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>          rte_mtr_color_in_protocol_priority_get;
>          rte_mtr_color_in_protocol_set;
>          rte_mtr_meter_vlan_table_update;
> +       rte_eth_rx_hw_desc_dump;
> +       rte_eth_tx_hw_desc_dump;

These new APIs should go after "# added in 22.11" comment, if you rebase 
on top of latest HEAD, comment is already there.

Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Andrew Rybchenko]

On 10/4/22 01:40, Ferruh Yigit wrote:
> On 9/23/2022 8:43 AM, Dongdong Liu wrote:
> 
>>
>> From: "Min Hu (Connor)" <humin29@huawei.com>
>>
>> Added the ethdev Rx/Tx desc dump API which provides functions for query
>> descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different
> 
> Overall OK to have these new APIs, please find comments below.
> 
> Do you think does it worth to list this as one of the PMD future in 
> future list, in 'doc/guides/nics/features.rst' ?

IMHO it does not deserve entry in features.
It is a deep debugging using vendor-specific information.

> 
>   between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> 
> <...>
> 
>>   int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior 
>> notice
>> + *
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param num
>> + *   The number of the descriptors to dump.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t 
>> queue_id,
>> +                           uint16_t num);
> 
> There are other HW desc related APIs, like 
> 'rte_eth_rx_descriptor_status()'.
> Should this APIs follow same naming convention:
> 'rte_eth_rx_descriptor_dump()'
> 'rte_eth_tx_descriptor_dump()'

+1 on naming, it should not be bound to HW. SW parts could be
interesting as well.

> 
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior 
>> notice
>> + *
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param num
>> + *   The number of the descriptors to dump.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t 
>> queue_id,
>> +                           uint16_t num);
> 
> 'num' is provided, does it assume it starts from offset 0, what do you 
> think to provide 'offset' as parameter?
> It may be a use case to start from where tail/head pointer is.
> 
>> +
>> +
>>   #include <rte_ethdev_core.h>
>>
>>   /**
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 03f52fee91..3c7c75b582 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>>          rte_mtr_color_in_protocol_priority_get;
>>          rte_mtr_color_in_protocol_set;
>>          rte_mtr_meter_vlan_table_update;
>> +       rte_eth_rx_hw_desc_dump;
>> +       rte_eth_tx_hw_desc_dump;
> 
> These new APIs should go after "# added in 22.11" comment, if you rebase 
> on top of latest HEAD, comment is already there.
> 

Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Dongdong Liu]

Hi Ferruh

Many thanks for your review.
On 2022/10/4 6:40, Ferruh Yigit wrote:
> On 9/23/2022 8:43 AM, Dongdong Liu wrote:
>
>>
>> From: "Min Hu (Connor)" <humin29@huawei.com>
>>
>> Added the ethdev Rx/Tx desc dump API which provides functions for query
>> descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different
>
> Overall OK to have these new APIs, please find comments below.
>
> Do you think does it worth to list this as one of the PMD future in
> future list, in 'doc/guides/nics/features.rst' ?
As Andrew said
It does not deserve entry in features.
It is a deep debugging using vendor-specific information.

I agree with him.
>
>  between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>
> <...>
>
>>   int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param num
>> + *   The number of the descriptors to dump.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> +                           uint16_t num);
>
> There are other HW desc related APIs, like
> 'rte_eth_rx_descriptor_status()'.
> Should this APIs follow same naming convention:
> 'rte_eth_rx_descriptor_dump()'
> 'rte_eth_tx_descriptor_dump()'
Looks good, will do.
>
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param queue_id
>> + *   The selected queue.
>> + * @param num
>> + *   The number of the descriptors to dump.
>> + * @return
>> + *   - On success, zero.
>> + *   - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> +                           uint16_t num);
>
> 'num' is provided, does it assume it starts from offset 0, what do you
> think to provide 'offset' as parameter?
> It may be a use case to start from where tail/head pointer is.
It will be ok to provide 'offset' as parameter. will change as below.

/**
  * @warning
  * @b EXPERIMENTAL: this API may change, or be removed, without prior 
notice
  *
  * Dump ethdev Rx descriptor info to a file. 
 
 

  *
  * This API is used for debugging, not a dataplane API.
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param queue_id
  *   A Rx queue identifier on this port.
  * @param offset
  *  The offset of the descriptor starting from tail. (0 is the next
  *  packet to be received by the driver).
  * @param num
  *   The number of the descriptors to dump.
  * @param file
  *   A pointer to a file for output.
  * @return
  *   - On success, zero.
  *   - On failure, a negative value.
  */
__rte_experimental
int rte_eth_rx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
                                uint16_t offset, uint16_t num, FILE *file);
>
>> +
>> +
>>   #include <rte_ethdev_core.h>
>>
>>   /**
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 03f52fee91..3c7c75b582 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>>          rte_mtr_color_in_protocol_priority_get;
>>          rte_mtr_color_in_protocol_set;
>>          rte_mtr_meter_vlan_table_update;
>> +       rte_eth_rx_hw_desc_dump;
>> +       rte_eth_tx_hw_desc_dump;
>
> These new APIs should go after "# added in 22.11" comment, if you rebase
> on top of latest HEAD, comment is already there.
Will rebase on top of latest HEAD.

Thanks,
Dongdong
>
> .
>

Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Stephen Hemminger]

On Thu, 6 Oct 2022 16:26:24 +0800
Dongdong Liu <liudongdong3@huawei.com> wrote:

> > Do you think does it worth to list this as one of the PMD future in
> > future list, in 'doc/guides/nics/features.rst' ?  
> As Andrew said
> It does not deserve entry in features.
> It is a deep debugging using vendor-specific information.
> 
> I agree with him.

This should never be a stable API either. Format may change.

Re: [PATCH v4 1/3] ethdev: introduce ethdev desc dump API

[Ferruh Yigit]

On 10/6/2022 6:13 PM, Stephen Hemminger wrote:
> On Thu, 6 Oct 2022 16:26:24 +0800
> Dongdong Liu <liudongdong3@huawei.com> wrote:
> 
>>> Do you think does it worth to list this as one of the PMD future in
>>> future list, in 'doc/guides/nics/features.rst' ?
>> As Andrew said
>> It does not deserve entry in features.
>> It is a deep debugging using vendor-specific information.
>>
>> I agree with him.
> 
> This should never be a stable API either. Format may change.

Format is not defined in the API, it is vendor specific. But the API 
itself can be stable I think, after we have more PMDs implement it.

[PATCH v4 2/3] net/hns3: support Rx/Tx bd dump

[Dongdong Liu]

From: "Min Hu (Connor)" <humin29@huawei.com>

This patch support query HW descriptor from hns3 device. HW descriptor
is also called BD(buffer description) which is shared memory between
software and hardware.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/net/hns3/hns3_dump.c      | 80 +++++++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h      |  5 ++
 drivers/net/hns3/hns3_ethdev.c    |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c |  2 +
 4 files changed, 89 insertions(+)

diff --git a/drivers/net/hns3/hns3_dump.c b/drivers/net/hns3/hns3_dump.c
index 95b64f8896..ce26e5b65f 100644
--- a/drivers/net/hns3/hns3_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -10,6 +10,9 @@
 #include "hns3_rxtx.h"
 #include "hns3_dump.h"
 
+#define HNS3_BD_DW_NUM 8
+#define HNS3_BD_ADDRESS_LAST_DW 2
+
 static const char *
 hns3_get_adapter_state_name(enum hns3_adapter_state state)
 {
@@ -931,3 +934,80 @@ hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file)
 
 	return 0;
 }
+
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t num)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_rx_queue *rxq = dev->data->rx_queues[queue_id];
+	uint32_t *bd_data;
+	uint16_t count = 0;
+	uint16_t desc_id;
+	int i;
+
+	if (num > rxq->nb_rx_desc) {
+		hns3_err(hw, "Invalid BD num=%u\n", num);
+		return -EINVAL;
+	}
+
+	while (count < num) {
+		desc_id = (rxq->next_to_use + count) % rxq->nb_rx_desc;
+		bd_data = (uint32_t *)(&rxq->rx_ring[desc_id]);
+		fprintf(file, "Rx queue id:%u BD id:%u\n", queue_id, desc_id);
+		for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+			/*
+			 * For the sake of security, first 8 bytes of BD which
+			 * stands for physical address of packet should not be
+			 * shown.
+			 */
+			if (i < HNS3_BD_ADDRESS_LAST_DW) {
+				fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, 0);
+				continue;
+			}
+			fprintf(file, "RX BD WORD[%d]:0x%08x\n", i,
+				*(bd_data + i));
+		}
+		count++;
+	}
+
+	return 0;
+}
+
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t num)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_tx_queue *txq = dev->data->tx_queues[queue_id];
+	uint32_t *bd_data;
+	uint16_t count = 0;
+	uint16_t desc_id;
+	int i;
+
+	if (num > txq->nb_tx_desc) {
+		hns3_err(hw, "Invalid BD num=%u\n", num);
+		return -EINVAL;
+	}
+
+	while (count < num) {
+		desc_id = (txq->next_to_use + count) % txq->nb_tx_desc;
+		bd_data = (uint32_t *)(&txq->tx_ring[desc_id]);
+		fprintf(file, "Tx queue id:%u BD id:%u\n", queue_id, desc_id);
+		for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+			/*
+			 * For the sake of security, first 8 bytes of BD which
+			 * stands for physical address of packet should not be
+			 * shown.
+			 */
+			if (i < HNS3_BD_ADDRESS_LAST_DW) {
+				fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
+				continue;
+			}
+
+			fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i,
+				*(bd_data + i));
+		}
+		count++;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
index b0fe37ee21..f92d8af880 100644
--- a/drivers/net/hns3/hns3_dump.h
+++ b/drivers/net/hns3/hns3_dump.h
@@ -10,4 +10,9 @@
 #include <ethdev_driver.h>
 
 int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+
+int hns3_rx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t num);
+int hns3_tx_hw_desc_dump(FILE *file, const struct rte_eth_dev *dev,
+			 uint16_t queue_id, uint16_t num);
 #endif /* _HNS3_DUMP_H_ */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 7cd1ff53e8..43a604c0fd 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6567,6 +6567,8 @@ static const struct eth_dev_ops hns3_eth_dev_ops = {
 	.timesync_read_time         = hns3_timesync_read_time,
 	.timesync_write_time        = hns3_timesync_write_time,
 	.eth_dev_priv_dump          = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump        = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump        = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3_reset_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index a72535eb7d..b1997c42f4 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2308,6 +2308,8 @@ static const struct eth_dev_ops hns3vf_eth_dev_ops = {
 	.dev_supported_ptypes_get = hns3_dev_supported_ptypes_get,
 	.tx_done_cleanup    = hns3_tx_done_cleanup,
 	.eth_dev_priv_dump  = hns3_eth_dev_priv_dump,
+	.eth_rx_hw_desc_dump = hns3_rx_hw_desc_dump,
+	.eth_tx_hw_desc_dump = hns3_tx_hw_desc_dump,
 };
 
 static const struct hns3_reset_ops hns3vf_reset_ops = {
-- 
2.22.0

[PATCH v4 3/3] app/procinfo: support descriptor dump

[Dongdong Liu]

From: "Min Hu (Connor)" <humin29@huawei.com>

This patch support HW Rx/Tx descriptor dump

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

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

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(+)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 1bfba5f60d..3907af3213 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)(FILE *file, uint16_t port_id, uint16_t queue_id,
+			   uint16_t num);
+
 /**< mask of enabled ports */
 static unsigned long enabled_port_mask;
 /**< Enable stats. */
@@ -102,6 +105,12 @@ static char *mempool_iter_name;
 /**< Enable dump regs. */
 static uint32_t enable_dump_regs;
 static char *dump_regs_file_prefix;
+/* Enable dump buffer descriptor. */
+#define MAX_NB_ITEM 2
+static uint16_t rx_nb_item;
+static uint16_t tx_nb_item;
+static uint16_t rx_item_opt[MAX_NB_ITEM];
+static uint16_t tx_item_opt[MAX_NB_ITEM];
 
 /**< display usage */
 static void
@@ -130,6 +139,8 @@ 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:num: to display ports Rx buffer description by queue id and num\n"
+		"  --show-tx-descriptor queue_id:num: to display ports Tx buffer description by queue id and num\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 +193,34 @@ parse_xstats_ids(char *list, uint64_t *ids, int limit) {
 	return length;
 }
 
+/*
+ * Parse ids value list into array
+ */
+static int
+parse_descriptor_param(char *list, uint16_t *item_opt, int limit)
+{
+	int length;
+	char *token;
+	char *ctx = NULL;
+	char *endptr;
+
+	length = 0;
+	token = strtok_r(list, ":", &ctx);
+	while (token != NULL) {
+		item_opt[length] = strtoul(token, &endptr, 10);
+		if (*endptr != '\0')
+			return -EINVAL;
+
+		length++;
+		if (length > limit)
+			return -E2BIG;
+
+		token = strtok_r(NULL, ":", &ctx);
+	}
+
+	return length;
+}
+
 static int
 proc_info_preparse_args(int argc, char **argv)
 {
@@ -242,6 +281,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 +375,26 @@ 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_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Rx descriptor param parse error.\n");
+					return -1;
+				}
+				rx_nb_item = ret;
+			} else if (!strncmp(long_option[option_index].name,
+				"show-tx-descriptor", MAX_LONG_OPT_SZ)) {
+				int ret = parse_descriptor_param(optarg,
+								 tx_item_opt,
+								 MAX_NB_ITEM);
+				if (ret < MAX_NB_ITEM) {
+					printf("Tx descriptor param parse error.\n");
+					return -1;
+				}
+				tx_nb_item = ret;
 			}
 			break;
 		default:
@@ -1476,6 +1537,19 @@ dump_regs(char *file_prefix)
 	}
 }
 
+static void
+nic_descriptor_display(uint16_t port_id, uint16_t *item_opt,
+		       desc_dump_t desc_dump)
+{
+	static const char *nic_desc_border = "###";
+	uint16_t queue_id = item_opt[0];
+	uint16_t num = item_opt[1];
+
+	printf("%s NIC descriptor for port %u %s\n",
+		   nic_desc_border, port_id, nic_desc_border);
+
+	desc_dump(stdout, port_id, queue_id, num);
+}
 int
 main(int argc, char **argv)
 {
@@ -1559,6 +1633,12 @@ main(int argc, char **argv)
 		else if (nb_xstats_ids > 0)
 			nic_xstats_by_ids_display(i, xstats_ids,
 						  nb_xstats_ids);
+		else if (rx_nb_item > 0)
+			nic_descriptor_display(i, rx_item_opt,
+					       rte_eth_rx_hw_desc_dump);
+		else if (tx_nb_item > 0)
+			nic_descriptor_display(i, tx_item_opt,
+					       rte_eth_tx_hw_desc_dump);
 #ifdef RTE_LIB_METRICS
 		else if (enable_metrics)
 			metrics_display(i);
-- 
2.22.0

RE: [PATCH v4 3/3] app/procinfo: support descriptor dump

[Pattan, Reshma]

> -----Original Message-----
> From: Dongdong Liu <liudongdong3@huawei.com>
> Subject: [PATCH v4 3/3] app/procinfo: support descriptor dump
> 
> From: "Min Hu (Connor)" <humin29@huawei.com>
> 
> This patch support HW Rx/Tx descriptor dump
> 
> The command is like:
> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-rx-descriptor
> queue_id:num


What is num here?  You need to describe about this in commit message.
Is num means number of descriptors? Better use the descriptor word here in num

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

Same here.

> +/* Enable dump buffer descriptor. */
> +#define MAX_NB_ITEM 2
> +static uint16_t rx_nb_item;
> +static uint16_t tx_nb_item;
> +static uint16_t rx_item_opt[MAX_NB_ITEM]; static uint16_t


Instead of using array to keep queid and number of descriptors info , better have structure with 2 parameters one for queue and one for the number of descriptors.
And You can use  the same structure to declare tx_item.

> +tx_item_opt[MAX_NB_ITEM];
> 

> +		"  --show-rx-descriptor queue_id:num: to display ports Rx
> buffer description by queue id and num\n"
> +		"  --show-tx-descriptor queue_id:num: to display ports Tx
> buffer description by queue id and num\n"

Here also use enough description what is num means or use the another name.

>+				if (ret < MAX_NB_ITEM) {

You can check ret < 0 no need to use MAX_NB_ITEM again.
>+					printf("Rx descriptor param parse error.\n");
>+					return -1;
>+				}

Instead of saying parse error. You might need to give clear reason of failure.
That is invalid queueid:num passed by user or user passed more than one pair.

>+				int ret = parse_descriptor_param(optarg,
>+								 rx_item_opt,
>+								 MAX_NB_ITEM);

Do we need to really pass the MAX_NB_ITEM> I don't think we need you can directly use the macro in the called function.


>+				if (ret < MAX_NB_ITEM) {

You can check ret < 0 no need to use MAX_NB_ITEM again.

>+					printf("Tx descriptor param parse error.\n");
>+					return -1;
>+				}

Same here.

Thanks,
Reshma

Re: [PATCH v4 3/3] app/procinfo: support descriptor dump

[Dongdong Liu]

Hi Reshma

Many thanks for your view.
On 2022/9/23 20:18, Pattan, Reshma wrote:
>
>
>> -----Original Message-----
>> From: Dongdong Liu <liudongdong3@huawei.com>
>> Subject: [PATCH v4 3/3] app/procinfo: support descriptor dump
>>
>> From: "Min Hu (Connor)" <humin29@huawei.com>
>>
>> This patch support HW Rx/Tx descriptor dump
>>
>> The command is like:
>> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-rx-descriptor
>> queue_id:num
>
>
> What is num here?  You need to describe about this in commit message.
> Is num means number of descriptors? Better use the descriptor word here in num

Yes, It means number of descriptors. How about rename it to desc_num.
>
>>
>> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-tx-descriptor
>> queue_id:num
>
> Same here.
Will fix.
>
>> +/* Enable dump buffer descriptor. */
>> +#define MAX_NB_ITEM 2
>> +static uint16_t rx_nb_item;
>> +static uint16_t tx_nb_item;
>> +static uint16_t rx_item_opt[MAX_NB_ITEM]; static uint16_t
>
>
> Instead of using array to keep queid and number of descriptors info , better have structure with 2 parameters one for queue and one for the number of descriptors.
> And You can use  the same structure to declare tx_item.
Good point, will do.
>
>> +tx_item_opt[MAX_NB_ITEM];
>>
>
>> +		"  --show-rx-descriptor queue_id:num: to display ports Rx
>> buffer description by queue id and num\n"
>> +		"  --show-tx-descriptor queue_id:num: to display ports Tx
>> buffer description by queue id and num\n"
>
> Here also use enough description what is num means or use the another name.
Will do.
>
>> +				if (ret < MAX_NB_ITEM) {
>
> You can check ret < 0 no need to use MAX_NB_ITEM again.
>> +					printf("Rx descriptor param parse error.\n");
>> +					return -1;
>> +				}
>
> Instead of saying parse error. You might need to give clear reason of failure.
> That is invalid queueid:num passed by user or user passed more than one pair.
Will do.
>
>> +				int ret = parse_descriptor_param(optarg,
>> +								 rx_item_opt,
>> +								 MAX_NB_ITEM);
>
> Do we need to really pass the MAX_NB_ITEM> I don't think we need you can directly use the macro in the called function.
>
>
>> +				if (ret < MAX_NB_ITEM) {
>
> You can check ret < 0 no need to use MAX_NB_ITEM again.
Will fix.
>
>> +					printf("Tx descriptor param parse error.\n");
>> +					return -1;
>> +				}
>
> Same here.
Will do.

Thanks,
Dongdong
>
> Thanks,
> Reshma
>
>
> .
>

[PATCH v5 0/3] support ethdev Rx/Tx descriptor dump

[Dongdong Liu]

Support ethdev Rx/Tx descriptor dump by using procinfo tool.

Thanks to Ferruh, Andrew and Reshma help to review the patchset.

NOTE:
October 1st to October 7th is China's National Day holiday.
I don't have a test environment available at the moment.
I will test this patchset on October 8th.  Current compile is ok.

v4->v5:
- Rename the Rx/Tx descriptor dump API and provide 'offset' parameter.
- Refactor procinfo dump descriptor code as Reshma suggested.

v3->v4:
- Modify the desc dump API to dump specified number of descriptors.
- Modify the hn3 pmd implementation and procinfo part

Dongdong Liu (3):
  ethdev: introduce ethdev desc dump API
  net/hns3: support Rx/Tx bd dump
  app/procinfo: support descriptor dump

 app/proc-info/main.c                   | 80 +++++++++++++++++++++++
 doc/guides/rel_notes/release_22_11.rst |  6 ++
 drivers/net/hns3/hns3_dump.c           | 88 ++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h           |  5 ++
 drivers/net/hns3/hns3_ethdev.c         |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c      |  2 +
 lib/ethdev/ethdev_driver.h             | 53 ++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 52 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 55 ++++++++++++++++
 lib/ethdev/version.map                 |  2 +
 10 files changed, 345 insertions(+)

--
2.22.0

[PATCH v5 1/3] ethdev: introduce ethdev desc dump API

[Dongdong Liu]

Added the ethdev Rx/Tx desc dump API which provides functions for query
descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 doc/guides/rel_notes/release_22_11.rst |  6 +++
 lib/ethdev/ethdev_driver.h             | 53 +++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 52 ++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 55 ++++++++++++++++++++++++++
 lib/ethdev/version.map                 |  2 +
 5 files changed, 168 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index f1cc129933..453562fe69 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -144,6 +144,12 @@ New Features
   into single event containing ``rte_event_vector``
   whose event type is ``RTE_EVENT_TYPE_CRYPTODEV_VECTOR``.
 
+* **Added ethdev desc dump API, to dump Rx/Tx desc info from device.**
+  Added the ethdev Rx/Tx desc dump API which provides functions for query
+  descriptor from device. The descriptor info differs in different NICs.
+  The information demonstrates I/O process which is important for debug.
+  As the information is different between NICs, the new API is introduced.
+  The dump format is vendor-specific.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index ea93f0418a..6505772a24 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -1094,6 +1094,54 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
 					uint16_t *rx_queue_id,
 					uint8_t *avail_thresh);
 
+/**
+ * @internal
+ * Dump Rx descriptor info to a file.
+ *
+ * It is used for debugging, not a dataplane API.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   A Rx queue identifier on this port.
+ * @param offset
+ *   The offset of the descriptor starting from tail. (0 is the next
+ *   packet to be received by the driver).
+ * @param num
+ *   The number of the descriptors to dump.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_descriptor_dump_t)(const struct rte_eth_dev *dev,
+					uint16_t queue_id, uint16_t offset,
+					uint16_t num, FILE *file);
+
+/**
+ * @internal
+ * Dump Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param queue_id
+ *   A Tx queue identifier on this port.
+ * @param offset
+ *   The offset of the descriptor starting from tail. (0 is the place where
+ *   the next packet will be send).
+ * @param num
+ *   The number of the descriptors to dump.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_descriptor_dump_t)(const struct rte_eth_dev *dev,
+					uint16_t queue_id, uint16_t offset,
+					uint16_t num, FILE *file);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1309,6 +1357,11 @@ struct eth_dev_ops {
 	eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
 	/** Query Rx queue available descriptors threshold event */
 	eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
+
+	/** Dump Rx descriptor info */
+	eth_rx_descriptor_dump_t eth_rx_descriptor_dump;
+	/** Dump Tx descriptor info */
+	eth_tx_descriptor_dump_t eth_tx_descriptor_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 01c141a039..c34d943631 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6044,6 +6044,58 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
 	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
 }
 
+int
+rte_eth_rx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
+			   uint16_t offset, uint16_t num, FILE *file)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_rx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	if (*dev->dev_ops->eth_rx_descriptor_dump == NULL)
+		return -ENOTSUP;
+
+	return eth_err(port_id, (*dev->dev_ops->eth_rx_descriptor_dump)(dev,
+						queue_id, offset, num, file));
+}
+
+int
+rte_eth_tx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
+			   uint16_t offset, uint16_t num, FILE *file)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (queue_id >= dev->data->nb_tx_queues) {
+		RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+		return -EINVAL;
+	}
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	if (*dev->dev_ops->eth_tx_descriptor_dump == NULL)
+		return -ENOTSUP;
+
+	return eth_err(port_id, (*dev->dev_ops->eth_tx_descriptor_dump)(dev,
+						queue_id, offset, num, file));
+}
+
 RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
 RTE_INIT(ethdev_init_telemetry)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index e0158480dd..7e6571b25a 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5312,6 +5312,61 @@ typedef struct {
 __rte_experimental
 int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *   A Rx queue identifier on this port.
+ * @param offset
+ *  The offset of the descriptor starting from tail. (0 is the next
+ *  packet to be received by the driver).
+ * @param num
+ *   The number of the descriptors to dump.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
+			       uint16_t offset, uint16_t num, FILE *file);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *   A Tx queue identifier on this port.
+ * @param offset
+ *  The offset of the descriptor starting from tail. (0 is the place where
+ *  the next packet will be send).
+ * @param num
+ *   The number of the descriptors to dump.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
+			       uint16_t offset, uint16_t num, FILE *file);
+
+
 #include <rte_ethdev_core.h>
 
 /**
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 3651ceb234..375e30adeb 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -290,6 +290,8 @@ EXPERIMENTAL {
 	rte_flow_async_action_handle_query;
 	rte_mtr_meter_policy_get;
 	rte_mtr_meter_profile_get;
+	rte_eth_rx_descriptor_dump;
+	rte_eth_tx_descriptor_dump;
 };
 
 INTERNAL {
-- 
2.22.0

Re: [PATCH v5 1/3] ethdev: introduce ethdev desc dump API

[Ferruh Yigit]

On 10/6/2022 1:05 PM, Dongdong Liu wrote:
> Added the ethdev Rx/Tx desc dump API which provides functions for query
> descriptor from device. HW descriptor info differs in different NICs.
> The information demonstrates I/O process which is important for debug.
> As the information is different between NICs, the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@amd.com>

[PATCH v5 2/3] net/hns3: support Rx/Tx bd dump

[Dongdong Liu]

This patch support query HW descriptor from hns3 device. HW descriptor
is also called BD(buffer description) which is shared memory between
software and hardware.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/net/hns3/hns3_dump.c      | 88 +++++++++++++++++++++++++++++++
 drivers/net/hns3/hns3_dump.h      |  5 ++
 drivers/net/hns3/hns3_ethdev.c    |  2 +
 drivers/net/hns3/hns3_ethdev_vf.c |  2 +
 4 files changed, 97 insertions(+)

diff --git a/drivers/net/hns3/hns3_dump.c b/drivers/net/hns3/hns3_dump.c
index 95b64f8896..ae62bb56c8 100644
--- a/drivers/net/hns3/hns3_dump.c
+++ b/drivers/net/hns3/hns3_dump.c
@@ -10,6 +10,9 @@
 #include "hns3_rxtx.h"
 #include "hns3_dump.h"
 
+#define HNS3_BD_DW_NUM 8
+#define HNS3_BD_ADDRESS_LAST_DW 2
+
 static const char *
 hns3_get_adapter_state_name(enum hns3_adapter_state state)
 {
@@ -931,3 +934,88 @@ hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file)
 
 	return 0;
 }
+
+int
+hns3_rx_descriptor_dump(const struct rte_eth_dev *dev, uint16_t queue_id,
+			uint16_t offset, uint16_t num, FILE *file)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_rx_queue *rxq = dev->data->rx_queues[queue_id];
+	uint32_t *bd_data;
+	uint16_t count = 0;
+	uint16_t desc_id;
+	int i;
+
+	if (offset >= rxq->nb_rx_desc)
+		return -EINVAL;
+
+	if (num > rxq->nb_rx_desc) {
+		hns3_err(hw, "Invalid BD num=%u\n", num);
+		return -EINVAL;
+	}
+
+	while (count < num) {
+		desc_id = (rxq->next_to_use + offset + count) % rxq->nb_rx_desc;
+		bd_data = (uint32_t *)(&rxq->rx_ring[desc_id]);
+		fprintf(file, "Rx queue id:%u BD id:%u\n", queue_id, desc_id);
+		for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+			/*
+			 * For the sake of security, first 8 bytes of BD which
+			 * stands for physical address of packet should not be
+			 * shown.
+			 */
+			if (i < HNS3_BD_ADDRESS_LAST_DW) {
+				fprintf(file, "RX BD WORD[%d]:0x%08x\n", i, 0);
+				continue;
+			}
+			fprintf(file, "RX BD WORD[%d]:0x%08x\n", i,
+				*(bd_data + i));
+		}
+		count++;
+	}
+
+	return 0;
+}
+
+int
+hns3_tx_descriptor_dump(const struct rte_eth_dev *dev, uint16_t queue_id,
+			uint16_t offset, uint16_t num, FILE *file)
+{
+	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct hns3_tx_queue *txq = dev->data->tx_queues[queue_id];
+	uint32_t *bd_data;
+	uint16_t count = 0;
+	uint16_t desc_id;
+	int i;
+
+	if (offset >= txq->nb_tx_desc)
+		return -EINVAL;
+
+	if (num > txq->nb_tx_desc) {
+		hns3_err(hw, "Invalid BD num=%u\n", num);
+		return -EINVAL;
+	}
+
+	while (count < num) {
+		desc_id = (txq->next_to_use + offset + count) % txq->nb_tx_desc;
+		bd_data = (uint32_t *)(&txq->tx_ring[desc_id]);
+		fprintf(file, "Tx queue id:%u BD id:%u\n", queue_id, desc_id);
+		for (i = 0; i < HNS3_BD_DW_NUM; i++) {
+			/*
+			 * For the sake of security, first 8 bytes of BD which
+			 * stands for physical address of packet should not be
+			 * shown.
+			 */
+			if (i < HNS3_BD_ADDRESS_LAST_DW) {
+				fprintf(file, "TX BD WORD[%d]:0x%08x\n", i, 0);
+				continue;
+			}
+
+			fprintf(file, "Tx BD WORD[%d]:0x%08x\n", i,
+				*(bd_data + i));
+		}
+		count++;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/hns3/hns3_dump.h b/drivers/net/hns3/hns3_dump.h
index 43e983a42f..54f6c67e63 100644
--- a/drivers/net/hns3/hns3_dump.h
+++ b/drivers/net/hns3/hns3_dump.h
@@ -10,4 +10,9 @@
 #include <ethdev_driver.h>
 
 int hns3_eth_dev_priv_dump(struct rte_eth_dev *dev, FILE *file);
+
+int hns3_rx_descriptor_dump(const struct rte_eth_dev *dev, uint16_t queue_id,
+			    uint16_t offset, uint16_t num, FILE *file);
+int hns3_tx_descriptor_dump(const struct rte_eth_dev *dev, uint16_t queue_id,
+			    uint16_t offset, uint16_t num, FILE *file);
 #endif /* HNS3_DUMP_H */
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 813fcedc6a..b4365b78be 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -6558,6 +6558,8 @@ static const struct eth_dev_ops hns3_eth_dev_ops = {
 	.timesync_read_time         = hns3_timesync_read_time,
 	.timesync_write_time        = hns3_timesync_write_time,
 	.eth_dev_priv_dump          = hns3_eth_dev_priv_dump,
+	.eth_rx_descriptor_dump     = hns3_rx_descriptor_dump,
+	.eth_tx_descriptor_dump     = hns3_tx_descriptor_dump,
 };
 
 static const struct hns3_reset_ops hns3_reset_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index c1bbcf42b1..d220522c43 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2302,6 +2302,8 @@ static const struct eth_dev_ops hns3vf_eth_dev_ops = {
 	.dev_supported_ptypes_get = hns3_dev_supported_ptypes_get,
 	.tx_done_cleanup    = hns3_tx_done_cleanup,
 	.eth_dev_priv_dump  = hns3_eth_dev_priv_dump,
+	.eth_rx_descriptor_dump = hns3_rx_descriptor_dump,
+	.eth_tx_descriptor_dump = hns3_tx_descriptor_dump,
 };
 
 static const struct hns3_reset_ops hns3vf_reset_ops = {
-- 
2.22.0

Re: [PATCH v5 2/3] net/hns3: support Rx/Tx bd dump

[Ferruh Yigit]

On 10/6/2022 1:05 PM, Dongdong Liu wrote:
> This patch support query HW descriptor from hns3 device. HW descriptor
> is also called BD(buffer description) which is shared memory between
> software and hardware.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

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

[Dongdong Liu]

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(+)

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
-- 
2.22.0

RE: [PATCH v5 3/3] app/procinfo: support descriptor dump

[Pattan, Reshma]

> -----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.

Re: [PATCH v5 3/3] app/procinfo: support descriptor dump

[Dongdong Liu]

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.
>
> .
>

Re: [PATCH v5 0/3] support ethdev Rx/Tx descriptor dump

[Ferruh Yigit]

On 10/6/2022 1:05 PM, Dongdong Liu wrote:
> Support ethdev Rx/Tx descriptor dump by using procinfo tool.
> 
> Thanks to Ferruh, Andrew and Reshma help to review the patchset.
> 
> NOTE:
> October 1st to October 7th is China's National Day holiday.
> I don't have a test environment available at the moment.
> I will test this patchset on October 8th.  Current compile is ok.
> 
> v4->v5:
> - Rename the Rx/Tx descriptor dump API and provide 'offset' parameter.
> - Refactor procinfo dump descriptor code as Reshma suggested.
> 
> v3->v4:
> - Modify the desc dump API to dump specified number of descriptors.
> - Modify the hn3 pmd implementation and procinfo part
> 
> Dongdong Liu (3):
>    ethdev: introduce ethdev desc dump API
>    net/hns3: support Rx/Tx bd dump
>    app/procinfo: support descriptor dump
> 

I will wait review from Reshma for procinfo, but will merge rest to make 
them available for -rc1, procinfo can get after -rc1,

Except from procinfo patch, 3/3,
Series applied to dpdk-next-net/main, thanks.


Release notes and .map file order updated while merging.

Re: [PATCH v5 0/3] support ethdev Rx/Tx descriptor dump

[Dongdong Liu]

On 2022/10/7 0:42, Ferruh Yigit wrote:
> On 10/6/2022 1:05 PM, Dongdong Liu wrote:
>> Support ethdev Rx/Tx descriptor dump by using procinfo tool.
>>
>> Thanks to Ferruh, Andrew and Reshma help to review the patchset.
>>
>> NOTE:
>> October 1st to October 7th is China's National Day holiday.
>> I don't have a test environment available at the moment.
>> I will test this patchset on October 8th.  Current compile is ok.
I have tested the patchset and it works ok.
>>
>> v4->v5:
>> - Rename the Rx/Tx descriptor dump API and provide 'offset' parameter.
>> - Refactor procinfo dump descriptor code as Reshma suggested.
>>
>> v3->v4:
>> - Modify the desc dump API to dump specified number of descriptors.
>> - Modify the hn3 pmd implementation and procinfo part
>>
>> Dongdong Liu (3):
>>    ethdev: introduce ethdev desc dump API
>>    net/hns3: support Rx/Tx bd dump
>>    app/procinfo: support descriptor dump
>>
>
> I will wait review from Reshma for procinfo, but will merge rest to make
> them available for -rc1, procinfo can get after -rc1,
>
> Except from procinfo patch, 3/3,
> Series applied to dpdk-next-net/main, thanks.
>
>
> Release notes and .map file order updated while merging.
Thanks for the work.

I have sent out the below patchset for the procinfo.

[PATCH v9 0/8] app/procinfo: add some extended features
https://patches.dpdk.org/project/dpdk/list/?series=25048

[v9,7/8] app/procinfo: support descriptor dump
https://patches.dpdk.org/project/dpdk/patch/20221008105353.18195-8-liudongdong3@huawei.com/

Thanks,
Dongdong.
>
> .
>