[RFC] ethdev: introduce ethdev dump API

[RFC] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Added the ethdev dump API which provides functions for query private info
from device. There exists many private properties in different PMD drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
these properties is important for debug. As the information is private,
the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 doc/guides/rel_notes/release_22_03.rst |  6 ++++++
 lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..9b51da899a 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,12 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+   * **Added the ethdev dump API, for query private info of ethdev.**
+
+     Added the ethdev dump API which provides functions for query private info
+     from device. There exists many private properties in different PMD
+     drivers. The information of these properties is important for debug. As
+     the information is private, the new API is introduced.
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index d95605a355..ac7fa5eae2 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Get ethdev private info.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param file
+ *   A pointer to a file for output.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1200,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump ethdev private info */
+	eth_dev_dump_t eth_dev_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a1d475a292..4bbe444045 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_dump(uint16_t port_id, FILE *file)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_dev_dump)(dev, file);
+
+	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 fa299c8ad7..918bd3116f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get ethdev private info.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+__rte_experimental
+int rte_eth_dev_dump(uint16_t port_id, FILE *file);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.33.0

RE: [RFC] ethdev: introduce ethdev dump API

[Morten Brørup]

> From: Min Hu (Connor) [mailto:humin29@huawei.com]
> Sent: Tuesday, 11 January 2022 12.55
> 
> Added the ethdev dump API which provides functions for query private
> info
> from device. There exists many private properties in different PMD
> drivers,
> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> information of
> these properties is important for debug. As the information is private,
> the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
>  doc/guides/rel_notes/release_22_03.rst |  6 ++++++
>  lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>  lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>  4 files changed, 54 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_03.rst
> b/doc/guides/rel_notes/release_22_03.rst
> index 6d99d1eaa9..9b51da899a 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -55,6 +55,12 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
> 
> +   * **Added the ethdev dump API, for query private info of ethdev.**
> +
> +     Added the ethdev dump API which provides functions for query
> private info
> +     from device. There exists many private properties in different
> PMD
> +     drivers. The information of these properties is important for
> debug. As
> +     the information is private, the new API is introduced.
> 
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index d95605a355..ac7fa5eae2 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> rte_eth_dev *dev,
>  typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>  				       uint64_t *features);
> 
> +/**
> + * @internal
> + * Get ethdev private info.
> + *
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param file
> + *   A pointer to a file for output.
> + *
> + * @return
> + *   Negative errno value on error, positive value on success.
> + */
> +typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);
> +
>  /**
>   * @internal A structure containing the functions exported by an
> Ethernet driver.
>   */
> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>  	 * kinds of metadata to the PMD
>  	 */
>  	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> +
> +	/** Dump ethdev private info */
> +	eth_dev_dump_t eth_dev_dump;
>  };
> 
>  /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index a1d475a292..4bbe444045 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
> uint64_t *features)
>  		       (*dev->dev_ops->rx_metadata_negotiate)(dev,
> features));
>  }
> 
> +int
> +rte_eth_dev_dump(uint16_t port_id, FILE *file)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_dump, -ENOTSUP);
> +	ret = (*dev->dev_ops->eth_dev_dump)(dev, file);
> +
> +	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 fa299c8ad7..918bd3116f 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> queue_id,
>  	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>  }
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Get ethdev private info.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param file
> + *   A pointer to a file for output.
> + * @return
> + *   Negative errno value on error, positive value on success.
> + */
> +__rte_experimental
> +int rte_eth_dev_dump(uint16_t port_id, FILE *file);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.33.0
> 

Good idea.

Two comments:

1. This function dumps private ethdev information. It should be named and described as such, e.g. eth_dev_priv_dump().

It should be a generic ethdev dump function, which dumps the common ethdev information, and also calls the eth_dev_priv_dump() function, if present.

2. Please make FILE* the first parameter, like similar dump functions.

Re: [RFC] ethdev: introduce ethdev dump API

[Thomas Monjalon]

Please use --cc-cmd devtools/get-maintainer.sh so all maintainers are Cc'ed.

11/01/2022 12:54, Min Hu (Connor):
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -55,6 +55,12 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
>  
> +   * **Added the ethdev dump API, for query private info of ethdev.**
> +
> +     Added the ethdev dump API which provides functions for query private info
> +     from device. There exists many private properties in different PMD
> +     drivers. The information of these properties is important for debug. As
> +     the information is private, the new API is introduced.
>  

A blank line is missing.
Also please check the comment above asking to start the actual text at the margin.

[...]
> +typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);

There is a dump function for rte_flow: rte_flow_dev_dump().
This one should have a clear scope: private device infos?

[RFC v2] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Added the ethdev dump API which provides functions for query private info
from device. There exists many private properties in different PMD drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
these properties is important for debug. As the information is private,
the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
v2:
* fix dump API name
* adjust description in doc.
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
 4 files changed, 55 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..4f97df942d 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,13 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added the private ethdev dump API, for query private info of ethdev.**
+
+  Added the private ethdev dump API which provides functions for query
+  private info from device. There exists many private properties in
+  different PMD drivers. The information of these properties is important
+  for debug. As the information is private, the new API is introduced.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index d95605a355..e75ff3f15b 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Get ethdev private info.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1200,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump ethdev private info */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a1d475a292..9fc6d91d76 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
+
+	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 fa299c8ad7..8e33e6927f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get ethdev private info.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.33.0

Re: [RFC] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Morten,
	thanks for your reply, all is fixed in v2.

在 2022/1/11 20:10, Morten Brørup 写道:
>> From: Min Hu (Connor) [mailto:humin29@huawei.com]
>> Sent: Tuesday, 11 January 2022 12.55
>>
>> Added the ethdev dump API which provides functions for query private
>> info
>> from device. There exists many private properties in different PMD
>> drivers,
>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>> information of
>> these properties is important for debug. As the information is private,
>> the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>>   doc/guides/rel_notes/release_22_03.rst |  6 ++++++
>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>   4 files changed, 54 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_03.rst
>> b/doc/guides/rel_notes/release_22_03.rst
>> index 6d99d1eaa9..9b51da899a 100644
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -55,6 +55,12 @@ New Features
>>        Also, make sure to start the actual text at the margin.
>>        =======================================================
>>
>> +   * **Added the ethdev dump API, for query private info of ethdev.**
>> +
>> +     Added the ethdev dump API which provides functions for query
>> private info
>> +     from device. There exists many private properties in different
>> PMD
>> +     drivers. The information of these properties is important for
>> debug. As
>> +     the information is private, the new API is introduced.
>>
>>   Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index d95605a355..ac7fa5eae2 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
>> rte_eth_dev *dev,
>>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>   				       uint64_t *features);
>>
>> +/**
>> + * @internal
>> + * Get ethdev private info.
>> + *
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param file
>> + *   A pointer to a file for output.
>> + *
>> + * @return
>> + *   Negative errno value on error, positive value on success.
>> + */
>> +typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an
>> Ethernet driver.
>>    */
>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>>   	 * kinds of metadata to the PMD
>>   	 */
>>   	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>> +
>> +	/** Dump ethdev private info */
>> +	eth_dev_dump_t eth_dev_dump;
>>   };
>>
>>   /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index a1d475a292..4bbe444045 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
>> uint64_t *features)
>>   		       (*dev->dev_ops->rx_metadata_negotiate)(dev,
>> features));
>>   }
>>
>> +int
>> +rte_eth_dev_dump(uint16_t port_id, FILE *file)
>> +{
>> +	struct rte_eth_dev *dev;
>> +	int ret;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_dump, -ENOTSUP);
>> +	ret = (*dev->dev_ops->eth_dev_dump)(dev, file);
>> +
>> +	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 fa299c8ad7..918bd3116f 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
>> queue_id,
>>   	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>   }
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Get ethdev private info.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @return
>> + *   Negative errno value on error, positive value on success.
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_dump(uint16_t port_id, FILE *file);
>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> --
>> 2.33.0
>>
> 
> Good idea.
> 
> Two comments:
> 
> 1. This function dumps private ethdev information. It should be named and described as such, e.g. eth_dev_priv_dump().
> 
> It should be a generic ethdev dump function, which dumps the common ethdev information, and also calls the eth_dev_priv_dump() function, if present.
> 
> 2. Please make FILE* the first parameter, like similar dump functions.
> 
> 
> .
> 

Re: [RFC] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Thomas,
	fixed in v2, thanks.

在 2022/1/11 20:48, Thomas Monjalon 写道:
> Please use --cc-cmd devtools/get-maintainer.sh so all maintainers are Cc'ed.
> 
> 11/01/2022 12:54, Min Hu (Connor):
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -55,6 +55,12 @@ New Features
>>        Also, make sure to start the actual text at the margin.
>>        =======================================================
>>   
>> +   * **Added the ethdev dump API, for query private info of ethdev.**
>> +
>> +     Added the ethdev dump API which provides functions for query private info
>> +     from device. There exists many private properties in different PMD
>> +     drivers. The information of these properties is important for debug. As
>> +     the information is private, the new API is introduced.
>>   
> 
> A blank line is missing.
> Also please check the comment above asking to start the actual text at the margin.
> 
> [...]
>> +typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);
> 
> There is a dump function for rte_flow: rte_flow_dev_dump().
> This one should have a clear scope: private device infos?
> 
> 
> 
> .
> 

Re: [RFC] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Thomas,

在 2022/1/11 20:48, Thomas Monjalon 写道:
> Please use --cc-cmd devtools/get-maintainer.sh so all maintainers are Cc'edlike this ?
  git send-email -to dev@dpdk.org -cc ferruh.yigit@intel.com -cc 
thomas@monjalon.net --cc-cmd devtools/get-maintainer.sh  *.patch 
--suppress-cc=all

I did this, but it doesn't work. It only Cc ferruh and you.


> 
> 11/01/2022 12:54, Min Hu (Connor):
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -55,6 +55,12 @@ New Features
>>        Also, make sure to start the actual text at the margin.
>>        =======================================================
>>   
>> +   * **Added the ethdev dump API, for query private info of ethdev.**
>> +
>> +     Added the ethdev dump API which provides functions for query private info
>> +     from device. There exists many private properties in different PMD
>> +     drivers. The information of these properties is important for debug. As
>> +     the information is private, the new API is introduced.
>>   
> 
> A blank line is missing.
> Also please check the comment above asking to start the actual text at the margin.
> 
> [...]
>> +typedef int (*eth_dev_dump_t)(struct rte_eth_dev *dev, FILE *file);
> 
> There is a dump function for rte_flow: rte_flow_dev_dump().
> This one should have a clear scope: private device infos?
> 
> 
> 
> .
> 

RE: [RFC v2] ethdev: introduce ethdev dump API

[Morten Brørup]

> From: Min Hu (Connor) [mailto:humin29@huawei.com]
> Sent: Wednesday, 12 January 2022 03.40
> 
> Added the ethdev dump API which provides functions for query private
> info
> from device. There exists many private properties in different PMD
> drivers,
> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> information of
> these properties is important for debug. As the information is private,
> the new API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> ---
> v2:
> * fix dump API name
> * adjust description in doc.
> ---
>  doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>  lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>  lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>  4 files changed, 55 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_03.rst
> b/doc/guides/rel_notes/release_22_03.rst
> index 6d99d1eaa9..4f97df942d 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -55,6 +55,13 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
> 
> +* **Added the private ethdev dump API, for query private info of
> ethdev.**
> +
> +  Added the private ethdev dump API which provides functions for query
> +  private info from device. There exists many private properties in
> +  different PMD drivers. The information of these properties is
> important
> +  for debug. As the information is private, the new API is introduced.
> +
> 
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index d95605a355..e75ff3f15b 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> rte_eth_dev *dev,
>  typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>  				       uint64_t *features);
> 
> +/**
> + * @internal
> + * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + *
> + * @return
> + *   Negative errno value on error, positive value on success.
> + */
> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
> *dev);
> +
>  /**
>   * @internal A structure containing the functions exported by an
> Ethernet driver.
>   */
> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>  	 * kinds of metadata to the PMD
>  	 */
>  	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> +
> +	/** Dump ethdev private info */
> +	eth_dev_priv_dump_t eth_dev_priv_dump;
>  };
> 
>  /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index a1d475a292..9fc6d91d76 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
> uint64_t *features)
>  		       (*dev->dev_ops->rx_metadata_negotiate)(dev,
> features));
>  }
> 
> +int
> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -
> ENOTSUP);
> +	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
> +
> +	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 fa299c8ad7..8e33e6927f 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> queue_id,
>  	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>  }
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> notice
> + *
> + * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

> + *
> + * @param file
> + *   A pointer to a file for output.
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @return
> + *   Negative errno value on error, positive value on success.
> + */
> +__rte_experimental
> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.33.0
> 

You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.

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

Re: [RFC] ethdev: introduce ethdev dump API

[Thomas Monjalon]

12/01/2022 03:44, Min Hu (Connor):
> Hi, Thomas,
> 
> 在 2022/1/11 20:48, Thomas Monjalon 写道:
> > Please use --cc-cmd devtools/get-maintainer.sh so all maintainers are Cc'edlike this ?
>   git send-email -to dev@dpdk.org -cc ferruh.yigit@intel.com -cc 
> thomas@monjalon.net --cc-cmd devtools/get-maintainer.sh  *.patch 
> --suppress-cc=all
> 
> I did this, but it doesn't work. It only Cc ferruh and you.

You should read the documentation about the tools:
https://doc.dpdk.org/guides/contributing/patches.html#checking-the-patches

In my file ~/.config/dpdk/devel.config, I have these lines:
export DPDK_GETMAINTAINER_PATH=$root/linux/scripts/get_maintainer.pl
export DPDK_CHECKPATCH_PATH=$root/linux/scripts/checkpatch.pl
export DPDK_CHECKPATCH_CODESPELL=$root/codespell/dictionary.txt

Did you set DPDK_GETMAINTAINER_PATH?

Re: [RFC] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Thanks Thomas, I will have a try.

在 2022/1/12 18:13, Thomas Monjalon 写道:
> 12/01/2022 03:44, Min Hu (Connor):
>> Hi, Thomas,
>>
>> 在 2022/1/11 20:48, Thomas Monjalon 写道:
>>> Please use --cc-cmd devtools/get-maintainer.sh so all maintainers are Cc'edlike this ?
>>    git send-email -to dev@dpdk.org -cc ferruh.yigit@intel.com -cc
>> thomas@monjalon.net --cc-cmd devtools/get-maintainer.sh  *.patch
>> --suppress-cc=all
>>
>> I did this, but it doesn't work. It only Cc ferruh and you.
> 
> You should read the documentation about the tools:
> https://doc.dpdk.org/guides/contributing/patches.html#checking-the-patches
> 
> In my file ~/.config/dpdk/devel.config, I have these lines:
> export DPDK_GETMAINTAINER_PATH=$root/linux/scripts/get_maintainer.pl
> export DPDK_CHECKPATCH_PATH=$root/linux/scripts/checkpatch.pl
> export DPDK_CHECKPATCH_CODESPELL=$root/codespell/dictionary.txt
> 
> Did you set DPDK_GETMAINTAINER_PATH?
> 
> 
> .
> 

[RFC v3] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 4529 bytes --]

Added the ethdev dump API which provides functions for query private info
from device. There exists many private properties in different PMD drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
these properties is important for debug. As the information is private,
the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
v3:
* fix comment.
* add rte_eth_dev_priv_dump to version.map file.

