[RFC] ethdev: introduce ethdev dump API

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

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

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?
  

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

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

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

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?
  

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