[v4,1/3] ethdev: introduce ethdev desc dump API
Checks
Commit Message
From: "Min Hu (Connor)" <humin29@huawei.com>
Added the ethdev Rx/Tx desc dump API which provides functions for query
descriptor from device. HW descriptor info differs in different NICs.
The information demonstrates I/O process which is important for debug.
As the information is different between NICs, the new API is introduced.
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Acked-by: Ray Kinsella <mdr@ashroe.eu>
---
doc/guides/rel_notes/release_22_11.rst | 7 ++++
lib/ethdev/ethdev_driver.h | 46 +++++++++++++++++++++++
lib/ethdev/rte_ethdev.c | 52 ++++++++++++++++++++++++++
lib/ethdev/rte_ethdev.h | 49 ++++++++++++++++++++++++
lib/ethdev/version.map | 2 +
5 files changed, 156 insertions(+)
Comments
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.
On 10/4/22 01:40, Ferruh Yigit wrote:
> On 9/23/2022 8:43 AM, Dongdong Liu wrote:
>
>>
>> From: "Min Hu (Connor)" <humin29@huawei.com>
>>
>> Added the ethdev Rx/Tx desc dump API which provides functions for query
>> descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different
>
> Overall OK to have these new APIs, please find comments below.
>
> Do you think does it worth to list this as one of the PMD future in
> future list, in 'doc/guides/nics/features.rst' ?
IMHO it does not deserve entry in features.
It is a deep debugging using vendor-specific information.
>
> between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>
> <...>
>
>> int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + * A pointer to a file for output.
>> + * @param dev
>> + * Port (ethdev) handle.
>> + * @param queue_id
>> + * The selected queue.
>> + * @param num
>> + * The number of the descriptors to dump.
>> + * @return
>> + * - On success, zero.
>> + * - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> + uint16_t num);
>
> There are other HW desc related APIs, like
> 'rte_eth_rx_descriptor_status()'.
> Should this APIs follow same naming convention:
> 'rte_eth_rx_descriptor_dump()'
> 'rte_eth_tx_descriptor_dump()'
+1 on naming, it should not be bound to HW. SW parts could be
interesting as well.
>
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + * A pointer to a file for output.
>> + * @param dev
>> + * Port (ethdev) handle.
>> + * @param queue_id
>> + * The selected queue.
>> + * @param num
>> + * The number of the descriptors to dump.
>> + * @return
>> + * - On success, zero.
>> + * - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> + uint16_t num);
>
> 'num' is provided, does it assume it starts from offset 0, what do you
> think to provide 'offset' as parameter?
> It may be a use case to start from where tail/head pointer is.
>
>> +
>> +
>> #include <rte_ethdev_core.h>
>>
>> /**
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 03f52fee91..3c7c75b582 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>> rte_mtr_color_in_protocol_priority_get;
>> rte_mtr_color_in_protocol_set;
>> rte_mtr_meter_vlan_table_update;
>> + rte_eth_rx_hw_desc_dump;
>> + rte_eth_tx_hw_desc_dump;
>
> These new APIs should go after "# added in 22.11" comment, if you rebase
> on top of latest HEAD, comment is already there.
>
Hi Ferruh
Many thanks for your review.
On 2022/10/4 6:40, Ferruh Yigit wrote:
> On 9/23/2022 8:43 AM, Dongdong Liu wrote:
>
>>
>> From: "Min Hu (Connor)" <humin29@huawei.com>
>>
>> Added the ethdev Rx/Tx desc dump API which provides functions for query
>> descriptor from device. HW descriptor info differs in different NICs.
>> The information demonstrates I/O process which is important for debug.
>> As the information is different
>
> Overall OK to have these new APIs, please find comments below.
>
> Do you think does it worth to list this as one of the PMD future in
> future list, in 'doc/guides/nics/features.rst' ?
As Andrew said
It does not deserve entry in features.
It is a deep debugging using vendor-specific information.
I agree with him.
>
> between NICs, the new API is introduced.
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Acked-by: Ray Kinsella <mdr@ashroe.eu>
>
> <...>
>
>> int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
>>
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Rx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + * A pointer to a file for output.
>> + * @param dev
>> + * Port (ethdev) handle.
>> + * @param queue_id
>> + * The selected queue.
>> + * @param num
>> + * The number of the descriptors to dump.
>> + * @return
>> + * - On success, zero.
>> + * - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> + uint16_t num);
>
> There are other HW desc related APIs, like
> 'rte_eth_rx_descriptor_status()'.
> Should this APIs follow same naming convention:
> 'rte_eth_rx_descriptor_dump()'
> 'rte_eth_tx_descriptor_dump()'
Looks good, will do.
>
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
>> notice
>> + *
>> + * Dump ethdev Tx descriptor info to a file.
>> + *
>> + * This API is used for debugging, not a dataplane API.
>> + *
>> + * @param file
>> + * A pointer to a file for output.
>> + * @param dev
>> + * Port (ethdev) handle.
>> + * @param queue_id
>> + * The selected queue.
>> + * @param num
>> + * The number of the descriptors to dump.
>> + * @return
>> + * - On success, zero.
>> + * - On failure, a negative value.
>> + */
>> +__rte_experimental
>> +int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t
>> queue_id,
>> + uint16_t num);
>
> 'num' is provided, does it assume it starts from offset 0, what do you
> think to provide 'offset' as parameter?
> It may be a use case to start from where tail/head pointer is.
It will be ok to provide 'offset' as parameter. will change as below.
/**
* @warning
* @b EXPERIMENTAL: this API may change, or be removed, without prior
notice
*
* Dump ethdev Rx descriptor info to a file.
*
* This API is used for debugging, not a dataplane API.
*
* @param port_id
* The port identifier of the Ethernet device.
* @param queue_id
* A Rx queue identifier on this port.
* @param offset
* The offset of the descriptor starting from tail. (0 is the next
* packet to be received by the driver).
* @param num
* The number of the descriptors to dump.
* @param file
* A pointer to a file for output.
* @return
* - On success, zero.
* - On failure, a negative value.
*/
__rte_experimental
int rte_eth_rx_descriptor_dump(uint16_t port_id, uint16_t queue_id,
uint16_t offset, uint16_t num, FILE *file);
>
>> +
>> +
>> #include <rte_ethdev_core.h>
>>
>> /**
>> diff --git a/lib/ethdev/version.map b/lib/ethdev/version.map
>> index 03f52fee91..3c7c75b582 100644
>> --- a/lib/ethdev/version.map
>> +++ b/lib/ethdev/version.map
>> @@ -285,6 +285,8 @@ EXPERIMENTAL {
>> rte_mtr_color_in_protocol_priority_get;
>> rte_mtr_color_in_protocol_set;
>> rte_mtr_meter_vlan_table_update;
>> + rte_eth_rx_hw_desc_dump;
>> + rte_eth_tx_hw_desc_dump;
>
> These new APIs should go after "# added in 22.11" comment, if you rebase
> on top of latest HEAD, comment is already there.
Will rebase on top of latest HEAD.
Thanks,
Dongdong
>
> .
>
On Thu, 6 Oct 2022 16:26:24 +0800
Dongdong Liu <liudongdong3@huawei.com> wrote:
> > Do you think does it worth to list this as one of the PMD future in
> > future list, in 'doc/guides/nics/features.rst' ?
> As Andrew said
> It does not deserve entry in features.
> It is a deep debugging using vendor-specific information.
>
> I agree with him.
This should never be a stable API either. Format may change.
On 10/6/2022 6:13 PM, Stephen Hemminger wrote:
> On Thu, 6 Oct 2022 16:26:24 +0800
> Dongdong Liu <liudongdong3@huawei.com> wrote:
>
>>> Do you think does it worth to list this as one of the PMD future in
>>> future list, in 'doc/guides/nics/features.rst' ?
>> As Andrew said
>> It does not deserve entry in features.
>> It is a deep debugging using vendor-specific information.
>>
>> I agree with him.
>
> This should never be a stable API either. Format may change.
Format is not defined in the API, it is vendor specific. But the API
itself can be stable I think, after we have more PMDs implement it.
@@ -55,6 +55,13 @@ New Features
Also, make sure to start the actual text at the margin.
=======================================================
+* **Added ethdev desc dump API, to dump Rx/Tx desc info from device.**
+
+Added the ethdev Rx/Tx desc dump API which provides functions for query
+descriptor from device. The descriptor info differs in different NICs.
+The information demonstrates I/O process which is important for debug.
+As the information is different between NICs, the new API is introduced.
+The dump format is vendor-specific.
Removed Items
-------------
@@ -1093,6 +1093,47 @@ typedef int (*eth_rx_queue_avail_thresh_query_t)(struct rte_eth_dev *dev,
uint16_t *rx_queue_id,
uint8_t *avail_thresh);
+
+/**
+ * @internal
+ * Dump Rx descriptor info to a file.
+ *
+ * It is used for debugging, not a dataplane API.
+ *
+ * @param file
+ * A pointer to a file for output.
+ * @param dev
+ * Port (ethdev) handle.
+ * @param queue_id
+ * The selected queue.
+ * @param num
+ * The number of the descriptors to dump.
+ * @return
+ * Negative errno value on error, zero on success.
+ */
+typedef int (*eth_rx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+ uint16_t queue_id, uint16_t num);
+
+/**
+ * @internal
+ * Dump Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ * A pointer to a file for output.
+ * @param dev
+ * Port (ethdev) handle.
+ * @param queue_id
+ * The selected queue.
+ * @param num
+ * The number of the descriptors to dump.
+ * @return
+ * Negative errno value on error, zero on success.
+ */
+typedef int (*eth_tx_hw_desc_dump_t)(FILE *file, const struct rte_eth_dev *dev,
+ uint16_t queue_id, uint16_t num);
+
/**
* @internal A structure containing the functions exported by an Ethernet driver.
*/
@@ -1308,6 +1349,11 @@ struct eth_dev_ops {
eth_rx_queue_avail_thresh_set_t rx_queue_avail_thresh_set;
/** Query Rx queue available descriptors threshold event */
eth_rx_queue_avail_thresh_query_t rx_queue_avail_thresh_query;
+
+ /** Dump Rx descriptor info */
+ eth_rx_hw_desc_dump_t eth_rx_hw_desc_dump;
+ /** Dump Tx descriptor info */
+ eth_tx_hw_desc_dump_t eth_tx_hw_desc_dump;
};
/**
@@ -5917,6 +5917,58 @@ rte_eth_dev_priv_dump(uint16_t port_id, FILE *file)
return eth_err(port_id, (*dev->dev_ops->eth_dev_priv_dump)(dev, file));
}
+int
+rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+ uint16_t num)
+{
+ struct rte_eth_dev *dev;
+ int ret;
+
+ if (file == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+ return -EINVAL;
+ }
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (queue_id >= dev->data->nb_rx_queues) {
+ RTE_ETHDEV_LOG(ERR, "Invalid Rx queue_id=%u\n", queue_id);
+ return -EINVAL;
+ }
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_rx_hw_desc_dump, -ENOTSUP);
+ ret = (*dev->dev_ops->eth_rx_hw_desc_dump)(file, dev, queue_id, num);
+
+ return ret;
+}
+
+int
+rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+ uint16_t num)
+{
+ struct rte_eth_dev *dev;
+ int ret;
+
+ if (file == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Invalid file (NULL)\n");
+ return -EINVAL;
+ }
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+
+ if (queue_id >= dev->data->nb_tx_queues) {
+ RTE_ETHDEV_LOG(ERR, "Invalid Tx queue_id=%u\n", queue_id);
+ return -EINVAL;
+ }
+
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->eth_tx_hw_desc_dump, -ENOTSUP);
+ ret = (*dev->dev_ops->eth_tx_hw_desc_dump)(file, dev, queue_id, num);
+
+ return ret;
+}
+
RTE_LOG_REGISTER_DEFAULT(rte_eth_dev_logtype, INFO);
RTE_INIT(ethdev_init_telemetry)
@@ -5221,6 +5221,55 @@ typedef struct {
__rte_experimental
int rte_eth_dev_priv_dump(uint16_t port_id, FILE *file);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Rx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ * A pointer to a file for output.
+ * @param dev
+ * Port (ethdev) handle.
+ * @param queue_id
+ * The selected queue.
+ * @param num
+ * The number of the descriptors to dump.
+ * @return
+ * - On success, zero.
+ * - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_rx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+ uint16_t num);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Dump ethdev Tx descriptor info to a file.
+ *
+ * This API is used for debugging, not a dataplane API.
+ *
+ * @param file
+ * A pointer to a file for output.
+ * @param dev
+ * Port (ethdev) handle.
+ * @param queue_id
+ * The selected queue.
+ * @param num
+ * The number of the descriptors to dump.
+ * @return
+ * - On success, zero.
+ * - On failure, a negative value.
+ */
+__rte_experimental
+int rte_eth_tx_hw_desc_dump(FILE *file, uint16_t port_id, uint16_t queue_id,
+ uint16_t num);
+
+
#include <rte_ethdev_core.h>
/**
@@ -285,6 +285,8 @@ EXPERIMENTAL {
rte_mtr_color_in_protocol_priority_get;
rte_mtr_color_in_protocol_set;
rte_mtr_meter_vlan_table_update;
+ rte_eth_rx_hw_desc_dump;
+ rte_eth_tx_hw_desc_dump;
};
INTERNAL {