v2:
* fix dump API name
* adjust description in doc.
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 5 files changed, 58 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..4f97df942d 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,13 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added the private ethdev dump API, for query private info of ethdev.**
+
+  Added the private ethdev dump API which provides functions for query
+  private info from device. There exists many private properties in
+  different PMD drivers. The information of these properties is important
+  for debug. As the information is private, the new API is introduced.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index d95605a355..0e3d99b07e 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1200,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump ethdev private info */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a1d475a292..9fc6d91d76 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
+
+	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 fa299c8ad7..d17e0f177d 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index c2fb0669a4..f29c60eda4 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,9 @@ EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	# added in 22.03
+	rte_eth_dev_priv_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [RFC v2] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Thanks Morten, fixed in v3.

在 2022/1/12 15:20, Morten Brørup 写道:
>> From: Min Hu (Connor) [mailto:humin29@huawei.com]
>> Sent: Wednesday, 12 January 2022 03.40
>>
>> Added the ethdev dump API which provides functions for query private
>> info
>> from device. There exists many private properties in different PMD
>> drivers,
>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>> information of
>> these properties is important for debug. As the information is private,
>> the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> ---
>> v2:
>> * fix dump API name
>> * adjust description in doc.
>> ---
>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>   4 files changed, 55 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_03.rst
>> b/doc/guides/rel_notes/release_22_03.rst
>> index 6d99d1eaa9..4f97df942d 100644
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -55,6 +55,13 @@ New Features
>>        Also, make sure to start the actual text at the margin.
>>        =======================================================
>>
>> +* **Added the private ethdev dump API, for query private info of
>> ethdev.**
>> +
>> +  Added the private ethdev dump API which provides functions for query
>> +  private info from device. There exists many private properties in
>> +  different PMD drivers. The information of these properties is
>> important
>> +  for debug. As the information is private, the new API is introduced.
>> +
>>
>>   Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index d95605a355..e75ff3f15b 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
>> rte_eth_dev *dev,
>>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>   				       uint64_t *features);
>>
>> +/**
>> + * @internal
>> + * Get ethdev private info.
> 
> Suggestion: Dump ethdev private info to a file.
> 
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + *
>> + * @return
>> + *   Negative errno value on error, positive value on success.
>> + */
>> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
>> *dev);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an
>> Ethernet driver.
>>    */
>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>>   	 * kinds of metadata to the PMD
>>   	 */
>>   	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>> +
>> +	/** Dump ethdev private info */
>> +	eth_dev_priv_dump_t eth_dev_priv_dump;
>>   };
>>
>>   /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index a1d475a292..9fc6d91d76 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
>> uint64_t *features)
>>   		       (*dev->dev_ops->rx_metadata_negotiate)(dev,
>> features));
>>   }
>>
>> +int
>> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
>> +{
>> +	struct rte_eth_dev *dev;
>> +	int ret;
>> +
>> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +	dev = &rte_eth_devices[port_id];
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -
>> ENOTSUP);
>> +	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
>> +
>> +	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 fa299c8ad7..8e33e6927f 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
>> queue_id,
>>   	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>   }
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Get ethdev private info.
> 
> Suggestion: Dump ethdev private info to a file.
> 
>> + *
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @return
>> + *   Negative errno value on error, positive value on success.
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> --
>> 2.33.0
>>
> 
> You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> .
> 

Re: [RFC v3] ethdev: introduce ethdev dump API

[Ray Kinsella]

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

> Added the ethdev dump API which provides functions for query private info
> from device. There exists many private properties in different PMD drivers,
> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
> these properties is important for debug. As the information is private,
> the new API is introduced.
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
> ---
> v3:
> * fix comment.
> * add rte_eth_dev_priv_dump to version.map file.
>
> v2:
> * fix dump API name
> * adjust description in doc.
> ---
>  doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>  lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>  lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>  lib/ethdev/version.map                 |  3 +++
>  5 files changed, 58 insertions(+)
>

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

Re: [RFC v2] ethdev: introduce ethdev dump API

[Ajit Khaparde]

On Wed, Jan 12, 2022 at 3:15 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> Thanks Morten, fixed in v3.
>
> 在 2022/1/12 15:20, Morten Brørup 写道:
> >> From: Min Hu (Connor) [mailto:humin29@huawei.com]
> >> Sent: Wednesday, 12 January 2022 03.40
> >>
> >> Added the ethdev dump API which provides functions for query private
> >> info
> >> from device. There exists many private properties in different PMD
> >> drivers,
> >> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> >> information of
> >> these properties is important for debug. As the information is private,
> >> the new API is introduced.
Do you have any changes to testpmd to use this API?

> >>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> ---
> >> v2:
> >> * fix dump API name
> >> * adjust description in doc.
> >> ---
> >>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
> >>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
> >>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
> >>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
> >>   4 files changed, 55 insertions(+)
> >>
> >> diff --git a/doc/guides/rel_notes/release_22_03.rst
> >> b/doc/guides/rel_notes/release_22_03.rst
> >> index 6d99d1eaa9..4f97df942d 100644
> >> --- a/doc/guides/rel_notes/release_22_03.rst
> >> +++ b/doc/guides/rel_notes/release_22_03.rst
> >> @@ -55,6 +55,13 @@ New Features
> >>        Also, make sure to start the actual text at the margin.
> >>        =======================================================
> >>
> >> +* **Added the private ethdev dump API, for query private info of
> >> ethdev.**
> >> +
> >> +  Added the private ethdev dump API which provides functions for query
> >> +  private info from device. There exists many private properties in
> >> +  different PMD drivers. The information of these properties is
> >> important
> >> +  for debug. As the information is private, the new API is introduced.
> >> +
> >>
> >>   Removed Items
> >>   -------------
> >> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> >> index d95605a355..e75ff3f15b 100644
> >> --- a/lib/ethdev/ethdev_driver.h
> >> +++ b/lib/ethdev/ethdev_driver.h
> >> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> >> rte_eth_dev *dev,
> >>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
> >>                                     uint64_t *features);
> >>
> >> +/**
> >> + * @internal
> >> + * Get ethdev private info.
> >
> > Suggestion: Dump ethdev private info to a file.
> >
> >> + *
> >> + * @param file
> >> + *   A pointer to a file for output.
> >> + * @param dev
> >> + *   Port (ethdev) handle.
> >> + *
> >> + * @return
> >> + *   Negative errno value on error, positive value on success.
> >> + */
> >> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
> >> *dev);
> >> +
> >>   /**
> >>    * @internal A structure containing the functions exported by an
> >> Ethernet driver.
> >>    */
> >> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
> >>       * kinds of metadata to the PMD
> >>       */
> >>      eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> >> +
> >> +    /** Dump ethdev private info */
> >> +    eth_dev_priv_dump_t eth_dev_priv_dump;
> >>   };
> >>
> >>   /**
> >> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> >> index a1d475a292..9fc6d91d76 100644
> >> --- a/lib/ethdev/rte_ethdev.c
> >> +++ b/lib/ethdev/rte_ethdev.c
> >> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
> >> uint64_t *features)
> >>                     (*dev->dev_ops->rx_metadata_negotiate)(dev,
> >> features));
> >>   }
> >>
> >> +int
> >> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
> >> +{
> >> +    struct rte_eth_dev *dev;
> >> +    int ret;
> >> +
> >> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >> +    dev = &rte_eth_devices[port_id];
> >> +
> >> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -
> >> ENOTSUP);
> >> +    ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
> >> +
> >> +    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 fa299c8ad7..8e33e6927f 100644
> >> --- a/lib/ethdev/rte_ethdev.h
> >> +++ b/lib/ethdev/rte_ethdev.h
> >> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> >> queue_id,
> >>      return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
> >>   }
> >>
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> >> notice
> >> + *
> >> + * Get ethdev private info.
> >
> > Suggestion: Dump ethdev private info to a file.
> >
> >> + *
> >> + * @param file
> >> + *   A pointer to a file for output.
> >> + * @param port_id
> >> + *   The port identifier of the Ethernet device.
> >> + * @return
> >> + *   Negative errno value on error, positive value on success.
> >> + */
> >> +__rte_experimental
> >> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> >> +
> >>   #ifdef __cplusplus
> >>   }
> >>   #endif
> >> --
> >> 2.33.0
> >>
> >
> > You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.
> >
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > .
> >

Re: [RFC v2] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Ajit,

在 2022/1/15 1:56, Ajit Khaparde 写道:
> On Wed, Jan 12, 2022 at 3:15 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>>
>> Thanks Morten, fixed in v3.
>>
>> 在 2022/1/12 15:20, Morten Brørup 写道:
>>>> From: Min Hu (Connor) [mailto:humin29@huawei.com]
>>>> Sent: Wednesday, 12 January 2022 03.40
>>>>
>>>> Added the ethdev dump API which provides functions for query private
>>>> info
>>>> from device. There exists many private properties in different PMD
>>>> drivers,
>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>> information of
>>>> these properties is important for debug. As the information is private,
>>>> the new API is introduced.
> Do you have any changes to testpmd to use this API?
> 
No changes to testpmd.
BTW, I will add some options for proc-info to use this API,
and this is my next-step plan.

>>>>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> ---
>>>> v2:
>>>> * fix dump API name
>>>> * adjust description in doc.
>>>> ---
>>>>    doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>    lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>>    lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>>    lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>>    4 files changed, 55 insertions(+)
>>>>
>>>> diff --git a/doc/guides/rel_notes/release_22_03.rst
>>>> b/doc/guides/rel_notes/release_22_03.rst
>>>> index 6d99d1eaa9..4f97df942d 100644
>>>> --- a/doc/guides/rel_notes/release_22_03.rst
>>>> +++ b/doc/guides/rel_notes/release_22_03.rst
>>>> @@ -55,6 +55,13 @@ New Features
>>>>         Also, make sure to start the actual text at the margin.
>>>>         =======================================================
>>>>
>>>> +* **Added the private ethdev dump API, for query private info of
>>>> ethdev.**
>>>> +
>>>> +  Added the private ethdev dump API which provides functions for query
>>>> +  private info from device. There exists many private properties in
>>>> +  different PMD drivers. The information of these properties is
>>>> important
>>>> +  for debug. As the information is private, the new API is introduced.
>>>> +
>>>>
>>>>    Removed Items
>>>>    -------------
>>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>>>> index d95605a355..e75ff3f15b 100644
>>>> --- a/lib/ethdev/ethdev_driver.h
>>>> +++ b/lib/ethdev/ethdev_driver.h
>>>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
>>>> rte_eth_dev *dev,
>>>>    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>>>                                      uint64_t *features);
>>>>
>>>> +/**
>>>> + * @internal
>>>> + * Get ethdev private info.
>>>
>>> Suggestion: Dump ethdev private info to a file.
>>>
>>>> + *
>>>> + * @param file
>>>> + *   A pointer to a file for output.
>>>> + * @param dev
>>>> + *   Port (ethdev) handle.
>>>> + *
>>>> + * @return
>>>> + *   Negative errno value on error, positive value on success.
>>>> + */
>>>> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
>>>> *dev);
>>>> +
>>>>    /**
>>>>     * @internal A structure containing the functions exported by an
>>>> Ethernet driver.
>>>>     */
>>>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>>>>        * kinds of metadata to the PMD
>>>>        */
>>>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>>>> +
>>>> +    /** Dump ethdev private info */
>>>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
>>>>    };
>>>>
>>>>    /**
>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>>> index a1d475a292..9fc6d91d76 100644
>>>> --- a/lib/ethdev/rte_ethdev.c
>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
>>>> uint64_t *features)
>>>>                      (*dev->dev_ops->rx_metadata_negotiate)(dev,
>>>> features));
>>>>    }
>>>>
>>>> +int
>>>> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
>>>> +{
>>>> +    struct rte_eth_dev *dev;
>>>> +    int ret;
>>>> +
>>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>>> +    dev = &rte_eth_devices[port_id];
>>>> +
>>>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -
>>>> ENOTSUP);
>>>> +    ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
>>>> +
>>>> +    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 fa299c8ad7..8e33e6927f 100644
>>>> --- a/lib/ethdev/rte_ethdev.h
>>>> +++ b/lib/ethdev/rte_ethdev.h
>>>> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
>>>> queue_id,
>>>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>>>    }
>>>>
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>>>> notice
>>>> + *
>>>> + * Get ethdev private info.
>>>
>>> Suggestion: Dump ethdev private info to a file.
>>>
>>>> + *
>>>> + * @param file
>>>> + *   A pointer to a file for output.
>>>> + * @param port_id
>>>> + *   The port identifier of the Ethernet device.
>>>> + * @return
>>>> + *   Negative errno value on error, positive value on success.
>>>> + */
>>>> +__rte_experimental
>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>> +
>>>>    #ifdef __cplusplus
>>>>    }
>>>>    #endif
>>>> --
>>>> 2.33.0
>>>>
>>>
>>> You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.
>>>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>
>>> .
>>>
> .
> 

Re: [RFC v2] ethdev: introduce ethdev dump API

[Ajit Khaparde]

On Fri, Jan 14, 2022 at 4:25 PM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> Hi, Ajit,
>
> 在 2022/1/15 1:56, Ajit Khaparde 写道:
> > On Wed, Jan 12, 2022 at 3:15 AM Min Hu (Connor) <humin29@huawei.com> wrote:
> >>
> >> Thanks Morten, fixed in v3.
> >>
> >> 在 2022/1/12 15:20, Morten Brørup 写道:
> >>>> From: Min Hu (Connor) [mailto:humin29@huawei.com]
> >>>> Sent: Wednesday, 12 January 2022 03.40
> >>>>
> >>>> Added the ethdev dump API which provides functions for query private
> >>>> info
> >>>> from device. There exists many private properties in different PMD
> >>>> drivers,
> >>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> >>>> information of
> >>>> these properties is important for debug. As the information is private,
> >>>> the new API is introduced.
> > Do you have any changes to testpmd to use this API?
> >
> No changes to testpmd.
> BTW, I will add some options for proc-info to use this API,
> and this is my next-step plan.
Ok. proc-info is a good place for this.

>
> >>>>
> >>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >>>> ---
> >>>> v2:
> >>>> * fix dump API name
> >>>> * adjust description in doc.
> >>>> ---
> >>>>    doc/guides/rel_notes/release_22_03.rst |  7 +++++++
> >>>>    lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
> >>>>    lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
> >>>>    lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
> >>>>    4 files changed, 55 insertions(+)
> >>>>
> >>>> diff --git a/doc/guides/rel_notes/release_22_03.rst
> >>>> b/doc/guides/rel_notes/release_22_03.rst
> >>>> index 6d99d1eaa9..4f97df942d 100644
> >>>> --- a/doc/guides/rel_notes/release_22_03.rst
> >>>> +++ b/doc/guides/rel_notes/release_22_03.rst
> >>>> @@ -55,6 +55,13 @@ New Features
> >>>>         Also, make sure to start the actual text at the margin.
> >>>>         =======================================================
> >>>>
> >>>> +* **Added the private ethdev dump API, for query private info of
> >>>> ethdev.**
> >>>> +
> >>>> +  Added the private ethdev dump API which provides functions for query
> >>>> +  private info from device. There exists many private properties in
> >>>> +  different PMD drivers. The information of these properties is
> >>>> important
> >>>> +  for debug. As the information is private, the new API is introduced.
> >>>> +
> >>>>
> >>>>    Removed Items
> >>>>    -------------
> >>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> >>>> index d95605a355..e75ff3f15b 100644
> >>>> --- a/lib/ethdev/ethdev_driver.h
> >>>> +++ b/lib/ethdev/ethdev_driver.h
> >>>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> >>>> rte_eth_dev *dev,
> >>>>    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
> >>>>                                      uint64_t *features);
> >>>>
> >>>> +/**
> >>>> + * @internal
> >>>> + * Get ethdev private info.
> >>>
> >>> Suggestion: Dump ethdev private info to a file.
> >>>
> >>>> + *
> >>>> + * @param file
> >>>> + *   A pointer to a file for output.
> >>>> + * @param dev
> >>>> + *   Port (ethdev) handle.
> >>>> + *
> >>>> + * @return
> >>>> + *   Negative errno value on error, positive value on success.
> >>>> + */
> >>>> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev
> >>>> *dev);
> >>>> +
> >>>>    /**
> >>>>     * @internal A structure containing the functions exported by an
> >>>> Ethernet driver.
> >>>>     */
> >>>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
> >>>>        * kinds of metadata to the PMD
> >>>>        */
> >>>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> >>>> +
> >>>> +    /** Dump ethdev private info */
> >>>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
> >>>>    };
> >>>>
> >>>>    /**
> >>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> >>>> index a1d475a292..9fc6d91d76 100644
> >>>> --- a/lib/ethdev/rte_ethdev.c
> >>>> +++ b/lib/ethdev/rte_ethdev.c
> >>>> @@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id,
> >>>> uint64_t *features)
> >>>>                      (*dev->dev_ops->rx_metadata_negotiate)(dev,
> >>>> features));
> >>>>    }
> >>>>
> >>>> +int
> >>>> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
> >>>> +{
> >>>> +    struct rte_eth_dev *dev;
> >>>> +    int ret;
> >>>> +
> >>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >>>> +    dev = &rte_eth_devices[port_id];
> >>>> +
> >>>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -
> >>>> ENOTSUP);
> >>>> +    ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
> >>>> +
> >>>> +    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 fa299c8ad7..8e33e6927f 100644
> >>>> --- a/lib/ethdev/rte_ethdev.h
> >>>> +++ b/lib/ethdev/rte_ethdev.h
> >>>> @@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t
> >>>> queue_id,
> >>>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
> >>>>    }
> >>>>
> >>>> +/**
> >>>> + * @warning
> >>>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> >>>> notice
> >>>> + *
> >>>> + * Get ethdev private info.
> >>>
> >>> Suggestion: Dump ethdev private info to a file.
> >>>
> >>>> + *
> >>>> + * @param file
> >>>> + *   A pointer to a file for output.
> >>>> + * @param port_id
> >>>> + *   The port identifier of the Ethernet device.
> >>>> + * @return
> >>>> + *   Negative errno value on error, positive value on success.
> >>>> + */
> >>>> +__rte_experimental
> >>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> >>>> +
> >>>>    #ifdef __cplusplus
> >>>>    }
> >>>>    #endif
> >>>> --
> >>>> 2.33.0
> >>>>
> >>>
> >>> You should probably also add rte_eth_dev_priv_dump to the EXPERIMENTAL section of the version.map file.
> >>>
> >>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>>
> >>> .
> >>>
> > .
> >

Re: [RFC v3] ethdev: introduce ethdev dump API

[Ajit Khaparde]

On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>
>
> Min Hu (Connor) <humin29@huawei.com> writes:
>
> > Added the ethdev dump API which provides functions for query private info
> > from device. There exists many private properties in different PMD drivers,
> > such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
> > these properties is important for debug. As the information is private,
> > the new API is introduced.
> >
> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> > Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
> > ---
> > v3:
> > * fix comment.
> > * add rte_eth_dev_priv_dump to version.map file.
> >
> > v2:
> > * fix dump API name
> > * adjust description in doc.
> > ---
> >  doc/guides/rel_notes/release_22_03.rst |  7 +++++++
> >  lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
> >  lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
> >  lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
> >  lib/ethdev/version.map                 |  3 +++
> >  5 files changed, 58 insertions(+)
> >
>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

Re: [RFC v3] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 1/18/2022 3:34 PM, Ajit Khaparde wrote:
> On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>>
>>
>> Min Hu (Connor) <humin29@huawei.com> writes:
>>
>>> Added the ethdev dump API which provides functions for query private info
>>> from device. There exists many private properties in different PMD drivers,
>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
>>> these properties is important for debug. As the information is private,
>>> the new API is introduced.
>>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
>>> ---
>>> v3:
>>> * fix comment.
>>> * add rte_eth_dev_priv_dump to version.map file.
>>>
>>> v2:
>>> * fix dump API name
>>> * adjust description in doc.
>>> ---
>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>   lib/ethdev/version.map                 |  3 +++
>>>   5 files changed, 58 insertions(+)
>>>
>>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


I have a concern and this "private info", it can be useful for debug
but still it will lead to a PMD specific applications, I wonder if some
common information can be provided. Is there any list in your mind what
can be part of this private info?

