ethdev: introduce ethdev dump API

From: Min Hu (Connor) [mailto:[email protected]]
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) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +3 -0

index 6d99d1eaa9..9b51da899a 100644
@@ -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

doc/guides/rel_notes/release_22_03.rst +1 -0

+     the information is private, the new API is introduced.
 
 Removed Items
 -------------

lib/ethdev/ethdev_driver.h +14 -0

index d95605a355..ac7fa5eae2 100644
@@ -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.
*/

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
 };
 
 /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..4bbe444045 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..918bd3116f 100644
@@ -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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_dump(uint16_t port_id, FILE *file);
+
 #ifdef __cplusplus
 }
 #endif
--

2.33.0

Quoted 136 lines

From: Min Hu (Connor) [mailto:[email protected]]
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) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +3 -0

index 6d99d1eaa9..9b51da899a 100644
@@ -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

doc/guides/rel_notes/release_22_03.rst +1 -0

+     the information is private, the new API is introduced.
 
  Removed Items
  -------------

lib/ethdev/ethdev_driver.h +14 -0

index d95605a355..ac7fa5eae2 100644
@@ -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.
*/

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
  };
 
  /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..4bbe444045 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..918bd3116f 100644
@@ -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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_dump(uint16_t port_id, FILE *file);
+
  #ifdef __cplusplus
  }
  #endif
--

Quoted 2 lines

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.

.

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

11/01/2022 12:54, Min Hu (Connor):

doc/guides/rel_notes/release_22_03.rst New Features · line 64

Quoted 13 lines

doc/guides/rel_notes/release_22_03.rst +6 -0

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

[...]

doc/guides/rel_notes/release_22_03.rst New Features · line 65

Quoted 1 line

doc/guides/rel_notes/release_22_03.rst +1 -0

+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?

.

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

11/01/2022 12:54, Min Hu (Connor):

doc/guides/rel_notes/release_22_03.rst New Features · line 64

Quoted 13 lines

doc/guides/rel_notes/release_22_03.rst +6 -0

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

[...]

doc/guides/rel_notes/release_22_03.rst New Features · line 65

Quoted 1 line

doc/guides/rel_notes/release_22_03.rst +1 -0

+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?

.

Hi, Thomas,

Quoted 1 line · Thomas Monjalon

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

git send-email -to [email protected] -cc [email protected] -cc
[email protected] --cc-cmd devtools/get-maintainer.sh *.patch --suppress-cc=all

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

12/01/2022 03:44, Min Hu (Connor):

Quoted 9 lines

Hi, Thomas,

Quoted 1 line · Thomas Monjalon

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

git send-email -to [email protected] -cc [email protected] -cc
[email protected] --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?

.

From: Min Hu (Connor) [mailto:[email protected]]
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) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6d99d1eaa9..4f97df942d 100644
@@ -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.**

doc/guides/rel_notes/release_22_03.rst +4 -0

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

doc/guides/rel_notes/release_22_03.rst +2 -0

+  for debug. As the information is private, the new API is introduced.
+
 
 Removed Items
 -------------

lib/ethdev/ethdev_driver.h +0 -0

index d95605a355..e75ff3f15b 100644

lib/ethdev/ethdev_driver.h +11 -0

+ *
+ * @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.
*/

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
 };
 
 /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..9fc6d91d76 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..8e33e6927f 100644
@@ -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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
 #ifdef __cplusplus
 }
 #endif
--

2.33.0

Quoted 50 lines

From: Min Hu (Connor) [mailto:[email protected]]
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) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6d99d1eaa9..4f97df942d 100644
@@ -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.**

doc/guides/rel_notes/release_22_03.rst +4 -0

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

doc/guides/rel_notes/release_22_03.rst +2 -0

+  for debug. As the information is private, the new API is introduced.
+
 
  Removed Items
  -------------

lib/ethdev/ethdev_driver.h +0 -0

index d95605a355..e75ff3f15b 100644

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct · line 996

Quoted 7 lines

lib/ethdev/ethdev_driver.h +3 -0

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

Quoted 67 lines

lib/ethdev/ethdev_driver.h +11 -0

+ *
+ * @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.
*/

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
  };
 
  /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..9fc6d91d76 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..8e33e6927f 100644
@@ -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

Quoted 3 lines

notice
+ *
+ * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

Quoted 15 lines

+ *
+ * @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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
  #ifdef __cplusplus
  }
  #endif
--

Quoted 2 lines

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 <[email protected]>

.

Thanks Morten, fixed in v3.

Quoted 11 lines · Morten Brørup

