* [PATCH] net/mlx5: use port index as representor index
From: Dariusz Sosnowski @ 2026-05-22 10:19 UTC (permalink / raw)
To: Viacheslav Ovsiienko, Bing Zhao, Ori Kam, Suanming Mou,
Matan Azrad
Cc: dev, stable
Since the offending commit, mlx5 driver supports probing
representors on BlueField DPUs with Socket Direct (SD).
Such card can be connected to 2 different CPUs on the host system.
On DPU, user would see the following network devices:
- p0 and p1 - physical ports
- pf0hpf and pf2hpf - PF0 on CPU 0 and CPU 1 respectively
- pf1hpf and pf3hpf - PF1 on CPU 0 and CPU 1 respectively
mlx5 driver finds the relevant netdev by matching information
provided in representor devarg to phys_port_name
reported by Linux kernel.
For the above interfaces phys_port_name's would be reported
and probed as:
- p0 -> p0, no need for representor devarg
- p1 -> p1, with representor=pf1
- pf0hpf -> c1pf0, with representor=c1pf0vf65535
- pf1hpf -> c1pf1, with representor=c1pf1vf65535
- pf2hpf -> c2pf0, with representor=c2pf0vf65535
- pf3hpf -> c2pf1, with representor=c2pf1vf65535
Although hot-plugging all these representors is successful,
RTE_ETH_FOREACH_MATCHING_DEV() macro would find DPDK ports.
This is caused missing information reported by mlx5 driver,
through rte_eth_representor_info_get() API.
Specifically, mlx5 driver did not report controller index for all
representor ranges.
Until now mlx5 driver used static encoding for 16-bit representor_id:
- 2 bits for representor type
- 2 bits for PF index
- 2 bits for representor index (either VF or SF number)
Controller index was not encoded. This caused the mentioned issue
and on top of that:
- limits the number of PFs
- limits the number of SFs
This patch changes the mlx5 driver logic for
rte_eth_representor_info_get().
Instead of static encoding:
- representor_id's will be dynamically assigned
to each probed representor.
- rte_eth_representor_info_get() will report N ranges:
- N == number of probed ports on single embedded switch
- Each range will define single representor_id
for given controller/PF/VF/SF.
Fixes: 2f7cdd821b1b ("net/mlx5: fix probing to allow BlueField Socket Direct")
Cc: stable@dpdk.org
Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Bing Zhao <bingz@nvidia.com>
---
drivers/net/mlx5/linux/mlx5_os.c | 6 +-
drivers/net/mlx5/mlx5.h | 19 +++
drivers/net/mlx5/mlx5_ethdev.c | 284 +++++++++++++++++++------------
3 files changed, 199 insertions(+), 110 deletions(-)
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 0fc721592b..5305523c1b 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1677,9 +1677,13 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
err = ENOMEM;
goto error;
}
+ priv->port_info.type = spawn->info.name_type;
+ priv->port_info.ctrl_num = spawn->info.ctrl_num;
+ priv->port_info.pf_num = spawn->info.pf_num;
+ priv->port_info.port_num = spawn->info.port_name;
if (priv->representor) {
eth_dev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR;
- eth_dev->data->representor_id = priv->representor_id;
+ eth_dev->data->representor_id = eth_dev->data->port_id;
MLX5_ETH_FOREACH_DEV(port_id, dpdk_dev) {
struct mlx5_priv *opriv =
rte_eth_devices[port_id].data->dev_private;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 49a0c03544..23803b450b 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1984,6 +1984,24 @@ struct mlx5_quota_ctx {
struct mlx5_indexed_pool *quota_ipool; /* Manage quota objects */
};
+/* Stores info parsed from phys_port_name related to given DPDK port. */
+struct mlx5_representor_info {
+ enum mlx5_nl_phys_port_name_type type;
+ /* PCI controller index. 0 if no controller was reported in phys_port_name. */
+ int32_t ctrl_num;
+ /* PF index. */
+ int32_t pf_num;
+ /*
+ * Representor number:
+ *
+ * - For VF/SF - VF/SF index.
+ * - For PFHPF - -1.
+ * - For uplink - physical port index.
+ * - For others - VF representor is assumed, so VF index.
+ */
+ int32_t port_num;
+};
+
struct mlx5_nta_sample_ctx;
struct mlx5_priv {
struct rte_eth_dev_data *dev_data; /* Pointer to device data. */
@@ -2019,6 +2037,7 @@ struct mlx5_priv {
uint32_t vport_meta_tag; /* Used for vport index match ove VF LAG. */
uint32_t vport_meta_mask; /* Used for vport index field match mask. */
uint16_t representor_id; /* UINT16_MAX if not a representor. */
+ struct mlx5_representor_info port_info;
int32_t pf_bond; /* >=0, representor owner PF index in bonding. */
int32_t mpesw_owner; /* >=0, representor owner PF index in MPESW. */
int32_t mpesw_port; /* Related port index of MPESW device. < 0 - no MPESW. */
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index a29cdeeb50..e14b7f148b 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -345,6 +345,23 @@ mlx5_dev_get_max_wq_size(struct mlx5_dev_ctx_shared *sh)
return max_wqe;
}
+/**
+ * Get switch port ID for given DPDK port.
+ *
+ * @param dev
+ * Pointer to Ethernet device structure.
+ * @return
+ * Switch port ID reported through rte_eth_dev_info_get().
+ */
+static uint16_t
+mlx5_dev_switch_info_port_id_get(struct rte_eth_dev *dev)
+{
+ if (rte_eth_dev_is_repr(dev))
+ return dev->data->port_id;
+
+ return UINT16_MAX;
+}
+
/**
* DPDK callback to get information about the device.
*
@@ -401,7 +418,7 @@ mlx5_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
info->dev_capa |= RTE_ETH_DEV_CAPA_RXQ_SHARE;
info->switch_info.name = dev->data->name;
info->switch_info.domain_id = priv->domain_id;
- info->switch_info.port_id = priv->representor_id;
+ info->switch_info.port_id = mlx5_dev_switch_info_port_id_get(dev);
info->switch_info.rx_domain = 0; /* No sub Rx domains. */
if (priv->representor) {
uint16_t port_id;
@@ -472,14 +489,162 @@ mlx5_representor_id_encode(const struct mlx5_switch_info *info,
return MLX5_REPRESENTOR_ID(pf, type, repr);
}
+static unsigned int
+mlx5_representor_info_count_one(struct mlx5_priv *priv)
+{
+ switch (priv->port_info.type) {
+ case MLX5_PHYS_PORT_NAME_TYPE_PFHPF:
+ return 2;
+ case MLX5_PHYS_PORT_NAME_TYPE_UPLINK:
+ /* Only representor uplinks should be reported */
+ if (!priv->representor)
+ return 0;
+ return 1;
+ case MLX5_PHYS_PORT_NAME_TYPE_NOTSET:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_LEGACY:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_PFVF:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_PFSF:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN:
+ /* FALLTHROUGH */
+ default:
+ return 1;
+ }
+}
+
+static unsigned int
+mlx5_representor_info_count(struct rte_eth_dev *dev)
+{
+ struct mlx5_priv *priv = dev->data->dev_private;
+ uint16_t port_id;
+ unsigned int count = 0;
+
+ MLX5_ETH_FOREACH_DEV(port_id, dev->device) {
+ struct mlx5_priv *opriv = rte_eth_devices[port_id].data->dev_private;
+
+ if (!opriv ||
+ opriv->sh != priv->sh ||
+ opriv->domain_id != priv->domain_id)
+ continue;
+
+ count += mlx5_representor_info_count_one(opriv);
+ }
+
+ return count;
+}
+
+static void
+mlx5_representor_info_fill_one(struct mlx5_priv *priv,
+ struct rte_eth_representor_info *info)
+{
+ struct rte_eth_representor_range *range;
+ unsigned int count;
+
+ count = mlx5_representor_info_count_one(priv);
+ if (count == 0)
+ return;
+
+ if (info->nb_ranges + count > info->nb_ranges_alloc) {
+ DRV_LOG(ERR, "port %u representor info already full", priv->dev_data->port_id);
+ return;
+ }
+
+ range = &info->ranges[info->nb_ranges];
+
+ switch (priv->port_info.type) {
+ case MLX5_PHYS_PORT_NAME_TYPE_UPLINK:
+ range->type = RTE_ETH_REPRESENTOR_PF;
+ range->controller = priv->port_info.ctrl_num;
+ range->pf = priv->port_info.port_num;
+ range->id_base = priv->dev_data->port_id;
+ range->id_end = range->id_base;
+ snprintf(range->name, sizeof(range->name), "pf%d", range->pf);
+ break;
+ case MLX5_PHYS_PORT_NAME_TYPE_PFSF:
+ /* Secondly, fill in SF variant. */
+ range->type = RTE_ETH_REPRESENTOR_SF;
+ range->controller = priv->port_info.ctrl_num;
+ range->pf = priv->port_info.pf_num;
+ range->sf = priv->port_info.port_num;
+ range->id_base = priv->dev_data->port_id;
+ range->id_end = range->id_base;
+ snprintf(range->name, sizeof(range->name), "pf%dsf", range->pf);
+ break;
+ case MLX5_PHYS_PORT_NAME_TYPE_PFHPF:
+ /*
+ * Host PF can be probed either through VF(0xffff) or SF(0xffff).
+ * Firstly fill in VF variant.
+ */
+ range->type = RTE_ETH_REPRESENTOR_VF;
+ range->controller = priv->port_info.ctrl_num;
+ range->pf = priv->port_info.pf_num;
+ range->vf = UINT16_MAX;
+ range->id_base = priv->dev_data->port_id;
+ range->id_end = range->id_base;
+ snprintf(range->name, sizeof(range->name), "pf%dvf", range->pf);
+
+ /* Move the SF variant. */
+ range++;
+
+ /* Fill in SF variant. */
+ range->type = RTE_ETH_REPRESENTOR_SF;
+ range->controller = priv->port_info.ctrl_num;
+ range->pf = priv->port_info.pf_num;
+ range->sf = UINT16_MAX;
+ range->id_base = priv->dev_data->port_id;
+ range->id_end = range->id_base;
+ snprintf(range->name, sizeof(range->name), "pf%dsf", range->pf);
+ break;
+ case MLX5_PHYS_PORT_NAME_TYPE_PFVF:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_NOTSET:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_LEGACY:
+ /* FALLTHROUGH */
+ case MLX5_PHYS_PORT_NAME_TYPE_UNKNOWN:
+ range->type = RTE_ETH_REPRESENTOR_VF;
+ range->controller = priv->port_info.ctrl_num;
+ range->pf = priv->port_info.pf_num;
+ range->vf = priv->port_info.port_num;
+ range->id_base = priv->dev_data->port_id;
+ range->id_end = range->id_base;
+ snprintf(range->name, sizeof(range->name), "pf%dvf", range->pf);
+ break;
+ }
+
+ info->nb_ranges += count;
+}
+
+static unsigned int
+mlx5_representor_info_fill(struct rte_eth_dev *dev,
+ struct rte_eth_representor_info *info)
+{
+ struct mlx5_priv *priv = dev->data->dev_private;
+ uint16_t port_id;
+
+ info->controller = priv->port_info.ctrl_num;
+ info->pf = RTE_DEV_TO_PCI(dev->device)->addr.function;
+
+ MLX5_ETH_FOREACH_DEV(port_id, dev->device) {
+ struct mlx5_priv *opriv = rte_eth_devices[port_id].data->dev_private;
+
+ if (!opriv ||
+ opriv->sh != priv->sh ||
+ opriv->domain_id != priv->domain_id)
+ continue;
+
+ mlx5_representor_info_fill_one(opriv, info);
+ }
+
+ return info->nb_ranges;
+}
+
/**
* DPDK callback to get information about representor.
*
- * Representor ID bits definition:
- * vf/sf: 12
- * type: 2
- * pf: 2
- *
* @param dev
* Pointer to Ethernet device structure.
* @param[out] info
@@ -492,110 +657,11 @@ int
mlx5_representor_info_get(struct rte_eth_dev *dev,
struct rte_eth_representor_info *info)
{
- struct mlx5_priv *priv = dev->data->dev_private;
- /* Representor types: PF, VF, HPF@VF, SF and HPF@SF, total 5. */
- int n_type = RTE_ETH_REPRESENTOR_PF + 2; /* Maximal type + 2 for HPFs. */
- int n_pf = 8; /* Maximal number of PFs. */
- int i = 0, pf;
- int n_entries;
-
if (info == NULL)
- goto out;
-
- n_entries = n_type * n_pf;
- if ((uint32_t)n_entries > info->nb_ranges_alloc)
- n_entries = info->nb_ranges_alloc;
-
- info->controller = 0;
- info->pf = 0;
- if (mlx5_is_port_on_mpesw_device(priv)) {
- info->pf = priv->mpesw_port;
- for (i = 0; i < n_pf; i++) {
- /* PF range, both ports will show the same information. */
- info->ranges[i].type = RTE_ETH_REPRESENTOR_PF;
- info->ranges[i].controller = 0;
- info->ranges[i].pf = priv->mpesw_owner + i + 1;
- info->ranges[i].vf = 0;
- /*
- * The representor indexes should be the values set of "priv->mpesw_port".
- * In the real case now, only 1 PF/UPLINK representor is supported.
- * The port index will always be the value of "owner + 1".
- */
- info->ranges[i].id_base =
- MLX5_REPRESENTOR_ID(priv->mpesw_owner,
- info->ranges[i].type,
- info->ranges[i].pf);
- info->ranges[i].id_end =
- MLX5_REPRESENTOR_ID(priv->mpesw_owner,
- info->ranges[i].type,
- info->ranges[i].pf);
- snprintf(info->ranges[i].name,
- sizeof(info->ranges[i].name),
- "pf%d", info->ranges[i].pf);
- }
- } else if (priv->pf_bond >= 0)
- info->pf = priv->pf_bond;
- for (pf = 0; pf < n_pf; ++pf) {
- /* VF range. */
- info->ranges[i].type = RTE_ETH_REPRESENTOR_VF;
- info->ranges[i].controller = 0;
- info->ranges[i].pf = pf;
- info->ranges[i].vf = 0;
- info->ranges[i].id_base =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, 0);
- info->ranges[i].id_end =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- snprintf(info->ranges[i].name,
- sizeof(info->ranges[i].name), "pf%dvf", pf);
- i++;
- if (i == n_entries)
- break;
- /* HPF range of VF type. */
- info->ranges[i].type = RTE_ETH_REPRESENTOR_VF;
- info->ranges[i].controller = 0;
- info->ranges[i].pf = pf;
- info->ranges[i].vf = UINT16_MAX;
- info->ranges[i].id_base =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- info->ranges[i].id_end =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- snprintf(info->ranges[i].name,
- sizeof(info->ranges[i].name), "pf%dvf", pf);
- i++;
- if (i == n_entries)
- break;
- /* SF range. */
- info->ranges[i].type = RTE_ETH_REPRESENTOR_SF;
- info->ranges[i].controller = 0;
- info->ranges[i].pf = pf;
- info->ranges[i].vf = 0;
- info->ranges[i].id_base =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, 0);
- info->ranges[i].id_end =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- snprintf(info->ranges[i].name,
- sizeof(info->ranges[i].name), "pf%dsf", pf);
- i++;
- if (i == n_entries)
- break;
- /* HPF range of SF type. */
- info->ranges[i].type = RTE_ETH_REPRESENTOR_SF;
- info->ranges[i].controller = 0;
- info->ranges[i].pf = pf;
- info->ranges[i].vf = UINT16_MAX;
- info->ranges[i].id_base =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- info->ranges[i].id_end =
- MLX5_REPRESENTOR_ID(pf, info->ranges[i].type, -1);
- snprintf(info->ranges[i].name,
- sizeof(info->ranges[i].name), "pf%dsf", pf);
- i++;
- if (i == n_entries)
- break;
- }
- info->nb_ranges = i;
-out:
- return n_type * n_pf;
+ return mlx5_representor_info_count(dev);
+
+ return mlx5_representor_info_fill(dev, info);
+
}
/**
--
2.47.3
^ permalink raw reply related
* RE: [PATCH v1 1/1] net/iavf: fix large VF IRQ mapping
From: Morten Brørup @ 2026-05-22 9:58 UTC (permalink / raw)
To: Burakov, Anatoly, Vladimir Medvedkin, Bruce Richardson
Cc: dev, David Marchand
In-Reply-To: <8b0e6d7f-c7c1-4d0d-8b76-ea2f1fd6655c@intel.com>
> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> Sent: Friday, 22 May 2026 11.46
>
> On 5/8/2026 11:16 AM, Morten Brørup wrote:
> >> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
> >> Sent: Thursday, 7 May 2026 10.09
> >>
> >> On 5/6/2026 5:58 PM, David Marchand wrote:
> >>> On Wed, 6 May 2026 at 16:07, Anatoly Burakov
> >> <anatoly.burakov@intel.com> wrote:
> >>>>
> >>>> The PF will check buffer size for being too big, and the chunk
> >> sizing code
> >>>> correctly calls that out. However, the size was actually still too
> >> big
> >>>> because `struct virtchnl_queue_vector_maps` already had one queue
> >> vector
> >>>> as part of its definition, so `chunk_sz` was too big by 1.
> >>>>
> >>>> Fixes: 292d3b781ac4 ("net/iavf: replace unnecessary hugepage
> memory
> >> allocations")
> >>>>
> >>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>> ---
> >>>> drivers/net/intel/iavf/iavf_vchnl.c | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c
> >> b/drivers/net/intel/iavf/iavf_vchnl.c
> >>>> index c2f340db81..dd09b0fa61 100644
> >>>> --- a/drivers/net/intel/iavf/iavf_vchnl.c
> >>>> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
> >>>> @@ -1528,7 +1528,7 @@ iavf_config_irq_map_lv_chunk(struct
> >> iavf_adapter *adapter,
> >>>>
> >>>> /* for some reason PF side checks for buffer being too
> big,
> >> so adjust it down */
> >>>
> >>> The comment above can be removed?
> >>
> >> No, it's still relevant, because it refers to the fact that we're
> >> adjusting the total length downwards as opposed to leaving it at max
> >> size.
> >>
> >>>
> >>>> buf_len = sizeof(struct virtchnl_queue_vector_maps) +
> >>>> - sizeof(struct virtchnl_queue_vector) * chunk_sz;
> >>>> + sizeof(struct virtchnl_queue_vector) * (chunk_sz
> -
> >> 1);
> >>>
> >>> - did you make sure you did not break compat with previous version
> of
> >>> Intel out of tree PF driver (since this concerns configuring "Large
> >>> VF")?
> >>
> >> The commit in question *did* break things with previous out of tree
> PF
> >> driver. This commit fixes the breakage introduced in that commit.
> The
> >> commit being fixed was a refactor, which specified size as N-1.
> >>
> >>>
> >>> - all those virtchnl list struct have the same elems[1] issue.
> >>> Kernel side did some cleanups some time ago, maybe time for DPDK to
> >> do
> >>> the same...?
> >>>
> >>
> >> Yes, it is indeed time to do the same, but not as part of this
> >> patchset,
> >> and not before the base driver code is updated to do the same. There
> is
> >> some background work happening on that front already, but there are
> a
> >> lot of dependencies and moving parts, so we can't just change this
> >> willy
> >> nilly.
> >
> > Is there a timeline for this fix?
> >
>
> No specific timeline, unfortunately.
The drivers were very recently updated to use memcpy() instead of rte_memcpy(), so now we can enable stringop-overflow warnings in rte_memcpy().
What really annoyed me was the need to disable these warnings in a general library function due to some drivers.
Since this problem has now been resolved, and the scope of the elems[1] problem has shrunk from system-wide to Intel drivers only, I'm not that concerned about timeline anymore. ;-)
-Morten
^ permalink raw reply
* Re: [PATCH v1 1/1] net/iavf: fix large VF IRQ mapping
From: Burakov, Anatoly @ 2026-05-22 9:45 UTC (permalink / raw)
To: Morten Brørup, Vladimir Medvedkin, Bruce Richardson
Cc: dev, David Marchand
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35F65860@smartserver.smartshare.dk>
On 5/8/2026 11:16 AM, Morten Brørup wrote:
>> From: Burakov, Anatoly [mailto:anatoly.burakov@intel.com]
>> Sent: Thursday, 7 May 2026 10.09
>>
>> On 5/6/2026 5:58 PM, David Marchand wrote:
>>> On Wed, 6 May 2026 at 16:07, Anatoly Burakov
>> <anatoly.burakov@intel.com> wrote:
>>>>
>>>> The PF will check buffer size for being too big, and the chunk
>> sizing code
>>>> correctly calls that out. However, the size was actually still too
>> big
>>>> because `struct virtchnl_queue_vector_maps` already had one queue
>> vector
>>>> as part of its definition, so `chunk_sz` was too big by 1.
>>>>
>>>> Fixes: 292d3b781ac4 ("net/iavf: replace unnecessary hugepage memory
>> allocations")
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>> drivers/net/intel/iavf/iavf_vchnl.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/intel/iavf/iavf_vchnl.c
>> b/drivers/net/intel/iavf/iavf_vchnl.c
>>>> index c2f340db81..dd09b0fa61 100644
>>>> --- a/drivers/net/intel/iavf/iavf_vchnl.c
>>>> +++ b/drivers/net/intel/iavf/iavf_vchnl.c
>>>> @@ -1528,7 +1528,7 @@ iavf_config_irq_map_lv_chunk(struct
>> iavf_adapter *adapter,
>>>>
>>>> /* for some reason PF side checks for buffer being too big,
>> so adjust it down */
>>>
>>> The comment above can be removed?
>>
>> No, it's still relevant, because it refers to the fact that we're
>> adjusting the total length downwards as opposed to leaving it at max
>> size.
>>
>>>
>>>> buf_len = sizeof(struct virtchnl_queue_vector_maps) +
>>>> - sizeof(struct virtchnl_queue_vector) * chunk_sz;
>>>> + sizeof(struct virtchnl_queue_vector) * (chunk_sz -
>> 1);
>>>
>>> - did you make sure you did not break compat with previous version of
>>> Intel out of tree PF driver (since this concerns configuring "Large
>>> VF")?
>>
>> The commit in question *did* break things with previous out of tree PF
>> driver. This commit fixes the breakage introduced in that commit. The
>> commit being fixed was a refactor, which specified size as N-1.
>>
>>>
>>> - all those virtchnl list struct have the same elems[1] issue.
>>> Kernel side did some cleanups some time ago, maybe time for DPDK to
>> do
>>> the same...?
>>>
>>
>> Yes, it is indeed time to do the same, but not as part of this
>> patchset,
>> and not before the base driver code is updated to do the same. There is
>> some background work happening on that front already, but there are a
>> lot of dependencies and moving parts, so we can't just change this
>> willy
>> nilly.
>
> Is there a timeline for this fix?
>
No specific timeline, unfortunately.
--
Thanks,
Anatoly
^ permalink raw reply
* RE: [PATCH] net/txgbe: fix AML mailbox lock acquisition
From: Zaiyu Wang @ 2026-05-22 8:54 UTC (permalink / raw)
To: 'Stephen Hemminger', dev; +Cc: stable, 'Jiawen Wu'
In-Reply-To: <20260522050032.1354427-1-stephen@networkplumber.org>
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, May 22, 2026 1:01 PM
> To: dev@dpdk.org; dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; stable@dpdk.org;
> Jiawen Wu <jiawenwu@trustnetic.com>; Zaiyu Wang
> <zaiyuwang@trustnetic.com>; Stephen Hemminger
> <stephen@networkplumber.org>; stable@dpdk.org; Jiawen Wu
> <jiawenwu@trustnetic.com>; Zaiyu Wang <zaiyuwang@trustnetic.com>
> Subject: [PATCH] net/txgbe: fix AML mailbox lock acquisition
>
> The try-lock spin loop in txgbe_host_interface_command_aml() has the
condition
> inverted. rte_atomic32_test_and_set returns non-zero on successful
acquisition (0
> -> 1), zero when the lock was already held. Walk through the two cases:
>
> swfw_busy was 0 (free): test_and_set returns 1, sets to 1, loop
> body runs and sleeps 1ms. Next iteration finds 1, returns 0, loop
> exits. Lock held, after an unnecessary 1ms sleep.
>
> swfw_busy was 1 (busy): test_and_set returns 0, loop exits
> immediately. The caller proceeds without holding the lock,
> racing with the in-flight host interface command.
>
> Invert the condition so the loop spins while the lock remains held and
exits only
> when acquisition succeeds, matching the intent of the surrounding timeout
> machinery.
>
> Fixes: 6a139ade82e7 ("net/txgbe: add new SW-FW mailbox interface")
> Cc: stable@dpdk.org
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> drivers/net/txgbe/base/txgbe_mng.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/txgbe/base/txgbe_mng.c
> b/drivers/net/txgbe/base/txgbe_mng.c
> index a1974820b6..dcd8f58a68 100644
> --- a/drivers/net/txgbe/base/txgbe_mng.c
> +++ b/drivers/net/txgbe/base/txgbe_mng.c
> @@ -185,7 +185,7 @@ txgbe_host_interface_command_aml(struct txgbe_hw
> *hw, u32 *buffer,
> }
>
> /* try to get lock */
> - while (rte_atomic32_test_and_set(&hw->swfw_busy)) {
> + while (rte_atomic32_test_and_set(&hw->swfw_busy) == 0) {
> timeout--;
> if (!timeout)
> return TXGBE_ERR_TIMEOUT;
> --
> 2.53.0
Thanks for the patch and the detailed analysis.
The fix is correct. This code was originally ported from our out-of-tree
Linux kernel driver, where it used a classic test_and_set_bit() that returns
the old bit value (0 on success). When adapting it for DPDK, we overlooked
that rte_atomic32_test_and_set() returns the opposite semantic.
Acked-by: Zaiyu Wang <zaiyuwang@trustnetic.com>
^ permalink raw reply
* Re: [PATCH 0/2] extend interactive telemetry script
From: Bruce Richardson @ 2026-05-22 7:51 UTC (permalink / raw)
To: fengchengwen; +Cc: dev
In-Reply-To: <628f817f-7f6a-41c5-b21f-e7b28224fb51@huawei.com>
On Fri, May 22, 2026 at 08:44:55AM +0800, fengchengwen wrote:
> On 5/21/2026 11:39 PM, Bruce Richardson wrote:
> > To simplify interactive telemetry script for general use, i.e. not from
> > other scripts, we can add two new features to it:
> >
> > 1. Support for FOREACH to allow gathering a set of output values across
> > a list of ports or devices, e.g. ethdevs or rawdevs.
> > 2. Support having predefined aliases in a file in the user's home
> > directory to simplify the use of more complicated FOREACH commands.
>
> +1
> How about add help command to introducing above FOREACH and aliases?
>
Sure, not a bad idea if we are extending the script to make it more
powerful. Will add in a V2.
/Bruce
^ permalink raw reply
* [PATCH v3 2/2] net/mana: add documentation for device reset support
From: Wei Hu @ 2026-05-22 6:59 UTC (permalink / raw)
To: dev, stephen; +Cc: longli, weh
In-Reply-To: <20260522065943.126703-1-weh@linux.microsoft.com>
From: Wei Hu <weh@microsoft.com>
Add device reset section to mana.rst describing the two-phase
reset flow, recovery events, and PCI remove event dependency.
Add release note entry for the 26.07 release.
Signed-off-by: Wei Hu <weh@microsoft.com>
---
doc/guides/nics/mana.rst | 33 ++++++++++++++++++++++++++
doc/guides/rel_notes/release_26_07.rst | 8 +++++++
2 files changed, 41 insertions(+)
diff --git a/doc/guides/nics/mana.rst b/doc/guides/nics/mana.rst
index 0fcab6e2f6..a89338291a 100644
--- a/doc/guides/nics/mana.rst
+++ b/doc/guides/nics/mana.rst
@@ -71,3 +71,36 @@ The user can specify below argument in devargs.
The default value is not set,
meaning all the NICs will be probed and loaded.
User can specify multiple mac=xx:xx:xx:xx:xx:xx arguments for up to 8 NICs.
+
+Device Reset Support
+--------------------
+
+The MANA PMD supports automatic recovery from hardware service reset events.
+When the MANA kernel driver receives a hardware service event,
+it initiates a device reset and notifies userspace
+via ``IBV_EVENT_DEVICE_FATAL``.
+
+The driver handles this transparently through a two-phase reset flow:
+
+* **Enter phase**: The driver stops the data path,
+ waits for all threads to reach a quiescent state using RCU,
+ tears down IB resources and queues,
+ and unmaps secondary process doorbell pages.
+
+* **Exit phase**: After a delay for hardware recovery,
+ a control thread re-probes the PCI device,
+ reinstalls the interrupt handler,
+ reinitializes resources, and restarts queues.
+
+The driver emits the following ethdev recovery events
+to notify upper layers (e.g. netvsc) of the reset lifecycle:
+
+* ``RTE_ETH_EVENT_ERR_RECOVERING`` — reset has started
+* ``RTE_ETH_EVENT_RECOVERY_SUCCESS`` — device has recovered successfully
+* ``RTE_ETH_EVENT_RECOVERY_FAILED`` — recovery failed
+
+To distinguish a PCI hot-remove from a service reset,
+the driver registers for PCI device removal events.
+This requires the application to call ``rte_dev_event_monitor_start()``
+for removal events to be delivered
+(e.g. testpmd ``--hot-plug-handling`` option).
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 92c90673bc..114bc09c5d 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -77,6 +77,14 @@ New Features
Added network driver for the Linkdata Network Adapters.
+* **Added device reset support to the MANA PMD.**
+
+ Added automatic recovery from hardware service reset events
+ in the MANA poll mode driver. The driver uses ethdev recovery events
+ (``RTE_ETH_EVENT_ERR_RECOVERING``, ``RTE_ETH_EVENT_RECOVERY_SUCCESS``,
+ ``RTE_ETH_EVENT_RECOVERY_FAILED``) to notify upper layers of the
+ reset lifecycle.
+
Removed Items
-------------
--
2.34.1
^ permalink raw reply related
* [PATCH v3 1/2] net/mana: add device reset support
From: Wei Hu @ 2026-05-22 6:59 UTC (permalink / raw)
To: dev, stephen; +Cc: longli, weh
In-Reply-To: <20260522065943.126703-1-weh@linux.microsoft.com>
From: Wei Hu <weh@microsoft.com>
Add support for handling hardware reset events in the MANA driver.
When the MANA kernel driver receives a hardware service event, it
initiates a device reset and notifies userspace via
IBV_EVENT_DEVICE_FATAL. The DPDK driver handles this by performing
an automatic teardown and recovery sequence.
The reset flow has two phases. In the enter phase, running on the
EAL interrupt thread, the driver transitions the device state,
waits for data path threads to reach a quiescent state using RCU,
stops queues, tears down IB resources, and frees per-queue MR
caches. A control thread is then spawned to handle the exit phase:
it waits for the hardware to recover, unregisters the interrupt
handler, re-probes the PCI device, reinitializes MR caches, and
restarts queues.
A per-device spinlock serializes the reset path with ethdev
operations. Operations that cannot wait (configure, queue setup)
return -EBUSY during reset, while dev_stop and dev_close join the
reset thread and use a blocking lock to ensure proper sequencing.
Multi-process support is included: secondary processes unmap and
remap doorbell pages via IPC during the reset enter and exit
phases. Data path functions in both primary and secondary
processes check the device state atomically and return early when
the device is not active.
The driver uses ethdev recovery events to notify upper layers
(e.g. netvsc) of the reset lifecycle: RTE_ETH_EVENT_ERR_RECOVERING
on entry, RTE_ETH_EVENT_RECOVERY_SUCCESS or
RTE_ETH_EVENT_RECOVERY_FAILED on completion. A PCI device removal
event callback distinguishes hot-remove from service reset.
Signed-off-by: Wei Hu <weh@microsoft.com>
---
drivers/net/mana/mana.c | 995 ++++++++++++++++++++++++++++++++---
drivers/net/mana/mana.h | 33 +-
drivers/net/mana/meson.build | 2 +-
drivers/net/mana/mp.c | 89 +++-
drivers/net/mana/mr.c | 6 +-
drivers/net/mana/rx.c | 24 +-
drivers/net/mana/tx.c | 40 +-
7 files changed, 1082 insertions(+), 107 deletions(-)
diff --git a/drivers/net/mana/mana.c b/drivers/net/mana/mana.c
index 67396cda1f..898d9314d3 100644
--- a/drivers/net/mana/mana.c
+++ b/drivers/net/mana/mana.c
@@ -13,7 +13,10 @@
#include <ethdev_pci.h>
#include <rte_kvargs.h>
#include <rte_eal_paging.h>
+#include <rte_alarm.h>
#include <rte_pci.h>
+#include <rte_rcu_qsbr.h>
+#include <rte_lock_annotations.h>
#include <infiniband/verbs.h>
#include <infiniband/manadv.h>
@@ -103,6 +106,23 @@ mana_dev_configure(struct rte_eth_dev *dev)
RTE_ETH_RX_OFFLOAD_VLAN_STRIP);
priv->num_queues = dev->data->nb_rx_queues;
+ DRV_LOG(DEBUG, "priv %p, port %u, dev port %u, num_queues: %u",
+ priv, priv->port_id, priv->dev_port, priv->num_queues);
+
+ /*
+ * Register data path thread IDs (rx and tx) with the RCU
+ * quiescent state variable for device state synchronization.
+ */
+ for (int i = 0; i < 2 * priv->num_queues; i++) {
+ if (rte_rcu_qsbr_thread_register(priv->dev_state_qsv, i) != 0) {
+ DRV_LOG(ERR, "Failed to register rcu qsv thread %d of total %d",
+ i, 2 * priv->num_queues - 1);
+ return -EINVAL;
+ }
+ DRV_LOG(DEBUG,
+ "Register thread 0x%x for priv %p, port %u",
+ i, priv, priv->port_id);
+ }
manadv_set_context_attr(priv->ib_ctx, MANADV_CTX_ATTR_BUF_ALLOCATORS,
(void *)((uintptr_t)&(struct manadv_ctx_allocators){
@@ -214,8 +234,8 @@ mana_dev_start(struct rte_eth_dev *dev)
DRV_LOG(INFO, "TX/RX queues have started");
- /* Enable datapath for secondary processes */
- mana_mp_req_on_rxtx(dev, MANA_MP_REQ_START_RXTX);
+ /* Intentionally ignore errors — secondary may not be running */
+ (void)mana_mp_req_on_rxtx(dev, MANA_MP_REQ_START_RXTX);
ret = rxq_intr_enable(priv);
if (ret) {
@@ -242,26 +262,33 @@ mana_dev_stop(struct rte_eth_dev *dev)
{
int ret;
struct mana_priv *priv = dev->data->dev_private;
-
- rxq_intr_disable(priv);
+ enum mana_device_state state;
+
+ state = rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire);
+ if (state == MANA_DEV_ACTIVE ||
+ state == MANA_DEV_RESET_FAILED) {
+ rxq_intr_disable(priv);
+ DRV_LOG(DEBUG, "rxq_intr_disable called");
+ }
dev->tx_pkt_burst = mana_tx_burst_removed;
dev->rx_pkt_burst = mana_rx_burst_removed;
- /* Stop datapath on secondary processes */
- mana_mp_req_on_rxtx(dev, MANA_MP_REQ_STOP_RXTX);
+ /* Intentionally ignore errors — secondary may not be running */
+ (void)mana_mp_req_on_rxtx(dev, MANA_MP_REQ_STOP_RXTX);
rte_wmb();
ret = mana_stop_tx_queues(dev);
if (ret) {
- DRV_LOG(ERR, "failed to stop tx queues");
+ DRV_LOG(ERR, "failed to stop tx queues, ret %d", ret);
return ret;
}
ret = mana_stop_rx_queues(dev);
if (ret) {
- DRV_LOG(ERR, "failed to stop tx queues");
+ DRV_LOG(ERR, "failed to stop rx queues, ret %d", ret);
return ret;
}
@@ -275,36 +302,69 @@ mana_dev_close(struct rte_eth_dev *dev)
{
struct mana_priv *priv = dev->data->dev_private;
int ret;
+ enum mana_device_state state;
+ DRV_LOG(DEBUG, "Free MR for priv %p", priv);
mana_remove_all_mr(priv);
- ret = mana_intr_uninstall(priv);
- if (ret)
- return ret;
+ state = rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire);
+ if (state == MANA_DEV_ACTIVE ||
+ state == MANA_DEV_RESET_FAILED) {
+ ret = mana_intr_uninstall(priv);
+ if (ret)
+ return ret;
+ }
if (priv->ib_parent_pd) {
- int err = ibv_dealloc_pd(priv->ib_parent_pd);
- if (err)
- DRV_LOG(ERR, "Failed to deallocate parent PD: %d", err);
+ ret = ibv_dealloc_pd(priv->ib_parent_pd);
+ if (ret)
+ DRV_LOG(ERR,
+ "Failed to deallocate parent PD: %d", ret);
priv->ib_parent_pd = NULL;
}
if (priv->ib_pd) {
- int err = ibv_dealloc_pd(priv->ib_pd);
- if (err)
- DRV_LOG(ERR, "Failed to deallocate PD: %d", err);
+ ret = ibv_dealloc_pd(priv->ib_pd);
+ if (ret)
+ DRV_LOG(ERR, "Failed to deallocate PD: %d", ret);
priv->ib_pd = NULL;
}
- ret = ibv_close_device(priv->ib_ctx);
- if (ret) {
- ret = errno;
- return ret;
+ state = rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire);
+ if (state == MANA_DEV_ACTIVE ||
+ state == MANA_DEV_RESET_FAILED) {
+ if (priv->ib_ctx) {
+ ret = ibv_close_device(priv->ib_ctx);
+ if (ret) {
+ ret = errno;
+ return ret;
+ }
+ priv->ib_ctx = NULL;
+ }
}
return 0;
}
+/*
+ * Called from mana_pci_remove to free resources allocated
+ * during probe that are not freed by dev_close.
+ */
+static void
+mana_dev_free_resources(struct rte_eth_dev *dev)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+
+ if (priv->dev_state_qsv) {
+ rte_free(priv->dev_state_qsv);
+ priv->dev_state_qsv = NULL;
+ }
+ pthread_mutex_destroy(&priv->reset_cond_mutex);
+ pthread_cond_destroy(&priv->reset_cond);
+}
+
static int
mana_dev_info_get(struct rte_eth_dev *dev,
struct rte_eth_dev_info *dev_info)
@@ -391,6 +451,27 @@ mana_dev_info_get(struct rte_eth_dev *dev,
return 0;
}
+static int
+mana_dev_info_get_lock(struct rte_eth_dev *dev,
+ struct rte_eth_dev_info *dev_info)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+ int ret;
+
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) {
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return -EBUSY;
+ }
+ ret = mana_dev_info_get(dev, dev_info);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ } else {
+ ret = -EBUSY;
+ }
+ return ret;
+}
+
static void
mana_dev_tx_queue_info(struct rte_eth_dev *dev, uint16_t queue_id,
struct rte_eth_txq_info *qinfo)
@@ -552,6 +633,29 @@ mana_dev_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
return ret;
}
+static int
+mana_dev_tx_queue_setup_lock(struct rte_eth_dev *dev, uint16_t queue_idx,
+ uint16_t nb_desc, unsigned int socket_id,
+ const struct rte_eth_txconf *tx_conf)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+ int ret;
+
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) {
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return -EBUSY;
+ }
+ ret = mana_dev_tx_queue_setup(dev, queue_idx,
+ nb_desc, socket_id, tx_conf);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ } else {
+ ret = -EBUSY;
+ }
+ return ret;
+}
+
static void
mana_dev_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
{
@@ -629,6 +733,30 @@ mana_dev_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
return ret;
}
+static int
+mana_dev_rx_queue_setup_lock(struct rte_eth_dev *dev, uint16_t queue_idx,
+ uint16_t nb_desc, unsigned int socket_id,
+ const struct rte_eth_rxconf *rx_conf __rte_unused,
+ struct rte_mempool *mp)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+ int ret;
+
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) {
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return -EBUSY;
+ }
+ ret = mana_dev_rx_queue_setup(dev, queue_idx, nb_desc,
+ socket_id, rx_conf, mp);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ } else {
+ ret = -EBUSY;
+ }
+ return ret;
+}
+
static void
mana_dev_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid)
{
@@ -820,33 +948,221 @@ mana_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
return mana_ifreq(priv, SIOCSIFMTU, &request);
}
+#define MANA_OPS_1_LOCK(_func) \
+static int \
+_func##_lock(struct rte_eth_dev *dev) \
+{ \
+ struct mana_priv *priv = dev->data->dev_private; \
+ int ret; \
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) { \
+ if (rte_atomic_load_explicit(&priv->dev_state, \
+ rte_memory_order_acquire) != \
+ MANA_DEV_ACTIVE) { \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ return -EBUSY; \
+ } \
+ ret = _func(dev); \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ } else { \
+ ret = -EBUSY; \
+ } \
+ return ret; \
+}
+
+MANA_OPS_1_LOCK(mana_dev_configure)
+
+MANA_OPS_1_LOCK(mana_dev_start)
+
+#undef MANA_OPS_1_LOCK
+
+/*
+ * Custom lock wrappers for dev_stop and dev_close.
+ * These join any active reset thread and use a blocking lock (not
+ * trylock) so they wait for any in-progress reset processing to
+ * finish, rather than returning -EBUSY. When the device is not in
+ * MANA_DEV_ACTIVE state, they transition state to MANA_DEV_ACTIVE.
+ */
+static int
+mana_dev_stop_lock(struct rte_eth_dev *dev)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+ int ret;
+
+ /* Signal reset thread to stop by setting state, then wait for it.
+ * Must be done before acquiring the lock to avoid deadlock
+ * (reset thread also acquires the lock).
+ */
+ if (rte_atomic_load_explicit(&priv->reset_thread_active,
+ rte_memory_order_acquire)) {
+ pthread_mutex_lock(&priv->reset_cond_mutex);
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_ACTIVE, rte_memory_order_release);
+ pthread_cond_signal(&priv->reset_cond);
+ pthread_mutex_unlock(&priv->reset_cond_mutex);
+ rte_thread_join(priv->reset_thread, NULL);
+ rte_atomic_store_explicit(&priv->reset_thread_active,
+ false, rte_memory_order_release);
+ }
+
+ rte_spinlock_lock(&priv->reset_ops_lock);
+
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_ACTIVE, rte_memory_order_release);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return 0;
+ }
+
+ ret = mana_dev_stop(dev);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return ret;
+}
+
+static int
+mana_dev_close_lock(struct rte_eth_dev *dev)
+{
+ struct mana_priv *priv = dev->data->dev_private;
+ int ret;
+
+ /* Signal reset thread to stop by setting state, then wait for it.
+ * Must be done before acquiring the lock to avoid deadlock
+ * (reset thread also acquires the lock).
+ */
+ if (rte_atomic_load_explicit(&priv->reset_thread_active,
+ rte_memory_order_acquire)) {
+ pthread_mutex_lock(&priv->reset_cond_mutex);
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_ACTIVE, rte_memory_order_release);
+ pthread_cond_signal(&priv->reset_cond);
+ pthread_mutex_unlock(&priv->reset_cond_mutex);
+ rte_thread_join(priv->reset_thread, NULL);
+ rte_atomic_store_explicit(&priv->reset_thread_active,
+ false, rte_memory_order_release);
+ }
+
+ rte_spinlock_lock(&priv->reset_ops_lock);
+
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE) {
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_ACTIVE, rte_memory_order_release);
+ }
+
+ ret = mana_dev_close(dev);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return ret;
+}
+
+#define MANA_OPS_2_LOCK(_func) \
+static int \
+_func##_lock(struct rte_eth_dev *dev, \
+ struct rte_eth_rss_conf *rss_conf) \
+{ \
+ struct mana_priv *priv = dev->data->dev_private; \
+ int ret; \
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) { \
+ if (rte_atomic_load_explicit(&priv->dev_state, \
+ rte_memory_order_acquire) != \
+ MANA_DEV_ACTIVE) { \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ return -EBUSY; \
+ } \
+ ret = _func(dev, rss_conf); \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ } else { \
+ ret = -EBUSY; \
+ } \
+ return ret; \
+}
+
+MANA_OPS_2_LOCK(mana_rss_hash_update)
+
+MANA_OPS_2_LOCK(mana_rss_hash_conf_get)
+#undef MANA_OPS_2_LOCK
+
+#define MANA_OPS_3_LOCK(_func, _arg) \
+static void \
+_func##_lock(struct rte_eth_dev *dev, uint16_t _arg) \
+{ \
+ struct mana_priv *priv = dev->data->dev_private; \
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) { \
+ if (rte_atomic_load_explicit(&priv->dev_state, \
+ rte_memory_order_acquire) != \
+ MANA_DEV_ACTIVE) { \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ DRV_LOG(ERR, "Device reset in progress, " \
+ "%s not called", #_func); \
+ return; \
+ } \
+ _func(dev, _arg); \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ } else { \
+ DRV_LOG(ERR, "Device reset in progress, " \
+ "%s not called", #_func); \
+ } \
+}
+
+MANA_OPS_3_LOCK(mana_dev_tx_queue_release, qid)
+
+MANA_OPS_3_LOCK(mana_dev_rx_queue_release, qid)
+#undef MANA_OPS_3_LOCK
+
+#define MANA_OPS_4_LOCK(_func, _arg) \
+static int \
+_func##_lock(struct rte_eth_dev *dev, uint16_t _arg) \
+{ \
+ struct mana_priv *priv = dev->data->dev_private; \
+ int ret; \
+ if (rte_spinlock_trylock(&priv->reset_ops_lock)) { \
+ if (rte_atomic_load_explicit(&priv->dev_state, \
+ rte_memory_order_acquire) != \
+ MANA_DEV_ACTIVE) { \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ return -EBUSY; \
+ } \
+ ret = _func(dev, _arg); \
+ rte_spinlock_unlock(&priv->reset_ops_lock); \
+ } else { \
+ ret = -EBUSY; \
+ } \
+ return ret; \
+}
+
+MANA_OPS_4_LOCK(mana_rx_intr_enable, rx_queue_id)
+
+MANA_OPS_4_LOCK(mana_rx_intr_disable, rx_queue_id)
+
+MANA_OPS_4_LOCK(mana_mtu_set, mtu)
+#undef MANA_OPS_4_LOCK
+
static const struct eth_dev_ops mana_dev_ops = {
- .dev_configure = mana_dev_configure,
- .dev_start = mana_dev_start,
- .dev_stop = mana_dev_stop,
- .dev_close = mana_dev_close,
- .dev_infos_get = mana_dev_info_get,
+ .dev_configure = mana_dev_configure_lock,
+ .dev_start = mana_dev_start_lock,
+ .dev_stop = mana_dev_stop_lock,
+ .dev_close = mana_dev_close_lock,
+ .dev_infos_get = mana_dev_info_get_lock,
.txq_info_get = mana_dev_tx_queue_info,
.rxq_info_get = mana_dev_rx_queue_info,
.dev_supported_ptypes_get = mana_supported_ptypes,
- .rss_hash_update = mana_rss_hash_update,
- .rss_hash_conf_get = mana_rss_hash_conf_get,
- .tx_queue_setup = mana_dev_tx_queue_setup,
- .tx_queue_release = mana_dev_tx_queue_release,
- .rx_queue_setup = mana_dev_rx_queue_setup,
- .rx_queue_release = mana_dev_rx_queue_release,
- .rx_queue_intr_enable = mana_rx_intr_enable,
- .rx_queue_intr_disable = mana_rx_intr_disable,
+ .rss_hash_update = mana_rss_hash_update_lock,
+ .rss_hash_conf_get = mana_rss_hash_conf_get_lock,
+ .tx_queue_setup = mana_dev_tx_queue_setup_lock,
+ .tx_queue_release = mana_dev_tx_queue_release_lock,
+ .rx_queue_setup = mana_dev_rx_queue_setup_lock,
+ .rx_queue_release = mana_dev_rx_queue_release_lock,
+ .rx_queue_intr_enable = mana_rx_intr_enable_lock,
+ .rx_queue_intr_disable = mana_rx_intr_disable_lock,
.link_update = mana_dev_link_update,
.stats_get = mana_dev_stats_get,
.stats_reset = mana_dev_stats_reset,
- .mtu_set = mana_mtu_set,
+ .mtu_set = mana_mtu_set_lock,
};
static const struct eth_dev_ops mana_dev_secondary_ops = {
.stats_get = mana_dev_stats_get,
.stats_reset = mana_dev_stats_reset,
- .dev_infos_get = mana_dev_info_get,
+ .dev_infos_get = mana_dev_info_get_lock,
};
uint16_t
@@ -1031,28 +1347,434 @@ mana_ibv_device_to_pci_addr(const struct ibv_device *device,
return 0;
}
+static int mana_pci_probe(struct rte_pci_driver *pci_drv,
+ struct rte_pci_device *pci_dev);
+static void mana_intr_handler(void *arg);
+static void mana_reset_exit(struct mana_priv *priv);
+
+/* Delay before initiating reset exit after reset enter completes */
+#define MANA_RESET_TIMER_US (15 * 1000000ULL) /* 15 seconds */
+
/*
- * Interrupt handler from IB layer to notify this device is being removed.
+ * Callback for PCI device removal events from EAL.
+ * If the device is in reset (RESET_EXIT state), this means the PCI
+ * device was hot-removed rather than a service reset. Wake the reset
+ * thread via condvar and notify netvsc via RTE_ETH_EVENT_INTR_RMV.
+ */
+static void
+mana_pci_remove_event_cb(const char *device_name,
+ enum rte_dev_event_type event, void *cb_arg)
+ __rte_no_thread_safety_analysis
+{
+ struct mana_priv *priv = cb_arg;
+ struct rte_eth_dev *dev;
+
+ if (event != RTE_DEV_EVENT_REMOVE)
+ return;
+
+ DRV_LOG(INFO, "PCI device %s removed", device_name);
+
+ /* Wake the reset thread immediately */
+ pthread_mutex_lock(&priv->reset_cond_mutex);
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_RESET_FAILED, rte_memory_order_release);
+ pthread_cond_signal(&priv->reset_cond);
+ pthread_mutex_unlock(&priv->reset_cond_mutex);
+
+ rte_spinlock_lock(&priv->reset_ops_lock);
+
+ dev = &rte_eth_devices[priv->port_id];
+ DRV_LOG(INFO, "Sending RTE_ETH_EVENT_INTR_RMV for port %u",
+ priv->port_id);
+ rte_eth_dev_callback_process(dev,
+ RTE_ETH_EVENT_INTR_RMV, NULL);
+
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+}
+
+/*
+ * Reset thread: sleeps for the reset timer period, then performs
+ * the reset exit sequence. Runs on a control thread so it can call
+ * rte_intr_callback_unregister (which fails from alarm/intr thread).
+ */
+static uint32_t
+mana_reset_thread(void *arg)
+ __rte_no_thread_safety_analysis
+{
+ struct mana_priv *priv = (struct mana_priv *)arg;
+ struct timespec ts;
+
+ DRV_LOG(INFO, "Reset thread started, waiting %us",
+ (unsigned int)(MANA_RESET_TIMER_US / 1000000));
+
+ /* Wait on condvar with timeout — can be woken early by PCI remove */
+ clock_gettime(CLOCK_REALTIME, &ts);
+ ts.tv_sec += MANA_RESET_TIMER_US / 1000000;
+
+ pthread_mutex_lock(&priv->reset_cond_mutex);
+ pthread_cond_timedwait(&priv->reset_cond, &priv->reset_cond_mutex, &ts);
+ pthread_mutex_unlock(&priv->reset_cond_mutex);
+
+ rte_spinlock_lock(&priv->reset_ops_lock);
+
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_RESET_EXIT) {
+ DRV_LOG(INFO, "Reset thread: dev_state=%d, skipping",
+ (int)rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire));
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return 0;
+ }
+
+ DRV_LOG(INFO, "Reset thread: initiating reset exit");
+ mana_reset_exit(priv);
+ /* Lock is released by mana_reset_exit_delay at the end of
+ * the reset exit processing.
+ *
+ * reset_thread_active is NOT cleared here — the joiner
+ * (dev_stop_lock/dev_close_lock) is responsible for joining
+ * and clearing the flag to avoid leaking the thread.
+ */
+ return 0;
+}
+
+static void
+mana_reset_enter(struct mana_priv *priv)
+ __rte_no_thread_safety_analysis
+{
+ int ret;
+ uint64_t ticket;
+ struct rte_eth_dev *dev = &rte_eth_devices[priv->port_id];
+
+ /*
+ * Lock ownership for reset_ops_lock through the reset path:
+ *
+ * mana_intr_handler — acquires the lock
+ * mana_reset_enter — called with lock held, releases it
+ * after spawning the reset thread
+ * mana_reset_thread — re-acquires the lock after the
+ * recovery delay
+ * mana_reset_exit — called with lock held, passes it
+ * to mana_reset_exit_delay
+ * mana_reset_exit_delay — called with lock held, releases it
+ * on completion
+ */
+
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_ENTER,
+ rte_memory_order_release);
+
+ DRV_LOG(DEBUG, "Entering into device reset state");
+ DRV_LOG(DEBUG, "Resetting dev = %p, priv = %p", dev, priv);
+
+ ticket = rte_rcu_qsbr_start(priv->dev_state_qsv);
+
+ while (rte_rcu_qsbr_check(priv->dev_state_qsv, ticket, false) == 0)
+ rte_pause();
+
+ DRV_LOG(DEBUG, "All threads are quiescent");
+
+ /* Stop data path on primary and secondary before unmapping doorbell */
+ ret = mana_dev_stop(dev);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to stop mana dev ret %d", ret);
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ goto reset_failed;
+ }
+
+ /* Unmap secondary doorbell pages after data path is stopped */
+ ret = mana_mp_req_on_rxtx(dev, MANA_MP_REQ_RESET_ENTER);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to reset secondary processes ret = %d",
+ ret);
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ goto reset_failed;
+ }
+
+ ret = mana_dev_close(dev);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to close mana dev ret %d", ret);
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ goto reset_failed;
+ }
+
+ for (int i = 0; i < priv->num_queues; i++) {
+ struct mana_rxq *rxq = dev->data->rx_queues[i];
+ struct mana_txq *txq = dev->data->tx_queues[i];
+
+ DRV_LOG(DEBUG, "Free MR for priv = %p, rxq %u, txq %u",
+ priv, rxq->rxq_idx, txq->txq_idx);
+ mana_mr_btree_free(&rxq->mr_btree);
+ mana_mr_btree_free(&txq->mr_btree);
+ }
+
+ DRV_LOG(DEBUG, "Reset processing exited successfully");
+
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_EXIT,
+ rte_memory_order_release);
+
+ /* Join previous reset thread if it completed but was not joined */
+ if (rte_atomic_load_explicit(&priv->reset_thread_active,
+ rte_memory_order_acquire)) {
+ rte_thread_join(priv->reset_thread, NULL);
+ rte_atomic_store_explicit(&priv->reset_thread_active,
+ false, rte_memory_order_release);
+ }
+
+ ret = rte_thread_create_internal_control(&priv->reset_thread,
+ "mana-reset",
+ mana_reset_thread, priv);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to create reset thread ret %d", ret);
+ rte_atomic_store_explicit(&priv->dev_state,
+ MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return;
+ }
+ rte_atomic_store_explicit(&priv->reset_thread_active,
+ true, rte_memory_order_release);
+
+ DRV_LOG(DEBUG, "Reset thread started");
+
+ /* Release the lock so the application can call dev_stop/dev_close */
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return;
+
+reset_failed:
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+}
+
+static uint32_t
+mana_reset_exit_delay(void *arg)
+ __rte_no_thread_safety_analysis
+{
+ struct mana_priv *priv = (struct mana_priv *)arg;
+ uint32_t ret = 0;
+ int i;
+ struct rte_eth_dev *dev;
+ struct rte_pci_device *pci_dev;
+
+ DRV_LOG(DEBUG, "Delayed mana device reset complete processing");
+
+ /* If the app called dev_stop/dev_close during the timer window,
+ * state is no longer RESET_EXIT. Nothing to do.
+ */
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_RESET_EXIT) {
+ DRV_LOG(DEBUG, "State is not RESET_EXIT, skipping");
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return ret;
+ }
+
+ dev = &rte_eth_devices[priv->port_id];
+ pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+
+ DRV_LOG(DEBUG, "Resetting dev = %p, priv = %p", dev, priv);
+
+ ret = ibv_close_device(priv->ib_ctx);
+ priv->ib_ctx = NULL;
+ if (ret) {
+ DRV_LOG(ERR, "Failed to close ibv device %d", ret);
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ goto out;
+ }
+
+ ret = mana_pci_probe(NULL, pci_dev);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to probe mana pci dev ret %d", ret);
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+ goto out;
+ }
+
+ /*
+ * Init the local MR caches.
+ */
+ for (i = 0; i < priv->num_queues; i++) {
+ struct mana_rxq *rxq = dev->data->rx_queues[i];
+ struct mana_txq *txq = dev->data->tx_queues[i];
+
+ ret = mana_mr_btree_init(&rxq->mr_btree,
+ MANA_MR_BTREE_PER_QUEUE_N,
+ rxq->socket);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to init RXQ %d MR btree "
+ "on socket %u, ret %d", i, rxq->socket, ret);
+ goto mr_init_failed_rxq;
+ }
+
+ ret = mana_mr_btree_init(&txq->mr_btree,
+ MANA_MR_BTREE_PER_QUEUE_N,
+ txq->socket);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to init TXQ %d MR btree "
+ "on socket %u, ret %d", i, txq->socket, ret);
+ goto mr_init_failed_txq;
+ }
+ }
+ DRV_LOG(DEBUG, "priv %p, num_queues %u", priv, priv->num_queues);
+
+ /* Start secondaries */
+ ret = mana_mp_req_on_rxtx(dev, MANA_MP_REQ_RESET_EXIT);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to start secondary processes ret = %d",
+ ret);
+ goto mr_init_failed_all;
+ }
+
+ ret = mana_dev_start(dev);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to start mana dev ret %d", ret);
+ goto mr_init_failed_all;
+ }
+
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_ACTIVE,
+ rte_memory_order_release);
+
+ DRV_LOG(DEBUG, "Exiting the reset complete processing");
+
+ DRV_LOG(INFO, "Sending RTE_ETH_EVENT_RECOVERY_SUCCESS for port %u",
+ priv->port_id);
+ rte_eth_dev_callback_process(dev,
+ RTE_ETH_EVENT_RECOVERY_SUCCESS, NULL);
+
+out:
+ if (ret) {
+ DRV_LOG(INFO, "Sending RTE_ETH_EVENT_RECOVERY_FAILED for port %u",
+ priv->port_id);
+ rte_eth_dev_callback_process(dev,
+ RTE_ETH_EVENT_RECOVERY_FAILED, NULL);
+ }
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return ret;
+
+mr_init_failed_all:
+ i = priv->num_queues;
+ goto mr_init_failed_rxq;
+
+mr_init_failed_txq:
+ /* RXQ btree at index i was initialized, free it */
+ mana_mr_btree_free(&((struct mana_rxq *)
+ dev->data->rx_queues[i])->mr_btree);
+
+mr_init_failed_rxq:
+ /* Free all fully initialized btrees for indices < i */
+ for (int j = 0; j < i; j++) {
+ struct mana_rxq *rxq = dev->data->rx_queues[j];
+ struct mana_txq *txq = dev->data->tx_queues[j];
+
+ mana_mr_btree_free(&rxq->mr_btree);
+ mana_mr_btree_free(&txq->mr_btree);
+ }
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_RESET_FAILED,
+ rte_memory_order_release);
+
+ DRV_LOG(INFO, "Sending RTE_ETH_EVENT_RECOVERY_FAILED (MR init) for port %u",
+ priv->port_id);
+ rte_eth_dev_callback_process(dev,
+ RTE_ETH_EVENT_RECOVERY_FAILED, NULL);
+
+ rte_spinlock_unlock(&priv->reset_ops_lock);
+ return ret;
+}
+
+static void
+mana_reset_exit(struct mana_priv *priv)
+ __rte_no_thread_safety_analysis
+{
+ int ret;
+
+ if (!priv) {
+ DRV_LOG(ERR, "Private structure invalid");
+ return;
+ }
+ DRV_LOG(DEBUG, "Entering into device reset complete processing");
+
+ rxq_intr_disable(priv);
+
+ /* Unregister the interrupt handler. Since mana_reset_exit is always
+ * called from mana_reset_thread (a non-interrupt thread), the
+ * interrupt source is inactive and rte_intr_callback_unregister
+ * succeeds directly.
+ */
+ if (priv->intr_handle) {
+ ret = rte_intr_callback_unregister(priv->intr_handle,
+ mana_intr_handler, priv);
+ if (ret < 0)
+ DRV_LOG(ERR, "Failed to unregister intr callback ret %d",
+ ret);
+ else
+ DRV_LOG(DEBUG, "%d intr callback(s) removed", ret);
+
+ rte_intr_instance_free(priv->intr_handle);
+ priv->intr_handle = NULL;
+ }
+
+ /* Proceed directly to reset exit delay (re-probe and restart).
+ * No need for a separate thread - we are already on
+ * mana_reset_thread which is a non-interrupt control thread.
+ */
+ mana_reset_exit_delay(priv);
+}
+
+/*
+ * Interrupt handler from IB layer to notify this device is
+ * being removed or reset.
*/
static void
mana_intr_handler(void *arg)
+ __rte_no_thread_safety_analysis
{
struct mana_priv *priv = arg;
struct ibv_context *ctx = priv->ib_ctx;
- struct ibv_async_event event;
+ struct ibv_async_event event = { 0 };
+ struct rte_eth_dev *dev;
/* Read and ack all messages from IB device */
while (true) {
if (ibv_get_async_event(ctx, &event))
break;
- if (event.event_type == IBV_EVENT_DEVICE_FATAL) {
- struct rte_eth_dev *dev;
-
- dev = &rte_eth_devices[priv->port_id];
- if (dev->data->dev_conf.intr_conf.rmv)
+ switch (event.event_type) {
+ case IBV_EVENT_DEVICE_FATAL:
+ DRV_LOG(INFO, "IBV_EVENT_DEVICE_FATAL received, dev_state=%d",
+ (int)rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire));
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) == MANA_DEV_ACTIVE) {
+ rte_spinlock_lock(&priv->reset_ops_lock);
+
+ /* Re-check after lock to avoid racing with
+ * mana_pci_remove_event_cb which may have
+ * set RESET_FAILED while we waited.
+ */
+ if (rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) !=
+ MANA_DEV_ACTIVE) {
+ rte_spinlock_unlock(
+ &priv->reset_ops_lock);
+ break;
+ }
+ mana_reset_enter(priv);
+
+ dev = &rte_eth_devices[priv->port_id];
+ DRV_LOG(INFO, "Sending RTE_ETH_EVENT_ERR_RECOVERING for port %u",
+ priv->port_id);
rte_eth_dev_callback_process(dev,
- RTE_ETH_EVENT_INTR_RMV, NULL);
+ RTE_ETH_EVENT_ERR_RECOVERING, NULL);
+ } else {
+ DRV_LOG(ERR, "Already in reset handling, dev_state=%d",
+ (int)rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire));
+ }
+ break;
+
+ default:
+ break;
}
ibv_ack_async_event(&event);
@@ -1063,6 +1785,20 @@ static int
mana_intr_uninstall(struct mana_priv *priv)
{
int ret;
+ struct rte_eth_dev *dev;
+
+ if (!priv->intr_handle)
+ return 0;
+
+ /* Unregister PCI device removal event callback */
+ dev = &rte_eth_devices[priv->port_id];
+ if (dev->device) {
+ do {
+ ret = rte_dev_event_callback_unregister(
+ dev->device->name,
+ mana_pci_remove_event_cb, priv);
+ } while (ret == -EAGAIN);
+ }
ret = rte_intr_callback_unregister(priv->intr_handle,
mana_intr_handler, priv);
@@ -1072,6 +1808,7 @@ mana_intr_uninstall(struct mana_priv *priv)
}
rte_intr_instance_free(priv->intr_handle);
+ priv->intr_handle = NULL;
return 0;
}
@@ -1127,6 +1864,16 @@ mana_intr_install(struct rte_eth_dev *eth_dev, struct mana_priv *priv)
goto free_intr;
}
+ /* Register for PCI device removal events to distinguish
+ * PCI hot-remove from service reset. This requires the
+ * application to call rte_dev_event_monitor_start() for
+ * events to be delivered (e.g. testpmd --hot-plug-handling).
+ */
+ ret = rte_dev_event_callback_register(eth_dev->device->name,
+ mana_pci_remove_event_cb, priv);
+ if (ret)
+ DRV_LOG(WARNING, "Failed to register PCI remove event callback");
+
eth_dev->intr_handle = priv->intr_handle;
return 0;
@@ -1156,7 +1903,7 @@ mana_proc_priv_init(struct rte_eth_dev *dev)
/*
* Map the doorbell page for the secondary process through IB device handle.
*/
-static int
+int
mana_map_doorbell_secondary(struct rte_eth_dev *eth_dev, int fd)
{
struct mana_process_priv *priv = eth_dev->process_private;
@@ -1294,17 +2041,30 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
char name[RTE_ETH_NAME_MAX_LEN];
int ret;
struct ibv_context *ctx = NULL;
+ size_t sz;
+ bool is_reset = false;
+ pthread_mutexattr_t mattr;
+ pthread_condattr_t cattr;
rte_ether_format_addr(address, sizeof(address), addr);
- DRV_LOG(INFO, "device located port %u address %s", port, address);
- priv = rte_zmalloc_socket(NULL, sizeof(*priv), RTE_CACHE_LINE_SIZE,
- SOCKET_ID_ANY);
- if (!priv)
- return -ENOMEM;
+ DRV_LOG(DEBUG, "device located port %u address %s", port, address);
snprintf(name, sizeof(name), "%s_port%d", pci_dev->device.name, port);
+ eth_dev = rte_eth_dev_allocated(name);
+ if (eth_dev) {
+ is_reset = true;
+ priv = eth_dev->data->dev_private;
+ DRV_LOG(DEBUG, "Device reset for eth_dev %p priv %p",
+ eth_dev, priv);
+ } else {
+ priv = rte_zmalloc_socket(NULL, sizeof(*priv), RTE_CACHE_LINE_SIZE,
+ SOCKET_ID_ANY);
+ if (!priv)
+ return -ENOMEM;
+ }
+
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
int fd;
@@ -1317,6 +2077,7 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
eth_dev->device = &pci_dev->device;
eth_dev->dev_ops = &mana_dev_secondary_ops;
+
ret = mana_proc_priv_init(eth_dev);
if (ret)
goto failed;
@@ -1336,7 +2097,7 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
goto failed;
}
- /* fd is no not used after mapping doorbell */
+ /* fd is not used after mapping doorbell */
close(fd);
eth_dev->tx_pkt_burst = mana_tx_burst;
@@ -1355,22 +2116,6 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
goto failed;
}
- eth_dev = rte_eth_dev_allocate(name);
- if (!eth_dev) {
- ret = -ENOMEM;
- goto failed;
- }
-
- eth_dev->data->mac_addrs =
- rte_calloc("mana_mac", 1,
- sizeof(struct rte_ether_addr), 0);
- if (!eth_dev->data->mac_addrs) {
- ret = -ENOMEM;
- goto failed;
- }
-
- rte_ether_addr_copy(addr, eth_dev->data->mac_addrs);
-
priv->ib_pd = ibv_alloc_pd(ctx);
if (!priv->ib_pd) {
DRV_LOG(ERR, "ibv_alloc_pd failed port %d", port);
@@ -1390,10 +2135,6 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
}
priv->ib_ctx = ctx;
- priv->port_id = eth_dev->data->port_id;
- priv->dev_port = port;
- eth_dev->data->dev_private = priv;
- priv->dev_data = eth_dev->data;
priv->max_rx_queues = dev_attr->orig_attr.max_qp;
priv->max_tx_queues = dev_attr->orig_attr.max_qp;
@@ -1415,23 +2156,93 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
name, priv->max_rx_queues, priv->max_rx_desc,
priv->max_send_sge, priv->max_mr_size);
+ if (!is_reset) {
+ eth_dev = rte_eth_dev_allocate(name);
+ if (!eth_dev) {
+ ret = -ENOMEM;
+ goto failed;
+ }
+
+ eth_dev->data->mac_addrs =
+ rte_calloc("mana_mac", 1,
+ sizeof(struct rte_ether_addr), 0);
+ if (!eth_dev->data->mac_addrs) {
+ ret = -ENOMEM;
+ goto failed;
+ }
+
+ rte_ether_addr_copy(addr, eth_dev->data->mac_addrs);
+ } else {
+ /*
+ * Reset path.
+ */
+ rte_ether_format_addr(address, RTE_ETHER_ADDR_FMT_SIZE,
+ eth_dev->data->mac_addrs);
+ DRV_LOG(DEBUG, "Found existing eth_dev %p with mac addr %s",
+ eth_dev, address);
+ DRV_LOG(DEBUG, "ib_ctx = %p", priv->ib_ctx);
+ goto out;
+ }
+
+ priv->port_id = eth_dev->data->port_id;
+ priv->dev_port = port;
+ eth_dev->data->dev_private = priv;
+ priv->dev_data = eth_dev->data;
+ rte_atomic_store_explicit(&priv->dev_state, MANA_DEV_ACTIVE,
+ rte_memory_order_release);
+
rte_eth_copy_pci_info(eth_dev, pci_dev);
- /* Create async interrupt handler */
- ret = mana_intr_install(eth_dev, priv);
- if (ret) {
- DRV_LOG(ERR, "Failed to install intr handler");
+ /*
+ * Now we've got maximum queues. Init the qsv to be the
+ * double of maximum queues for both rx and tx queues.
+ */
+ sz = rte_rcu_qsbr_get_memsize(2 * priv->max_rx_queues);
+ priv->dev_state_qsv = rte_zmalloc_socket("mana_rcu", sz,
+ RTE_CACHE_LINE_SIZE,
+ SOCKET_ID_ANY);
+ if (!priv->dev_state_qsv) {
+ DRV_LOG(ERR, "No memory for dev_state_qsv");
+ ret = -ENOMEM;
+ goto failed;
+ }
+ ret = rte_rcu_qsbr_init(priv->dev_state_qsv, 2 * priv->max_rx_queues);
+ if (ret < 0) {
+ DRV_LOG(ERR, "Init dev_state_qsv failed ret %d", ret);
goto failed;
}
- eth_dev->device = &pci_dev->device;
+ rte_spinlock_init(&priv->reset_ops_lock);
+
+ pthread_mutexattr_init(&mattr);
+ pthread_mutexattr_setpshared(&mattr, PTHREAD_PROCESS_SHARED);
+ pthread_mutex_init(&priv->reset_cond_mutex, &mattr);
+ pthread_mutexattr_destroy(&mattr);
- DRV_LOG(INFO, "device %s at port %u", name, eth_dev->data->port_id);
+ pthread_condattr_init(&cattr);
+ pthread_condattr_setpshared(&cattr, PTHREAD_PROCESS_SHARED);
+ pthread_cond_init(&priv->reset_cond, &cattr);
+ pthread_condattr_destroy(&cattr);
+
+ eth_dev->device = &pci_dev->device;
eth_dev->rx_pkt_burst = mana_rx_burst_removed;
eth_dev->tx_pkt_burst = mana_tx_burst_removed;
eth_dev->dev_ops = &mana_dev_ops;
+out:
+ /* Create async interrupt handler */
+ ret = mana_intr_install(eth_dev, priv);
+ if (ret) {
+ DRV_LOG(ERR, "Failed to install intr handler, ret %d", ret);
+ goto failed;
+ } else {
+ DRV_LOG(INFO, "mana_intr_install succeeded");
+ }
+
+ DRV_LOG(INFO, "device %s priv %p dev port %d at port %u",
+ name, priv, priv->dev_port, eth_dev->data->port_id);
+
rte_eth_dev_probing_finish(eth_dev);
return 0;
@@ -1439,20 +2250,32 @@ mana_probe_port(struct ibv_device *ibdev, struct ibv_device_attr_ex *dev_attr,
failed:
/* Free the resource for the port failed */
if (priv) {
- if (priv->ib_parent_pd)
+ if (!is_reset && priv->dev_state_qsv)
+ rte_free(priv->dev_state_qsv);
+
+ if (priv->ib_parent_pd) {
ibv_dealloc_pd(priv->ib_parent_pd);
+ priv->ib_parent_pd = NULL;
+ }
- if (priv->ib_pd)
+ if (priv->ib_pd) {
ibv_dealloc_pd(priv->ib_pd);
+ priv->ib_pd = NULL;
+ }
}
- if (eth_dev)
- rte_eth_dev_release_port(eth_dev);
+ if (!is_reset) {
+ if (eth_dev)
+ rte_eth_dev_release_port(eth_dev);
- rte_free(priv);
+ rte_free(priv);
+ }
- if (ctx)
+ if (ctx) {
ibv_close_device(ctx);
+ if (is_reset && priv)
+ priv->ib_ctx = NULL;
+ }
return ret;
}
@@ -1617,7 +2440,11 @@ mana_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
static int
mana_dev_uninit(struct rte_eth_dev *dev)
{
- return mana_dev_close(dev);
+ int ret;
+
+ ret = mana_dev_close(dev);
+ mana_dev_free_resources(dev);
+ return ret;
}
/*
diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
index 79cc47b6ab..df4832fd75 100644
--- a/drivers/net/mana/mana.h
+++ b/drivers/net/mana/mana.h
@@ -5,6 +5,8 @@
#ifndef __MANA_H__
#define __MANA_H__
+#include <pthread.h>
+
#define PCI_VENDOR_ID_MICROSOFT 0x1414
#define PCI_DEVICE_ID_MICROSOFT_MANA_PF 0x00b9
#define PCI_DEVICE_ID_MICROSOFT_MANA 0x00ba
@@ -337,6 +339,20 @@ struct mana_process_priv {
void *db_page;
};
+enum mana_device_state {
+ /* Normal running */
+ MANA_DEV_ACTIVE = 0,
+ /* In reset enter processing */
+ MANA_DEV_RESET_ENTER = 1,
+ /*
+ * Reset enter processing completed.
+ * Waiting for reset exit or in reset exit processing.
+ */
+ MANA_DEV_RESET_EXIT = 2,
+ /* Reset failed */
+ MANA_DEV_RESET_FAILED = 3,
+};
+
struct mana_priv {
struct rte_eth_dev_data *dev_data;
struct mana_process_priv *process_priv;
@@ -368,6 +384,16 @@ struct mana_priv {
uint64_t max_mr_size;
struct mana_mr_btree mr_btree;
rte_spinlock_t mr_btree_lock;
+ RTE_ATOMIC(enum mana_device_state) dev_state;
+ struct rte_rcu_qsbr *dev_state_qsv;
+ /* lock for synchronizing mana reset and some mana_dev_ops callbacks */
+ rte_spinlock_t reset_ops_lock;
+ /* Reset thread ID, valid when reset_thread_active is true */
+ rte_thread_t reset_thread;
+ RTE_ATOMIC(bool) reset_thread_active;
+ /* Condvar to wake reset thread early on PCI remove */
+ pthread_mutex_t reset_cond_mutex;
+ pthread_cond_t reset_cond;
};
struct mana_txq_desc {
@@ -427,6 +453,7 @@ struct mana_txq {
struct mana_mr_btree mr_btree;
struct mana_stats stats;
unsigned int socket;
+ unsigned int txq_idx;
};
struct mana_rxq {
@@ -462,6 +489,7 @@ struct mana_rxq {
struct mana_mr_btree mr_btree;
unsigned int socket;
+ unsigned int rxq_idx;
};
extern int mana_logtype_driver;
@@ -543,6 +571,8 @@ enum mana_mp_req_type {
MANA_MP_REQ_CREATE_MR,
MANA_MP_REQ_START_RXTX,
MANA_MP_REQ_STOP_RXTX,
+ MANA_MP_REQ_RESET_ENTER,
+ MANA_MP_REQ_RESET_EXIT,
};
/* Pameters for IPC. */
@@ -563,8 +593,9 @@ void mana_mp_uninit_primary(void);
void mana_mp_uninit_secondary(void);
int mana_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev);
int mana_mp_req_mr_create(struct mana_priv *priv, uintptr_t addr, uint32_t len);
+int mana_map_doorbell_secondary(struct rte_eth_dev *eth_dev, int fd);
-void mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type);
+int mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type);
void *mana_alloc_verbs_buf(size_t size, void *data);
void mana_free_verbs_buf(void *ptr, void *data __rte_unused);
diff --git a/drivers/net/mana/meson.build b/drivers/net/mana/meson.build
index 19d4b3695e..5b01d9f57e 100644
--- a/drivers/net/mana/meson.build
+++ b/drivers/net/mana/meson.build
@@ -7,7 +7,7 @@ if not is_linux or not (dpdk_conf.has('RTE_ARCH_X86') or dpdk_conf.has('RTE_ARCH
subdir_done()
endif
-deps += ['pci', 'bus_pci', 'net', 'eal', 'kvargs']
+deps += ['pci', 'bus_pci', 'net', 'eal', 'kvargs', 'rcu']
sources += files(
'gdma.c',
diff --git a/drivers/net/mana/mp.c b/drivers/net/mana/mp.c
index 72417fc0c7..fc4e10ac64 100644
--- a/drivers/net/mana/mp.c
+++ b/drivers/net/mana/mp.c
@@ -2,10 +2,13 @@
* Copyright 2022 Microsoft Corporation
*/
+#include <sys/mman.h>
#include <rte_malloc.h>
#include <ethdev_driver.h>
#include <rte_log.h>
+#include <rte_eal_paging.h>
#include <stdlib.h>
+#include <unistd.h>
#include <infiniband/verbs.h>
@@ -119,6 +122,23 @@ mana_mp_primary_handle(const struct rte_mp_msg *mp_msg, const void *peer)
return ret;
}
+static int
+mana_mp_reset_enter(struct rte_eth_dev *dev)
+{
+ struct mana_process_priv *proc_priv = dev->process_private;
+
+ void *addr = proc_priv->db_page;
+
+ /* Reset the db_page to NULL */
+ proc_priv->db_page = NULL;
+
+ if (addr)
+ (void)munmap(addr, rte_mem_page_size());
+
+ DRV_LOG(DEBUG, "All secondary threads are quiescent");
+ return 0;
+}
+
static int
mana_mp_secondary_handle(const struct rte_mp_msg *mp_msg, const void *peer)
{
@@ -171,6 +191,49 @@ mana_mp_secondary_handle(const struct rte_mp_msg *mp_msg, const void *peer)
ret = rte_mp_reply(&mp_res, peer);
break;
+ case MANA_MP_REQ_RESET_ENTER:
+ DRV_LOG(INFO, "Port %u reset enter", dev->data->port_id);
+ res->result = mana_mp_reset_enter(dev);
+
+ ret = rte_mp_reply(&mp_res, peer);
+ break;
+
+ case MANA_MP_REQ_RESET_EXIT:
+ DRV_LOG(INFO, "Port %u reset exit", dev->data->port_id);
+ {
+ struct mana_process_priv *proc_priv =
+ dev->process_private;
+
+ if (proc_priv->db_page != NULL) {
+ DRV_LOG(DEBUG,
+ "Secondary doorbell already "
+ "mapped to %p",
+ proc_priv->db_page);
+ res->result = 0;
+ } else if (mp_msg->num_fds < 1) {
+ DRV_LOG(ERR,
+ "No FD in RESET_EXIT message");
+ res->result = -EINVAL;
+ } else {
+ int fd = mp_msg->fds[0];
+
+ ret = mana_map_doorbell_secondary(dev,
+ fd);
+ if (ret) {
+ DRV_LOG(ERR,
+ "Failed secondary "
+ "doorbell map %d",
+ fd);
+ res->result = -ENODEV;
+ } else {
+ res->result = 0;
+ }
+ close(fd);
+ }
+ }
+ ret = rte_mp_reply(&mp_res, peer);
+ break;
+
default:
DRV_LOG(ERR, "Port %u unknown secondary MP type %u",
param->port_id, param->type);
@@ -254,7 +317,7 @@ mana_mp_req_verbs_cmd_fd(struct rte_eth_dev *dev)
}
ret = mp_res->fds[0];
- DRV_LOG(ERR, "port %u command FD from primary is %d",
+ DRV_LOG(DEBUG, "port %u command FD from primary is %d",
dev->data->port_id, ret);
exit:
free(mp_rep.msgs);
@@ -298,27 +361,36 @@ mana_mp_req_mr_create(struct mana_priv *priv, uintptr_t addr, uint32_t len)
return ret;
}
-void
+int
mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type)
{
struct rte_mp_msg mp_req = { 0 };
struct rte_mp_msg *mp_res;
- struct rte_mp_reply mp_rep;
+ struct rte_mp_reply mp_rep = { 0 };
struct mana_mp_param *res;
struct timespec ts = {.tv_sec = MANA_MP_REQ_TIMEOUT_SEC, .tv_nsec = 0};
- int i, ret;
+ int i, ret = 0;
- if (type != MANA_MP_REQ_START_RXTX && type != MANA_MP_REQ_STOP_RXTX) {
+ if (type != MANA_MP_REQ_START_RXTX && type != MANA_MP_REQ_STOP_RXTX &&
+ type != MANA_MP_REQ_RESET_ENTER && type != MANA_MP_REQ_RESET_EXIT) {
DRV_LOG(ERR, "port %u unknown request (req_type %d)",
dev->data->port_id, type);
- return;
+ return -EINVAL;
}
if (rte_atomic_load_explicit(&mana_shared_data->secondary_cnt, rte_memory_order_relaxed) == 0)
- return;
+ return 0;
mp_init_msg(&mp_req, type, dev->data->port_id);
+ /* Include IB cmd FD for secondary doorbell remap */
+ if (type == MANA_MP_REQ_RESET_EXIT) {
+ struct mana_priv *priv = dev->data->dev_private;
+
+ mp_req.num_fds = 1;
+ mp_req.fds[0] = priv->ib_ctx->cmd_fd;
+ }
+
ret = rte_mp_request_sync(&mp_req, &mp_rep, &ts);
if (ret) {
if (rte_errno != ENOTSUP)
@@ -329,6 +401,7 @@ mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type)
if (mp_rep.nb_sent != mp_rep.nb_received) {
DRV_LOG(ERR, "port %u not all secondaries responded (%d)",
dev->data->port_id, type);
+ ret = -ETIMEDOUT;
goto exit;
}
for (i = 0; i < mp_rep.nb_received; i++) {
@@ -337,9 +410,11 @@ mana_mp_req_on_rxtx(struct rte_eth_dev *dev, enum mana_mp_req_type type)
if (res->result) {
DRV_LOG(ERR, "port %u request failed on secondary %d",
dev->data->port_id, i);
+ ret = res->result;
goto exit;
}
}
exit:
free(mp_rep.msgs);
+ return ret;
}
diff --git a/drivers/net/mana/mr.c b/drivers/net/mana/mr.c
index c4045141bc..8914f4cf04 100644
--- a/drivers/net/mana/mr.c
+++ b/drivers/net/mana/mr.c
@@ -314,8 +314,10 @@ mana_mr_btree_init(struct mana_mr_btree *bt, int n, int socket)
void
mana_mr_btree_free(struct mana_mr_btree *bt)
{
- rte_free(bt->table);
- memset(bt, 0, sizeof(*bt));
+ if (bt && bt->table) {
+ rte_free(bt->table);
+ memset(bt, 0, sizeof(*bt));
+ }
}
int
diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
index 1b8ba1f3a9..ae05d8dd2f 100644
--- a/drivers/net/mana/rx.c
+++ b/drivers/net/mana/rx.c
@@ -2,6 +2,7 @@
* Copyright 2022 Microsoft Corporation
*/
#include <ethdev_driver.h>
+#include <rte_rcu_qsbr.h>
#include <infiniband/verbs.h>
#include <infiniband/manadv.h>
@@ -36,6 +37,11 @@ mana_rq_ring_doorbell(struct mana_rxq *rxq)
db_page = process_priv->db_page;
}
+ if (!db_page) {
+ DP_LOG(ERR, "db_page is NULL, cannot ring RX doorbell");
+ return -EINVAL;
+ }
+
/* Hardware Spec specifies that software client should set 0 for
* wqe_cnt for Receive Queues.
*/
@@ -172,7 +178,7 @@ mana_stop_rx_queues(struct rte_eth_dev *dev)
for (i = 0; i < priv->num_queues; i++)
if (dev->data->rx_queue_state[i] == RTE_ETH_QUEUE_STATE_STOPPED)
- return -EINVAL;
+ return 0;
if (priv->rwq_qp) {
ret = ibv_destroy_qp(priv->rwq_qp);
@@ -256,6 +262,9 @@ mana_start_rx_queues(struct rte_eth_dev *dev)
struct mana_rxq *rxq = dev->data->rx_queues[i];
struct ibv_wq_init_attr wq_attr = {};
+ rxq->rxq_idx = i;
+ DRV_LOG(DEBUG, "assigning rxq_idx to %d", i);
+
manadv_set_context_attr(priv->ib_ctx,
MANADV_CTX_ATTR_BUF_ALLOCATORS,
(void *)((uintptr_t)&(struct manadv_ctx_allocators){
@@ -451,6 +460,17 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
uint32_t pkt_len;
uint32_t i;
int polled = 0;
+ struct rte_rcu_qsbr *dstate_qsv = priv->dev_state_qsv;
+ unsigned int tid = rxq->rxq_idx;
+
+ rte_rcu_qsbr_thread_online(dstate_qsv, tid);
+
+ if (unlikely(rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE)) {
+ /* Device reset occurred. */
+ rte_rcu_qsbr_thread_offline(dstate_qsv, tid);
+ return 0;
+ }
repoll:
/* Polling on new completions if we have no backlog */
@@ -592,6 +612,8 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
wqe_consumed, ret);
}
+ rte_rcu_qsbr_thread_offline(dstate_qsv, tid);
+
return pkt_received;
}
diff --git a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c
index 57dbbc3651..3b07a8b9a6 100644
--- a/drivers/net/mana/tx.c
+++ b/drivers/net/mana/tx.c
@@ -3,6 +3,7 @@
*/
#include <ethdev_driver.h>
+#include <rte_rcu_qsbr.h>
#include <infiniband/verbs.h>
#include <infiniband/manadv.h>
@@ -17,7 +18,7 @@ mana_stop_tx_queues(struct rte_eth_dev *dev)
for (i = 0; i < priv->num_queues; i++)
if (dev->data->tx_queue_state[i] == RTE_ETH_QUEUE_STATE_STOPPED)
- return -EINVAL;
+ return 0;
for (i = 0; i < priv->num_queues; i++) {
struct mana_txq *txq = dev->data->tx_queues[i];
@@ -83,6 +84,9 @@ mana_start_tx_queues(struct rte_eth_dev *dev)
txq = dev->data->tx_queues[i];
+ txq->txq_idx = i;
+ DRV_LOG(DEBUG, "assigning txq_idx to %d", txq->txq_idx);
+
manadv_set_context_attr(priv->ib_ctx,
MANADV_CTX_ATTR_BUF_ALLOCATORS,
(void *)((uintptr_t)&(struct manadv_ctx_allocators){
@@ -190,10 +194,30 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
void *db_page;
uint16_t pkt_sent = 0;
uint32_t num_comp, i;
+ unsigned int tid = priv->num_queues + txq->txq_idx;
+ struct rte_rcu_qsbr *dstate_qsv = priv->dev_state_qsv;
#ifdef RTE_ARCH_32
uint32_t wqe_count = 0;
#endif
+ db_page = priv->db_page;
+ if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ struct rte_eth_dev *dev =
+ &rte_eth_devices[priv->dev_data->port_id];
+ struct mana_process_priv *process_priv = dev->process_private;
+
+ db_page = process_priv->db_page;
+ }
+
+ rte_rcu_qsbr_thread_online(dstate_qsv, tid);
+
+ if (unlikely(rte_atomic_load_explicit(&priv->dev_state,
+ rte_memory_order_acquire) != MANA_DEV_ACTIVE || !db_page)) {
+ /* Device reset event occurred. */
+ rte_rcu_qsbr_thread_offline(dstate_qsv, tid);
+ return 0;
+ }
+
/* Process send completions from GDMA */
num_comp = gdma_poll_completion_queue(&txq->gdma_cq,
txq->gdma_comp_buf, txq->num_desc);
@@ -216,7 +240,8 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
}
if (!desc->pkt) {
- DP_LOG(ERR, "mana_txq_desc has a NULL pkt");
+ DP_LOG(ERR, "mana_txq_desc has a NULL pkt, priv %p, "
+ "txq = %d", priv, txq->txq_idx);
} else {
txq->stats.bytes += desc->pkt->pkt_len;
rte_pktmbuf_free(desc->pkt);
@@ -474,15 +499,6 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
}
/* Ring hardware door bell */
- db_page = priv->db_page;
- if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
- struct rte_eth_dev *dev =
- &rte_eth_devices[priv->dev_data->port_id];
- struct mana_process_priv *process_priv = dev->process_private;
-
- db_page = process_priv->db_page;
- }
-
if (pkt_sent) {
#ifdef RTE_ARCH_32
ret = mana_ring_short_doorbell(db_page, GDMA_QUEUE_SEND,
@@ -501,5 +517,7 @@ mana_tx_burst(void *dpdk_txq, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
DP_LOG(ERR, "mana_ring_doorbell failed ret %d", ret);
}
+ rte_rcu_qsbr_thread_offline(dstate_qsv, tid);
+
return pkt_sent;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v3 0/2] net/mana: add device reset support
From: Wei Hu @ 2026-05-22 6:59 UTC (permalink / raw)
To: dev, stephen; +Cc: longli, weh
From: Wei Hu <weh@microsoft.com>
Add support for handling hardware service reset events in the
MANA driver. When the MANA kernel driver receives a hardware
service event, it initiates a device reset and notifies userspace
via IBV_EVENT_DEVICE_FATAL. The MANA PMD handles this by
performing an automatic teardown and recovery sequence.
The driver uses ethdev recovery events (ERR_RECOVERING,
RECOVERY_SUCCESS, RECOVERY_FAILED) to notify upper layers of
the reset lifecycle, and a PCI device removal event callback
to distinguish hot-remove from service reset.
Changes since v2:
- Fixed dev_state_qsv memory leak on device removal
- Fixed reset thread TCB/stack leak: reset_thread_active is now
only cleared by the joiner, not the thread itself
- Fixed second reset crash: removed reset thread join logic from
mana_dev_close (inner function) to avoid corrupting dev_state
when called from mana_reset_enter
- Made reset_thread_active RTE_ATOMIC(bool) with explicit ordering
- Added retry loop for rte_dev_event_callback_unregister on -EAGAIN
- Initialized condvar/mutex with PTHREAD_PROCESS_SHARED since priv
is in hugepage shared memory
- Added re-check of dev_state after lock acquisition in
mana_intr_handler to prevent racing with pci_remove_event_cb
- Replaced (void *)0 with NULL in mp.c
- Added lock ownership comment block at mana_reset_enter
- Documented rte_dev_event_monitor_start() requirement
- Added mana.rst documentation and release note (patch 2/2)
Changes since v1:
- Removed net/netvsc patch from this series
- Simplified reset exit: mana_reset_exit calls
mana_reset_exit_delay directly instead of spawning a thread
- Added __rte_no_thread_safety_analysis annotations for clang
- Switched to rte_thread_create_internal_control
- Fixed declaration-after-statement style issues
- Removed unnecessary blank lines and stale comments
Wei Hu (2):
net/mana: add device reset support
net/mana: add documentation for device reset support
doc/guides/nics/mana.rst | 33 +
doc/guides/rel_notes/release_26_07.rst | 8 +
drivers/net/mana/mana.c | 995 ++++++++++++++++++++++---
drivers/net/mana/mana.h | 33 +-
drivers/net/mana/meson.build | 2 +-
drivers/net/mana/mp.c | 89 ++-
drivers/net/mana/mr.c | 6 +-
drivers/net/mana/rx.c | 24 +-
drivers/net/mana/tx.c | 40 +-
9 files changed, 1123 insertions(+), 107 deletions(-)
--
2.34.1
^ permalink raw reply
* [PATCH] net/txgbe: fix AML mailbox lock acquisition
From: Stephen Hemminger @ 2026-05-22 5:00 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, stable, Jiawen Wu, Zaiyu Wang
The try-lock spin loop in txgbe_host_interface_command_aml() has
the condition inverted. rte_atomic32_test_and_set returns non-zero
on successful acquisition (0 -> 1), zero when the lock was already
held. Walk through the two cases:
swfw_busy was 0 (free): test_and_set returns 1, sets to 1, loop
body runs and sleeps 1ms. Next iteration finds 1, returns 0, loop
exits. Lock held, after an unnecessary 1ms sleep.
swfw_busy was 1 (busy): test_and_set returns 0, loop exits
immediately. The caller proceeds without holding the lock,
racing with the in-flight host interface command.
Invert the condition so the loop spins while the lock remains held
and exits only when acquisition succeeds, matching the intent of
the surrounding timeout machinery.
Fixes: 6a139ade82e7 ("net/txgbe: add new SW-FW mailbox interface")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/txgbe/base/txgbe_mng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/txgbe/base/txgbe_mng.c b/drivers/net/txgbe/base/txgbe_mng.c
index a1974820b6..dcd8f58a68 100644
--- a/drivers/net/txgbe/base/txgbe_mng.c
+++ b/drivers/net/txgbe/base/txgbe_mng.c
@@ -185,7 +185,7 @@ txgbe_host_interface_command_aml(struct txgbe_hw *hw, u32 *buffer,
}
/* try to get lock */
- while (rte_atomic32_test_and_set(&hw->swfw_busy)) {
+ while (rte_atomic32_test_and_set(&hw->swfw_busy) == 0) {
timeout--;
if (!timeout)
return TXGBE_ERR_TIMEOUT;
--
2.53.0
^ permalink raw reply related
* [PATCH v3 10/10] power: add lcore ID check for PMD mgmt
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
The pmd_mgmt lib is mainly used together with the data plane of ethdev
PMD. The core in data plane is ROLE_RTE. So use rte_lcore_is_enabled
to verify it.
Fixes: 426511683762 ("power: add get/set min/max scaling frequencies API")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
lib/power/rte_power_pmd_mgmt.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/lib/power/rte_power_pmd_mgmt.c b/lib/power/rte_power_pmd_mgmt.c
index a4d53aac2a..a5fc1c3a94 100644
--- a/lib/power/rte_power_pmd_mgmt.c
+++ b/lib/power/rte_power_pmd_mgmt.c
@@ -511,7 +511,8 @@ rte_power_ethdev_pmgmt_queue_enable(unsigned int lcore_id, uint16_t port_id,
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
- if (queue_id >= RTE_MAX_QUEUES_PER_PORT || lcore_id >= RTE_MAX_LCORE) {
+ if (queue_id >= RTE_MAX_QUEUES_PER_PORT ||
+ !rte_lcore_is_enabled(lcore_id)) {
ret = -EINVAL;
goto end;
}
@@ -627,7 +628,7 @@ rte_power_ethdev_pmgmt_queue_disable(unsigned int lcore_id,
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
- if (lcore_id >= RTE_MAX_LCORE || queue_id >= RTE_MAX_QUEUES_PER_PORT)
+ if (!rte_lcore_is_enabled(lcore_id) || queue_id >= RTE_MAX_QUEUES_PER_PORT)
return -EINVAL;
/* check if the queue is stopped */
@@ -729,8 +730,8 @@ RTE_EXPORT_SYMBOL(rte_power_pmd_mgmt_set_scaling_freq_min)
int
rte_power_pmd_mgmt_set_scaling_freq_min(unsigned int lcore, unsigned int min)
{
- if (lcore >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID: %u", lcore);
+ if (!rte_lcore_is_enabled(lcore)) {
+ POWER_LOG(ERR, "lcore id %u is not enabled", lcore);
return -EINVAL;
}
@@ -747,8 +748,8 @@ RTE_EXPORT_SYMBOL(rte_power_pmd_mgmt_set_scaling_freq_max)
int
rte_power_pmd_mgmt_set_scaling_freq_max(unsigned int lcore, unsigned int max)
{
- if (lcore >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID: %u", lcore);
+ if (!rte_lcore_is_enabled(lcore)) {
+ POWER_LOG(ERR, "lcore id %u is not enabled", lcore);
return -EINVAL;
}
@@ -769,8 +770,8 @@ RTE_EXPORT_SYMBOL(rte_power_pmd_mgmt_get_scaling_freq_min)
int
rte_power_pmd_mgmt_get_scaling_freq_min(unsigned int lcore)
{
- if (lcore >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID: %u", lcore);
+ if (!rte_lcore_is_enabled(lcore)) {
+ POWER_LOG(ERR, "lcore id %u is not enabled", lcore);
return -EINVAL;
}
@@ -784,8 +785,8 @@ RTE_EXPORT_SYMBOL(rte_power_pmd_mgmt_get_scaling_freq_max)
int
rte_power_pmd_mgmt_get_scaling_freq_max(unsigned int lcore)
{
- if (lcore >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID: %u", lcore);
+ if (!rte_lcore_is_enabled(lcore)) {
+ POWER_LOG(ERR, "lcore id %u is not enabled", lcore);
return -EINVAL;
}
--
2.33.0
^ permalink raw reply related
* [PATCH v3 09/10] power: allow the service core to config power QoS
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
The lcore ID verification in power QoS API used to use
rte_lcore_is_enabled(), which only accepts the lcore with
ROLE_RTE role. But service core thread (ROLE_SERVICE) can
also use power QoS API.
So this patch replaces rte_lcore_is_enabled() with the new
helper rte_lcore_is_eal_managed() by using the common macro
RTE_POWER_VALID_LCOREID_OR_ERR_RET. This change makes the
power QoS API accept both ROLE_RTE and ROLE_SERVICE lcores.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
doc/guides/rel_notes/release_26_07.rst | 6 ++++++
lib/power/rte_power_qos.c | 10 ++--------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 98a7cf0203..0eca0261b3 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -76,6 +76,12 @@ New Features
that the lcore ID is EAL-managed (``ROLE_RTE`` or ``ROLE_SERVICE``)
before proceeding, replacing per-driver range checks.
+ The power QoS library also updated its lcore validation to use the
+ new helper, so service cores (``ROLE_SERVICE``) are now permitted
+ to configure power QoS parameters via ``rte_power_qos_set_cpu_resume_latency()``
+ and ``rte_power_qos_get_cpu_resume_latency()``, in addition to
+ regular DPDK lcores (``ROLE_RTE``).
+
Removed Items
-------------
diff --git a/lib/power/rte_power_qos.c b/lib/power/rte_power_qos.c
index be230d1c50..f6630b08f7 100644
--- a/lib/power/rte_power_qos.c
+++ b/lib/power/rte_power_qos.c
@@ -27,10 +27,7 @@ rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
FILE *f;
int ret;
- if (!rte_lcore_is_enabled(lcore_id)) {
- POWER_LOG(ERR, "lcore id %u is not enabled", lcore_id);
- return -EINVAL;
- }
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
ret = power_get_lcore_mapped_cpu_id(lcore_id, &cpu_id);
if (ret != 0)
return ret;
@@ -82,10 +79,7 @@ rte_power_qos_get_cpu_resume_latency(uint16_t lcore_id)
FILE *f;
int ret;
- if (!rte_lcore_is_enabled(lcore_id)) {
- POWER_LOG(ERR, "lcore id %u is not enabled", lcore_id);
- return -EINVAL;
- }
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -EINVAL);
ret = power_get_lcore_mapped_cpu_id(lcore_id, &cpu_id);
if (ret != 0)
return ret;
--
2.33.0
^ permalink raw reply related
* [PATCH v3 06/10] power/cppc: remove redundant lcore ID checks
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Now that the cpufreq framework validates the lcore ID using
rte_lcore_is_eal_managed() before dispatching to any driver,
each individual cpufreq driver no longer needs its own range
check against RTE_MAX_LCORE.
Remove the duplicated lcore ID checks from the cppc cpufreq
driver ops.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/power/cppc/cppc_cpufreq.c | 65 -------------------------------
1 file changed, 65 deletions(-)
diff --git a/drivers/power/cppc/cppc_cpufreq.c b/drivers/power/cppc/cppc_cpufreq.c
index 3cd4165c83..529f68e574 100644
--- a/drivers/power/cppc/cppc_cpufreq.c
+++ b/drivers/power/cppc/cppc_cpufreq.c
@@ -346,12 +346,6 @@ power_cppc_cpufreq_init(unsigned int lcore_id)
return -1;
}
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
exp_state = POWER_IDLE;
/* The power in use state works as a guard variable between
@@ -429,11 +423,6 @@ power_cppc_cpufreq_exit(unsigned int lcore_id)
struct cppc_power_info *pi;
uint32_t exp_state;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
pi = &lcore_power_info[lcore_id];
exp_state = POWER_USED;
/* The power in use state works as a guard variable between
@@ -479,11 +468,6 @@ power_cppc_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return 0;
- }
-
if (freqs == NULL) {
POWER_LOG(ERR, "NULL buffer supplied");
return 0;
@@ -502,22 +486,12 @@ power_cppc_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
uint32_t
power_cppc_cpufreq_get_freq(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return RTE_POWER_INVALID_FREQ_INDEX;
- }
-
return lcore_power_info[lcore_id].curr_idx;
}
int
power_cppc_cpufreq_set_freq(unsigned int lcore_id, uint32_t index)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
return set_freq_internal(&(lcore_power_info[lcore_id]), index);
}
@@ -526,11 +500,6 @@ power_cppc_cpufreq_freq_down(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx + 1 == pi->nb_freqs)
return 0;
@@ -544,11 +513,6 @@ power_cppc_cpufreq_freq_up(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx == 0 || (pi->curr_idx == 1 &&
pi->turbo_available && !pi->turbo_enable))
@@ -561,11 +525,6 @@ power_cppc_cpufreq_freq_up(unsigned int lcore_id)
int
power_cppc_cpufreq_freq_max(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
/* Frequencies in the array are from high to low. */
if (lcore_power_info[lcore_id].turbo_available) {
if (lcore_power_info[lcore_id].turbo_enable)
@@ -585,11 +544,6 @@ power_cppc_cpufreq_freq_min(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
/* Frequencies in the array are from high to low. */
@@ -601,11 +555,6 @@ power_cppc_turbo_status(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
return pi->turbo_enable;
@@ -616,11 +565,6 @@ power_cppc_enable_turbo(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->turbo_available)
@@ -652,11 +596,6 @@ power_cppc_disable_turbo(unsigned int lcore_id)
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
pi->turbo_enable = 0;
@@ -680,10 +619,6 @@ power_cppc_get_capabilities(unsigned int lcore_id,
{
struct cppc_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
if (caps == NULL) {
POWER_LOG(ERR, "Invalid argument");
return -1;
--
2.33.0
^ permalink raw reply related
* [PATCH v3 08/10] power/kvm_vm: remove redundant lcore ID checks
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Now that the cpufreq framework validates the lcore ID using
rte_lcore_is_eal_managed() before dispatching to any driver,
each individual cpufreq driver no longer needs its own range
check against RTE_MAX_LCORE.
Remove the duplicated lcore ID checks from the kvm_vm cpufreq
driver and its guest_channel helper.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/power/kvm_vm/guest_channel.c | 22 ----------------------
drivers/power/kvm_vm/kvm_vm.c | 10 ----------
2 files changed, 32 deletions(-)
diff --git a/drivers/power/kvm_vm/guest_channel.c b/drivers/power/kvm_vm/guest_channel.c
index 42bfcedb56..dc8fe05fef 100644
--- a/drivers/power/kvm_vm/guest_channel.c
+++ b/drivers/power/kvm_vm/guest_channel.c
@@ -61,11 +61,6 @@ guest_channel_host_connect(const char *path, unsigned int lcore_id)
char fd_path[PATH_MAX];
int fd = -1;
- if (lcore_id >= RTE_MAX_LCORE) {
- GUEST_CHANNEL_LOG(ERR, "Channel(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
/* check if path is already open */
if (global_fds[lcore_id] != -1) {
GUEST_CHANNEL_LOG(ERR, "Channel(%u) is already open with fd %d",
@@ -127,12 +122,6 @@ guest_channel_send_msg(struct rte_power_channel_packet *pkt,
int ret, buffer_len = sizeof(*pkt);
void *buffer = pkt;
- if (lcore_id >= RTE_MAX_LCORE) {
- GUEST_CHANNEL_LOG(ERR, "Channel(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
-
if (global_fds[lcore_id] < 0) {
GUEST_CHANNEL_LOG(ERR, "Channel is not connected");
return -1;
@@ -169,12 +158,6 @@ int power_guest_channel_read_msg(void *pkt,
if (pkt_len == 0 || pkt == NULL)
return -1;
- if (lcore_id >= RTE_MAX_LCORE) {
- GUEST_CHANNEL_LOG(ERR, "Channel(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
-
if (global_fds[lcore_id] < 0) {
GUEST_CHANNEL_LOG(ERR, "Channel is not connected");
return -1;
@@ -225,11 +208,6 @@ int rte_power_guest_channel_receive_msg(void *pkt,
void
guest_channel_host_disconnect(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- GUEST_CHANNEL_LOG(ERR, "Channel(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return;
- }
if (global_fds[lcore_id] < 0)
return;
close(global_fds[lcore_id]);
diff --git a/drivers/power/kvm_vm/kvm_vm.c b/drivers/power/kvm_vm/kvm_vm.c
index 5754a441cd..e8b454bb55 100644
--- a/drivers/power/kvm_vm/kvm_vm.c
+++ b/drivers/power/kvm_vm/kvm_vm.c
@@ -24,11 +24,6 @@ power_kvm_vm_check_supported(void)
int
power_kvm_vm_init(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Core(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
pkt[lcore_id].command = RTE_POWER_CPU_POWER;
pkt[lcore_id].resource_id = lcore_id;
return guest_channel_host_connect(FD_PATH, lcore_id);
@@ -73,11 +68,6 @@ send_msg(unsigned int lcore_id, uint32_t scale_direction)
{
int ret;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Core(%u) is out of range 0...%d",
- lcore_id, RTE_MAX_LCORE-1);
- return -1;
- }
pkt[lcore_id].unit = scale_direction;
ret = guest_channel_send_msg(&pkt[lcore_id], lcore_id);
if (ret == 0)
--
2.33.0
^ permalink raw reply related
* [PATCH v3 07/10] power/intel_pstate: remove redundant lcore ID checks
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Now that the cpufreq framework validates the lcore ID using
rte_lcore_is_eal_managed() before dispatching to any driver,
each individual cpufreq driver no longer needs its own range
check against RTE_MAX_LCORE.
Remove the duplicated lcore ID checks from the intel_pstate
cpufreq driver ops.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
.../power/intel_pstate/intel_pstate_cpufreq.c | 65 -------------------
1 file changed, 65 deletions(-)
diff --git a/drivers/power/intel_pstate/intel_pstate_cpufreq.c b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
index 8e27570e3c..e7ed9c8260 100644
--- a/drivers/power/intel_pstate/intel_pstate_cpufreq.c
+++ b/drivers/power/intel_pstate/intel_pstate_cpufreq.c
@@ -548,12 +548,6 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
return -1;
}
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceed %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
exp_state = POWER_IDLE;
/* The power in use state works as a guard variable between
@@ -630,11 +624,6 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
struct pstate_power_info *pi;
uint32_t exp_state;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
pi = &lcore_power_info[lcore_id];
exp_state = POWER_USED;
@@ -688,11 +677,6 @@ power_pstate_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return 0;
- }
-
if (freqs == NULL) {
POWER_LOG(ERR, "NULL buffer supplied");
return 0;
@@ -711,11 +695,6 @@ power_pstate_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
uint32_t
power_pstate_cpufreq_get_freq(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return RTE_POWER_INVALID_FREQ_INDEX;
- }
-
return lcore_power_info[lcore_id].curr_idx;
}
@@ -723,11 +702,6 @@ power_pstate_cpufreq_get_freq(unsigned int lcore_id)
int
power_pstate_cpufreq_set_freq(unsigned int lcore_id, uint32_t index)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
return set_freq_internal(&(lcore_power_info[lcore_id]), index);
}
@@ -736,11 +710,6 @@ power_pstate_cpufreq_freq_up(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx == 0 ||
(pi->curr_idx == 1 && pi->turbo_available && !pi->turbo_enable))
@@ -755,11 +724,6 @@ power_pstate_cpufreq_freq_down(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx + 1 == pi->nb_freqs)
return 0;
@@ -771,11 +735,6 @@ power_pstate_cpufreq_freq_down(unsigned int lcore_id)
int
power_pstate_cpufreq_freq_max(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
/* Frequencies in the array are from high to low. */
if (lcore_power_info[lcore_id].turbo_available) {
if (lcore_power_info[lcore_id].turbo_enable)
@@ -796,11 +755,6 @@ power_pstate_cpufreq_freq_min(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
/* Frequencies in the array are from high to low. */
@@ -813,11 +767,6 @@ power_pstate_turbo_status(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
return pi->turbo_enable;
@@ -828,11 +777,6 @@ power_pstate_enable_turbo(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->turbo_available)
@@ -854,11 +798,6 @@ power_pstate_disable_turbo(unsigned int lcore_id)
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
pi->turbo_enable = 0;
@@ -882,10 +821,6 @@ int power_pstate_get_capabilities(unsigned int lcore_id,
{
struct pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
if (caps == NULL) {
POWER_LOG(ERR, "Invalid argument");
return -1;
--
2.33.0
^ permalink raw reply related
* [PATCH v3 04/10] power/acpi: remove redundant lcore ID checks
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Now that the cpufreq framework validates the lcore ID using
rte_lcore_is_eal_managed() before dispatching to any driver,
each individual cpufreq driver no longer needs its own range
check against RTE_MAX_LCORE.
Remove the duplicated lcore ID checks from the acpi cpufreq
driver ops.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/power/acpi/acpi_cpufreq.c | 65 -------------------------------
1 file changed, 65 deletions(-)
diff --git a/drivers/power/acpi/acpi_cpufreq.c b/drivers/power/acpi/acpi_cpufreq.c
index 81a5e3f6ea..9c78b4cb1a 100644
--- a/drivers/power/acpi/acpi_cpufreq.c
+++ b/drivers/power/acpi/acpi_cpufreq.c
@@ -242,12 +242,6 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
return -1;
}
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
exp_state = POWER_IDLE;
/* The power in use state works as a guard variable between
@@ -319,11 +313,6 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
struct acpi_power_info *pi;
uint32_t exp_state;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
pi = &lcore_power_info[lcore_id];
exp_state = POWER_USED;
/* The power in use state works as a guard variable between
@@ -373,11 +362,6 @@ power_acpi_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return 0;
- }
-
if (freqs == NULL) {
POWER_LOG(ERR, "NULL buffer supplied");
return 0;
@@ -396,22 +380,12 @@ power_acpi_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t num)
uint32_t
power_acpi_cpufreq_get_freq(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return RTE_POWER_INVALID_FREQ_INDEX;
- }
-
return lcore_power_info[lcore_id].curr_idx;
}
int
power_acpi_cpufreq_set_freq(unsigned int lcore_id, uint32_t index)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
return set_freq_internal(&(lcore_power_info[lcore_id]), index);
}
@@ -420,11 +394,6 @@ power_acpi_cpufreq_freq_down(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx + 1 == pi->nb_freqs)
return 0;
@@ -438,11 +407,6 @@ power_acpi_cpufreq_freq_up(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx == 0 ||
(pi->curr_idx == 1 && pi->turbo_available && !pi->turbo_enable))
@@ -455,11 +419,6 @@ power_acpi_cpufreq_freq_up(unsigned int lcore_id)
int
power_acpi_cpufreq_freq_max(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
/* Frequencies in the array are from high to low. */
if (lcore_power_info[lcore_id].turbo_available) {
if (lcore_power_info[lcore_id].turbo_enable)
@@ -479,11 +438,6 @@ power_acpi_cpufreq_freq_min(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
/* Frequencies in the array are from high to low. */
@@ -496,11 +450,6 @@ power_acpi_turbo_status(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
return pi->turbo_enable;
@@ -512,11 +461,6 @@ power_acpi_enable_turbo(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->turbo_available)
@@ -545,11 +489,6 @@ power_acpi_disable_turbo(unsigned int lcore_id)
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
pi->turbo_enable = 0;
@@ -572,10 +511,6 @@ int power_acpi_get_capabilities(unsigned int lcore_id,
{
struct acpi_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
if (caps == NULL) {
POWER_LOG(ERR, "Invalid argument");
return -1;
--
2.33.0
^ permalink raw reply related
* [PATCH v3 05/10] power/amd_pstate: remove redundant lcore ID checks
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Now that the cpufreq framework validates the lcore ID using
rte_lcore_is_eal_managed() before dispatching to any driver,
each individual cpufreq driver no longer needs its own range
check against RTE_MAX_LCORE.
Remove the duplicated lcore ID checks from the amd_pstate
cpufreq driver ops.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/power/amd_pstate/amd_pstate_cpufreq.c | 65 -------------------
1 file changed, 65 deletions(-)
diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
index 95495bff7d..8ca27d689d 100644
--- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
+++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
@@ -360,12 +360,6 @@ power_amd_pstate_cpufreq_init(unsigned int lcore_id)
return -1;
}
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
exp_state = POWER_IDLE;
/* The power in use state works as a guard variable between
@@ -443,11 +437,6 @@ power_amd_pstate_cpufreq_exit(unsigned int lcore_id)
struct amd_pstate_power_info *pi;
uint32_t exp_state;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
- lcore_id, RTE_MAX_LCORE - 1U);
- return -1;
- }
pi = &lcore_power_info[lcore_id];
exp_state = POWER_USED;
/* The power in use state works as a guard variable between
@@ -493,11 +482,6 @@ power_amd_pstate_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return 0;
- }
-
if (freqs == NULL) {
POWER_LOG(ERR, "NULL buffer supplied");
return 0;
@@ -516,22 +500,12 @@ power_amd_pstate_cpufreq_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t
uint32_t
power_amd_pstate_cpufreq_get_freq(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return RTE_POWER_INVALID_FREQ_INDEX;
- }
-
return lcore_power_info[lcore_id].curr_idx;
}
int
power_amd_pstate_cpufreq_set_freq(unsigned int lcore_id, uint32_t index)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
return set_freq_internal(&(lcore_power_info[lcore_id]), index);
}
@@ -540,11 +514,6 @@ power_amd_pstate_cpufreq_freq_down(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx + 1 == pi->nb_freqs)
return 0;
@@ -558,11 +527,6 @@ power_amd_pstate_cpufreq_freq_up(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->curr_idx == 0 || (pi->curr_idx == pi->nom_idx &&
pi->turbo_available && !pi->turbo_enable))
@@ -575,11 +539,6 @@ power_amd_pstate_cpufreq_freq_up(unsigned int lcore_id)
int
power_amd_pstate_cpufreq_freq_max(unsigned int lcore_id)
{
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
/* Frequencies in the array are from high to low. */
if (lcore_power_info[lcore_id].turbo_available) {
if (lcore_power_info[lcore_id].turbo_enable)
@@ -600,11 +559,6 @@ power_amd_pstate_cpufreq_freq_min(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
/* Frequencies in the array are from high to low. */
@@ -616,11 +570,6 @@ power_amd_pstate_turbo_status(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
return pi->turbo_enable;
@@ -631,11 +580,6 @@ power_amd_pstate_enable_turbo(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
if (pi->turbo_available)
@@ -667,11 +611,6 @@ power_amd_pstate_disable_turbo(unsigned int lcore_id)
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
-
pi = &lcore_power_info[lcore_id];
pi->turbo_enable = 0;
@@ -695,10 +634,6 @@ power_amd_pstate_get_capabilities(unsigned int lcore_id,
{
struct amd_pstate_power_info *pi;
- if (lcore_id >= RTE_MAX_LCORE) {
- POWER_LOG(ERR, "Invalid lcore ID");
- return -1;
- }
if (caps == NULL) {
POWER_LOG(ERR, "Invalid argument");
return -1;
--
2.33.0
^ permalink raw reply related
* [PATCH v3 00/10] power: centralize lcore ID verification in cpufreq framework
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
This series centralizes the lcore ID verification in the power cpufreq
framework, replacing the per-driver range checks with a common validation
using the new EAL helper rte_lcore_is_eal_managed().
Background
----------
Currently, various cpufreq drivers implement their own lcore ID checks,
which are limited to simple range validation against RTE_MAX_LCORE and
involve significant code duplication across 12+ functions per driver.
The checks are duplicated across all drivers — any change requires
updating 5+ drivers identically. Moreover, these checks do not verify
whether the lcore is actually managed by the application (i.e., its
role is ROLE_RTE or ROLE_SERVICE).
The lcore ID validation requirements differ by power sub-component:
- For cpufreq-related APIs and power QoS APIs, although service cores
do not typically invoke these APIs, they may operate in polling modes
where power management is required. To maintain compatibility with
applications using service cores, the validation logic now explicitly
accepts both ROLE_RTE and ROLE_SERVICE, using the new
rte_lcore_is_eal_managed() helper.
- For PMD power management APIs, the lcore must be ROLE_RTE because
these are used together with the data plane of ethdev PMD. Hence,
rte_lcore_is_enabled() is used for validation.
Key Changes:
------------
Patch 1: Add rte_lcore_is_eal_managed() in EAL to check whether a
lcore is managed by EAL (ROLE_RTE or ROLE_SERVICE).
Patch 2: Add a common macro RTE_POWER_VALID_LCOREID_OR_ERR_RET()
in power_common.h for use across the power library.
Patch 3: Add the centralized lcore ID verification to the cpufreq
framework dispatch layer, so all driver ops are guarded.
Patches 4-8: Remove the now-redundant lcore_id >= RTE_MAX_LCORE
checks from acpi, amd_pstate, cppc, intel_pstate, and
kvm_vm drivers.
Patch 9: Update power QoS to use the new validation, allowing
service cores to configure QoS parameters.
Patch 10: Add lcore validation to PMD management functions.
Note:
- Passed devtools/checkpatches.sh (9/10 valid, 1 acceptable
MACRO_WITH_FLOW_CONTROL warning).
Changes:
-------
v3:
- update release note.
- add __rte_experimental for new helper function.
- restructure this patch set to facilitate review.
v2:
- allow the service cores to set power API.
--------
Huisong Li (10):
eal: add interface to check if lcore is EAL managed
power: add a common macro to verify lcore ID
power/cpufreq: add the lcore ID verification to framework
power/acpi: remove redundant lcore ID checks
power/amd_pstate: remove redundant lcore ID checks
power/cppc: remove redundant lcore ID checks
power/intel_pstate: remove redundant lcore ID checks
power/kvm_vm: remove redundant lcore ID checks
power: allow the service core to config power QoS
power: add lcore ID check for PMD mgmt
doc/guides/rel_notes/release_26_07.rst | 19 ++++++
drivers/power/acpi/acpi_cpufreq.c | 65 -------------------
drivers/power/amd_pstate/amd_pstate_cpufreq.c | 65 -------------------
drivers/power/cppc/cppc_cpufreq.c | 65 -------------------
.../power/intel_pstate/intel_pstate_cpufreq.c | 65 -------------------
drivers/power/kvm_vm/guest_channel.c | 22 -------
drivers/power/kvm_vm/kvm_vm.c | 10 ---
lib/eal/common/eal_common_lcore.c | 11 ++++
lib/eal/include/rte_lcore.h | 12 ++++
lib/power/power_common.h | 7 ++
lib/power/rte_power_cpufreq.c | 13 ++++
lib/power/rte_power_pmd_mgmt.c | 21 +++---
lib/power/rte_power_qos.c | 10 +--
13 files changed, 75 insertions(+), 310 deletions(-)
--
2.33.0
^ permalink raw reply
* [PATCH v3 03/10] power/cpufreq: add the lcore ID verification to framework
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Currently, many cpufreq drivers verify the lcore ID in their
own driver and just restrict it to the RTE_MAX_LCORE range.
Actually, the lcore ID verification is common for each cpufreq
driver. It is better to add this verification to framework.
In addition, the lcore ID for cpufreq library is used by the
application and managed by the EAL layer. So this patch use
rte_lcore_is_eal_managed to verify the lcore ID in cpufreq
core.
Please note the lcore ID for cpufreq library must be ROLE_RTE
or ROLE_SERVICE after this patch.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
doc/guides/rel_notes/release_26_07.rst | 7 +++++++
lib/power/rte_power_cpufreq.c | 13 +++++++++++++
2 files changed, 20 insertions(+)
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index be5ccecd46..98a7cf0203 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -69,6 +69,13 @@ New Features
to check whether a logical core is managed by EAL
(i.e., its role is ``ROLE_RTE`` or ``ROLE_SERVICE``).
+* **Added lcore ID verification to power cpufreq framework.**
+
+ Added centralized lcore ID validation using ``rte_lcore_is_eal_managed()``
+ in the power cpufreq framework. All cpufreq API functions now verify
+ that the lcore ID is EAL-managed (``ROLE_RTE`` or ``ROLE_SERVICE``)
+ before proceeding, replacing per-driver range checks.
+
Removed Items
-------------
diff --git a/lib/power/rte_power_cpufreq.c b/lib/power/rte_power_cpufreq.c
index f63e976dc2..8394cbc77f 100644
--- a/lib/power/rte_power_cpufreq.c
+++ b/lib/power/rte_power_cpufreq.c
@@ -116,6 +116,7 @@ rte_power_init(unsigned int lcore_id)
struct rte_power_cpufreq_ops *ops;
uint8_t env;
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
if (global_default_env != PM_ENV_NOT_SET)
return global_cpufreq_ops->init(lcore_id);
@@ -147,6 +148,7 @@ RTE_EXPORT_SYMBOL(rte_power_exit)
int
rte_power_exit(unsigned int lcore_id)
{
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
if (global_default_env != PM_ENV_NOT_SET)
return global_cpufreq_ops->exit(lcore_id);
@@ -161,6 +163,7 @@ uint32_t
rte_power_freqs(unsigned int lcore_id, uint32_t *freqs, uint32_t n)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, 0);
return global_cpufreq_ops->get_avail_freqs(lcore_id, freqs, n);
}
@@ -169,6 +172,7 @@ uint32_t
rte_power_get_freq(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, RTE_POWER_INVALID_FREQ_INDEX);
return global_cpufreq_ops->get_freq(lcore_id);
}
@@ -177,6 +181,7 @@ uint32_t
rte_power_set_freq(unsigned int lcore_id, uint32_t index)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->set_freq(lcore_id, index);
}
@@ -185,6 +190,7 @@ int
rte_power_freq_up(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->freq_up(lcore_id);
}
@@ -193,6 +199,7 @@ int
rte_power_freq_down(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->freq_down(lcore_id);
}
@@ -201,6 +208,7 @@ int
rte_power_freq_max(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->freq_max(lcore_id);
}
@@ -209,6 +217,7 @@ int
rte_power_freq_min(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->freq_min(lcore_id);
}
@@ -217,6 +226,7 @@ int
rte_power_turbo_status(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->turbo_status(lcore_id);
}
@@ -225,6 +235,7 @@ int
rte_power_freq_enable_turbo(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->enable_turbo(lcore_id);
}
@@ -233,6 +244,7 @@ int
rte_power_freq_disable_turbo(unsigned int lcore_id)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->disable_turbo(lcore_id);
}
@@ -242,5 +254,6 @@ rte_power_get_capabilities(unsigned int lcore_id,
struct rte_power_core_capabilities *caps)
{
RTE_ASSERT(global_cpufreq_ops != NULL);
+ RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, -1);
return global_cpufreq_ops->get_caps(lcore_id, caps);
}
--
2.33.0
^ permalink raw reply related
* [PATCH v3 01/10] eal: add interface to check if lcore is EAL managed
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
Add a new helper function rte_lcore_is_eal_managed() to determine
if a logical core is managed by EAL.
This interface returns true if the lcore role is either ROLE_RTE
(standard worker/main cores) or ROLE_SERVICE (service cores).
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
doc/guides/rel_notes/release_26_07.rst | 6 ++++++
lib/eal/common/eal_common_lcore.c | 11 +++++++++++
lib/eal/include/rte_lcore.h | 12 ++++++++++++
3 files changed, 29 insertions(+)
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index f012d47a4b..be5ccecd46 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -63,6 +63,12 @@ New Features
``rte_eal_init`` and the application is responsible for probing each device,
* ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
+* **Added lcore EAL-managed status check.**
+
+ Added ``rte_lcore_is_eal_managed()`` helper function in EAL lcore library
+ to check whether a logical core is managed by EAL
+ (i.e., its role is ``ROLE_RTE`` or ``ROLE_SERVICE``).
+
Removed Items
-------------
diff --git a/lib/eal/common/eal_common_lcore.c b/lib/eal/common/eal_common_lcore.c
index 39411f9370..ab7b5e8393 100644
--- a/lib/eal/common/eal_common_lcore.c
+++ b/lib/eal/common/eal_common_lcore.c
@@ -102,6 +102,17 @@ int rte_lcore_is_enabled(unsigned int lcore_id)
return cfg->lcore_role[lcore_id] == ROLE_RTE;
}
+RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_lcore_is_eal_managed, 26.07)
+int rte_lcore_is_eal_managed(unsigned int lcore_id)
+{
+ struct rte_config *cfg = rte_eal_get_configuration();
+
+ if (lcore_id >= RTE_MAX_LCORE)
+ return 0;
+ return cfg->lcore_role[lcore_id] == ROLE_RTE ||
+ cfg->lcore_role[lcore_id] == ROLE_SERVICE;
+}
+
RTE_EXPORT_SYMBOL(rte_get_next_lcore)
unsigned int rte_get_next_lcore(unsigned int i, int skip_main, int wrap)
{
diff --git a/lib/eal/include/rte_lcore.h b/lib/eal/include/rte_lcore.h
index 10f965b4f0..9e6bb5230a 100644
--- a/lib/eal/include/rte_lcore.h
+++ b/lib/eal/include/rte_lcore.h
@@ -195,6 +195,18 @@ rte_cpuset_t rte_lcore_cpuset(unsigned int lcore_id);
*/
int rte_lcore_is_enabled(unsigned int lcore_id);
+/**
+ * Test if an lcore is ROLE_RTE or ROLE_SERVICE.
+ *
+ * @param lcore_id
+ * The identifier of the lcore, which MUST be between 0 and
+ * RTE_MAX_LCORE-1.
+ * @return
+ * True if the given lcore is ROLE_RTE or ROLE_SERVICE; false otherwise.
+ */
+__rte_experimental
+int rte_lcore_is_eal_managed(unsigned int lcore_id);
+
/**
* Get the next enabled lcore ID.
*
--
2.33.0
^ permalink raw reply related
* [PATCH v3 02/10] power: add a common macro to verify lcore ID
From: Huisong Li @ 2026-05-22 4:11 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260522041110.2023062-1-lihuisong@huawei.com>
There are many places to verify lcore ID in power. So add a common macro
to do this to simplify code.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
lib/power/power_common.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/lib/power/power_common.h b/lib/power/power_common.h
index 3f56b1103d..78cebef59b 100644
--- a/lib/power/power_common.h
+++ b/lib/power/power_common.h
@@ -23,6 +23,13 @@ extern int rte_power_logtype;
#define POWER_DEBUG_LOG(...)
#endif
+#define RTE_POWER_VALID_LCOREID_OR_ERR_RET(lcore_id, retval) do { \
+ if (!rte_lcore_is_eal_managed(lcore_id)) { \
+ POWER_LOG(ERR, "lcore id %u is invalid", lcore_id); \
+ return retval; \
+ } \
+} while (0)
+
/* check if scaling driver matches one we want */
__rte_internal
int cpufreq_check_scaling_driver(const char *driver);
--
2.33.0
^ permalink raw reply related
* Re: [PATCH 1/2] bus/uacce: support driver forward compatibility
From: fengchengwen @ 2026-05-22 1:05 UTC (permalink / raw)
To: David Marchand; +Cc: thomas, dev, qianweili, liuyonglong
In-Reply-To: <CAJFAV8wbj3oKRoF-0+jr1ceWMgVDZT-a9Rd5Ptdx9=N+1_WA7w@mail.gmail.com>
On 5/21/2026 9:06 PM, David Marchand wrote:
> Hello Chengwen,
>
> On Mon, 2 Mar 2026 at 12:02, Chengwen Feng <fengchengwen@huawei.com> wrote:
>>
>> As we know, the uacce driver (e.g. hisi_acc DMA driver) reads the API of
>> the hardware device (through /sysfs/class/uacce/xxx/api) and compares it
>> with the API supported by the driver to match the corresponding hardware
>> device.
>>
>> Hardware devices will continue to evolve, which means their APIs will
>> change, but business requirements demand that they support old
>> programming interfaces as much as possible.
>>
>> To adapt to this situation, this commit supports forward compatibility
>> of driver APIs. For example, if the driver supports the hisi_qm_v5 API,
>> it can drive the hardware device that supports the hisi_qm_v6 or
>> hisi_qm_v7 API.
>>
>> In addition, a driver flag (RTE_UACCE_DRV_FORWARD_COMPATIBILITY_DEV) is
>> introduced. The driver supports forward compatibility of APIs only when
>> this flag is defined.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>> drivers/bus/uacce/bus_uacce_driver.h | 5 +++
>> drivers/bus/uacce/uacce.c | 51 ++++++++++++++++++++++++++--
>> 2 files changed, 53 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/bus/uacce/bus_uacce_driver.h b/drivers/bus/uacce/bus_uacce_driver.h
>> index 618e0f9b76..c7445778a6 100644
>> --- a/drivers/bus/uacce/bus_uacce_driver.h
>> +++ b/drivers/bus/uacce/bus_uacce_driver.h
>> @@ -50,6 +50,7 @@ struct rte_uacce_device {
>> char dev_root[RTE_UACCE_DEV_PATH_SIZE]; /**< Sysfs path with device name. */
>> char cdev_path[RTE_UACCE_DEV_PATH_SIZE]; /**< Device path in devfs. */
>> char api[RTE_UACCE_API_NAME_SIZE]; /**< Device context type. */
>> + uint32_t api_ver; /**< Device api version used for compatibility. */
>
> I have some problem with this field.
> See below.
...
>> +
>> static bool
>> -uacce_match(const struct rte_uacce_driver *dr, const struct rte_uacce_device *dev)
>> +uacce_match(const struct rte_uacce_driver *dr, struct rte_uacce_device *dev)
>
> A matching helper should only match, and not modify the object.
Yes, you are right
>
>> {
>> + bool forward_compat = !!(dr->drv_flags & RTE_UACCE_DRV_FORWARD_COMPATIBILITY_DEV);
>> + uint32_t api_ver = uacce_calc_api_ver(dev->api, NULL);
>
> This conversion from a string to integer could be placed in the scanning step.
> Why place it here?
I think it's OK to place in the scanning step.
Should I submit a commit to fix this?
>
>
> The dev->api_ver has no in-tree user.
> This field was (silently?) dropped by my best AI friend in the bus
> refactoring series I posted.
> https://patchwork.dpdk.org/project/dpdk/patch/20260506155201.2709810-12-david.marchand@redhat.com/
>
> Please advise if I can drop this field (it is just the integer value
> extracted from dev->api afaiu), or if it should be moved to the
> scanning step.
Please keep this field.
This field mainly used for driver to know which api-version the device support, so driver could do
bug-fix for specific device by judging this field.
Thanks
>
>
^ permalink raw reply
* Re: [PATCH 0/2] extend interactive telemetry script
From: fengchengwen @ 2026-05-22 0:44 UTC (permalink / raw)
To: Bruce Richardson, dev
In-Reply-To: <20260521153913.82634-1-bruce.richardson@intel.com>
On 5/21/2026 11:39 PM, Bruce Richardson wrote:
> To simplify interactive telemetry script for general use, i.e. not from
> other scripts, we can add two new features to it:
>
> 1. Support for FOREACH to allow gathering a set of output values across
> a list of ports or devices, e.g. ethdevs or rawdevs.
> 2. Support having predefined aliases in a file in the user's home
> directory to simplify the use of more complicated FOREACH commands.
+1
How about add help command to introducing above FOREACH and aliases?
>
> Putting these together, we can create new commands such as "eth_names".
>
> bruce@host:$ cat ~/.dpdk_telemetry_aliases
> eth_names=FOREACH index /ethdev/list /ethdev/info,$index .name
>
> bruce@host:$ echo eth_names | ./usertools/dpdk-telemetry.py | jq
> [
> {
> "index": 0,
> "name": "0000:16:00.0"
> },
> {
> "index": 1,
> "name": "0000:16:00.1"
> }
> ]
>
> Bruce Richardson (2):
> usertools/telemetry: add a FOREACH command
> usertools/telemetry: support using aliases for long commands
>
> doc/guides/howto/telemetry.rst | 76 +++++++++++++
> usertools/dpdk-telemetry.py | 201 ++++++++++++++++++++++++++++++++-
> 2 files changed, 271 insertions(+), 6 deletions(-)
>
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH v2 2/2] ethdev: add telemetry endpoint for list names
From: fengchengwen @ 2026-05-22 0:42 UTC (permalink / raw)
To: Bruce Richardson, Stephen Hemminger
Cc: Morten Brørup, thomas, dev, andrew.rybchenko
In-Reply-To: <ag8oZodLjw0cp5Kf@bricha3-mobl1.ger.corp.intel.com>
On 5/21/2026 11:44 PM, Bruce Richardson wrote:
> On Thu, May 21, 2026 at 04:21:59PM +0100, Bruce Richardson wrote:
>> On Thu, May 21, 2026 at 07:37:16AM -0700, Stephen Hemminger wrote:
>>> On Thu, 21 May 2026 14:40:47 +0200 Morten Brørup
>>> <mb@smartsharesystems.com> wrote:
>>>
>>>>> From: fengchengwen [mailto:fengchengwen@huawei.com] Sent: Thursday,
>>>>> 21 May 2026 14.25
>>>>>
>>>>> Thanks for the feedback.
>>>>>
>>>>> I intend to keep the current dict format. This concise ID-name
>>>>> mapping is quite helpful and easy to read especially when there are
>>>>> massive ports, which is exactly the main purpose why I submitted
>>>>> this patch.
>>>>>
>>>>> In my opinion, adopting OData-style query would require
>>>>> architecture- level refactoring of telemetry framework, which is
>>>>> way too heavy for this simple requirement.
>>>>
>>>> Agree. Refactoring the telemetry framework is different task, not
>>>> related to this patch.
>>>>
>>>>> For complex query demands, we can implement them by extending the
>>>>> upper-layer Python telemetry script instead.
>>>>>
>>>>> So I suggest we keep this simple form here.
>>>>
>>>> If it is generally acceptable for DPDK telemetry that a request for a
>>>> list does not return a list type, but returns an object type with
>>>> "index": "value" fields instead, then Series-acked-by: Morten Brørup
>>>> <mb@smartsharesystems.com>
>>>
>>> It is necessary since port list may have holes due to hotplug or the
>>> ownership API. It would be good to have a more complete query function
>>> that returns more about each ethdev. I wouldn't worry about the size
>>> of the response. This is JSON and it is meant to be read by scripts not
>>> directly by humans.
>>
>> That's where I think we have a difference - if the output is meant to be
>> read by scripts, we should not need extra commands like this, since the
>> information is already available via existing commands. The only
>> compelling reason that I can see for adding a new command to return a set
>> of ethdev names is for human interactive use.
>>
>> Personally, I think the output should be just used by other scripts, but
>> it does appear that this quick-and-dirty telemetry script is being used
>> extensively for human interactive querying. Therefore, I'm working on
>> extending the script to make it more useful for such cases. I'd prefer to
>> add the extra smarts into the script rather than seeing a proliferation
>> of endpoints providing the same data in different formats.
>>
> Here [1] is my proposed generalised solution, quickly prototyped with copilot,
> composed of two parts: firstly, support for querying values across a range
> of ports using a foreach command, and then secondly, support for aliases to
> make the use of those foreach commands easier on the user. It's not
> intended as a mergable set of patches as-is, but rather to demonstrate how
> we might be able to come up with a more general solution (that keeps to the
> DRY principle) entirely within the python script rather than extending the
> C code.
Great
With the newly introduced FOREACH compound query capability, the port ID and
device name mapping can be fully acquired by existing telemetry commands.
The new command satisfies my original requirement perfectly.
>
> /Bruce
>
> [1] https://patches.dpdk.org/project/dpdk/list/?series=38197
>
^ permalink raw reply
* [PATCH v15 7/7] MAINTAINERS: add section for AI review tools
From: Stephen Hemminger @ 2026-05-21 22:44 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Aaron Conole, Thomas Monjalon
In-Reply-To: <20260521224550.927338-1-stephen@networkplumber.org>
Add maintainer entries for the AI-assisted code review tooling:
AGENTS.md, analyze-patch.py, compare-reviews.sh, and
review-doc.py.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Aaron Conole <aconole@redhat.com>
---
MAINTAINERS | 6 ++++++
doc/guides/rel_notes/release_26_07.rst | 5 +++++
2 files changed, 11 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b390446ed3..61a8a9ee68 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -109,6 +109,12 @@ F: license/
F: .editorconfig
F: .mailmap
+AI review tools
+M: Stephen Hemminger <stephen@networkplumber.org>
+M: Aaron Conole <aconole@redhat.com>
+F: AGENTS.md
+F: devtools/ai/
+
Linux kernel uAPI headers
M: Maxime Coquelin <maxime.coquelin@redhat.com>
F: devtools/linux-uapi.sh
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index f012d47a4b..32fb5a4c55 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -63,6 +63,11 @@ New Features
``rte_eal_init`` and the application is responsible for probing each device,
* ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
+* **Added scripts and prompts for AI review.**
+
+ Added AGENTS.md file for AI review and supporting scripts to analyze
+ patches and review documentation.
+
Removed Items
-------------
--
2.53.0
^ permalink raw reply related
* [PATCH v15 6/7] doc: add AI-assisted patch review to contributing guide
From: Stephen Hemminger @ 2026-05-21 22:44 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, Aaron Conole
In-Reply-To: <20260521224550.927338-1-stephen@networkplumber.org>
Add a new section to the contributing guide describing the
analyze-patch.py script which uses AI providers to review patches
against DPDK coding standards before submission to the mailing list.
The new section covers basic usage, provider selection, patch series
handling, LTS release review, and output format options. A note
clarifies that AI review supplements but does not replace human
review.
Also add a reference to the script in the new driver guide's
test tools checklist.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Aaron Conole <aconole@redhat.com>
---
doc/guides/contributing/new_driver.rst | 2 +
doc/guides/contributing/patches.rst | 59 ++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/doc/guides/contributing/new_driver.rst b/doc/guides/contributing/new_driver.rst
index 555e875329..6c0d356cfd 100644
--- a/doc/guides/contributing/new_driver.rst
+++ b/doc/guides/contributing/new_driver.rst
@@ -210,3 +210,5 @@ Be sure to run the following test tools per patch in a patch series:
* `check-doc-vs-code.sh`
* `check-spdx-tag.sh`
* Build documentation and validate how output looks
+* Optionally run ``analyze-patch.py`` for AI-assisted review
+ (see :ref:`ai_assisted_review` in the Contributing Guide)
diff --git a/doc/guides/contributing/patches.rst b/doc/guides/contributing/patches.rst
index 5f554d47e6..52001570c6 100644
--- a/doc/guides/contributing/patches.rst
+++ b/doc/guides/contributing/patches.rst
@@ -183,6 +183,10 @@ Make your planned changes in the cloned ``dpdk`` repo. Here are some guidelines
* Code and related documentation must be updated atomically in the same patch.
+* Consider running the :ref:`AI-assisted review <ai_assisted_review>` tool
+ before submitting to catch common issues early.
+ This is encouraged but not required.
+
Once the changes have been made you should commit them to your local repo.
For small changes, that do not require specific explanations, it is better to keep things together in the
@@ -503,6 +507,61 @@ Additionally, when contributing to the DTS tool, patches should also be checked
the ``dts-check-format.sh`` script in the ``devtools`` directory of the DPDK repo.
To run the script, extra :ref:`Python dependencies <dts_deps>` are needed.
+
+.. _ai_assisted_review:
+
+AI-Assisted Patch Review
+------------------------
+
+Contributors may optionally use the ``analyze-patch.py`` script
+to get an AI-assisted review of patches before submitting them to the mailing list.
+The script checks patches against the DPDK coding standards and contribution
+guidelines documented in ``AGENTS.md``.
+
+The script supports multiple AI providers (Anthropic Claude, OpenAI ChatGPT,
+xAI Grok, Google Gemini). An API key for the chosen provider must be set
+in the corresponding environment variable (see ``--list-providers``).
+
+Basic usage::
+
+ # Review a single patch (default provider: Anthropic Claude)
+ devtools/ai/analyze-patch.py my-patch.patch
+
+ # Use a different provider
+ devtools/ai/analyze-patch.py -p openai my-patch.patch
+
+ # Review for an LTS branch (enables stricter rules)
+ devtools/ai/analyze-patch.py -r 24.11 my-patch.patch
+
+ # List available providers and their API key variables
+ devtools/ai/analyze-patch.py --list-providers
+
+For a patch series in an mbox file, the ``--split-patches`` option reviews
+each patch individually::
+
+ devtools/ai/analyze-patch.py --split-patches series.mbox
+
+ # Review only a range of patches
+ devtools/ai/analyze-patch.py --split-patches --patch-range 1-5 series.mbox
+
+When reviewing for a Long Term Stable (LTS) release, use the ``-r`` option
+with the target version. Any DPDK release with minor version ``.11``
+(e.g., 23.11, 24.11) is automatically recognized as LTS,
+and the script will enforce stricter rules: bug fixes only, no new features or APIs.
+
+Output can be formatted as plain text (default), Markdown, HTML, or JSON::
+
+ devtools/ai/analyze-patch.py -f markdown -o review.md my-patch.patch
+
+The review guidelines in ``AGENTS.md`` focus on correctness bug detection
+and other DPDK-specific requirements. Commit message formatting and
+SPDX/copyright compliance are checked by ``checkpatches.sh`` and are
+not duplicated in the AI review.
+
+.. note::
+
+ Always verify AI suggestions before acting on them.
+
.. _contrib_check_compilation:
Checking Compilation
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox