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

[Ferruh Yigit <ferruh.yigit@amd.com>]

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

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

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

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

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

<...>

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

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

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

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

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

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