Quoted 11 lines

From: Min Hu (Connor) [mailto:[email protected]]
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.

Quoted 146 lines

Quoted 39 lines

Signed-off-by Min Hu (Connor) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6d99d1eaa9..4f97df942d 100644
@@ -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.**

doc/guides/rel_notes/release_22_03.rst +4 -0

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

doc/guides/rel_notes/release_22_03.rst +2 -0

+  for debug. As the information is private, the new API is introduced.
+
 
  Removed Items
  -------------

lib/ethdev/ethdev_driver.h +0 -0

index d95605a355..e75ff3f15b 100644

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct · line 996

Quoted 7 lines

lib/ethdev/ethdev_driver.h +3 -0

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

Quoted 67 lines

lib/ethdev/ethdev_driver.h +11 -0

+ *
+ * @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.
*/

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
  };
 
  /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..9fc6d91d76 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..8e33e6927f 100644
@@ -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

Quoted 3 lines

notice
+ *
+ * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

Quoted 15 lines

+ *
+ * @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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
  #ifdef __cplusplus
  }
  #endif
--

Quoted 2 lines

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 <[email protected]>

.

Quoted 15 lines · Min Hu (Connor)

Thanks Morten, fixed in v3.

Quoted 11 lines · Morten Brørup

Quoted 11 lines

From: Min Hu (Connor) [mailto:[email protected]]
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?

Quoted 142 lines

Quoted 142 lines

Quoted 39 lines

Signed-off-by Min Hu (Connor) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6d99d1eaa9..4f97df942d 100644
@@ -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.**

doc/guides/rel_notes/release_22_03.rst +4 -0

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

doc/guides/rel_notes/release_22_03.rst +2 -0

+  for debug. As the information is private, the new API is introduced.
+
 
   Removed Items
   -------------

lib/ethdev/ethdev_driver.h +0 -0

index d95605a355..e75ff3f15b 100644

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct · line 996

Quoted 7 lines

lib/ethdev/ethdev_driver.h +3 -0

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

Quoted 67 lines

lib/ethdev/ethdev_driver.h +11 -0

+ *
+ * @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.

    */

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
   };
 
   /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..9fc6d91d76 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..8e33e6927f 100644
@@ -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

Quoted 3 lines

notice
+ *
+ * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

Quoted 15 lines

+ *
+ * @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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
   #ifdef __cplusplus
   }
   #endif
--

Quoted 2 lines

2.33.0

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

Quoted 4 lines

Quoted 4 lines

Acked-by Morten Brørup <[email protected]>

.

.

Hi, Ajit,

Quoted 16 lines · Ajit Khaparde

Quoted 15 lines · Min Hu (Connor)

Thanks Morten, fixed in v3.

Quoted 11 lines · Morten Brørup

Quoted 11 lines

From: Min Hu (Connor) [mailto:[email protected]]
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.

Quoted 2 lines

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.

Quoted 149 lines

Quoted 143 lines

Quoted 143 lines

Quoted 40 lines

Signed-off-by Min Hu (Connor) <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +1 -0

index 6d99d1eaa9..4f97df942d 100644
@@ -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.**

doc/guides/rel_notes/release_22_03.rst +4 -0

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

doc/guides/rel_notes/release_22_03.rst +2 -0

+  for debug. As the information is private, the new API is introduced.
+
 
   Removed Items
   -------------

lib/ethdev/ethdev_driver.h +0 -0

index d95605a355..e75ff3f15b 100644

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct · line 996

Quoted 7 lines

lib/ethdev/ethdev_driver.h +3 -0

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

Quoted 67 lines

lib/ethdev/ethdev_driver.h +11 -0

+ *
+ * @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.

    */

lib/ethdev/ethdev_driver.h +3 -0

@@ -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;
   };
 
   /**

lib/ethdev/rte_ethdev.c +15 -0

index a1d475a292..9fc6d91d76 100644
@@ -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)

lib/ethdev/rte_ethdev.h +3 -0

index fa299c8ad7..8e33e6927f 100644
@@ -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

Quoted 3 lines

notice
+ *
+ * Get ethdev private info.

Suggestion: Dump ethdev private info to a file.

Quoted 15 lines

+ *
+ * @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.
+ */

lib/ethdev/rte_ethdev.h +3 -1

+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+
   #ifdef __cplusplus
   }
   #endif
--

Quoted 2 lines

2.33.0

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

Quoted 4 lines

Quoted 4 lines

Acked-by Morten Brørup <[email protected]>

.

.

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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 16 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