Also why not use existing xstats or telemetry to get more data from the
drivers?
As synced with Ciara (cc'ed), she mentioned more information can be get
via registering callbacks to ethdev.

Re: [RFC v3] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 1/25/2022 12:56 PM, Ferruh Yigit wrote:
> On 1/18/2022 3:34 PM, Ajit Khaparde wrote:
>> On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>>>
>>>
>>> Min Hu (Connor) <humin29@huawei.com> writes:
>>>
>>>> Added the ethdev dump API which provides functions for query private info
>>>> from device. There exists many private properties in different PMD drivers,
>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
>>>> these properties is important for debug. As the information is private,
>>>> the new API is introduced.
>>>>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
>>>> ---
>>>> v3:
>>>> * fix comment.
>>>> * add rte_eth_dev_priv_dump to version.map file.
>>>>
>>>> v2:
>>>> * fix dump API name
>>>> * adjust description in doc.
>>>> ---
>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>>   lib/ethdev/version.map                 |  3 +++
>>>>   5 files changed, 58 insertions(+)
>>>>
>>>
>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
> 
> I have a concern and this "private info", it can be useful for debug
> but still it will lead to a PMD specific applications, I wonder if some
> common information can be provided. Is there any list in your mind what
> can be part of this private info?
> 
> Also why not use existing xstats or telemetry to get more data from the
> drivers?
> As synced with Ciara (cc'ed), she mentioned more information can be get
> via registering callbacks to ethdev.

Opps, Ciara (Power) cc'ed now.

Re: [RFC v3] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/1/25 20:58, Ferruh Yigit 写道:
> On 1/25/2022 12:56 PM, Ferruh Yigit wrote:
>> On 1/18/2022 3:34 PM, Ajit Khaparde wrote:
>>> On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>>>>
>>>>
>>>> Min Hu (Connor) <humin29@huawei.com> writes:
>>>>
>>>>> Added the ethdev dump API which provides functions for query 
>>>>> private info
>>>>> from device. There exists many private properties in different PMD 
>>>>> drivers,
>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The 
>>>>> information of
>>>>> these properties is important for debug. As the information is 
>>>>> private,
>>>>> the new API is introduced.
>>>>>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
>>>>> ---
>>>>> v3:
>>>>> * fix comment.
>>>>> * add rte_eth_dev_priv_dump to version.map file.
>>>>>
>>>>> v2:
>>>>> * fix dump API name
>>>>> * adjust description in doc.
>>>>> ---
>>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>>>   lib/ethdev/version.map                 |  3 +++
>>>>>   5 files changed, 58 insertions(+)
>>>>>
>>>>
>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>
>>
>> I have a concern and this "private info", it can be useful for debug
>> but still it will lead to a PMD specific applications, I wonder if some
>> common information can be provided.
The purpose of introducing the API is to enhance dump ability for NICs.
It will be used in APP proc-info, I will add "show-port-private" option
for it. As for common information, I will add them in "show-port" option
which already exists.


  Is there any list in your mind what
>> can be part of this private info?
Private info for NICs, take HNS3 PMD NICS as an example,
   - Dev Capability:
           -- support DCB: no
           -- support COPPER: no
           -- support FD QUEUE REGION: no
           -- support PTP: no
           -- support TX PUSH: no
           -- support INDEP TXRX: no
           -- support STASH: no
           -- support SIMPLE BD: no
           -- support RXD Advanced Layout: no
           -- support OUTER UDP CKSUM: no
           -- support RAS IMP: no
           -- support TM: no
           -- support VF VLAN FILTER MOD: no
   - VLAN Config Info:
           -- Port VLAN filter configuration
                nic_ingress           :Disable
                nic_egress            :Disable
           -- VF VLAN filter configuration
                nic_ingress           :Disable
                nic_egress            :Disable
           -- RX VLAN configuration
                vlan1_strip_en        :Disable
                vlan2_strip_en        :Disable
                vlan1_vlan_prionly    :Disable
                vlan2_vlan_prionly    :Disable
                vlan1_strip_discard   :Disable
                vlan2_strip_discard   :Disable
           -- TX VLAN configuration
                accept_tag1           :Enable
                accept_untag1         :Enable
                insert_tag1_en        :Disable
                default_vlan_tag1 = 0, qos = 0
                accept_tag2           :Enable
                accept_untag2         :Enable
                insert_tag2_en        :Disable
                default_vlan_tag2 = 0, qos = 0
                vlan_shift_mode       :Disable
           -- pvid status: off
   - Fdir Info:
           -- mode=0 max_key_len=400 rule_num:512 cnt_num:32
           -- key_sel=1 tuple_active=0x3bdfd890 meta_data_active=0xd0
           -- ipv6_word_en: in_s=3 in_d=3 out_s=0 out_d=0
....

These info is private for hns3 PMD.


>>
>> Also why not use existing xstats or telemetry to get more data from the
>> drivers?
Xstats is used to query statistics rather than state. Like above info,
many keys is "enable" or "disable". It is not appropriated to put the
date in xstats.

Telemetry is used to query common info for NICs, not for private info.
  It is not also appropriated to put private date in telemetry.



>> As synced with Ciara (cc'ed), she mentioned more information can be get
>> via registering callbacks to ethdev.
> 
> Opps, Ciara (Power) cc'ed now.
> .

Re: [RFC v3] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 1/25/2022 1:45 PM, Min Hu (Connor) wrote:
> Hi, Ferruh,
> 
> 在 2022/1/25 20:58, Ferruh Yigit 写道:
>> On 1/25/2022 12:56 PM, Ferruh Yigit wrote:
>>> On 1/18/2022 3:34 PM, Ajit Khaparde wrote:
>>>> On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>>>>>
>>>>>
>>>>> Min Hu (Connor) <humin29@huawei.com> writes:
>>>>>
>>>>>> Added the ethdev dump API which provides functions for query private info
>>>>>> from device. There exists many private properties in different PMD drivers,
>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
>>>>>> these properties is important for debug. As the information is private,
>>>>>> the new API is introduced.
>>>>>>
>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
>>>>>> ---
>>>>>> v3:
>>>>>> * fix comment.
>>>>>> * add rte_eth_dev_priv_dump to version.map file.
>>>>>>
>>>>>> v2:
>>>>>> * fix dump API name
>>>>>> * adjust description in doc.
>>>>>> ---
>>>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>>>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>>>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>>>>   lib/ethdev/version.map                 |  3 +++
>>>>>>   5 files changed, 58 insertions(+)
>>>>>>
>>>>>
>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>
>>>
>>> I have a concern and this "private info", it can be useful for debug
>>> but still it will lead to a PMD specific applications, I wonder if some
>>> common information can be provided.
> The purpose of introducing the API is to enhance dump ability for NICs.
> It will be used in APP proc-info, I will add "show-port-private" option
> for it. As for common information, I will add them in "show-port" option
> which already exists.
> 

Hi Connor,

We briefly touch on this patch in today's release status meeting,
as far as I get general consensus was this can be good for debugging
issues offline (instead of application parsing the output of API).

So can you please send a new, non-RFC, version of the patch?
And please keep the existing ack/review tags.

> 
>   Is there any list in your mind what
>>> can be part of this private info?
> Private info for NICs, take HNS3 PMD NICS as an example,
>    - Dev Capability:
>            -- support DCB: no
>            -- support COPPER: no
>            -- support FD QUEUE REGION: no
>            -- support PTP: no
>            -- support TX PUSH: no
>            -- support INDEP TXRX: no
>            -- support STASH: no
>            -- support SIMPLE BD: no
>            -- support RXD Advanced Layout: no
>            -- support OUTER UDP CKSUM: no
>            -- support RAS IMP: no
>            -- support TM: no
>            -- support VF VLAN FILTER MOD: no
>    - VLAN Config Info:
>            -- Port VLAN filter configuration
>                 nic_ingress           :Disable
>                 nic_egress            :Disable
>            -- VF VLAN filter configuration
>                 nic_ingress           :Disable
>                 nic_egress            :Disable
>            -- RX VLAN configuration
>                 vlan1_strip_en        :Disable
>                 vlan2_strip_en        :Disable
>                 vlan1_vlan_prionly    :Disable
>                 vlan2_vlan_prionly    :Disable
>                 vlan1_strip_discard   :Disable
>                 vlan2_strip_discard   :Disable
>            -- TX VLAN configuration
>                 accept_tag1           :Enable
>                 accept_untag1         :Enable
>                 insert_tag1_en        :Disable
>                 default_vlan_tag1 = 0, qos = 0
>                 accept_tag2           :Enable
>                 accept_untag2         :Enable
>                 insert_tag2_en        :Disable
>                 default_vlan_tag2 = 0, qos = 0
>                 vlan_shift_mode       :Disable
>            -- pvid status: off
>    - Fdir Info:
>            -- mode=0 max_key_len=400 rule_num:512 cnt_num:32
>            -- key_sel=1 tuple_active=0x3bdfd890 meta_data_active=0xd0
>            -- ipv6_word_en: in_s=3 in_d=3 out_s=0 out_d=0
> ....
> 
> These info is private for hns3 PMD.
> 
> 
>>>
>>> Also why not use existing xstats or telemetry to get more data from the
>>> drivers?
> Xstats is used to query statistics rather than state. Like above info,
> many keys is "enable" or "disable". It is not appropriated to put the
> date in xstats.
> 
> Telemetry is used to query common info for NICs, not for private info.
>   It is not also appropriated to put private date in telemetry.
> 
> 
> 
>>> As synced with Ciara (cc'ed), she mentioned more information can be get
>>> via registering callbacks to ethdev.
>>
>> Opps, Ciara (Power) cc'ed now.
>> .

[RFC v3] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 4621 bytes --]

Added the ethdev dump API which provides functions for query private info
from device. There exists many private properties in different PMD drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
these properties is important for debug. As the information is private,
the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
v3:
* fix comment.
* add rte_eth_dev_priv_dump to version.map file.

v2:
* fix dump API name
* adjust description in doc.
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 5 files changed, 58 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index 6d99d1eaa9..4f97df942d 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -55,6 +55,13 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Added the private ethdev dump API, for query private info of ethdev.**
+
+  Added the private ethdev dump API which provides functions for query
+  private info from device. There exists many private properties in
+  different PMD drivers. The information of these properties is important
+  for debug. As the information is private, the new API is introduced.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index d95605a355..0e3d99b07e 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1200,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump ethdev private info */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index a1d475a292..9fc6d91d76 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6472,6 +6472,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
+
+	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 fa299c8ad7..d17e0f177d 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5888,6 +5888,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index c2fb0669a4..f29c60eda4 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,9 @@ EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	# added in 22.03
+	rte_eth_dev_priv_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [RFC v3] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

在 2022/2/3 21:21, Ferruh Yigit 写道:
> On 1/25/2022 1:45 PM, Min Hu (Connor) wrote:
>> Hi, Ferruh,
>>
>> 在 2022/1/25 20:58, Ferruh Yigit 写道:
>>> On 1/25/2022 12:56 PM, Ferruh Yigit wrote:
>>>> On 1/18/2022 3:34 PM, Ajit Khaparde wrote:
>>>>> On Wed, Jan 12, 2022 at 4:06 AM Ray Kinsella <mdr@ashroe.eu> wrote:
>>>>>>
>>>>>>
>>>>>> Min Hu (Connor) <humin29@huawei.com> writes:
>>>>>>
>>>>>>> Added the ethdev dump API which provides functions for query 
>>>>>>> private info
>>>>>>> from device. There exists many private properties in different 
>>>>>>> PMD drivers,
>>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The 
>>>>>>> information of
>>>>>>> these properties is important for debug. As the information is 
>>>>>>> private,
>>>>>>> the new API is introduced.
>>>>>>>
>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>> Acked-by: Morten BrÞrup <mb@smartsharesystems.com>
>>>>>>> ---
>>>>>>> v3:
>>>>>>> * fix comment.
>>>>>>> * add rte_eth_dev_priv_dump to version.map file.
>>>>>>>
>>>>>>> v2:
>>>>>>> * fix dump API name
>>>>>>> * adjust description in doc.
>>>>>>> ---
>>>>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>>>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>>>>>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>>>>>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>>>>>>   lib/ethdev/version.map                 |  3 +++
>>>>>>>   5 files changed, 58 insertions(+)
>>>>>>>
>>>>>>
>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>
>>>>
>>>> I have a concern and this "private info", it can be useful for debug
>>>> but still it will lead to a PMD specific applications, I wonder if some
>>>> common information can be provided.
>> The purpose of introducing the API is to enhance dump ability for NICs.
>> It will be used in APP proc-info, I will add "show-port-private" option
>> for it. As for common information, I will add them in "show-port" option
>> which already exists.
>>
> 
> Hi Connor,
> 
> We briefly touch on this patch in today's release status meeting,
> as far as I get general consensus was this can be good for debugging
> issues offline (instead of application parsing the output of API).
> 
> So can you please send a new, non-RFC, version of the patch?
> And please keep the existing ack/review tags.
Hi, Ferruh,
	Non-RFC version of the patch has been sent, please check it out.
Thanks.
> 
>>
>>   Is there any list in your mind what
>>>> can be part of this private info?
>> Private info for NICs, take HNS3 PMD NICS as an example,
>>    - Dev Capability:
>>            -- support DCB: no
>>            -- support COPPER: no
>>            -- support FD QUEUE REGION: no
>>            -- support PTP: no
>>            -- support TX PUSH: no
>>            -- support INDEP TXRX: no
>>            -- support STASH: no
>>            -- support SIMPLE BD: no
>>            -- support RXD Advanced Layout: no
>>            -- support OUTER UDP CKSUM: no
>>            -- support RAS IMP: no
>>            -- support TM: no
>>            -- support VF VLAN FILTER MOD: no
>>    - VLAN Config Info:
>>            -- Port VLAN filter configuration
>>                 nic_ingress           :Disable
>>                 nic_egress            :Disable
>>            -- VF VLAN filter configuration
>>                 nic_ingress           :Disable
>>                 nic_egress            :Disable
>>            -- RX VLAN configuration
>>                 vlan1_strip_en        :Disable
>>                 vlan2_strip_en        :Disable
>>                 vlan1_vlan_prionly    :Disable
>>                 vlan2_vlan_prionly    :Disable
>>                 vlan1_strip_discard   :Disable
>>                 vlan2_strip_discard   :Disable
>>            -- TX VLAN configuration
>>                 accept_tag1           :Enable
>>                 accept_untag1         :Enable
>>                 insert_tag1_en        :Disable
>>                 default_vlan_tag1 = 0, qos = 0
>>                 accept_tag2           :Enable
>>                 accept_untag2         :Enable
>>                 insert_tag2_en        :Disable
>>                 default_vlan_tag2 = 0, qos = 0
>>                 vlan_shift_mode       :Disable
>>            -- pvid status: off
>>    - Fdir Info:
>>            -- mode=0 max_key_len=400 rule_num:512 cnt_num:32
>>            -- key_sel=1 tuple_active=0x3bdfd890 meta_data_active=0xd0
>>            -- ipv6_word_en: in_s=3 in_d=3 out_s=0 out_d=0
>> ....
>>
>> These info is private for hns3 PMD.
>>
>>
>>>>
>>>> Also why not use existing xstats or telemetry to get more data from the
>>>> drivers?
>> Xstats is used to query statistics rather than state. Like above info,
>> many keys is "enable" or "disable". It is not appropriated to put the
>> date in xstats.
>>
>> Telemetry is used to query common info for NICs, not for private info.
>>   It is not also appropriated to put private date in telemetry.
>>
>>
>>
>>>> As synced with Ciara (cc'ed), she mentioned more information can be get
>>>> via registering callbacks to ethdev.
>>>
>>> Opps, Ciara (Power) cc'ed now.
>>> .
> 
> .

[PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Added the ethdev dump API which provides functions for query private info
from device. There exists many private properties in different PMD drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
these properties is important for debug. As the information is private,
the new API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
 lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 5 files changed, 58 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index b20716c521..5895194cde 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -92,6 +92,13 @@ New Features
   * Called ``rte_ipv4/6_udptcp_cksum_mbuf()`` functions in testpmd csum mode
     to support software UDP/TCP checksum over multiple segments.
 
+* **Added the private ethdev dump API, for query private info of ethdev.**
+
+  Added the private ethdev dump API which provides functions for query
+  private info from device. There exists many private properties in
+  different PMD drivers. The information of these properties is important
+  for debug. As the information is private, the new API is introduced.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 7d27781f7d..d41d8a2c9d 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param dev
+ *   Port (ethdev) handle.
+ *
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1200,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump ethdev private info */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 917a320afa..07838cc2d9 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6485,6 +6485,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
+
+	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 147cc1ced3..1c00fdbcd2 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5902,6 +5902,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev private info to a file.
+ *
+ * @param file
+ *   A pointer to a file for output.
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @return
+ *   Negative errno value on error, positive value on success.
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1f7359c846..376ea139aa 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,9 @@ EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	# added in 22.03
+	rte_eth_dev_priv_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [PATCH] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
> Added the ethdev dump API which provides functions for query private info

Isn't API and function are same thing in this contexts?

> from device. There exists many private properties in different PMD drivers,
> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The information of
> these properties is important for debug. As the information is private,
> the new API is introduced.>

In the patch title 'ethdev' is duplicated, can you fix it?

  
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>   lib/ethdev/version.map                 |  3 +++
>   5 files changed, 58 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
> index b20716c521..5895194cde 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -92,6 +92,13 @@ New Features
>     * Called ``rte_ipv4/6_udptcp_cksum_mbuf()`` functions in testpmd csum mode
>       to support software UDP/TCP checksum over multiple segments.
>   
> +* **Added the private ethdev dump API, for query private info of ethdev.**
> +
> +  Added the private ethdev dump API which provides functions for query
> +  private info from device. There exists many private properties in
> +  different PMD drivers. The information of these properties is important
> +  for debug. As the information is private, the new API is introduced.
> +
>   

Can you please move the update up, just above the ethdev PMD updates?

>   Removed Items
>   -------------
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 7d27781f7d..d41d8a2c9d 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>   				       uint64_t *features);
>   
> +/**
> + * @internal
> + * Dump ethdev private info to a file.
> + *

It doesn't dump the 'ethdev' private info, it dumps the private info from device.

> + * @param file
> + *   A pointer to a file for output.
> + * @param dev
> + *   Port (ethdev) handle.
> + *
> + * @return
> + *   Negative errno value on error, positive value on success.
> + */
> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
> +
>   /**
>    * @internal A structure containing the functions exported by an Ethernet driver.
>    */
> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>   	 * kinds of metadata to the PMD
>   	 */
>   	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> +
> +	/** Dump ethdev private info */

Ditto, not 'ethdev' private info.

> +	eth_dev_priv_dump_t eth_dev_priv_dump;
>   };
>   
>   /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 917a320afa..07838cc2d9 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -6485,6 +6485,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
>   		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
>   }
>   
> +int
> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
> +{
> +	struct rte_eth_dev *dev;
> +	int ret;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +

What do you think to check 'file' pointer against NULL before passing it to
dev_ops?

> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
> +	ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);

Should 'eth_err()' used here?

> +
> +	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 147cc1ced3..1c00fdbcd2 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5902,6 +5902,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>   	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>   }
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump ethdev private info to a file.
> + *

Ditto.

Should we highlight that provided data and the order depends on the PMD?

> + * @param file
> + *   A pointer to a file for output.
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @return
> + *   Negative errno value on error, positive value on success.


What does 'errno' mean here, does the API cause errno to be set and return it?


Also it is possible to list possible specific error values, as done with other
API documentation, like:
(0) if successful.
(-ENODEV) if *port_id* is invalid.
(-ENOTSUP) if hardware doesn't support.
....

> + */
> +__rte_experimental
> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> +

What do you think to have the 'port_id' as first argument to be consistent
with the other APIs?

>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 1f7359c846..376ea139aa 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -256,6 +256,9 @@ EXPERIMENTAL {
>   	rte_flow_flex_item_create;
>   	rte_flow_flex_item_release;
>   	rte_flow_pick_transfer_proxy;
> +
> +	# added in 22.03
> +	rte_eth_dev_priv_dump;
>   };
>   
>   INTERNAL {

RE: [PATCH] ethdev: introduce ethdev dump API

[Morten Brørup]

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, 7 February 2022 12.46
> 
> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
> > Added the ethdev dump API which provides functions for query private
> info
> 
> Isn't API and function are same thing in this contexts?
> 
> > from device. There exists many private properties in different PMD
> drivers,
> > such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> information of
> > these properties is important for debug. As the information is
> private,
> > the new API is introduced.>
> 
> In the patch title 'ethdev' is duplicated, can you fix it?
> 
> 
> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Ray Kinsella <mdr@ashroe.eu>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

[...]

> > @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
> rte_eth_dev *dev,
> >   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
> >   				       uint64_t *features);
> >
> > +/**
> > + * @internal
> > + * Dump ethdev private info to a file.
> > + *
> 
> It doesn't dump the 'ethdev' private info, it dumps the private info
> from device.

It seems perfectly clear to me. How would you prefer it phrased instead?

[...]

> 
> > + */
> > +__rte_experimental
> > +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> > +
> 
> What do you think to have the 'port_id' as first argument to be
> consistent
> with the other APIs?

The _dump APIs in other libraries have the file pointer as the first parameter, so let's follow that convention here too. No need to move the port_id parameter here.

Only rte_dma_dump() has the file pointer last, and I didn't catch it when the function was defined.

Re: [PATCH] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 2/7/2022 12:18 PM, Morten Brørup wrote:
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Monday, 7 February 2022 12.46
>>
>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>> Added the ethdev dump API which provides functions for query private
>> info
>>
>> Isn't API and function are same thing in this contexts?
>>
>>> from device. There exists many private properties in different PMD
>> drivers,
>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>> information of
>>> these properties is important for debug. As the information is
>> private,
>>> the new API is introduced.>
>>
>> In the patch title 'ethdev' is duplicated, can you fix it?
>>
>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 
> [...]
> 
>>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct
>> rte_eth_dev *dev,
>>>    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>>    				       uint64_t *features);
>>>
>>> +/**
>>> + * @internal
>>> + * Dump ethdev private info to a file.
>>> + *
>>
>> It doesn't dump the 'ethdev' private info, it dumps the private info
>> from device.
> 
> It seems perfectly clear to me. How would you prefer it phrased instead?
> 

What described in the document is more accurate,
"query private info from device".

What we are dumping here is not ethdev private info, it is device private info,
and we really don't know what that data may be in the ethdev layer.

Also there is a chance that 'ethdev private info' can be confused with
'ethdev->data->dev_private'

> [...]
> 
>>
>>> + */
>>> +__rte_experimental
>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>> +
>>
>> What do you think to have the 'port_id' as first argument to be
>> consistent
>> with the other APIs?
> 
> The _dump APIs in other libraries have the file pointer as the first parameter, so let's follow that convention here too. No need to move the port_id parameter here.
> 

Yes, for most of the _dump() APIs, file pointer seems is the first argument,
bu they are from various libraries.

Within the ethdev APIs, I think it makes sense that all APIs start with
'port_id' parameter for consistency, like done in:
rte_flow_dev_dump(uint16_t port_id, ...)

> Only rte_dma_dump() has the file pointer last, and I didn't catch it when the function was defined.
> 

RE: [PATCH] ethdev: introduce ethdev dump API

[Morten Brørup]

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, 7 February 2022 13.36
> 
> On 2/7/2022 12:18 PM, Morten Brørup wrote:
> >> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> >> Sent: Monday, 7 February 2022 12.46
> >>
> >> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
> >>> Added the ethdev dump API which provides functions for query
> private
> >> info
> >>
> >> Isn't API and function are same thing in this contexts?
> >>
> >>> from device. There exists many private properties in different PMD
> >> drivers,
> >>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
> >> information of
> >>> these properties is important for debug. As the information is
> >> private,
> >>> the new API is introduced.>
> >>
> >> In the patch title 'ethdev' is duplicated, can you fix it?
> >>
> >>
> >>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> >>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >
> > [...]
> >
> >>> @@ -990,6 +990,20 @@ typedef int
> (*eth_representor_info_get_t)(struct
> >> rte_eth_dev *dev,
> >>>    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
> *dev,
> >>>    				       uint64_t *features);
> >>>
> >>> +/**
> >>> + * @internal
> >>> + * Dump ethdev private info to a file.
> >>> + *
> >>
> >> It doesn't dump the 'ethdev' private info, it dumps the private info
> >> from device.
> >
> > It seems perfectly clear to me. How would you prefer it phrased
> instead?
> >
> 
> What described in the document is more accurate,
> "query private info from device".
> 
> What we are dumping here is not ethdev private info, it is device
> private info,
> and we really don't know what that data may be in the ethdev layer.
> 
> Also there is a chance that 'ethdev private info' can be confused with
> 'ethdev->data->dev_private'

OK. Now I got your point! The difference is very subtle.

> 
> > [...]
> >
> >>
> >>> + */
> >>> +__rte_experimental
> >>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
> >>> +
> >>
> >> What do you think to have the 'port_id' as first argument to be
> >> consistent
> >> with the other APIs?
> >
> > The _dump APIs in other libraries have the file pointer as the first
> parameter, so let's follow that convention here too. No need to move
> the port_id parameter here.
> >
> 
> Yes, for most of the _dump() APIs, file pointer seems is the first
> argument,
> bu they are from various libraries.
> 
> Within the ethdev APIs, I think it makes sense that all APIs start with
> 'port_id' parameter for consistency, like done in:
> rte_flow_dev_dump(uint16_t port_id, ...)
> 
> > Only rte_dma_dump() has the file pointer last, and I didn't catch it
> when the function was defined.
> >

OK. Then I agree with you about following the convention like rte_flow_dev_dump() with the port_id first.

