[v3,1/7] ethdev: support report register names and filter
Checks
Commit Message
This patch adds "filter" and "names" fields to "rte_dev_reg_info"
structure. Names of registers in data fields can be reported and
the registers can be filtered by their names.
The new API rte_eth_dev_get_reg_info_ext() is added to support
reporting names and filtering by names. And the original API
rte_eth_dev_get_reg_info() does not use the name and filter fields.
A local variable is used in rte_eth_dev_get_reg_info for
compatibility. If the drivers does not report the names, set them
to "offset_XXX".
Signed-off-by: Jie Hai <haijie1@huawei.com>
---
doc/guides/rel_notes/release_24_03.rst | 9 +++++++
lib/ethdev/rte_dev_info.h | 11 ++++++++
lib/ethdev/rte_ethdev.c | 36 ++++++++++++++++++++++++++
lib/ethdev/rte_ethdev.h | 28 ++++++++++++++++++++
lib/ethdev/version.map | 1 +
5 files changed, 85 insertions(+)
Comments
On Tue, 20 Feb 2024 18:58:17 +0800
Jie Hai <haijie1@huawei.com> wrote:
> +* **Added support for dumping regiters with names and filter.**
Spelling
On Tue, 20 Feb 2024 18:58:17 +0800
Jie Hai <haijie1@huawei.com> wrote:
> This patch adds "filter" and "names" fields to "rte_dev_reg_info"
> structure. Names of registers in data fields can be reported and
> the registers can be filtered by their names.
>
> The new API rte_eth_dev_get_reg_info_ext() is added to support
> reporting names and filtering by names. And the original API
> rte_eth_dev_get_reg_info() does not use the name and filter fields.
> A local variable is used in rte_eth_dev_get_reg_info for
> compatibility. If the drivers does not report the names, set them
> to "offset_XXX".
>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
> doc/guides/rel_notes/release_24_03.rst | 9 +++++++
> lib/ethdev/rte_dev_info.h | 11 ++++++++
> lib/ethdev/rte_ethdev.c | 36 ++++++++++++++++++++++++++
> lib/ethdev/rte_ethdev.h | 28 ++++++++++++++++++++
> lib/ethdev/version.map | 1 +
> 5 files changed, 85 insertions(+)
Could you add support to DPDK ethtool for displaying these?
On Tue, 20 Feb 2024 18:58:17 +0800
Jie Hai <haijie1@huawei.com> wrote:
> @@ -20,6 +25,12 @@ struct rte_dev_reg_info {
> uint32_t length; /**< Number of registers to fetch */
> uint32_t width; /**< Size of device register */
> uint32_t version; /**< Device version */
> + /**
> + * Filter for target subset of registers.
> + * This field could affects register selection for data/length/names.
> + */
> + char *filter;
> + struct rte_eth_reg_name *names; /**< Registers name saver */
> };
filter and names should be const, i.e won't get modified by call.
On Tue, 20 Feb 2024 18:58:17 +0800
Jie Hai <haijie1@huawei.com> wrote:
> int
> rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
> +{
> + struct rte_dev_reg_info reg_info = { 0 };
> + int ret;
> +
> + if (info == NULL) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Cannot get ethdev port %u register info to NULL",
> + port_id);
> + return -EINVAL;
> + }
> +
> + reg_info.length = info->length;
> + reg_info.data = info->data;
> + reg_info.names = NULL;
> + reg_info.filter = NULL;
Those NULL assignments are unnecessary, already initialized structure.
On 2024/2/20 23:09, Stephen Hemminger wrote:
> On Tue, 20 Feb 2024 18:58:17 +0800
> Jie Hai <haijie1@huawei.com> wrote:
>
>> +* **Added support for dumping regiters with names and filter.**
>
> Spelling
> .
Hi, Stephen,
Thanks for your review.
Will fix in V4.
Best Regards,
Jie Hai
On 2024/2/20 23:14, Stephen Hemminger wrote:
> On Tue, 20 Feb 2024 18:58:17 +0800
> Jie Hai <haijie1@huawei.com> wrote:
>
>> int
>> rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
>> +{
>> + struct rte_dev_reg_info reg_info = { 0 };
>> + int ret;
>> +
>> + if (info == NULL) {
>> + RTE_ETHDEV_LOG_LINE(ERR,
>> + "Cannot get ethdev port %u register info to NULL",
>> + port_id);
>> + return -EINVAL;
>> + }
>> +
>> + reg_info.length = info->length;
>> + reg_info.data = info->data;
>> + reg_info.names = NULL;
>> + reg_info.filter = NULL;
>
> Those NULL assignments are unnecessary, already initialized structure.
> .
Hi, Stephen,
Thanks for your review.
Will fix in V4.
Best Regards,
Jie Hai
On 2024/2/20 23:13, Stephen Hemminger wrote:
> On Tue, 20 Feb 2024 18:58:17 +0800
> Jie Hai <haijie1@huawei.com> wrote:
>
>> This patch adds "filter" and "names" fields to "rte_dev_reg_info"
>> structure. Names of registers in data fields can be reported and
>> the registers can be filtered by their names.
>>
>> The new API rte_eth_dev_get_reg_info_ext() is added to support
>> reporting names and filtering by names. And the original API
>> rte_eth_dev_get_reg_info() does not use the name and filter fields.
>> A local variable is used in rte_eth_dev_get_reg_info for
>> compatibility. If the drivers does not report the names, set them
>> to "offset_XXX".
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
>> doc/guides/rel_notes/release_24_03.rst | 9 +++++++
>> lib/ethdev/rte_dev_info.h | 11 ++++++++
>> lib/ethdev/rte_ethdev.c | 36 ++++++++++++++++++++++++++
>> lib/ethdev/rte_ethdev.h | 28 ++++++++++++++++++++
>> lib/ethdev/version.map | 1 +
>> 5 files changed, 85 insertions(+)
>
> Could you add support to DPDK ethtool for displaying these?
> .
Hi, Stephen,
Thanks for your review.
The app proc-info and ethtool already support dump registers with
the rte_eth_dev_get_reg_info API. For the use of the new API,
I think it's better to discuss whether to add or replace the
API of the two apps after the API of the new version is applied.
Best Regards,
Jie Hai
On 2024/2/20 23:14, Stephen Hemminger wrote:
> On Tue, 20 Feb 2024 18:58:17 +0800
> Jie Hai <haijie1@huawei.com> wrote:
>
>> @@ -20,6 +25,12 @@ struct rte_dev_reg_info {
>> uint32_t length; /**< Number of registers to fetch */
>> uint32_t width; /**< Size of device register */
>> uint32_t version; /**< Device version */
>> + /**
>> + * Filter for target subset of registers.
>> + * This field could affects register selection for data/length/names.
>> + */
>> + char *filter;
>> + struct rte_eth_reg_name *names; /**< Registers name saver */
>> };
>
> filter and names should be const, i.e won't get modified by call.
> .
The "filter" can be a pointer to a const string. It's OK to add const.
The "names" could be modified. The APPs are free to assign space for
regisger names and "names" point to the first element.
And the ethdev and drivers are free to modifiy each element.
@@ -102,6 +102,12 @@ New Features
* Added support for comparing result between packet fields or value.
+* **Added support for dumping regiters with names and filter.**
+
+ * Added new API functions ``rte_eth_dev_get_reg_info_ext()`` to and filter
+ the registers by their names and get the information of registers(names,
+ values and other attributes).
+
Removed Items
-------------
@@ -154,6 +160,9 @@ ABI Changes
* No ABI change that would break compatibility with 23.11.
+* ethdev: Added ``filter`` and ``names`` fields to ``rte_dev_reg_info``
+ structure for reporting names of registers and filtering them by names.
+
Known Issues
------------
@@ -11,6 +11,11 @@ extern "C" {
#include <stdint.h>
+#define RTE_ETH_REG_NAME_SIZE 128
+struct rte_eth_reg_name {
+ char name[RTE_ETH_REG_NAME_SIZE];
+};
+
/*
* Placeholder for accessing device registers
*/
@@ -20,6 +25,12 @@ struct rte_dev_reg_info {
uint32_t length; /**< Number of registers to fetch */
uint32_t width; /**< Size of device register */
uint32_t version; /**< Device version */
+ /**
+ * Filter for target subset of registers.
+ * This field could affects register selection for data/length/names.
+ */
+ char *filter;
+ struct rte_eth_reg_name *names; /**< Registers name saver */
};
/*
@@ -6388,8 +6388,39 @@ rte_eth_read_clock(uint16_t port_id, uint64_t *clock)
int
rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
+{
+ struct rte_dev_reg_info reg_info = { 0 };
+ int ret;
+
+ if (info == NULL) {
+ RTE_ETHDEV_LOG_LINE(ERR,
+ "Cannot get ethdev port %u register info to NULL",
+ port_id);
+ return -EINVAL;
+ }
+
+ reg_info.length = info->length;
+ reg_info.data = info->data;
+ reg_info.names = NULL;
+ reg_info.filter = NULL;
+
+ ret = rte_eth_dev_get_reg_info_ext(port_id, ®_info);
+ if (ret != 0)
+ return ret;
+
+ info->length = reg_info.length;
+ info->width = reg_info.width;
+ info->version = reg_info.version;
+ info->offset = reg_info.offset;
+
+ return 0;
+}
+
+int
+rte_eth_dev_get_reg_info_ext(uint16_t port_id, struct rte_dev_reg_info *info)
{
struct rte_eth_dev *dev;
+ uint32_t i;
int ret;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
@@ -6408,6 +6439,11 @@ rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info)
rte_ethdev_trace_get_reg_info(port_id, info, ret);
+ /* Report the default names if drivers not report. */
+ if (info->names != NULL && strlen(info->names[0].name) == 0)
+ for (i = 0; i < info->length; i++)
+ snprintf(info->names[i].name, RTE_ETH_REG_NAME_SIZE,
+ "offset_%x", info->offset + i * info->width);
return ret;
}
@@ -5066,6 +5066,34 @@ __rte_experimental
int rte_eth_get_monitor_addr(uint16_t port_id, uint16_t queue_id,
struct rte_power_monitor_cond *pmc);
+/**
+ * Retrieve the filtered device registers (values and names) and
+ * register attributes (number of registers and register size)
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param info
+ * Pointer to rte_dev_reg_info structure to fill in.
+ * If info->filter is not NULL and the driver does not support names or
+ * filter, return error. If info->filter is NULL, return info for all
+ * registers (seen as filter none).
+ * If info->data is NULL, the function fills in the width and length fields.
+ * If non-NULL, ethdev considers there are enough spaces to store the
+ * registers, and the values of registers whose name contains the filter
+ * string are put into the buffer pointed at by the data field. Do the same
+ * for the names of registers if info->names is not NULL. If drivers do not
+ * report names, default names are given by ethdev.
+ * @return
+ * - (0) if successful.
+ * - (-ENOTSUP) if hardware doesn't support.
+ * - (-EINVAL) if bad parameter.
+ * - (-ENODEV) if *port_id* invalid.
+ * - (-EIO) if device is removed.
+ * - others depends on the specific operations implementation.
+ */
+__rte_experimental
+int rte_eth_dev_get_reg_info_ext(uint16_t port_id, struct rte_dev_reg_info *info);
+
/**
* Retrieve device registers and register attributes (number of registers and
* register size)
@@ -325,6 +325,7 @@ EXPERIMENTAL {
rte_flow_template_table_resizable;
rte_flow_template_table_resize;
rte_flow_template_table_resize_complete;
+ rte_eth_dev_get_reg_info_ext;
};
INTERNAL {