---
v3:
* fix comment.
* add rte_eth_dev_priv_dump to version.map file.

v2:
* fix dump API name
* adjust description in doc.

Quoted 8 lines

---
 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 <[email protected]>

Quoted 29 lines · Ray Kinsella

Quoted 24 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 1 line

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

Quoted 30 lines · Ajit Khaparde

Quoted 29 lines · Ray Kinsella

Quoted 24 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 2 lines

Quoted 1 line

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

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.

Quoted 31 lines · Ferruh Yigit

Quoted 30 lines · Ajit Khaparde

Quoted 29 lines · Ray Kinsella

Quoted 24 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 7 lines

Quoted 2 lines

Quoted 1 line

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

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.

Quoted 1 line

can be part of this private info?

Quoted 3 lines

Also why not use existing xstats or telemetry to get more data from the drivers?

Quoted 2 lines

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

Opps, Ciara (Power) cc'ed now.
.

Hi, Ferruh,

Quoted 36 lines · Ferruh Yigit

Quoted 35 lines · Ferruh Yigit

Quoted 32 lines · Ajit Khaparde

Quoted 29 lines · Ray Kinsella

Quoted 22 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 2 lines

  5 files changed, 58 insertions(+)

Quoted 1 line

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

Quoted 3 lines

Quoted 3 lines

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

Quoted 1 line

Quoted 1 line

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.

Quoted 3 lines

Quoted 3 lines

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.

Quoted 5 lines

Quoted 2 lines

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

Opps, Ciara (Power) cc'ed now.
.

Quoted 39 lines · Min Hu (Connor)

Hi, Ferruh,

Quoted 36 lines · Ferruh Yigit

Quoted 35 lines · Ferruh Yigit

Quoted 32 lines · Ajit Khaparde

Quoted 29 lines · Ray Kinsella

Quoted 22 lines · 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) <[email protected]>

Acked-by Morten BrÞrup <[email protected]>

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

Quoted 2 lines

  5 files changed, 58 insertions(+)

Quoted 1 line

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

Quoted 8 lines

Quoted 3 lines

Quoted 3 lines

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.

Quoted 54 lines

  Is there any list in your mind what

Quoted 1 line

Quoted 1 line

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.

Quoted 1 line

Quoted 1 line

Quoted 16 lines

Quoted 2 lines

Quoted 2 lines

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.

Quoted 5 lines

Quoted 2 lines

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

Opps, Ciara (Power) cc'ed now.
.

.

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) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +0 -0

index b20716c521..5895194cde 100644
@@ -92,6 +92,13 @@ New Features
    * Called ``rte_ipv4/6_udptcp_cksum_mbuf()`` functions in testpmd csum mode

doc/guides/rel_notes/release_22_03.rst +0 -0

  Removed Items
  -------------

lib/ethdev/ethdev_driver.h +0 -0

index 7d27781f7d..d41d8a2c9d 100644

lib/ethdev/ethdev_driver.h +10 -0

+ * @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.
   */

lib/ethdev/ethdev_driver.h +1 -0

+	eth_dev_priv_dump_t eth_dev_priv_dump;
  };
 
  /**

lib/ethdev/rte_ethdev.c +0 -0

index 917a320afa..07838cc2d9 100644
@@ -6485,6 +6485,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features)
  		       (*dev->dev_ops->rx_metadata_negotiate)(dev, features));
  }

lib/ethdev/rte_ethdev.c +4 -0

+
+	return ret;
+}
+
  RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
 
  RTE_INIT(ethdev_init_telemetry)

lib/ethdev/rte_ethdev.h +0 -0

index 147cc1ced3..1c00fdbcd2 100644

lib/ethdev/rte_ethdev.h +0 -0

  #ifdef __cplusplus
  }
  #endif

lib/ethdev/version.map +3 -0

index 1f7359c846..376ea139aa 100644
@@ -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 {

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query private

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

Review typedef int (*eth_representor_info_get_t)(struct

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct

rte_eth_dev *dev,

Quoted 3 lines

lib/ethdev/ethdev_driver.h +0 -0

  typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
  				       uint64_t *features);
 

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev, · line 996

Quoted 4 lines

lib/ethdev/ethdev_driver.h +4 -0

+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *

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

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 5 lines

lib/ethdev/rte_ethdev.h +4 -0

+ */
+__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?

Quoted 24 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query private

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 10 lines