I even think Connor got it right the first time, and I proposed following the other convention.

It's not easy when there are two opposite conventions. :-)

-Morten

Re: [PATCH] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 2/7/2022 12:56 PM, Morten Brørup wrote:
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Monday, 7 February 2022 13.36
>>
>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>> Sent: Monday, 7 February 2022 12.46
>>>>
>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>> Added the ethdev dump API which provides functions for query
>> private
>>>> info
>>>>
>>>> Isn't API and function are same thing in this contexts?
>>>>
>>>>> from device. There exists many private properties in different PMD
>>>> drivers,
>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>> information of
>>>>> these properties is important for debug. As the information is
>>>> private,
>>>>> the new API is introduced.>
>>>>
>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>
>>>>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>
>>> [...]
>>>
>>>>> @@ -990,6 +990,20 @@ typedef int
>> (*eth_representor_info_get_t)(struct
>>>> rte_eth_dev *dev,
>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>> *dev,
>>>>>     				       uint64_t *features);
>>>>>
>>>>> +/**
>>>>> + * @internal
>>>>> + * Dump ethdev private info to a file.
>>>>> + *
>>>>
>>>> It doesn't dump the 'ethdev' private info, it dumps the private info
>>>> from device.
>>>
>>> It seems perfectly clear to me. How would you prefer it phrased
>> instead?
>>>
>>
>> What described in the document is more accurate,
>> "query private info from device".
>>
>> What we are dumping here is not ethdev private info, it is device
>> private info,
>> and we really don't know what that data may be in the ethdev layer.
>>
>> Also there is a chance that 'ethdev private info' can be confused with
>> 'ethdev->data->dev_private'
> 
> OK. Now I got your point! The difference is very subtle.
> 
>>
>>> [...]
>>>
>>>>
>>>>> + */
>>>>> +__rte_experimental
>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>> +
>>>>
>>>> What do you think to have the 'port_id' as first argument to be
>>>> consistent
>>>> with the other APIs?
>>>
>>> The _dump APIs in other libraries have the file pointer as the first
>> parameter, so let's follow that convention here too. No need to move
>> the port_id parameter here.
>>>
>>
>> Yes, for most of the _dump() APIs, file pointer seems is the first
>> argument,
>> bu they are from various libraries.
>>
>> Within the ethdev APIs, I think it makes sense that all APIs start with
>> 'port_id' parameter for consistency, like done in:
>> rte_flow_dev_dump(uint16_t port_id, ...)
>>
>>> Only rte_dma_dump() has the file pointer last, and I didn't catch it
>> when the function was defined.
>>>
> 
> OK. Then I agree with you about following the convention like rte_flow_dev_dump() with the port_id first.
> 
> I even think Connor got it right the first time, and I proposed following the other convention.
> 

Ahh, may bad I missed that, sorry for not commenting on time.


> It's not easy when there are two opposite conventions. :-)
> 

Yep, that is the main issue.