Review typedef int (*eth_representor_info_get_t)(struct

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int (*eth_representor_info_get_t)(struct

rte_eth_dev *dev,

Quoted 7 lines

lib/ethdev/ethdev_driver.h +4 -0

   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev *dev,
   				       uint64_t *features);
 
+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *

Quoted 2 lines

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?

[...]

Quoted 9 lines

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 5 lines

+ */

lib/ethdev/rte_ethdev.h +3 -0

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

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

(quoted diff) +0 -0

   typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev, · line 996

Quoted 6 lines

lib/ethdev/ethdev_driver.h +4 -0

   				       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?

Quoted 1 line

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'

Quoted 13 lines

[...]

Quoted 9 lines

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 5 lines

+ */

lib/ethdev/rte_ethdev.h +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

Quoted 54 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

(quoted diff) +0 -0

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct rte_eth_dev *dev, · line 996

Quoted 6 lines

lib/ethdev/ethdev_driver.h +4 -0

    				       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?

Quoted 1 line

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

Quoted 6 lines

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.

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 5 lines

+ */

lib/ethdev/rte_ethdev.h +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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

Quoted 48 lines · Morten Brørup

Quoted 48 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

                           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

Quoted 8 lines

Quoted 8 lines

instead?

Quoted 1 line

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,

Quoted 4 lines

Quoted 4 lines

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'

Quoted 29 lines

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

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 5 lines

+ */

lib/ethdev/rte_ethdev.h +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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.

Quoted 2 lines

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

Yep, that is the main issue.

.

Hi, Ferruh,

Quoted 52 lines · Ferruh Yigit

Quoted 51 lines · Morten Brørup

Quoted 51 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

                           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?

Quoted 1 line

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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?

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

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?

Quoted 47 lines

Quoted 29 lines

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

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

Review

Quoted 5 lines

+ */

(quoted diff) +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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.

Quoted 2 lines

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

Yep, that is the main issue.

.

Quoted 58 lines · Min Hu (Connor)

Hi, Ferruh,

Quoted 55 lines · Ferruh Yigit

Quoted 54 lines · Morten Brørup

Quoted 54 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

                           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?

Quoted 1 line

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

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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

device (PMD) private data. ethdev is common for all drivers.

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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

Quoted 42 lines

Quoted 42 lines

Quoted 30 lines

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

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

Review

Quoted 5 lines

+ */

(quoted diff) +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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.

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

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

Yep, that is the main issue.

.

.

Hi, Ferruh,

Quoted 59 lines · Ferruh Yigit

Quoted 58 lines · Min Hu (Connor)

Hi, Ferruh,

Quoted 55 lines · Ferruh Yigit

Quoted 54 lines · Morten Brørup

Quoted 54 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

                           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?

Quoted 1 line

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

Quoted 7 lines

Quoted 4 lines

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

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

Quoted 1 line

device (PMD) private data. ethdev is common for all drivers.

OK, we could treat it as convention in future.

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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?

Quoted 2 lines

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

Quoted 3 lines

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

Quoted 50 lines

Quoted 42 lines

Quoted 42 lines

Quoted 30 lines

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

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

Review

Quoted 5 lines

+ */

(quoted diff) +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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.

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

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

Yep, that is the main issue.

.

.

Quoted 7 lines · Ferruh Yigit

Quoted 2 lines

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

Quoted 3 lines

data
is represented in this struct), like if you want to dump queue state, this is out of 'ethdev->data->dev_private'.

Quoted 2 lines

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.

08/02/2022 13:59, Ferruh Yigit:

Quoted 11 lines · Min Hu (Connor)

Quoted 9 lines · Ferruh Yigit

Quoted 2 lines

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

Quoted 3 lines

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

Quoted 5 lines

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.

.

Quoted 69 lines · Min Hu (Connor)

Hi, Ferruh,

Quoted 66 lines · Ferruh Yigit

Quoted 55 lines · Min Hu (Connor)

Hi, Ferruh,

Quoted 52 lines · Ferruh Yigit

Quoted 51 lines · Morten Brørup

Quoted 51 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 13.36

Quoted 5 lines · Morten Brørup

Quoted 5 lines

From: Ferruh Yigit [mailto:[email protected]]
Sent: Monday, 7 February 2022 12.46

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query

private

Quoted 15 lines

Quoted 15 lines

info

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

Quoted 1 line

from device. There exists many private properties in different PMD

drivers,

Quoted 1 line

such as adapter state, Rx/Tx func algorithm in hns3 PMD. The

information of

Quoted 1 line

these properties is important for debug. As the information is

private,

Quoted 1 line

the new API is introduced.>

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

Quoted 8 lines

Quoted 4 lines

Quoted 4 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

[...]

Quoted 1 line

Review typedef int

Quoted 1 line

(quoted diff) +0 -0

@@ -990,6 +990,20 @@ typedef int

(*eth_representor_info_get_t)(struct

Quoted 2 lines

Quoted 2 lines

rte_eth_dev *dev,

Quoted 1 line

    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev

*dev,

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

                           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?

Quoted 1 line

Quoted 7 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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

Quoted 2 lines

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.

Quoted 12 lines

and device refers to ethdev

Quoted 1 line

device (PMD) private data. ethdev is common for all drivers.

OK, we could treat it as convention in future.

Quoted 9 lines

Quoted 8 lines

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

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?

Quoted 8 lines

Quoted 2 lines

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

Quoted 3 lines

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.

Quoted 42 lines

Quoted 42 lines

Quoted 42 lines

Quoted 42 lines

Quoted 30 lines

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

Quoted 26 lines

Quoted 13 lines

[...]

Quoted 9 lines

Review

Quoted 5 lines

+ */

(quoted diff) +3 -0

+__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?

Quoted 1 line

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.

Quoted 1 line

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

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Only rte_dma_dump() has the file pointer last, and I didn't catch it

when the function was defined.

Quoted 1 line

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.

Quoted 8 lines

Quoted 8 lines

Quoted 6 lines

Quoted 6 lines

Quoted 2 lines

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

Yep, that is the main issue.

.

.

.

Quoted 1 line · Min Hu (Connor)

Added the ethdev dump API which provides functions for query private info

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

Quoted 4 lines

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?

Quoted 20 lines

Signed-off-by Min Hu (Connor) <[email protected]>

Acked-by Morten Brørup <[email protected]>

Acked-by Ray Kinsella <[email protected]>

Acked-by Ajit Khaparde <[email protected]>

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

doc/guides/rel_notes/release_22_03.rst +0 -0

index b20716c521..5895194cde 100644
@@ -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

Quoted 8 lines

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?

Quoted 7 lines

  Removed Items
  -------------

lib/ethdev/ethdev_driver.h +0 -0

index 7d27781f7d..d41d8a2c9d 100644

lib/ethdev/ethdev_driver.h typedef int (*eth_representor_info_get_t)(struct · line 996

Quoted 7 lines

lib/ethdev/ethdev_driver.h +4 -0

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

Quoted 14 lines

lib/ethdev/ethdev_driver.h +10 -0

+ * @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.
    */

lib/ethdev/ethdev_driver.h struct eth_dev_ops { · line 1204

Quoted 5 lines

lib/ethdev/ethdev_driver.h +2 -0

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

Quoted 10 lines

lib/ethdev/ethdev_driver.h +1 -0

+    eth_dev_priv_dump_t eth_dev_priv_dump;
   };
   /**

lib/ethdev/rte_ethdev.c +0 -0

index 917a320afa..07838cc2d9 100644
@@ -6485,6 +6485,21 @@ rte_eth_rx_metadata_negotiate(uint16_t port_id, 
 uint64_t *features)
                  (*dev->dev_ops->rx_metadata_negotiate)(dev, features));

lib/ethdev/rte_ethdev.c rte_eth_rx_metadata_negotiate(uint16_t port_id, · line 6496

Quoted 10 lines

lib/ethdev/rte_ethdev.c +9 -0

   }
+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?

lib/ethdev/rte_ethdev.c rte_eth_rx_metadata_negotiate(uint16_t port_id, uint64_t *features) · line 6498

Quoted 2 lines

lib/ethdev/rte_ethdev.c +2 -0

+    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?

Quoted 17 lines

lib/ethdev/rte_ethdev.c +4 -0

+
+    return ret;
+}
+
   RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
   RTE_INIT(ethdev_init_telemetry)

lib/ethdev/rte_ethdev.h +3 -0

index 147cc1ced3..1c00fdbcd2 100644
@@ -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 

Quoted 4 lines

notice
+ *
+ * Dump ethdev private info to a file.
+ *

Ditto.

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

Quoted 6 lines

+ * @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.
....

lib/ethdev/rte_ethdev.h rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id, · line 5920

Quoted 4 lines

+ */

lib/ethdev/rte_ethdev.h +3 -0

+__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?

Quoted 8 lines

  #ifdef __cplusplus
  }
  #endif

lib/ethdev/version.map +0 -0

index 1f7359c846..376ea139aa 100644

lib/ethdev/version.map EXPERIMENTAL { · line 263

Quoted 8 lines

lib/ethdev/version.map +3 -0

@@ -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 {

.