Re: [PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/2/7 23:35, Ferruh Yigit 写道:
> On 2/7/2022 12:56 PM, Morten Brørup wrote:
>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>> Sent: Monday, 7 February 2022 13.36
>>>
>>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>> Sent: Monday, 7 February 2022 12.46
>>>>>
>>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>>> Added the ethdev dump API which provides functions for query
>>> private
>>>>> info
>>>>>
>>>>> Isn't API and function are same thing in this contexts?
>>>>>
>>>>>> from device. There exists many private properties in different PMD
>>>>> drivers,
>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>>> information of
>>>>>> these properties is important for debug. As the information is
>>>>> private,
>>>>>> the new API is introduced.>
>>>>>
>>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>>
>>>>>
>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>
>>>> [...]
>>>>
>>>>>> @@ -990,6 +990,20 @@ typedef int
>>> (*eth_representor_info_get_t)(struct
>>>>> rte_eth_dev *dev,
>>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>>> *dev,
>>>>>>                            uint64_t *features);
>>>>>>
>>>>>> +/**
>>>>>> + * @internal
>>>>>> + * Dump ethdev private info to a file.
>>>>>> + *
>>>>>
>>>>> It doesn't dump the 'ethdev' private info, it dumps the private info
>>>>> from device.
>>>>
>>>> It seems perfectly clear to me. How would you prefer it phrased
>>> instead?
>>>>
>>>
>>> What described in the document is more accurate,
>>> "query private info from device".
>>>
>>> What we are dumping here is not ethdev private info, it is device
>>> private info,

what is the difference between ethdev and device?
>>> and we really don't know what that data may be in the ethdev layer.
>>>
>>> Also there is a chance that 'ethdev private info' can be confused with
>>> 'ethdev->data->dev_private'
what I want to dump is exactly the 'ethdev->data->dev_private'.
'ethdev private info' means 'ethdev->data->dev_private'.
why confused?
>>
>> OK. Now I got your point! The difference is very subtle.
>>
>>>
>>>> [...]
>>>>
>>>>>
>>>>>> + */
>>>>>> +__rte_experimental
>>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>>> +
>>>>>
>>>>> What do you think to have the 'port_id' as first argument to be
>>>>> consistent
>>>>> with the other APIs?
>>>>
>>>> The _dump APIs in other libraries have the file pointer as the first
>>> parameter, so let's follow that convention here too. No need to move
>>> the port_id parameter here.
>>>>
>>>
>>> Yes, for most of the _dump() APIs, file pointer seems is the first
>>> argument,
>>> bu they are from various libraries.
>>>
>>> Within the ethdev APIs, I think it makes sense that all APIs start with
>>> 'port_id' parameter for consistency, like done in:
>>> rte_flow_dev_dump(uint16_t port_id, ...)
>>>
>>>> Only rte_dma_dump() has the file pointer last, and I didn't catch it
>>> when the function was defined.
>>>>
>>
>> OK. Then I agree with you about following the convention like 
>> rte_flow_dev_dump() with the port_id first.
>>
>> I even think Connor got it right the first time, and I proposed 
>> following the other convention.
>>
> 
> Ahh, may bad I missed that, sorry for not commenting on time.
> 
> 
>> It's not easy when there are two opposite conventions. :-)
>>
> 
> Yep, that is the main issue.
> 
> .

[PATCH v2] ethdev: introduce dump API

[Min Hu (Connor)]

Added the ethdev dump API which provides querying private info from ethdev.
There exists many private properties in different PMD drivers, such as
adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
properties is important for debug. As the information is private, the new
API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
v2:
* fixed comments from Ferruh.
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 5 files changed, 70 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index b20716c521..96c9b404c5 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -64,6 +64,13 @@ New Features
     - ``rte_ipv6_udptcp_cksum_mbuf()``
     - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
+* **Added the private ethdev dump API, for query private info of ethdev.**
+
+  Added the private ethdev dump API which provides functions for query
+  private info from device. There exists many private properties in
+  different PMD drivers. The information of these properties is important
+  for debug. As the information is private, the new API is introduced.
+
 * **Updated AF_XDP PMD**
 
   * Added support for libxdp >=v1.2.2.
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 7d27781f7d..75c3cd41cf 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,26 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump private info from ethdev to a file.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param file
+ *   A pointer to a file for output.
+ *
+ * @retval 0
+ *   Success
+ * @retval -EINVAL
+ *   Invalid file
+ * @retval -ENOTSUP
+ *   Not supported
+ * @retval -ENODEV
+ *   Invalid port_id
+ */
+typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1206,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump private info from ethdev */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 917a320afa..5ee316a3c0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6485,6 +6485,23 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, 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 147cc1ced3..ea60d4099b 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5902,6 +5902,26 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump private info from ethdev to a file. Provided data and the order depends
+ * on the PMD.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   - (-ENODEV) if *port_id* is invalid.
+ *   - (-EINVAL) if null file.
+ *   - (-ENOTSUP) if the device does not support this function.
+ *   - (0) on success
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1f7359c846..376ea139aa 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,9 @@ EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	# added in 22.03
+	rte_eth_dev_priv_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

HI, Ferruh,
   v2 has been sent according comments, please check it, thanks.

在 2022/2/7 19:46, Ferruh Yigit 写道:
> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>> Added the ethdev dump API which provides functions for query private info
> 
> Isn't API and function are same thing in this contexts?
> 
>> from device. There exists many private properties in different PMD 
>> drivers,
>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The 
>> information of
>> these properties is important for debug. As the information is private,
>> the new API is introduced.>
> 
> In the patch title 'ethdev' is duplicated, can you fix it?
> 
> 
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> ---
>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>   lib/ethdev/ethdev_driver.h             | 17 +++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 15 +++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 16 ++++++++++++++++
>>   lib/ethdev/version.map                 |  3 +++
>>   5 files changed, 58 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_03.rst 
>> b/doc/guides/rel_notes/release_22_03.rst
>> index b20716c521..5895194cde 100644
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -92,6 +92,13 @@ New Features
>>     * Called ``rte_ipv4/6_udptcp_cksum_mbuf()`` functions in testpmd 
>> csum mode
>>       to support software UDP/TCP checksum over multiple segments.
>> +* **Added the private ethdev dump API, for query private info of 
>> ethdev.**
>> +
>> +  Added the private ethdev dump API which provides functions for query
>> +  private info from device. There exists many private properties in
>> +  different PMD drivers. The information of these properties is 
>> important
>> +  for debug. As the information is private, the new API is introduced.
>> +
> 
> Can you please move the update up, just above the ethdev PMD updates?
> 
>>   Removed Items
>>   -------------
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 7d27781f7d..d41d8a2c9d 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct 
>> rte_eth_dev *dev,
>>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>                          uint64_t *features);
>> +/**
>> + * @internal
>> + * Dump ethdev private info to a file.
>> + *
> 
> It doesn't dump the 'ethdev' private info, it dumps the private info 
> from device.
> 
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + *
>> + * @return
>> + *   Negative errno value on error, positive value on success.
>> + */
>> +typedef int (*eth_dev_priv_dump_t)(FILE *file, struct rte_eth_dev *dev);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an 
>> Ethernet driver.
>>    */
>> @@ -1186,6 +1200,9 @@ struct eth_dev_ops {
>>        * kinds of metadata to the PMD
>>        */
>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>> +
>> +    /** Dump ethdev private info */
> 
> Ditto, not 'ethdev' private info.
> 
>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
>>   };
>>   /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 917a320afa..07838cc2d9 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -6485,6 +6485,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, 
>> uint64_t *features)
>>                  (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
>>   }
>> +int
>> +rte_eth_dev_priv_dump(FILE *file, uint16_t port_id)
>> +{
>> +    struct rte_eth_dev *dev;
>> +    int ret;
>> +
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +    dev = &rte_eth_devices[port_id];
>> +
> 
> What do you think to check 'file' pointer against NULL before passing it to
> dev_ops?
> 
>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
>> +    ret = (*dev->dev_ops->eth_dev_priv_dump)(file, dev);
> 
> Should 'eth_err()' used here?
> 
>> +
>> +    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 147cc1ced3..1c00fdbcd2 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -5902,6 +5902,22 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t 
>> queue_id,
>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>   }
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior 
>> notice
>> + *
>> + * Dump ethdev private info to a file.
>> + *
> 
> Ditto.
> 
> Should we highlight that provided data and the order depends on the PMD?
> 
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @return
>> + *   Negative errno value on error, positive value on success.
> 
> 
> What does 'errno' mean here, does the API cause errno to be set and 
> return it?
> 
> 
> Also it is possible to list possible specific error values, as done with 
> other
> API documentation, like:
> (0) if successful.
> (-ENODEV) if *port_id* is invalid.
> (-ENOTSUP) if hardware doesn't support.
> ....
> 
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>> +
> 
> What do you think to have the 'port_id' as first argument to be consistent
> with the other APIs?
> 
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 1f7359c846..376ea139aa 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -256,6 +256,9 @@ EXPERIMENTAL {
>>       rte_flow_flex_item_create;
>>       rte_flow_flex_item_release;
>>       rte_flow_pick_transfer_proxy;
>> +
>> +    # added in 22.03
>> +    rte_eth_dev_priv_dump;
>>   };
>>   INTERNAL {
> 
> .

Re: [PATCH] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 2/8/2022 12:39 AM, Min Hu (Connor) wrote:
> Hi, Ferruh,
> 
> 在 2022/2/7 23:35, Ferruh Yigit 写道:
>> On 2/7/2022 12:56 PM, Morten Brørup wrote:
>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>> Sent: Monday, 7 February 2022 13.36
>>>>
>>>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>> Sent: Monday, 7 February 2022 12.46
>>>>>>
>>>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>>>> Added the ethdev dump API which provides functions for query
>>>> private
>>>>>> info
>>>>>>
>>>>>> Isn't API and function are same thing in this contexts?
>>>>>>
>>>>>>> from device. There exists many private properties in different PMD
>>>>>> drivers,
>>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>>>> information of
>>>>>>> these properties is important for debug. As the information is
>>>>>> private,
>>>>>>> the new API is introduced.>
>>>>>>
>>>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>>>
>>>>>>
>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>
>>>>> [...]
>>>>>
>>>>>>> @@ -990,6 +990,20 @@ typedef int
>>>> (*eth_representor_info_get_t)(struct
>>>>>> rte_eth_dev *dev,
>>>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>>>> *dev,
>>>>>>>                            uint64_t *features);
>>>>>>>
>>>>>>> +/**
>>>>>>> + * @internal
>>>>>>> + * Dump ethdev private info to a file.
>>>>>>> + *
>>>>>>
>>>>>> It doesn't dump the 'ethdev' private info, it dumps the private info
>>>>>> from device.
>>>>>
>>>>> It seems perfectly clear to me. How would you prefer it phrased
>>>> instead?
>>>>>
>>>>
>>>> What described in the document is more accurate,
>>>> "query private info from device".
>>>>
>>>> What we are dumping here is not ethdev private info, it is device
>>>> private info,
> 
> what is the difference between ethdev and device?

It is not very clear, but for me 'ethdev' is refers to device abstract
layer (ethdev library) specific private data and device refers to ethdev
device (PMD) private data. ethdev is common for all drivers.

>>>> and we really don't know what that data may be in the ethdev layer.
>>>>
>>>> Also there is a chance that 'ethdev private info' can be confused with
>>>> 'ethdev->data->dev_private'
> what I want to dump is exactly the 'ethdev->data->dev_private'.
> 'ethdev private info' means 'ethdev->data->dev_private'.
> why confused?

What I understand was this API can return any device private information,
it is not limited to 'ethdev->data->dev_private', (although most of the data
is represented in this struct), like if you want to dump queue state,
this is out of 'ethdev->data->dev_private'.

>>>
>>> OK. Now I got your point! The difference is very subtle.
>>>
>>>>
>>>>> [...]
>>>>>
>>>>>>
>>>>>>> + */
>>>>>>> +__rte_experimental
>>>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>>>> +
>>>>>>
>>>>>> What do you think to have the 'port_id' as first argument to be
>>>>>> consistent
>>>>>> with the other APIs?
>>>>>
>>>>> The _dump APIs in other libraries have the file pointer as the first
>>>> parameter, so let's follow that convention here too. No need to move
>>>> the port_id parameter here.
>>>>>
>>>>
>>>> Yes, for most of the _dump() APIs, file pointer seems is the first
>>>> argument,
>>>> bu they are from various libraries.
>>>>
>>>> Within the ethdev APIs, I think it makes sense that all APIs start with
>>>> 'port_id' parameter for consistency, like done in:
>>>> rte_flow_dev_dump(uint16_t port_id, ...)
>>>>
>>>>> Only rte_dma_dump() has the file pointer last, and I didn't catch it
>>>> when the function was defined.
>>>>>
>>>
>>> OK. Then I agree with you about following the convention like rte_flow_dev_dump() with the port_id first.
>>>
>>> I even think Connor got it right the first time, and I proposed following the other convention.
>>>
>>
>> Ahh, may bad I missed that, sorry for not commenting on time.
>>
>>
>>> It's not easy when there are two opposite conventions. :-)
>>>
>>
>> Yep, that is the main issue.
>>
>> .

Re: [PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/2/8 18:21, Ferruh Yigit 写道:
> On 2/8/2022 12:39 AM, Min Hu (Connor) wrote:
>> Hi, Ferruh,
>>
>> 在 2022/2/7 23:35, Ferruh Yigit 写道:
>>> On 2/7/2022 12:56 PM, Morten Brørup wrote:
>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>> Sent: Monday, 7 February 2022 13.36
>>>>>
>>>>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>>> Sent: Monday, 7 February 2022 12.46
>>>>>>>
>>>>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>>>>> Added the ethdev dump API which provides functions for query
>>>>> private
>>>>>>> info
>>>>>>>
>>>>>>> Isn't API and function are same thing in this contexts?
>>>>>>>
>>>>>>>> from device. There exists many private properties in different PMD
>>>>>>> drivers,
>>>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>>>>> information of
>>>>>>>> these properties is important for debug. As the information is
>>>>>>> private,
>>>>>>>> the new API is introduced.>
>>>>>>>
>>>>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>> @@ -990,6 +990,20 @@ typedef int
>>>>> (*eth_representor_info_get_t)(struct
>>>>>>> rte_eth_dev *dev,
>>>>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>>>>> *dev,
>>>>>>>>                            uint64_t *features);
>>>>>>>>
>>>>>>>> +/**
>>>>>>>> + * @internal
>>>>>>>> + * Dump ethdev private info to a file.
>>>>>>>> + *
>>>>>>>
>>>>>>> It doesn't dump the 'ethdev' private info, it dumps the private info
>>>>>>> from device.
>>>>>>
>>>>>> It seems perfectly clear to me. How would you prefer it phrased
>>>>> instead?
>>>>>>
>>>>>
>>>>> What described in the document is more accurate,
>>>>> "query private info from device".
>>>>>
>>>>> What we are dumping here is not ethdev private info, it is device
>>>>> private info,
>>
>> what is the difference between ethdev and device?
> 
> It is not very clear, but for me 'ethdev' is refers to device abstract
> layer (ethdev library) specific private data 
Could you give an example for 'ethdev'specific private data ?

and device refers to ethdev
> device (PMD) private data. ethdev is common for all drivers.
OK, we could treat it as convention in future.
> 
>>>>> and we really don't know what that data may be in the ethdev layer.
>>>>>
>>>>> Also there is a chance that 'ethdev private info' can be confused with
>>>>> 'ethdev->data->dev_private'
>> what I want to dump is exactly the 'ethdev->data->dev_private'.
>> 'ethdev private info' means 'ethdev->data->dev_private'.
>> why confused?
> 
> What I understand was this API can return any device private information,
> it is not limited to 'ethdev->data->dev_private', (although most of the 
I think this API is limited to 'ethdev->data->dev_private'.
> data
> is represented in this struct), like if you want to dump queue state,
> this is out of 'ethdev->data->dev_private'.
Queue state can be dumped using API 'rte_eth_tx_queue_info_get'.

> 
>>>>
>>>> OK. Now I got your point! The difference is very subtle.
>>>>
>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>
>>>>>>>> + */
>>>>>>>> +__rte_experimental
>>>>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>>>>> +
>>>>>>>
>>>>>>> What do you think to have the 'port_id' as first argument to be
>>>>>>> consistent
>>>>>>> with the other APIs?
>>>>>>
>>>>>> The _dump APIs in other libraries have the file pointer as the first
>>>>> parameter, so let's follow that convention here too. No need to move
>>>>> the port_id parameter here.
>>>>>>
>>>>>
>>>>> Yes, for most of the _dump() APIs, file pointer seems is the first
>>>>> argument,
>>>>> bu they are from various libraries.
>>>>>
>>>>> Within the ethdev APIs, I think it makes sense that all APIs start 
>>>>> with
>>>>> 'port_id' parameter for consistency, like done in:
>>>>> rte_flow_dev_dump(uint16_t port_id, ...)
>>>>>
>>>>>> Only rte_dma_dump() has the file pointer last, and I didn't catch it
>>>>> when the function was defined.
>>>>>>
>>>>
>>>> OK. Then I agree with you about following the convention like 
>>>> rte_flow_dev_dump() with the port_id first.
>>>>
>>>> I even think Connor got it right the first time, and I proposed 
>>>> following the other convention.
>>>>
>>>
>>> Ahh, may bad I missed that, sorry for not commenting on time.
>>>
>>>
>>>> It's not easy when there are two opposite conventions. :-)
>>>>
>>>
>>> Yep, that is the main issue.
>>>
>>> .
> 
> .

Re: [PATCH] ethdev: introduce ethdev dump API

[Ferruh Yigit]

On 2/8/2022 11:14 AM, Min Hu (Connor) wrote:
> Hi, Ferruh,
> 
> 在 2022/2/8 18:21, Ferruh Yigit 写道:
>> On 2/8/2022 12:39 AM, Min Hu (Connor) wrote:
>>> Hi, Ferruh,
>>>
>>> 在 2022/2/7 23:35, Ferruh Yigit 写道:
>>>> On 2/7/2022 12:56 PM, Morten Brørup wrote:
>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>> Sent: Monday, 7 February 2022 13.36
>>>>>>
>>>>>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>>>> Sent: Monday, 7 February 2022 12.46
>>>>>>>>
>>>>>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>>>>>> Added the ethdev dump API which provides functions for query
>>>>>> private
>>>>>>>> info
>>>>>>>>
>>>>>>>> Isn't API and function are same thing in this contexts?
>>>>>>>>
>>>>>>>>> from device. There exists many private properties in different PMD
>>>>>>>> drivers,
>>>>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>>>>>> information of
>>>>>>>>> these properties is important for debug. As the information is
>>>>>>>> private,
>>>>>>>>> the new API is introduced.>
>>>>>>>>
>>>>>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>>>>>
>>>>>>>>
>>>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>> @@ -990,6 +990,20 @@ typedef int
>>>>>> (*eth_representor_info_get_t)(struct
>>>>>>>> rte_eth_dev *dev,
>>>>>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>>>>>> *dev,
>>>>>>>>>                            uint64_t *features);
>>>>>>>>>
>>>>>>>>> +/**
>>>>>>>>> + * @internal
>>>>>>>>> + * Dump ethdev private info to a file.
>>>>>>>>> + *
>>>>>>>>
>>>>>>>> It doesn't dump the 'ethdev' private info, it dumps the private info
>>>>>>>> from device.
>>>>>>>
>>>>>>> It seems perfectly clear to me. How would you prefer it phrased
>>>>>> instead?
>>>>>>>
>>>>>>
>>>>>> What described in the document is more accurate,
>>>>>> "query private info from device".
>>>>>>
>>>>>> What we are dumping here is not ethdev private info, it is device
>>>>>> private info,
>>>
>>> what is the difference between ethdev and device?
>>
>> It is not very clear, but for me 'ethdev' is refers to device abstract
>> layer (ethdev library) specific private data 
> Could you give an example for 'ethdev'specific private data ?
> 

I think 'struct rte_eth_dev' content can be a sample.

But I hear you, diff is not clear, it is subtle as Morten said,
when doc and commit log refers it as "private info from device",
I think we can use the same in the API documentation as well.

> and device refers to ethdev
>> device (PMD) private data. ethdev is common for all drivers.
> OK, we could treat it as convention in future.
>>
>>>>>> and we really don't know what that data may be in the ethdev layer.
>>>>>>
>>>>>> Also there is a chance that 'ethdev private info' can be confused with
>>>>>> 'ethdev->data->dev_private'
>>> what I want to dump is exactly the 'ethdev->data->dev_private'.
>>> 'ethdev private info' means 'ethdev->data->dev_private'.
>>> why confused?
>>
>> What I understand was this API can return any device private information,
>> it is not limited to 'ethdev->data->dev_private', (although most of the 
> I think this API is limited to 'ethdev->data->dev_private'.
>> data
>> is represented in this struct), like if you want to dump queue state,
>> this is out of 'ethdev->data->dev_private'.
> Queue state can be dumped using API 'rte_eth_tx_queue_info_get'.
> 

Yes it can be. But as far as I can see there is nothing prevents the dump()
API to provide the same, it is up to PMD.

If the intention is to limit what can be dump to 'ethdev->data->dev_private',
it is not clear from API documentation/implementation.

>>
>>>>>
>>>>> OK. Now I got your point! The difference is very subtle.
>>>>>
>>>>>>
>>>>>>> [...]
>>>>>>>
>>>>>>>>
>>>>>>>>> + */
>>>>>>>>> +__rte_experimental
>>>>>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>>>>>> +
>>>>>>>>
>>>>>>>> What do you think to have the 'port_id' as first argument to be
>>>>>>>> consistent
>>>>>>>> with the other APIs?
>>>>>>>
>>>>>>> The _dump APIs in other libraries have the file pointer as the first
>>>>>> parameter, so let's follow that convention here too. No need to move
>>>>>> the port_id parameter here.
>>>>>>>
>>>>>>
>>>>>> Yes, for most of the _dump() APIs, file pointer seems is the first
>>>>>> argument,
>>>>>> bu they are from various libraries.
>>>>>>
>>>>>> Within the ethdev APIs, I think it makes sense that all APIs start with
>>>>>> 'port_id' parameter for consistency, like done in:
>>>>>> rte_flow_dev_dump(uint16_t port_id, ...)
>>>>>>
>>>>>>> Only rte_dma_dump() has the file pointer last, and I didn't catch it
>>>>>> when the function was defined.
>>>>>>>
>>>>>
>>>>> OK. Then I agree with you about following the convention like rte_flow_dev_dump() with the port_id first.
>>>>>
>>>>> I even think Connor got it right the first time, and I proposed following the other convention.
>>>>>
>>>>
>>>> Ahh, may bad I missed that, sorry for not commenting on time.
>>>>
>>>>
>>>>> It's not easy when there are two opposite conventions. :-)
>>>>>
>>>>
>>>> Yep, that is the main issue.
>>>>
>>>> .
>>
>> .

Re: [PATCH] ethdev: introduce ethdev dump API

[Thomas Monjalon]

08/02/2022 13:59, Ferruh Yigit:
> On 2/8/2022 11:14 AM, Min Hu (Connor) wrote:
> > 在 2022/2/8 18:21, Ferruh Yigit 写道:
> >> What I understand was this API can return any device private information,
> >> it is not limited to 'ethdev->data->dev_private', (although most of the 
> > I think this API is limited to 'ethdev->data->dev_private'.
> >> data
> >> is represented in this struct), like if you want to dump queue state,
> >> this is out of 'ethdev->data->dev_private'.
> > Queue state can be dumped using API 'rte_eth_tx_queue_info_get'.
> > 
> 
> Yes it can be. But as far as I can see there is nothing prevents the dump()
> API to provide the same, it is up to PMD.
> 
> If the intention is to limit what can be dump to 'ethdev->data->dev_private',
> it is not clear from API documentation/implementation.

Why limiting?
The dump could even print debug infos which are nowhere else.

Re: [PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

Hi,

在 2022/2/8 20:59, Ferruh Yigit 写道:
> On 2/8/2022 11:14 AM, Min Hu (Connor) wrote:
>> Hi, Ferruh,
>>
>> 在 2022/2/8 18:21, Ferruh Yigit 写道:
>>> On 2/8/2022 12:39 AM, Min Hu (Connor) wrote:
>>>> Hi, Ferruh,
>>>>
>>>> 在 2022/2/7 23:35, Ferruh Yigit 写道:
>>>>> On 2/7/2022 12:56 PM, Morten Brørup wrote:
>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>>> Sent: Monday, 7 February 2022 13.36
>>>>>>>
>>>>>>> On 2/7/2022 12:18 PM, Morten Brørup wrote:
>>>>>>>>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>>>>>>>>> Sent: Monday, 7 February 2022 12.46
>>>>>>>>>
>>>>>>>>> On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
>>>>>>>>>> Added the ethdev dump API which provides functions for query
>>>>>>> private
>>>>>>>>> info
>>>>>>>>>
>>>>>>>>> Isn't API and function are same thing in this contexts?
>>>>>>>>>
>>>>>>>>>> from device. There exists many private properties in different 
>>>>>>>>>> PMD
>>>>>>>>> drivers,
>>>>>>>>>> such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
>>>>>>>>> information of
>>>>>>>>>> these properties is important for debug. As the information is
>>>>>>>>> private,
>>>>>>>>>> the new API is introduced.>
>>>>>>>>>
>>>>>>>>> In the patch title 'ethdev' is duplicated, can you fix it?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>>>>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>>>>>>>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>>>>>>>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>>> @@ -990,6 +990,20 @@ typedef int
>>>>>>> (*eth_representor_info_get_t)(struct
>>>>>>>>> rte_eth_dev *dev,
>>>>>>>>>>     typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
>>>>>>> *dev,
>>>>>>>>>>                            uint64_t *features);
>>>>>>>>>>
>>>>>>>>>> +/**
>>>>>>>>>> + * @internal
>>>>>>>>>> + * Dump ethdev private info to a file.
>>>>>>>>>> + *
>>>>>>>>>
>>>>>>>>> It doesn't dump the 'ethdev' private info, it dumps the private 
>>>>>>>>> info
>>>>>>>>> from device.
>>>>>>>>
>>>>>>>> It seems perfectly clear to me. How would you prefer it phrased
>>>>>>> instead?
>>>>>>>>
>>>>>>>
>>>>>>> What described in the document is more accurate,
>>>>>>> "query private info from device".
>>>>>>>
>>>>>>> What we are dumping here is not ethdev private info, it is device
>>>>>>> private info,
>>>>
>>>> what is the difference between ethdev and device?
>>>
>>> It is not very clear, but for me 'ethdev' is refers to device abstract
>>> layer (ethdev library) specific private data 
>> Could you give an example for 'ethdev'specific private data ?
>>
> 
> I think 'struct rte_eth_dev' content can be a sample.
> 
> But I hear you, diff is not clear, it is subtle as Morten said,
> when doc and commit log refers it as "private info from device",
> I think we can use the same in the API documentation as well.
> 
Agreed， I will fix it in release_22_03.rst.
>> and device refers to ethdev
>>> device (PMD) private data. ethdev is common for all drivers.
>> OK, we could treat it as convention in future.
>>>
>>>>>>> and we really don't know what that data may be in the ethdev layer.
>>>>>>>
>>>>>>> Also there is a chance that 'ethdev private info' can be confused 
>>>>>>> with
>>>>>>> 'ethdev->data->dev_private'
>>>> what I want to dump is exactly the 'ethdev->data->dev_private'.
>>>> 'ethdev private info' means 'ethdev->data->dev_private'.
>>>> why confused?
>>>
>>> What I understand was this API can return any device private 
>>> information,
>>> it is not limited to 'ethdev->data->dev_private', (although most of the 
>> I think this API is limited to 'ethdev->data->dev_private'.
>>> data
>>> is represented in this struct), like if you want to dump queue state,
>>> this is out of 'ethdev->data->dev_private'.
>> Queue state can be dumped using API 'rte_eth_tx_queue_info_get'.
>>
> 
> Yes it can be. But as far as I can see there is nothing prevents the dump()
> API to provide the same, it is up to PMD.
> 
> If the intention is to limit what can be dump to 
> 'ethdev->data->dev_private',
> it is not clear from API documentation/implementation.
> 
>>>
>>>>>>
>>>>>> OK. Now I got your point! The difference is very subtle.
>>>>>>
>>>>>>>
>>>>>>>> [...]
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> + */
>>>>>>>>>> +__rte_experimental
>>>>>>>>>> +int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
>>>>>>>>>> +
>>>>>>>>>
>>>>>>>>> What do you think to have the 'port_id' as first argument to be
>>>>>>>>> consistent
>>>>>>>>> with the other APIs?
>>>>>>>>
>>>>>>>> The _dump APIs in other libraries have the file pointer as the 
>>>>>>>> first
>>>>>>> parameter, so let's follow that convention here too. No need to move
>>>>>>> the port_id parameter here.
>>>>>>>>
>>>>>>>
>>>>>>> Yes, for most of the _dump() APIs, file pointer seems is the first
>>>>>>> argument,
>>>>>>> bu they are from various libraries.
>>>>>>>
>>>>>>> Within the ethdev APIs, I think it makes sense that all APIs 
>>>>>>> start with
>>>>>>> 'port_id' parameter for consistency, like done in:
>>>>>>> rte_flow_dev_dump(uint16_t port_id, ...)
>>>>>>>
>>>>>>>> Only rte_dma_dump() has the file pointer last, and I didn't 
>>>>>>>> catch it
>>>>>>> when the function was defined.
>>>>>>>>
>>>>>>
>>>>>> OK. Then I agree with you about following the convention like 
>>>>>> rte_flow_dev_dump() with the port_id first.
>>>>>>
>>>>>> I even think Connor got it right the first time, and I proposed 
>>>>>> following the other convention.
>>>>>>
>>>>>
>>>>> Ahh, may bad I missed that, sorry for not commenting on time.
>>>>>
>>>>>
>>>>>> It's not easy when there are two opposite conventions. :-)
>>>>>>
>>>>>
>>>>> Yep, that is the main issue.
>>>>>
>>>>> .
>>>
>>> .
> 
> .

Re: [PATCH] ethdev: introduce ethdev dump API

[Min Hu (Connor)]

在 2022/2/8 21:52, Thomas Monjalon 写道:
> 08/02/2022 13:59, Ferruh Yigit:
>> On 2/8/2022 11:14 AM, Min Hu (Connor) wrote:
>>> 在 2022/2/8 18:21, Ferruh Yigit 写道:
>>>> What I understand was this API can return any device private information,
>>>> it is not limited to 'ethdev->data->dev_private', (although most of the
>>> I think this API is limited to 'ethdev->data->dev_private'.
>>>> data
>>>> is represented in this struct), like if you want to dump queue state,
>>>> this is out of 'ethdev->data->dev_private'.
>>> Queue state can be dumped using API 'rte_eth_tx_queue_info_get'.
>>>
>>
>> Yes it can be. But as far as I can see there is nothing prevents the dump()
>> API to provide the same, it is up to PMD.
>>
>> If the intention is to limit what can be dump to 'ethdev->data->dev_private',
>> it is not clear from API documentation/implementation.
> 
> Why limiting?
> The dump could even print debug infos which are nowhere else.
Yes, no need to limit, it is up to PMD, just as Ferruh said.
> 
> 
> 
> .
> 

[PATCH v3] ethdev: introduce dump API

[Min Hu (Connor)]

Added the ethdev dump API which provides querying private info from ethdev.
There exists many private properties in different PMD drivers, such as
adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
properties is important for debug. As the information is private, the new
API is introduced.

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
v3:
* change 'ethdev' to 'device'
v2:
* fixed comments from Ferruh.
---
 doc/guides/rel_notes/release_22_03.rst |  7 +++++++
 lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
 lib/ethdev/version.map                 |  3 +++
 5 files changed, 70 insertions(+)

diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
index b20716c521..0e3d3ae365 100644
--- a/doc/guides/rel_notes/release_22_03.rst
+++ b/doc/guides/rel_notes/release_22_03.rst
@@ -64,6 +64,13 @@ New Features
     - ``rte_ipv6_udptcp_cksum_mbuf()``
     - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
 
+* **Added the private dump API, for query private info from device.**
+
+  Added the private dump API which provides querying private info from device.
+  There exists many private properties in different PMD drivers. The
+  information of these properties is important for debug. As the information
+  is private, the new API is introduced.
+
 * **Updated AF_XDP PMD**
 
   * Added support for libxdp >=v1.2.2.
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 7d27781f7d..a0bd066ab3 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -990,6 +990,26 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
 				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump private info from device to a file.
+ *
+ * @param dev
+ *   Port (ethdev) handle.
+ * @param file
+ *   A pointer to a file for output.
+ *
+ * @retval 0
+ *   Success
+ * @retval -EINVAL
+ *   Invalid file
+ * @retval -ENOTSUP
+ *   Not supported
+ * @retval -ENODEV
+ *   Invalid port_id
+ */
+typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1186,6 +1206,9 @@ struct eth_dev_ops {
 	 * kinds of metadata to the PMD
 	 */
 	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
+
+	/** Dump private info from device */
+	eth_dev_priv_dump_t eth_dev_priv_dump;
 };
 
 /**
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 917a320afa..5ee316a3c0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -6485,6 +6485,23 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
 		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
 }
 
+int
+rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
+{
+	struct rte_eth_dev *dev;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	dev = &rte_eth_devices[port_id];
+
+	if (file == NULL) {
+		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+		return -EINVAL;
+	}
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
+	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, 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 147cc1ced3..e6ea294402 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -5902,6 +5902,26 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump private info from device to a file. Provided data and the order depends
+ * on the PMD.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param file
+ *   A pointer to a file for output.
+ * @return
+ *   - (-ENODEV) if *port_id* is invalid.
+ *   - (-EINVAL) if null file.
+ *   - (-ENOTSUP) if the device does not support this function.
+ *   - (0) on success
+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
index 1f7359c846..376ea139aa 100644
--- a/lib/ethdev/version.map
+++ b/lib/ethdev/version.map
@@ -256,6 +256,9 @@ EXPERIMENTAL {
 	rte_flow_flex_item_create;
 	rte_flow_flex_item_release;
 	rte_flow_pick_transfer_proxy;
+
+	# added in 22.03
+	rte_eth_dev_priv_dump;
 };
 
 INTERNAL {
-- 
2.33.0

Re: [PATCH v3] ethdev: introduce dump API

[Ferruh Yigit]

On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
> Added the ethdev dump API which provides querying private info from ethdev.
> There exists many private properties in different PMD drivers, such as
> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
> properties is important for debug. As the information is private, the new
> API is introduced.
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Ray Kinsella <mdr@ashroe.eu>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> v3:
> * change 'ethdev' to 'device'
> v2:
> * fixed comments from Ferruh.
> ---
>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>   lib/ethdev/version.map                 |  3 +++
>   5 files changed, 70 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
> index b20716c521..0e3d3ae365 100644
> --- a/doc/guides/rel_notes/release_22_03.rst
> +++ b/doc/guides/rel_notes/release_22_03.rst
> @@ -64,6 +64,13 @@ New Features
>       - ``rte_ipv6_udptcp_cksum_mbuf()``
>       - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>   
> +* **Added the private dump API, for query private info from device.**
> +
> +  Added the private dump API which provides querying private info from device.
> +  There exists many private properties in different PMD drivers. The
> +  information of these properties is important for debug. As the information
> +  is private, the new API is introduced.
> +
>   * **Updated AF_XDP PMD**
>   
>     * Added support for libxdp >=v1.2.2.
> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
> index 7d27781f7d..a0bd066ab3 100644
> --- a/lib/ethdev/ethdev_driver.h
> +++ b/lib/ethdev/ethdev_driver.h
> @@ -990,6 +990,26 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>   				       uint64_t *features);
>   
> +/**
> + * @internal
> + * Dump private info from device to a file.
> + *
> + * @param dev
> + *   Port (ethdev) handle.
> + * @param file
> + *   A pointer to a file for output.
> + *
> + * @retval 0
> + *   Success
> + * @retval -EINVAL
> + *   Invalid file
> + * @retval -ENOTSUP
> + *   Not supported
> + * @retval -ENODEV
> + *   Invalid port_id

For dev_ops (eth_dev_priv_dump_t), I don't think '-ENOTSUP' and '-ENODEV'
are valid errors, those cases covered by API before dev_ops called.

Also '@return' is missing.

What about below:
   *
   * @return
   *   Negative value on error, 0 on success.
   *
   * @retval 0
   *   Success
   * @retval -EINVAL
   *   Invalid file
   */

> + */
> +typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
> +
>   /**
>    * @internal A structure containing the functions exported by an Ethernet driver.
>    */
> @@ -1186,6 +1206,9 @@ struct eth_dev_ops {
>   	 * kinds of metadata to the PMD
>   	 */
>   	eth_rx_metadata_negotiate_t rx_metadata_negotiate;
> +
> +	/** Dump private info from device */
> +	eth_dev_priv_dump_t eth_dev_priv_dump;
>   };
>   
>   /**
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 917a320afa..5ee316a3c0 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -6485,6 +6485,23 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
>   		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
>   }
>   
> +int
> +rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
> +{
> +	struct rte_eth_dev *dev;
> +
> +	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> +	dev = &rte_eth_devices[port_id];
> +
> +	if (file == NULL) {
> +		RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
> +		return -EINVAL;
> +	}
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
> +	return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
> +}
> +

Can you please move the function up in the file?
Bottom of the file is for static inline functions, APIs are above the
"#include <rte_ethdev_core.h>" line, can you move the function above
that?

>   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 147cc1ced3..e6ea294402 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -5902,6 +5902,26 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>   	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>   }
>   
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Dump private info from device to a file. Provided data and the order depends
> + * on the PMD.
> + *
> + * @param port_id
> + *   The port identifier of the Ethernet device.
> + * @param file
> + *   A pointer to a file for output.
> + * @return
> + *   - (-ENODEV) if *port_id* is invalid.
> + *   - (-EINVAL) if null file.
> + *   - (-ENOTSUP) if the device does not support this function.
> + *   - (0) on success

Can you please list "(0) on success" to be consistent with (most :( of the) other comments?

Also need to add "(-EIO) if device is removed." too, which can be
returned by 'eth_err()'.


> + */
> +__rte_experimental
> +int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
> index 1f7359c846..376ea139aa 100644
> --- a/lib/ethdev/version.map
> +++ b/lib/ethdev/version.map
> @@ -256,6 +256,9 @@ EXPERIMENTAL {
>   	rte_flow_flex_item_create;
>   	rte_flow_flex_item_release;
>   	rte_flow_pick_transfer_proxy;
> +
> +	# added in 22.03
> +	rte_eth_dev_priv_dump;

next version needs to be rebased on top of next-net, because of other
APIs merged, fyi

>   };
>   
>   INTERNAL {

Re: [PATCH v3] ethdev: introduce dump API

[Ferruh Yigit]

On 2/10/2022 12:32 PM, Ferruh Yigit wrote:
> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>> Added the ethdev dump API which provides querying private info from ethdev.
>> There exists many private properties in different PMD drivers, such as
>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
>> properties is important for debug. As the information is private, the new
>> API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> ---
>> v3:
>> * change 'ethdev' to 'device'
>> v2:
>> * fixed comments from Ferruh.
>> ---
>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>   lib/ethdev/version.map                 |  3 +++
>>   5 files changed, 70 insertions(+)
>>
>> diff --git a/doc/guides/rel_notes/release_22_03.rst b/doc/guides/rel_notes/release_22_03.rst
>> index b20716c521..0e3d3ae365 100644
>> --- a/doc/guides/rel_notes/release_22_03.rst
>> +++ b/doc/guides/rel_notes/release_22_03.rst
>> @@ -64,6 +64,13 @@ New Features
>>       - ``rte_ipv6_udptcp_cksum_mbuf()``
>>       - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>> +* **Added the private dump API, for query private info from device.**
>> +
>> +  Added the private dump API which provides querying private info from device.
>> +  There exists many private properties in different PMD drivers. The
>> +  information of these properties is important for debug. As the information
>> +  is private, the new API is introduced.
>> +
>>   * **Updated AF_XDP PMD**
>>     * Added support for libxdp >=v1.2.2.
>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>> index 7d27781f7d..a0bd066ab3 100644
>> --- a/lib/ethdev/ethdev_driver.h
>> +++ b/lib/ethdev/ethdev_driver.h
>> @@ -990,6 +990,26 @@ typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev,
>>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>                          uint64_t *features);
>> +/**
>> + * @internal
>> + * Dump private info from device to a file.
>> + *
>> + * @param dev
>> + *   Port (ethdev) handle.
>> + * @param file
>> + *   A pointer to a file for output.
>> + *
>> + * @retval 0
>> + *   Success
>> + * @retval -EINVAL
>> + *   Invalid file
>> + * @retval -ENOTSUP
>> + *   Not supported
>> + * @retval -ENODEV
>> + *   Invalid port_id
> 
> For dev_ops (eth_dev_priv_dump_t), I don't think '-ENOTSUP' and '-ENODEV'
> are valid errors, those cases covered by API before dev_ops called.
> 
> Also '@return' is missing.
> 
> What about below:
>    *
>    * @return
>    *   Negative value on error, 0 on success.
>    *
>    * @retval 0
>    *   Success
>    * @retval -EINVAL
>    *   Invalid file
>    */
> 
>> + */
>> +typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE *file);
>> +
>>   /**
>>    * @internal A structure containing the functions exported by an Ethernet driver.
>>    */
>> @@ -1186,6 +1206,9 @@ struct eth_dev_ops {
>>        * kinds of metadata to the PMD
>>        */
>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>> +
>> +    /** Dump private info from device */
>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
>>   };
>>   /**
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 917a320afa..5ee316a3c0 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -6485,6 +6485,23 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
>>                  (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
>>   }
>> +int
>> +rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
>> +{
>> +    struct rte_eth_dev *dev;
>> +
>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> +    dev = &rte_eth_devices[port_id];
>> +
>> +    if (file == NULL) {
>> +        RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, -ENOTSUP);
>> +    return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
>> +}
>> +
> 
> Can you please move the function up in the file?
> Bottom of the file is for static inline functions, APIs are above the
> "#include <rte_ethdev_core.h>" line, can you move the function above
> that?
> 
>>   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 147cc1ced3..e6ea294402 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -5902,6 +5902,26 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>   }
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Dump private info from device to a file. Provided data and the order depends
>> + * on the PMD.
>> + *
>> + * @param port_id
>> + *   The port identifier of the Ethernet device.
>> + * @param file
>> + *   A pointer to a file for output.
>> + * @return
>> + *   - (-ENODEV) if *port_id* is invalid.
>> + *   - (-EINVAL) if null file.
>> + *   - (-ENOTSUP) if the device does not support this function.
>> + *   - (0) on success
> 
> Can you please list "(0) on success" to be consistent with (most :( of the) other comments?
> 

Can you please list "(0) on success" as first item ...

* @return
*   - (0) on success
*   - (-ENODEV) if *port_id* is invalid.
*   - (-EINVAL) if null file.
...

> Also need to add "(-EIO) if device is removed." too, which can be
> returned by 'eth_err()'.
> 
> 
>> + */
>> +__rte_experimental
>> +int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>> +
>>   #ifdef __cplusplus
>>   }
>>   #endif
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 1f7359c846..376ea139aa 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -256,6 +256,9 @@ EXPERIMENTAL {
>>       rte_flow_flex_item_create;
>>       rte_flow_flex_item_release;
>>       rte_flow_pick_transfer_proxy;
>> +
>> +    # added in 22.03
>> +    rte_eth_dev_priv_dump;
> 
> next version needs to be rebased on top of next-net, because of other
> APIs merged, fyi
> 
>>   };
>>   INTERNAL {
> 

Re: [PATCH v3] ethdev: introduce dump API

[Ferruh Yigit]

On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
> Added the ethdev dump API which provides querying private info from ethdev.
> There exists many private properties in different PMD drivers, such as
> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
> properties is important for debug. As the information is private, the new
> API is introduced.
> 
> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>
> Acked-by: Morten Brørup<mb@smartsharesystems.com>
> Acked-by: Ray Kinsella<mdr@ashroe.eu>
> Acked-by: Ajit Khaparde<ajit.khaparde@broadcom.com>
> ---
> v3:
> * change 'ethdev' to 'device'
> v2:
> * fixed comments from Ferruh.
> ---
>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>   lib/ethdev/version.map                 |  3 +++


Btw, can you please confirm that there will be a PMD implementation
in this release, (it can be after -rc1)?
Because our policy requires at least one PMD implementation is
implemented to accept a new API.
(Thomas/David, please correct me if I remember the policy wrong)

Re: [PATCH v3] ethdev: introduce dump API

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/2/10 20:37, Ferruh Yigit 写道:
> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>> Added the ethdev dump API which provides querying private info from 
>> ethdev.
>> There exists many private properties in different PMD drivers, such as
>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
>> properties is important for debug. As the information is private, the new
>> API is introduced.
>>
>> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>
>> Acked-by: Morten Brørup<mb@smartsharesystems.com>
>> Acked-by: Ray Kinsella<mdr@ashroe.eu>
>> Acked-by: Ajit Khaparde<ajit.khaparde@broadcom.com>
>> ---
>> v3:
>> * change 'ethdev' to 'device'
>> v2:
>> * fixed comments from Ferruh.
>> ---
>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>   lib/ethdev/version.map                 |  3 +++
> 
> 
> Btw, can you please confirm that there will be a PMD implementation
> in this release, (it can be after -rc1)?YES, I will send a set of patches about hns3 PMD implementation once the
API is accepted.
> Because our policy requires at least one PMD implementation is
> implemented to accept a new API.
> (Thomas/David, please correct me if I remember the policy wrong)
> .

Re: [PATCH v3] ethdev: introduce dump API

[Ferruh Yigit]

On 2/10/2022 1:16 PM, Min Hu (Connor) wrote:
> Hi, Ferruh,
> 
> 在 2022/2/10 20:37, Ferruh Yigit 写道:
>> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>>> Added the ethdev dump API which provides querying private info from ethdev.
>>> There exists many private properties in different PMD drivers, such as
>>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
>>> properties is important for debug. As the information is private, the new
>>> API is introduced.
>>>
>>> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>
>>> Acked-by: Morten Brørup<mb@smartsharesystems.com>
>>> Acked-by: Ray Kinsella<mdr@ashroe.eu>
>>> Acked-by: Ajit Khaparde<ajit.khaparde@broadcom.com>
>>> ---
>>> v3:
>>> * change 'ethdev' to 'device'
>>> v2:
>>> * fixed comments from Ferruh.
>>> ---
>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>>   lib/ethdev/version.map                 |  3 +++
>>
>>
>> Btw, can you please confirm that there will be a PMD implementation
>> in this release, (it can be after -rc1)?

> YES, I will send a set of patches about hns3 PMD implementation once the
> API is accepted.

ack, thanks.

>> Because our policy requires at least one PMD implementation is
>> implemented to accept a new API.
>> (Thomas/David, please correct me if I remember the policy wrong)
>> .

Re: [PATCH v3] ethdev: introduce dump API

[Ferruh Yigit]

On 2/10/2022 1:22 PM, Ferruh Yigit wrote:
> On 2/10/2022 1:16 PM, Min Hu (Connor) wrote:
>> Hi, Ferruh,
>>
>> 在 2022/2/10 20:37, Ferruh Yigit 写道:
>>> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>>>> Added the ethdev dump API which provides querying private info from ethdev.
>>>> There exists many private properties in different PMD drivers, such as
>>>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of these
>>>> properties is important for debug. As the information is private, the new
>>>> API is introduced.
>>>>
>>>> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>
>>>> Acked-by: Morten Brørup<mb@smartsharesystems.com>
>>>> Acked-by: Ray Kinsella<mdr@ashroe.eu>
>>>> Acked-by: Ajit Khaparde<ajit.khaparde@broadcom.com>
>>>> ---
>>>> v3:
>>>> * change 'ethdev' to 'device'
>>>> v2:
>>>> * fixed comments from Ferruh.
>>>> ---
>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>>>   lib/ethdev/version.map                 |  3 +++
>>>
>>>
>>> Btw, can you please confirm that there will be a PMD implementation
>>> in this release, (it can be after -rc1)?
> 
>> YES, I will send a set of patches about hns3 PMD implementation once the
>> API is accepted.
> 
> ack, thanks.
> 

in fact process document [1] requires at least draft PMD implementation
ready to apply the API change [2].

Can be possible to send a draft, simple PMD implementation tomorrow, to
justify API design? It can be improved later after -rc1 with new versions.


[1]
doc/guides/contributing/patches.rst

[2]
* At least one PMD should implement the API.
   It may be a draft sent in a separate series.

Re: [PATCH v3] ethdev: introduce dump API

[Min Hu (Connor)]

Hi, Ferruh,

在 2022/2/10 23:50, Ferruh Yigit 写道:
> On 2/10/2022 1:22 PM, Ferruh Yigit wrote:
>> On 2/10/2022 1:16 PM, Min Hu (Connor) wrote:
>>> Hi, Ferruh,
>>>
>>> 在 2022/2/10 20:37, Ferruh Yigit 写道:
>>>> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>>>>> Added the ethdev dump API which provides querying private info from 
>>>>> ethdev.
>>>>> There exists many private properties in different PMD drivers, such as
>>>>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of 
>>>>> these
>>>>> properties is important for debug. As the information is private, 
>>>>> the new
>>>>> API is introduced.
>>>>>
>>>>> Signed-off-by: Min Hu (Connor)<humin29@huawei.com>
>>>>> Acked-by: Morten Brørup<mb@smartsharesystems.com>
>>>>> Acked-by: Ray Kinsella<mdr@ashroe.eu>
>>>>> Acked-by: Ajit Khaparde<ajit.khaparde@broadcom.com>
>>>>> ---
>>>>> v3:
>>>>> * change 'ethdev' to 'device'
>>>>> v2:
>>>>> * fixed comments from Ferruh.
>>>>> ---
>>>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>>>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>>>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>>>>   lib/ethdev/version.map                 |  3 +++
>>>>
>>>>
>>>> Btw, can you please confirm that there will be a PMD implementation
>>>> in this release, (it can be after -rc1)?
>>
>>> YES, I will send a set of patches about hns3 PMD implementation once the
>>> API is accepted.
>>
>> ack, thanks.
>>
> 
> in fact process document [1] requires at least draft PMD implementation
> ready to apply the API change [2].
> 
> Can be possible to send a draft, simple PMD implementation tomorrow, to
> justify API design? It can be improved later after -rc1 with new versions.
> 
I have sent a set of patches named 'dump device info', which includes:
a.[patch V4] ethdev: introduce dump API
b. a set of hns3 implementation:
   net/hns3: dump device basic info
   net/hns3: dump device feature capability
   net/hns3: dump device MAC info
   net/hns3: dump queue info
   net/hns3: dump VLAN configuration info
   net/hns3: dump flow director basic info
   net/hns3: dump TM configuration info
   net/hns3: dump flow control info

Please check it out, thanks.

> 
> [1]
> doc/guides/contributing/patches.rst
> 
> [2]
> * At least one PMD should implement the API.
>    It may be a draft sent in a separate series.
> .

Re: [PATCH v3] ethdev: introduce dump API

[Min Hu (Connor)]

Hi, Ferruh,
	all fixed in v4, please check it, thanks.

在 2022/2/10 20:34, Ferruh Yigit 写道:
> On 2/10/2022 12:32 PM, Ferruh Yigit wrote:
>> On 2/9/2022 1:21 AM, Min Hu (Connor) wrote:
>>> Added the ethdev dump API which provides querying private info from 
>>> ethdev.
>>> There exists many private properties in different PMD drivers, such as
>>> adapter state, Rx/Tx func algorithm in hns3 PMD. The information of 
>>> these
>>> properties is important for debug. As the information is private, the 
>>> new
>>> API is introduced.
>>>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> ---
>>> v3:
>>> * change 'ethdev' to 'device'
>>> v2:
>>> * fixed comments from Ferruh.
>>> ---
>>>   doc/guides/rel_notes/release_22_03.rst |  7 +++++++
>>>   lib/ethdev/ethdev_driver.h             | 23 +++++++++++++++++++++++
>>>   lib/ethdev/rte_ethdev.c                | 17 +++++++++++++++++
>>>   lib/ethdev/rte_ethdev.h                | 20 ++++++++++++++++++++
>>>   lib/ethdev/version.map                 |  3 +++
>>>   5 files changed, 70 insertions(+)
>>>
>>> diff --git a/doc/guides/rel_notes/release_22_03.rst 
>>> b/doc/guides/rel_notes/release_22_03.rst
>>> index b20716c521..0e3d3ae365 100644
>>> --- a/doc/guides/rel_notes/release_22_03.rst
>>> +++ b/doc/guides/rel_notes/release_22_03.rst
>>> @@ -64,6 +64,13 @@ New Features
>>>       - ``rte_ipv6_udptcp_cksum_mbuf()``
>>>       - ``rte_ipv6_udptcp_cksum_mbuf_verify()``
>>> +* **Added the private dump API, for query private info from device.**
>>> +
>>> +  Added the private dump API which provides querying private info 
>>> from device.
>>> +  There exists many private properties in different PMD drivers. The
>>> +  information of these properties is important for debug. As the 
>>> information
>>> +  is private, the new API is introduced.
>>> +
>>>   * **Updated AF_XDP PMD**
>>>     * Added support for libxdp >=v1.2.2.
>>> diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
>>> index 7d27781f7d..a0bd066ab3 100644
>>> --- a/lib/ethdev/ethdev_driver.h
>>> +++ b/lib/ethdev/ethdev_driver.h
>>> @@ -990,6 +990,26 @@ typedef int (*eth_representor_info_get_t)(struct 
>>> rte_eth_dev *dev,
>>>   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
>>>                          uint64_t *features);
>>> +/**
>>> + * @internal
>>> + * Dump private info from device to a file.
>>> + *
>>> + * @param dev
>>> + *   Port (ethdev) handle.
>>> + * @param file
>>> + *   A pointer to a file for output.
>>> + *
>>> + * @retval 0
>>> + *   Success
>>> + * @retval -EINVAL
>>> + *   Invalid file
>>> + * @retval -ENOTSUP
>>> + *   Not supported
>>> + * @retval -ENODEV
>>> + *   Invalid port_id
>>
>> For dev_ops (eth_dev_priv_dump_t), I don't think '-ENOTSUP' and '-ENODEV'
>> are valid errors, those cases covered by API before dev_ops called.
>>
>> Also '@return' is missing.
>>
>> What about below:
>>    *
>>    * @return
>>    *   Negative value on error, 0 on success.
>>    *
>>    * @retval 0
>>    *   Success
>>    * @retval -EINVAL
>>    *   Invalid file
>>    */
>>
>>> + */
>>> +typedef int (*eth_dev_priv_dump_t)(struct rte_eth_dev *dev, FILE 
>>> *file);
>>> +
>>>   /**
>>>    * @internal A structure containing the functions exported by an 
>>> Ethernet driver.
>>>    */
>>> @@ -1186,6 +1206,9 @@ struct eth_dev_ops {
>>>        * kinds of metadata to the PMD
>>>        */
>>>       eth_rx_metadata_negotiate_t rx_metadata_negotiate;
>>> +
>>> +    /** Dump private info from device */
>>> +    eth_dev_priv_dump_t eth_dev_priv_dump;
>>>   };
>>>   /**
>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index 917a320afa..5ee316a3c0 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -6485,6 +6485,23 @@ rte_eth_rx_metadata_negotiate(uint16_t 
>>> port_id, uint64_t *features)
>>>                  (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
>>>   }
>>> +int
>>> +rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
>>> +{
>>> +    struct rte_eth_dev *dev;
>>> +
>>> +    RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> +    dev = &rte_eth_devices[port_id];
>>> +
>>> +    if (file == NULL) {
>>> +        RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_dev_priv_dump, 
>>> -ENOTSUP);
>>> +    return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, 
>>> file));
>>> +}
>>> +
>>
>> Can you please move the function up in the file?
>> Bottom of the file is for static inline functions, APIs are above the
>> "#include <rte_ethdev_core.h>" line, can you move the function above
>> that?
>>
>>>   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 147cc1ced3..e6ea294402 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -5902,6 +5902,26 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t 
>>> queue_id,
>>>       return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>>   }
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change, or be removed, without 
>>> prior notice
>>> + *
>>> + * Dump private info from device to a file. Provided data and the 
>>> order depends
>>> + * on the PMD.
>>> + *
>>> + * @param port_id
>>> + *   The port identifier of the Ethernet device.
>>> + * @param file
>>> + *   A pointer to a file for output.
>>> + * @return
>>> + *   - (-ENODEV) if *port_id* is invalid.
>>> + *   - (-EINVAL) if null file.
>>> + *   - (-ENOTSUP) if the device does not support this function.
>>> + *   - (0) on success
>>
>> Can you please list "(0) on success" to be consistent with (most :( of 
>> the) other comments?
>>
> 
> Can you please list "(0) on success" as first item ...
> 
> * @return
> *   - (0) on success
> *   - (-ENODEV) if *port_id* is invalid.
> *   - (-EINVAL) if null file.
> ...
> 
>> Also need to add "(-EIO) if device is removed." too, which can be
>> returned by 'eth_err()'.
>>
>>
>>> + */
>>> +__rte_experimental
>>> +int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>> +
>>>   #ifdef __cplusplus
>>>   }
>>>   #endif
>>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>>> index 1f7359c846..376ea139aa 100644
>>> --- a/lib/ethdev/version.map
>>> +++ b/lib/ethdev/version.map
>>> @@ -256,6 +256,9 @@ EXPERIMENTAL {
>>>       rte_flow_flex_item_create;
>>>       rte_flow_flex_item_release;
>>>       rte_flow_pick_transfer_proxy;
>>> +
>>> +    # added in 22.03
>>> +    rte_eth_dev_priv_dump;
>>
>> next version needs to be rebased on top of next-net, because of other
>> APIs merged, fyi
>>
>>>   };
>>>   INTERNAL {
>>
> 
> .