* [PATCH v4] net/mlx5: prepend implicit items in sync flow creation path
From: Maxime Peim @ 2026-05-11 15:08 UTC (permalink / raw)
To: dev; +Cc: dsosnowski, viacheslavo, bingz, orika, suanmingm, matan
In-Reply-To: <20260427123217.510662-1-maxime.peim@gmail.com>
In eSwitch mode, the async (template) flow creation path automatically
prepends implicit match items to scope flow rules to the correct
representor port:
- Ingress: REPRESENTED_PORT item matching dev->data->port_id
- Egress: REG_C_0 TAG item matching the port's tx tag value
The sync path (flow_hw_list_create) was missing this logic, causing all
flow rules created via the non-template API to match traffic from all
ports rather than being scoped to the specific representor.
Add the same implicit item prepending to flow_hw_list_create, right
after pattern validation and before any branching (sample/RSS/single/
prefix), mirroring the behavior of flow_hw_pattern_template_create
and flow_hw_get_rule_items. The ingress case prepends
REPRESENTED_PORT with the current port_id; the egress case prepends
MLX5_RTE_FLOW_ITEM_TYPE_TAG with REG_C_0 value/mask (skipped when
user provides an explicit SQ item).
Also fix a pre-existing bug where 'return split' on metadata split
failure returned a negative int cast to uintptr_t, which callers
would treat as a valid flow handle instead of an error.
Fixes: e38776c36c8a ("net/mlx5: introduce HWS for non-template flow API")
Fixes: 821a6a5cc495 ("net/mlx5: add metadata split for compatibility")
Signed-off-by: Maxime Peim <maxime.peim@gmail.com>
---
v3:
- Factor the implicit-item prepend logic out of
flow_hw_pattern_template_create() into a new helper
flow_hw_adjust_pattern() and reuse it from flow_hw_list_create(),
instead of duplicating the prepend logic inline in the sync path.
- Zero-initialize item_flags in both callers. The validator is
read-modify-write on item_flags (reads MLX5_FLOW_LAYER_TUNNEL on
the first iteration), so leaving it uninitialized was UB.
- Call __flow_hw_pattern_validate() with nt_flow=true from the sync
path (was effectively nt_flow=false via the wrapper), restoring the
previous behavior that skips GENEVE_OPT TLV parser validation on
the non-template path.
- Document flow_hw_adjust_pattern(): the dual role of the nt_flow
parameter (template spec-left-zero vs. sync spec-filled + validator
flag), the three-way return, and the caller's ownership of
*copied_items across every exit path.
- Clarify the "omitting implicit REG_C_0 match" debug log now that
the helper runs on both the template and sync paths.
- Add Fixes: tags for the two original commits.
v4:
- Fix items in case splitted metadata are not needed.
drivers/net/mlx5/mlx5_flow_hw.c | 194 ++++++++++++++++++++++----------
1 file changed, 132 insertions(+), 62 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index bca5b2769e..6b3fcb43a7 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -9255,33 +9255,40 @@ pattern_template_validate(struct rte_eth_dev *dev,
return -ret;
}
-/**
- * Create flow item template.
+/*
+ * Validate the user-supplied items and, in eSwitch mode, prepend the implicit
+ * scoping item so the rule/template is bound to the current representor port:
+ * - ingress -> RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT (dev->data->port_id)
+ * - egress -> MLX5_RTE_FLOW_ITEM_TYPE_TAG on REG_C_0 (tx vport tag),
+ * skipped when the user already supplied an SQ item.
*
- * @param[in] dev
- * Pointer to the rte_eth_dev structure.
- * @param[in] attr
- * Pointer to the item template attributes.
- * @param[in] items
- * The template item pattern.
- * @param[out] error
- * Pointer to error structure.
+ * @param nt_flow
+ * Selects between the two call paths that share this helper:
+ * false -> pattern template creation (async API). The prepended item's
+ * spec is left zeroed so mlx5dr matches any value; the live
+ * port_id / tx-tag value is substituted later by
+ * flow_hw_get_rule_items() at rule-create time.
+ * true -> sync (non-template) flow creation. The prepended item's spec
+ * is filled immediately with the live values, and the flag is
+ * forwarded to __flow_hw_pattern_validate() so that validation
+ * paths gated on nt_flow (e.g. GENEVE_OPT TLV parser creation)
+ * take the non-template branch.
*
- * @return
- * Item template pointer on success, NULL otherwise and rte_errno is set.
+ * Return / ownership:
+ * - NULL on validation or allocation failure (error populated).
+ * - `items` unchanged when no prepending is required; *copied_items == NULL.
+ * - A newly-allocated array otherwise; also stored in *copied_items. The
+ * caller must mlx5_free(*copied_items) on every path (it is safe to call
+ * with NULL). Do not free the returned pointer directly.
*/
-static struct rte_flow_pattern_template *
-flow_hw_pattern_template_create(struct rte_eth_dev *dev,
- const struct rte_flow_pattern_template_attr *attr,
- const struct rte_flow_item items[],
- bool external,
- struct rte_flow_error *error)
+static const struct rte_flow_item *
+flow_hw_adjust_pattern(struct rte_eth_dev *dev, const struct rte_flow_pattern_template_attr *attr,
+ bool nt_flow, const struct rte_flow_item *items, uint64_t *item_flags,
+ uint64_t *nb_items, struct rte_flow_item **copied_items,
+ struct rte_flow_error *error)
{
struct mlx5_priv *priv = dev->data->dev_private;
- struct rte_flow_pattern_template *it;
- struct rte_flow_item *copied_items = NULL;
- const struct rte_flow_item *tmpl_items;
- uint64_t orig_item_nb, item_flags = 0;
+ struct rte_flow_item_ethdev port_spec = {.port_id = dev->data->port_id};
struct rte_flow_item port = {
.type = RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT,
.mask = &rte_flow_item_ethdev_mask,
@@ -9298,39 +9305,89 @@ flow_hw_pattern_template_create(struct rte_eth_dev *dev,
.type = (enum rte_flow_item_type)MLX5_RTE_FLOW_ITEM_TYPE_TAG,
.spec = &tag_v,
.mask = &tag_m,
- .last = NULL
+ .last = NULL,
};
- int it_items_size;
- unsigned int i = 0;
int rc;
+ if (!copied_items || !item_flags || !nb_items)
+ return NULL;
+
+ if (nt_flow) {
+ port.spec = &port_spec;
+ tag_v.data = flow_hw_tx_tag_regc_value(dev);
+ }
+
+ /*
+ * item_flags must be zero-initialized: __flow_hw_pattern_validate()
+ * OR-accumulates bits into it and reads it (MLX5_FLOW_LAYER_TUNNEL)
+ * on the first iteration.
+ */
+ *item_flags = 0;
+
/* Validate application items only */
- rc = flow_hw_pattern_validate(dev, attr, items, &item_flags, error);
+ rc = __flow_hw_pattern_validate(dev, attr, items, item_flags, nt_flow, error);
if (rc < 0)
return NULL;
- orig_item_nb = rc;
- if (priv->sh->config.dv_esw_en &&
- attr->ingress && !attr->egress && !attr->transfer) {
- copied_items = flow_hw_prepend_item(items, orig_item_nb, &port, error);
- if (!copied_items)
+ *nb_items = rc;
+
+ if (priv->sh->config.dv_esw_en && attr->ingress && !attr->egress && !attr->transfer) {
+ *copied_items = flow_hw_prepend_item(items, *nb_items, &port, error);
+ if (!*copied_items)
return NULL;
- tmpl_items = copied_items;
- } else if (priv->sh->config.dv_esw_en &&
- !attr->ingress && attr->egress && !attr->transfer) {
- if (item_flags & MLX5_FLOW_ITEM_SQ) {
- DRV_LOG(DEBUG, "Port %u omitting implicit REG_C_0 match for egress "
- "pattern template", dev->data->port_id);
- tmpl_items = items;
- goto setup_pattern_template;
+ return *copied_items;
+ } else if (priv->sh->config.dv_esw_en && !attr->ingress && attr->egress &&
+ !attr->transfer) {
+ if (*item_flags & MLX5_FLOW_ITEM_SQ) {
+ DRV_LOG(DEBUG,
+ "Port %u: explicit SQ item present, omitting implicit "
+ "REG_C_0 match for egress pattern",
+ dev->data->port_id);
+ return items;
}
- copied_items = flow_hw_prepend_item(items, orig_item_nb, &tag, error);
- if (!copied_items)
+ *copied_items = flow_hw_prepend_item(items, *nb_items, &tag, error);
+ if (!*copied_items)
return NULL;
- tmpl_items = copied_items;
- } else {
- tmpl_items = items;
+ return *copied_items;
}
-setup_pattern_template:
+ return items;
+}
+
+/**
+ * Create flow item template.
+ *
+ * @param[in] dev
+ * Pointer to the rte_eth_dev structure.
+ * @param[in] attr
+ * Pointer to the item template attributes.
+ * @param[in] items
+ * The template item pattern.
+ * @param[out] error
+ * Pointer to error structure.
+ *
+ * @return
+ * Item template pointer on success, NULL otherwise and rte_errno is set.
+ */
+static struct rte_flow_pattern_template *
+flow_hw_pattern_template_create(struct rte_eth_dev *dev,
+ const struct rte_flow_pattern_template_attr *attr,
+ const struct rte_flow_item items[],
+ bool external,
+ struct rte_flow_error *error)
+{
+ struct mlx5_priv *priv = dev->data->dev_private;
+ struct rte_flow_pattern_template *it;
+ struct rte_flow_item *copied_items = NULL;
+ const struct rte_flow_item *tmpl_items;
+ int it_items_size;
+ uint64_t orig_item_nb, item_flags;
+ unsigned int i = 0;
+ int rc;
+
+ tmpl_items = flow_hw_adjust_pattern(dev, attr, false, items, &orig_item_nb, &item_flags,
+ &copied_items, error);
+ if (!tmpl_items)
+ return NULL;
+
it = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*it), 0, SOCKET_ID_ANY);
if (!it) {
rte_flow_error_set(error, ENOMEM,
@@ -14272,7 +14329,6 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
struct rte_flow_hw *prfx_flow = NULL;
const struct rte_flow_action *qrss = NULL;
const struct rte_flow_action *mark = NULL;
- uint64_t item_flags = 0;
uint64_t action_flags = mlx5_flow_hw_action_flags_get(actions, &qrss, &mark,
&encap_idx, &actions_n, error);
struct mlx5_flow_hw_split_resource resource = {
@@ -14289,20 +14345,27 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
.egress = attr->egress,
.transfer = attr->transfer,
};
-
- /* Validate application items only */
- ret = __flow_hw_pattern_validate(dev, &pattern_template_attr, items,
- &item_flags, true, error);
- if (ret < 0)
- return 0;
+ struct rte_flow_item *copied_items = NULL;
+ const struct rte_flow_item *prepend_items;
+ uint64_t orig_item_nb, item_flags;
RTE_SET_USED(encap_idx);
if (!error)
error = &shadow_error;
+
+ prepend_items = flow_hw_adjust_pattern(dev, &pattern_template_attr, true, items,
+ &orig_item_nb, &item_flags, &copied_items, error);
+ if (!prepend_items)
+ return 0;
+
split = mlx5_flow_nta_split_metadata(dev, attr, actions, qrss, action_flags,
actions_n, external, &resource, error);
- if (split < 0)
- return split;
+ if (split < 0) {
+ mlx5_free(copied_items);
+ return 0;
+ } else if (!split) {
+ resource.suffix.items = prepend_items;
+ }
/* Update the metadata copy table - MLX5_FLOW_MREG_CP_TABLE_GROUP */
if (((attr->ingress && attr->group != MLX5_FLOW_MREG_CP_TABLE_GROUP) ||
@@ -14313,23 +14376,26 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
goto free;
}
if (action_flags & MLX5_FLOW_ACTION_SAMPLE) {
- flow = mlx5_nta_sample_flow_list_create(dev, type, attr, items, actions,
+ flow = mlx5_nta_sample_flow_list_create(dev, type, attr, prepend_items, actions,
item_flags, action_flags, error);
- if (flow != NULL)
+ if (flow != NULL) {
+ mlx5_free(copied_items);
return (uintptr_t)flow;
+ }
goto free;
}
if (action_flags & MLX5_FLOW_ACTION_RSS) {
const struct rte_flow_action_rss
*rss_conf = mlx5_flow_nta_locate_rss(dev, actions, error);
- flow = mlx5_flow_nta_handle_rss(dev, attr, items, actions, rss_conf,
- item_flags, action_flags, external,
- type, error);
+ flow = mlx5_flow_nta_handle_rss(dev, attr, prepend_items, actions, rss_conf,
+ item_flags, action_flags, external, type, error);
if (flow) {
flow->nt2hws->rix_mreg_copy = cpy_idx;
cpy_idx = 0;
- if (!split)
+ if (!split) {
+ mlx5_free(copied_items);
return (uintptr_t)flow;
+ }
goto prefix_flow;
}
goto free;
@@ -14343,12 +14409,14 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
if (flow) {
flow->nt2hws->rix_mreg_copy = cpy_idx;
cpy_idx = 0;
- if (!split)
+ if (!split) {
+ mlx5_free(copied_items);
return (uintptr_t)flow;
+ }
/* Fall Through to prefix flow creation. */
}
prefix_flow:
- ret = mlx5_flow_hw_create_flow(dev, type, attr, items, resource.prefix.actions,
+ ret = mlx5_flow_hw_create_flow(dev, type, attr, prepend_items, resource.prefix.actions,
item_flags, action_flags, external, &prfx_flow, error);
if (ret)
goto free;
@@ -14357,6 +14425,7 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
flow->nt2hws->chaned_flow = 1;
SLIST_INSERT_AFTER(prfx_flow, flow, nt2hws->next);
mlx5_flow_nta_split_resource_free(dev, &resource);
+ mlx5_free(copied_items);
return (uintptr_t)prfx_flow;
}
free:
@@ -14368,6 +14437,7 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
mlx5_flow_nta_del_copy_action(dev, cpy_idx);
if (split > 0)
mlx5_flow_nta_split_resource_free(dev, &resource);
+ mlx5_free(copied_items);
return 0;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 6/6] cmdline: add null checks for invalid input
From: Alex Michael @ 2026-05-07 22:25 UTC (permalink / raw)
To: bruce.richardson; +Cc: dev
pmd_buffer_scatter test suite failed on Ubuntu 24.04 LTS with an Intel XL710 40GbE NIC. Conversely it didn’t fail on Ubuntu 22.04 with an Intel E810 NIC, so the issue lies either with the OS (mbuf size or mempool configuration mismatch / buffer handling logic), the NIC (driver-level bug), the interaction between them or the test suite itself (as it should be unrelated to NULL conditionals).
^ permalink raw reply
* [PATCH v2] net/ixgbe: fix MAC/VLAN item validation for ntuple
From: Daniil Iskhakov @ 2026-05-07 13:21 UTC (permalink / raw)
To: Anatoly Burakov, Vladimir Medvedkin, Wei Zhao, Wenzhuo Lu
Cc: dev, stable, Daniil Iskhakov, Daniil Agalakov, sdl.dpdk, rrv
When parsing an ntuple filter, the code attempts to ensure that if the
first item is ETH or VLAN, its spec and mask are either absent or
contain only zero fields. The current check is:
if ((item->spec || item->mask) &&
(memcmp(spec, &null_struct, size) ||
memcmp(mask, &null_struct, size)))
This condition is logically incorrect. If item->spec points to a
zero-filled structure and item->mask is NULL, memcmp(mask) would
dereference a NULL pointer.
The intended behavior is to reject any non-zero spec or mask.
Guard each memcmp() call with a check of the corresponding pointer while
keeping a single error path.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 46ea969177f3 ("net/ixgbe: add ntuple support to flow parser")
Cc: stable@dpdk.org
Signed-off-by: Daniil Agalakov <ade@amicon.ru>
Signed-off-by: Daniil Iskhakov <dish@amicon.ru>
---
v2:
- Kept spec/mask validation as a single condition.
- Added explicit comparisons against NULL and zero.
- Fixed continuation indentation.
- Fixed the commit message.
Cc: wei.zhao1@intel.com
Cc: sdl.dpdk@linuxtesting.org
Cc: rrv@amicon.ru
---
drivers/net/intel/ixgbe/ixgbe_flow.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index 01cd4f9bde..a855cbfbd2 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -238,14 +238,10 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
}
/* if the first item is MAC, the content should be NULL */
- if ((item->spec || item->mask) &&
- (memcmp(eth_spec, ð_null,
- sizeof(struct rte_flow_item_eth)) ||
- memcmp(eth_mask, ð_null,
- sizeof(struct rte_flow_item_eth)))) {
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- item, "Not supported by ntuple filter");
+ if ((item->spec != NULL && memcmp(eth_spec, ð_null, sizeof(eth_null)) != 0) ||
+ (item->mask != NULL && memcmp(eth_mask, ð_null, sizeof(eth_null)) != 0)) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, item,
+ "Not supported by ntuple filter");
return -rte_errno;
}
/* check if the next not void item is IPv4 or Vlan */
@@ -271,15 +267,10 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
return -rte_errno;
}
/* the content should be NULL */
- if ((item->spec || item->mask) &&
- (memcmp(vlan_spec, &vlan_null,
- sizeof(struct rte_flow_item_vlan)) ||
- memcmp(vlan_mask, &vlan_null,
- sizeof(struct rte_flow_item_vlan)))) {
-
- rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ITEM,
- item, "Not supported by ntuple filter");
+ if ((item->spec != NULL && memcmp(vlan_spec, &vlan_null, sizeof(vlan_null)) != 0) ||
+ (item->mask != NULL && memcmp(vlan_mask, &vlan_null, sizeof(vlan_null)) != 0)) {
+ rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ITEM, item,
+ "Not supported by ntuple filter");
return -rte_errno;
}
/* check if the next not void item is IPv4 */
--
2.43.0
^ permalink raw reply related
* [PATCH] ethdev: fix pointer check in GENEVE and RAW flow copy
From: Denis Lyulin @ 2026-05-07 11:20 UTC (permalink / raw)
To: Ori Kam, Thomas Monjalon, Andrew Rybchenko, Ferruh Yigit,
Michael Baum
Cc: dev, stable, adrien.mazarguil, Denis Lyulin
When rte_flow_conv_item_spec() is called from rte_flow_conv_pattern(),
the spec, last and mask pointers are checked separately. If the API
is used incorrectly, the spec pointer may be NULL while last and mask
may be valid pointers.
In rte_flow_conv_item_spec() for GENVE_OPT and RAW item types the spec
pointer is used even if the function is called to copy last or mask.
It may cause a NULL pointer (spec) dereference.
This commit adds extra check of item->spec and if it is NULL, does not
copy further data relying on it
Fixes: 841a0445442d ("ethdev: fix GENEVE option item conversion")
Cc: michaelba@nvidia.com
Cc: adrien.mazarguil@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Denis Lyulin <lyulin.2003@mail.ru>
---
lib/ethdev/rte_flow.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
index fe8f43caff..7cf585e6f5 100644
--- a/lib/ethdev/rte_flow.c
+++ b/lib/ethdev/rte_flow.c
@@ -672,13 +672,17 @@ rte_flow_conv_item_spec(void *buf, const size_t size,
}),
size > sizeof(*dst.raw) ? sizeof(*dst.raw) : size);
off = sizeof(*dst.raw);
- if (type == RTE_FLOW_CONV_ITEM_SPEC ||
- (type == RTE_FLOW_CONV_ITEM_MASK &&
- ((spec.raw->length & mask.raw->length) >=
- (last.raw->length & mask.raw->length))))
- tmp = spec.raw->length & mask.raw->length;
- else
- tmp = last.raw->length & mask.raw->length;
+ if (spec.raw && last.raw) {
+ if (type == RTE_FLOW_CONV_ITEM_SPEC ||
+ (type == RTE_FLOW_CONV_ITEM_MASK &&
+ ((spec.raw->length & mask.raw->length) >=
+ (last.raw->length & mask.raw->length))))
+ tmp = spec.raw->length & mask.raw->length;
+ else
+ tmp = last.raw->length & mask.raw->length;
+ } else {
+ tmp = 0;
+ }
if (tmp) {
off = RTE_ALIGN_CEIL(off, sizeof(*dst.raw->pattern));
if (size >= off + tmp) {
@@ -696,8 +700,8 @@ rte_flow_conv_item_spec(void *buf, const size_t size,
spec.geneve_opt = item->spec;
src.geneve_opt = data;
dst.geneve_opt = buf;
- tmp = spec.geneve_opt->option_len << 2;
- if (size > 0 && src.geneve_opt->data) {
+ tmp = spec.geneve_opt ? (spec.geneve_opt->option_len << 2) : 0;
+ if (size > 0 && tmp > 0 && src.geneve_opt->data) {
deep_src = (void *)((uintptr_t)(dst.geneve_opt + 1));
dst.geneve_opt->data = rte_memcpy(deep_src,
src.geneve_opt->data,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 0/6] net/gve: add hardware timestamping support
From: Stephen Hemminger @ 2026-05-12 7:14 UTC (permalink / raw)
To: mark-blasko; +Cc: dev
In-Reply-To: <20260511224354.872997-1-blasko@google.com>
On Mon, 11 May 2026 22:43:48 +0000
mark-blasko <blasko@google.com> wrote:
> From: Mark Blasko <blasko@google.com>
>
> This patch series introduces support for GVE hardware timestamping on DQO
> queues. To support concurrent access, a mutex lock is introduced to protect
> admin queue operations. A mechanism is then added to periodically synchronize
> the NIC clock via AdminQ, and support is introduced for the read_clock ethdev
> operation. Finally, the RX datapath is updated to reconstruct full 64-bit
> timestamps from the 32-bit values in DQO descriptors.
>
> Mark Blasko (6):
> net/gve: add thread safety to admin queue
> net/gve: add device option support for HW timestamps
> net/gve: add AdminQ command for NIC timestamps
> net/gve: add periodic NIC clock synchronization
> net/gve: support read clock ethdev op
> net/gve: reconstruct HW timestamps from DQO
>
> doc/guides/nics/features/gve.ini | 1 +
> doc/guides/nics/gve.rst | 18 +++
> doc/guides/rel_notes/release_26_07.rst | 3 +
> drivers/net/gve/base/gve_adminq.c | 127 +++++++++++++++++----
> drivers/net/gve/base/gve_adminq.h | 29 +++++
> drivers/net/gve/base/gve_desc_dqo.h | 8 +-
> drivers/net/gve/gve_ethdev.c | 148 ++++++++++++++++++++++++-
> drivers/net/gve/gve_ethdev.h | 39 +++++++
> drivers/net/gve/gve_rx_dqo.c | 26 +++++
> 9 files changed, 377 insertions(+), 22 deletions(-)
>
Overall looks good, a little concerned about using ROBUST mutex.
AI review (with more opus 4.7)
Based on my review, here are the findings on this series. The patches mostly look reasonable but I have several concerns:
---
Some findings on this series.
[PATCH 1/6] net/gve: add thread safety to admin queue
The mutex is initialized with PTHREAD_MUTEX_ROBUST but pthread_mutex_lock
return values are never checked anywhere in the new code.
pthread_mutexattr_setrobust(&mutexattr, PTHREAD_MUTEX_ROBUST);
pthread_mutex_init(&priv->adminq_lock, &mutexattr);
...
pthread_mutex_lock(&priv->adminq_lock);
A robust mutex returns EOWNERDEAD on lock when the previous owner died
holding it, and transitions to permanent ENOTRECOVERABLE if
pthread_mutex_consistent() is not called before unlock. Either drop the
ROBUST attribute (matching the existing flow_rule_lock pattern in this
same driver, which uses only PROCESS_SHARED), or handle EOWNERDEAD and
ENOTRECOVERABLE explicitly.
[PATCH 3/6] net/gve: add AdminQ command for NIC timestamps
gve_adminq_alloc() explicitly zeroes every other adminq counter, but
the new adminq_report_nic_timestamp_cnt is not added to that list. The
counters get re-zeroed on every adminq_alloc, including from
gve_dev_reset() which calls gve_adminq_free() and then re-runs
gve_init_priv() -> gve_adminq_alloc(). After a device reset, every
counter except this new one will be back to zero, leaving inconsistent
stats. Add the assignment in gve_adminq_alloc().
[PATCH 4/6] net/gve: add periodic NIC clock synchronization
Unnecessary cast of void *:
priv->nic_ts_report = (struct gve_nic_ts_report *)priv->nic_ts_report_mz->addr;
The same applies to the function-arg cast in gve_read_nic_clock:
struct gve_priv *priv = (struct gve_priv *)arg;
Assignment from void * does not need a cast in C.
If the reschedule in gve_read_nic_clock fails:
err = rte_eal_alarm_set(GVE_NIC_CLOCK_READ_PERIOD_MS * 1000,
gve_read_nic_clock, priv);
if (err < 0)
PMD_DRV_LOG(ERR, "Failed to reschedule NIC clock read alarm, ret=%d", err);
no further callback will fire, no failures will accumulate, and
nic_ts_stale stays at whatever it was. If reschedule failure occurs
while stale is 0, the Rx datapath in 6/6 will keep attaching
reconstructed timestamps from an arbitrarily old base. Set nic_ts_stale
to 1 on reschedule failure so the Rx datapath stops trusting the cache.
gve_setup_nic_timestamp() discards the gve_alloc_nic_ts_report() error.
If the memzone allocation fails, priv->nic_timestamp_supported (set in
2/6) remains true, so dev_info_get() in 6/6 will advertise
RTE_ETH_RX_OFFLOAD_TIMESTAMP and read_clock() in 5/6 will return -EIO
on every call. Clear priv->nic_timestamp_supported on alloc failure so
the capability is advertised honestly.
[PATCH 6/6] net/gve: reconstruct HW timestamps from DQO
priv->mbuf_timestamp_offset sits in rte_zmalloc'd dev_private memory so
it starts at 0, but the fast path uses:
priv->mbuf_timestamp_offset >= 0
The check is dead under the rxq->timestamp_enabled gate today, but the
>= 0 form suggests an "offset registered" semantic that isn't actually
enforced - a zero-initialized offset passes. Initialize
priv->mbuf_timestamp_offset to -1 in gve_dev_init(), or remove the
inner check since timestamp_enabled is the real gate.
^ permalink raw reply
* RE: [PATCH v11 3/5] vhost_user: support function defines for back-end
From: Bathija, Pravin @ 2026-05-12 4:26 UTC (permalink / raw)
To: Maxime Coquelin, Stephen Hemminger
Cc: dev@dpdk.org, stephen@networkplumber.org, thomas@monjalon.net,
fengchengwen@huawei.com
In-Reply-To: <DM6PR19MB356485F24CFE411327BA5CDCF53F2@DM6PR19MB3564.namprd19.prod.outlook.com>
Hi Stephen, Maxime,
Any feedback on the latest patch-set v12 ? Please help merge this into mainline so we can catch the upcoming SPDK release.
Thanks,
Pravin
Internal Use - Confidential
> -----Original Message-----
> From: Bathija, Pravin
> Sent: Tuesday, May 5, 2026 9:00 PM
> To: 'Maxime Coquelin' <maxime.coquelin@redhat.com>
> Cc: dev@dpdk.org; stephen@networkplumber.org; thomas@monjalon.net;
> fengchengwen@huawei.com
> Subject: RE: [PATCH v11 3/5] vhost_user: support function defines for back-end
>
> Hi Maxime,
>
> The response are inline. I have also submitted patch-set v12 with the changes.
>
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, May 5, 2026 2:48 AM
> To: Bathija, Pravin <Pravin.Bathija@dell.com>
> Cc: dev@dpdk.org; stephen@networkplumber.org; thomas@monjalon.net;
> fengchengwen@huawei.com
> Subject: Re: [PATCH v11 3/5] vhost_user: support function defines for back-end
>
> [EXTERNAL EMAIL]
>
>
> On Tue, May 5, 2026 at 7:53 AM <mailto:pravin.bathija@dell.com> wrote:
> From: Pravin M Bathija <mailto:pravin.bathija@dell.com>
>
> Here we define support functions which are called from the various vhost-user
> back-end message functions like set memory table, get memory slots, add
> memory region, remove memory region. These are essentially common
> functions to initialize memory, unmap a set of memory regions, perform
> register copy, align memory addresses, dma map/unmap a single memory
> region and remove guest pages by removing all entries belonging to a given
> memory region.
>
> Signed-off-by: Pravin M Bathija <mailto:pravin.bathija@dell.com>
> ---
> lib/vhost/vhost_user.c | 146 ++++++++++++++++++++++++++++++++++++++--
> -
> 1 file changed, 136 insertions(+), 10 deletions(-)
>
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index
> 4bfb13fb98..1f96ecf963 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -171,6 +171,52 @@ get_blk_size(int fd)
> return ret == -1 ? (uint64_t)-1 : (uint64_t)stat.st_blksize;
> }
>
> +static int
> +async_dma_map_region(struct virtio_net *dev, struct
> +rte_vhost_mem_region *reg, bool do_map) {
> + uint32_t i;
> + int ret;
> + uint64_t reg_start = reg->host_user_addr;
> + uint64_t reg_end = reg_start + reg->size;
> +
> + for (i = 0; i < dev->nr_guest_pages; i++) {
> + struct guest_page *page = &dev->guest_pages[i];
> +
> + /* Only process pages belonging to this region */
> + if (page->host_user_addr < reg_start ||
> + page->host_user_addr >= reg_end)
> + continue;
> +
> + if (do_map) {
> + ret =
> +rte_vfio_container_dma_map(RTE_VFIO_DEFAULT_CONTAINER_FD,
> + page->host_user_addr,
> + page->host_iova,
> + page->size);
> + if (ret) {
> + if (rte_errno == ENODEV)
> + return 0;
> +
> + VHOST_CONFIG_LOG(dev->ifname, ERR, "DMA
> +engine map failed");
> + return -1;
> + }
> + } else {
> + ret =
> +rte_vfio_container_dma_unmap(RTE_VFIO_DEFAULT_CONTAINER_FD,
> + page->host_user_addr,
> + page->host_iova,
> + page->size);
> + if (ret) {
> + if (rte_errno == EINVAL)
> + return 0;
> +
> + VHOST_CONFIG_LOG(dev->ifname, ERR, "DMA
> +engine unmap failed");
> + return -1;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static void
> async_dma_map(struct virtio_net *dev, bool do_map)
> {
> @@ -225,7 +271,17 @@ async_dma_map(struct virtio_net *dev, bool
> do_map)
> }
>
> I think async_dma_map and async_dma_map_region should be refactored to
> avoid code duplication, What about something like this:
>
> static void
> async_dma_map(struct virtio_net *dev, bool do_map)
> {
> uint32_t i;
> struct rte_vhost_mem_region *reg;
>
> for (i = 0; i < VHOST_MEMORY_MAX_NREGIONS; i++) {
> reg = &dev->mem->regions[i];
> if (reg->host_user_addr == 0)
> continue;
> async_dma_map_region(dev, reg, do_map);
> }
> }
>
> Also, duplicating code and stripping comments is not ideal as they are
> important (i.e. to understand why we can ignore ENODEV and EINVAL)
>
> DMA refactoring: async_dma_map() now delegates to
> async_dma_map_region(), eliminating the duplicated DMA map/unmap logic.
> The original comments explaining ENODEV/EINVAL handling have been
> restored in async_dma_map_region().
>
>
> static void
> -free_mem_region(struct virtio_net *dev)
> +free_mem_region(struct rte_vhost_mem_region *reg) {
> + if (reg != NULL && reg->mmap_addr) {
> + munmap(reg->mmap_addr, reg->mmap_size);
> + close(reg->fd);
> + memset(reg, 0, sizeof(struct rte_vhost_mem_region));
> + }
> +}
> +
> +static void
> +free_all_mem_regions(struct virtio_net *dev)
> {
> uint32_t i;
> struct rte_vhost_mem_region *reg; @@ -236,12 +292,10 @@
> free_mem_region(struct virtio_net *dev)
> if (dev->async_copy && rte_vfio_is_enabled("vfio"))
> async_dma_map(dev, false);
>
> - for (i = 0; i < dev->mem->nregions; i++) {
> + for (i = 0; i < VHOST_MEMORY_MAX_NREGIONS; i++) {
> reg = &dev->mem->regions[i];
> - if (reg->host_user_addr) {
> - munmap(reg->mmap_addr, reg->mmap_size);
> - close(reg->fd);
> - }
> + if (reg->mmap_addr)
> + free_mem_region(reg);
> }
> }
>
> @@ -255,7 +309,7 @@ vhost_backend_cleanup(struct virtio_net *dev)
> vdpa_dev->ops->dev_cleanup(dev->vid);
>
> if (dev->mem) {
> - free_mem_region(dev);
> + free_all_mem_regions(dev);
> rte_free(dev->mem);
> dev->mem = NULL;
> }
> @@ -704,7 +758,7 @@ numa_realloc(struct virtio_net **pdev, struct
> vhost_virtqueue **pvq)
> vhost_devices[dev->vid] = dev;
>
> mem_size = sizeof(struct rte_vhost_memory) +
> - sizeof(struct rte_vhost_mem_region) * dev->mem->nregions;
> + sizeof(struct rte_vhost_mem_region) *
> +VHOST_MEMORY_MAX_NREGIONS;
> mem = rte_realloc_socket(dev->mem, mem_size, 0, node);
> if (!mem) {
> VHOST_CONFIG_LOG(dev->ifname, ERR, @@ -808,8 +862,10 @@
> hua_to_alignment(struct rte_vhost_memory *mem, void *ptr)
> uint32_t i;
> uintptr_t hua = (uintptr_t)ptr;
>
> - for (i = 0; i < mem->nregions; i++) {
> + for (i = 0; i < VHOST_MEMORY_MAX_NREGIONS; i++) {
> r = &mem->regions[i];
> + if (r->host_user_addr == 0)
> + continue;
> if (hua >= r->host_user_addr &&
> hua < r->host_user_addr + r->size) {
> return get_blk_size(r->fd); @@ -1136,6 +1192,24 @@
> add_guest_pages(struct virtio_net *dev, struct rte_vhost_mem_region *reg,
> return 0;
> }
>
> +static void
> +remove_guest_pages(struct virtio_net *dev, struct rte_vhost_mem_region
> +*reg) {
> + uint64_t reg_start = reg->host_user_addr;
> + uint64_t reg_end = reg_start + reg->size;
> + uint32_t i, j = 0;
> +
> + for (i = 0; i < dev->nr_guest_pages; i++) {
> + if (dev->guest_pages[i].host_user_addr >= reg_start &&
> + dev->guest_pages[i].host_user_addr < reg_end)
> + continue;
> + if (j != i)
> + dev->guest_pages[j] = dev->guest_pages[i];
> + j++;
> + }
> + dev->nr_guest_pages = j;
> +}
> +
> #ifdef RTE_LIBRTE_VHOST_DEBUG
> /* TODO: enable it only in debug mode? */
> static void
> @@ -1246,10 +1320,14 @@ vhost_user_postcopy_register(struct virtio_net
> *dev, int main_fd,
> * DPDK's virtual address with Qemu, so that Qemu can
> * retrieve the region offset when handling userfaults.
> */
> + int reg_msg_index = 0;
> memory = &ctx->msg.payload.memory;
> for (i = 0; i < memory->nregions; i++) {
> reg = &dev->mem->regions[i];
> - memory->regions[i].userspace_addr = reg->host_user_addr;
> + if (reg->host_user_addr == 0)
> + continue;
> + memory->regions[reg_msg_index].userspace_addr =
> +reg->host_user_addr;
> + reg_msg_index++;
> }
>
> /* Send the addresses back to qemu */ @@ -1278,6 +1356,8 @@
> vhost_user_postcopy_register(struct virtio_net *dev, int main_fd,
> /* Now userfault register and we can use the memory */
> for (i = 0; i < memory->nregions; i++) {
> reg = &dev->mem->regions[i];
> + if (reg->host_user_addr == 0)
> + continue;
> if (vhost_user_postcopy_region_register(dev, reg) < 0)
> return -1;
> }
> @@ -1382,6 +1462,52 @@ vhost_user_mmap_region(struct virtio_net *dev,
> return 0;
> }
>
> +static int
> +vhost_user_initialize_memory(struct virtio_net **pdev) {
> + struct virtio_net *dev = *pdev;
> + int numa_node = SOCKET_ID_ANY;
> +
> + if (dev->mem != NULL) {
> + VHOST_CONFIG_LOG(dev->ifname, ERR,
> + "memory already initialized, free it first");
> + return -1;
> + }
> +
> + /*
> + * If VQ 0 has already been allocated, try to allocate on the
> +same
> + * NUMA node. It can be reallocated later in numa_realloc().
> + */
> + if (dev->nr_vring > 0)
> + numa_node = dev->virtqueue[0]->numa_node;
> +
> + dev->nr_guest_pages = 0;
> + if (dev->guest_pages == NULL) {
> + dev->max_guest_pages = 8;
> + dev->guest_pages = rte_zmalloc_socket(NULL,
> + dev->max_guest_pages *
> + sizeof(struct guest_page),
> + RTE_CACHE_LINE_SIZE,
> + numa_node);
> + if (dev->guest_pages == NULL) {
> + VHOST_CONFIG_LOG(dev->ifname, ERR,
> + "failed to allocate memory for
> +dev->guest_pages");
> + return -1;
> + }
> + }
> +
> + dev->mem = rte_zmalloc_socket("vhost-mem-table", sizeof(struct
> +rte_vhost_memory) +
> + sizeof(struct rte_vhost_mem_region) *
> +VHOST_MEMORY_MAX_NREGIONS, 0, numa_node);
> + if (dev->mem == NULL) {
> + VHOST_CONFIG_LOG(dev->ifname, ERR, "failed to allocate
> +memory for dev->mem");
> + rte_free(dev->guest_pages);
> + dev->guest_pages = NULL;
> + return -1;
> + }
> +
> + return 0;
> +}
> +
>
> I think it should be in a dedicated patch, and in the same patch
> would vhost_user_set_mem_table() make use of it.
> The idea is to make it straitforward you are doing a refactoring, and easily
> check the code you are extracting out from
> vhost_user_set_mem_table() into a new function has not been changed in-
> between.
>
> vhost_user_initialize_memory() patch placement: Moved from patch 3 to patch
> 4, grouped with the
> vhost_user_set_mem_table() refactoring that uses it. This makes the
> extraction clearer to review as a pure refactor
> without mixing it with other changes.
>
> static int
> vhost_user_set_mem_table(struct virtio_net **pdev,
> struct vhu_msg_context *ctx,
> --
> 2.43.0
^ permalink raw reply
* [PATCH v1 6/7] power: add global uncore init and deinit interface
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
Currently, the uncore power library requires users to manually set
environment variables and call rte_power_uncore_init() one by one
in their applications. Since these steps are essentially part of
the driver initialization, they can be encapsulated in the uncore
core. So introduce two global API to initialize and deinitialize
uncore driver for application.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
lib/power/rte_power_uncore.c | 88 ++++++++++++++++++++++++++++++++++++
lib/power/rte_power_uncore.h | 20 ++++++++
2 files changed, 108 insertions(+)
diff --git a/lib/power/rte_power_uncore.c b/lib/power/rte_power_uncore.c
index a5d080e09e..0611a60d33 100644
--- a/lib/power/rte_power_uncore.c
+++ b/lib/power/rte_power_uncore.c
@@ -79,6 +79,12 @@ static int rte_power_probe_uncore_driver(void)
return global_uncore_ops ? 0 : -ENODEV;
}
+static void rte_power_remove_uncore_driver(void)
+{
+ global_uncore_ops = NULL;
+ global_uncore_env = RTE_UNCORE_PM_ENV_NOT_SET;
+}
+
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_power_set_uncore_env, 23.11)
int
rte_power_set_uncore_env(enum rte_uncore_power_mgmt_env env)
@@ -135,6 +141,88 @@ rte_power_get_uncore_env(void)
return global_uncore_env;
}
+RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_power_uncore_driver_init, 26.07)
+int
+rte_power_uncore_driver_init(void)
+{
+ int die, pkg, max_pkg, max_die;
+ int ret;
+
+ rte_spinlock_lock(&global_env_cfg_lock);
+ ret = rte_power_probe_uncore_driver();
+ if (ret) {
+ POWER_LOG(ERR, "Probe uncore driver failed, ret = %d.", ret);
+ goto out;
+ }
+
+ max_pkg = rte_power_uncore_get_num_pkgs();
+ if (max_pkg == 0) {
+ ret = -EINVAL;
+ goto remove_uncore_drv;
+ }
+
+ for (pkg = 0; pkg < max_pkg; pkg++) {
+ max_die = rte_power_uncore_get_num_dies(pkg);
+ if (max_die == 0) {
+ ret = -EINVAL;
+ goto remove_uncore_drv;
+ }
+
+ for (die = 0; die < max_die; die++) {
+ ret = rte_power_uncore_init(pkg, die);
+ if (ret) {
+ POWER_LOG(ERR, "Unable to initialize uncore for pkg-%d die-%d",
+ pkg, die);
+ goto uncore_exit;
+ }
+ }
+ }
+ rte_spinlock_unlock(&global_env_cfg_lock);
+ return 0;
+
+uncore_exit:
+ for (; pkg >= 0; pkg--) {
+ max_die = rte_power_uncore_get_num_dies(pkg);
+ for (die = 0; die < max_die; die++) {
+ ret = rte_power_uncore_exit(pkg, die);
+ if (ret)
+ POWER_LOG(ERR, "Failed to deinitialize uncore for pkg-%d die-%d",
+ pkg, die);
+ }
+ }
+
+remove_uncore_drv:
+ rte_power_remove_uncore_driver();
+out:
+ rte_spinlock_unlock(&global_env_cfg_lock);
+ return ret;
+}
+
+RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_power_uncore_driver_deinit, 26.07)
+void
+rte_power_uncore_driver_deinit(void)
+{
+ unsigned int die, pkg, max_pkg, max_die;
+
+ rte_spinlock_lock(&global_env_cfg_lock);
+ if (global_uncore_ops == NULL)
+ goto out;
+
+ max_pkg = rte_power_uncore_get_num_pkgs();
+ for (pkg = 0; pkg < max_pkg; pkg++) {
+ max_die = rte_power_uncore_get_num_dies(pkg);
+ for (die = 0; die < max_die; die++) {
+ if (rte_power_uncore_exit(pkg, die) != 0)
+ POWER_LOG(ERR, "Unable to deinitialize uncore for pkg-%02u die-%02u",
+ pkg, die);
+ }
+ }
+
+ rte_power_remove_uncore_driver();
+out:
+ rte_spinlock_unlock(&global_env_cfg_lock);
+}
+
RTE_EXPORT_SYMBOL(rte_power_uncore_init)
int
rte_power_uncore_init(unsigned int pkg, unsigned int die)
diff --git a/lib/power/rte_power_uncore.h b/lib/power/rte_power_uncore.h
index 66aea1b37f..cac9127894 100644
--- a/lib/power/rte_power_uncore.h
+++ b/lib/power/rte_power_uncore.h
@@ -26,6 +26,26 @@ enum rte_uncore_power_mgmt_env {
RTE_UNCORE_PM_ENV_AMD_HSMP
};
+/**
+ * Probing and Initializing the uncore driver on platform.
+ *
+ * @return
+ * - 0 on success.
+ * - Negative on error.
+ */
+__rte_experimental
+int rte_power_uncore_driver_init(void);
+
+/**
+ * Deinitializing the uncore driver on platform.
+ *
+ * @return
+ * - 0 on success.
+ * - Negative on error.
+ */
+__rte_experimental
+void rte_power_uncore_driver_deinit(void);
+
/**
* Set the default uncore power management implementation.
* This has to be called prior to calling any other rte_power_uncore_*() API.
--
2.33.0
^ permalink raw reply related
* [PATCH v1 7/7] examples/l3fwd-power: switch to new init/deinit uncore API
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
The new rte_power_uncore_driver_init/deinit() is friendly to user.
So replace the old way.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
examples/l3fwd-power/main.c | 34 ++++++----------------------------
1 file changed, 6 insertions(+), 28 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index a22634a04e..afcb352373 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2199,9 +2199,9 @@ power_uncore_init(void)
if (enabled_uncore == -1)
return 0;
- ret = rte_power_set_uncore_env(RTE_UNCORE_PM_ENV_AUTO_DETECT);
- if (ret < 0) {
- RTE_LOG(INFO, L3FWD_POWER, "Failed to set uncore env\n");
+ ret = rte_power_uncore_driver_init();
+ if (ret != 0) {
+ RTE_LOG(INFO, L3FWD_POWER, "Failed to initialize uncore driver.\n");
return ret;
}
@@ -2214,12 +2214,6 @@ power_uncore_init(void)
if (max_die == 0)
return -1;
for (die = 0; die < max_die; die++) {
- ret = rte_power_uncore_init(pkg, die);
- if (ret == -1) {
- RTE_LOG(INFO, L3FWD_POWER, "Unable to initialize uncore for pkg %02u die %02u\n"
- , pkg, die);
- return ret;
- }
if (g_uncore_cfg.uncore_choice == UNCORE_MIN) {
ret = rte_power_uncore_freq_min(pkg, die);
if (ret == -1) {
@@ -2331,7 +2325,7 @@ init_power_library(void)
static int
deinit_power_library(void)
{
- unsigned int lcore_id, max_pkg, max_die, die, pkg;
+ unsigned int lcore_id;
int ret = 0;
if (app_mode == APP_MODE_LEGACY) {
@@ -2348,24 +2342,8 @@ deinit_power_library(void)
}
/* if uncore option was set */
- if (enabled_uncore == 0) {
- max_pkg = rte_power_uncore_get_num_pkgs();
- if (max_pkg == 0)
- return -1;
- for (pkg = 0; pkg < max_pkg; pkg++) {
- max_die = rte_power_uncore_get_num_dies(pkg);
- if (max_die == 0)
- return -1;
- for (die = 0; die < max_die; die++) {
- ret = rte_power_uncore_exit(pkg, die);
- if (ret < 0) {
- RTE_LOG(ERR, L3FWD_POWER, "Failed to exit uncore deinit successfully for pkg %02u die %02u\n"
- , pkg, die);
- return -1;
- }
- }
- }
- }
+ if (enabled_uncore == 0)
+ rte_power_uncore_driver_deinit();
if (cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
--
2.33.0
^ permalink raw reply related
* [PATCH v1 4/7] examples/l3fwd-power: relocate uncore initialization
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
Currently, the deinitialization of uncore is in deinit_power_library().
But its initialization is located in the parameter parsing function,
which is not good to maintain.
So move this logic to init_power_library().
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
examples/l3fwd-power/main.c | 192 ++++++++++++++++++------------------
1 file changed, 97 insertions(+), 95 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 45b6697c85..a22634a04e 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -149,9 +149,6 @@ static struct rte_timer telemetry_timer;
/* stats index returned by metrics lib */
int telstats_index;
-/* flag to check if uncore option enabled */
-int enabled_uncore = -1;
-
struct telstats_name {
char name[RTE_ETH_XSTATS_NAME_SIZE];
};
@@ -170,12 +167,6 @@ enum busy_rate {
FULL = 100
};
-enum uncore_choice {
- UNCORE_MIN = 0,
- UNCORE_MAX = 1,
- UNCORE_IDX = 2
-};
-
/* reference poll count to measure core busyness */
#define DEFAULT_COUNT 10000
/*
@@ -212,6 +203,19 @@ enum freq_scale_hint_t
FREQ_HIGHEST = 2
};
+enum uncore_choice {
+ UNCORE_MIN = 0,
+ UNCORE_MAX = 1,
+ UNCORE_IDX = 2
+};
+
+/* flag to check if uncore option enabled */
+int enabled_uncore = -1;
+struct uncore_cfg {
+ enum uncore_choice uncore_choice;
+ uint32_t freq_idx;
+} g_uncore_cfg;
+
struct __rte_cache_aligned lcore_rx_queue {
uint16_t port_id;
uint16_t queue_id;
@@ -1552,80 +1556,6 @@ parse_uint(const char *opt, uint32_t max, uint32_t *res)
return 0;
}
-static int
-parse_uncore_options(enum uncore_choice choice, const char *argument)
-{
- unsigned int die, pkg, max_pkg, max_die;
- int ret = 0;
- ret = rte_power_set_uncore_env(RTE_UNCORE_PM_ENV_AUTO_DETECT);
- if (ret < 0) {
- RTE_LOG(INFO, L3FWD_POWER, "Failed to set uncore env\n");
- return ret;
- }
-
- max_pkg = rte_power_uncore_get_num_pkgs();
- if (max_pkg == 0)
- return -1;
-
- for (pkg = 0; pkg < max_pkg; pkg++) {
- max_die = rte_power_uncore_get_num_dies(pkg);
- if (max_die == 0)
- return -1;
- for (die = 0; die < max_die; die++) {
- ret = rte_power_uncore_init(pkg, die);
- if (ret == -1) {
- RTE_LOG(INFO, L3FWD_POWER, "Unable to initialize uncore for pkg %02u die %02u\n"
- , pkg, die);
- return ret;
- }
- if (choice == UNCORE_MIN) {
- ret = rte_power_uncore_freq_min(pkg, die);
- if (ret == -1) {
- RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set the uncore frequency to minimum value for pkg %02u die %02u\n"
- , pkg, die);
- return ret;
- }
- } else if (choice == UNCORE_MAX) {
- ret = rte_power_uncore_freq_max(pkg, die);
- if (ret == -1) {
- RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set uncore frequency to maximum value for pkg %02u die %02u\n"
- , pkg, die);
- return ret;
- }
- } else if (choice == UNCORE_IDX) {
- char *ptr = NULL;
- int frequency_index = strtol(argument, &ptr, 10);
- if (argument == ptr) {
- RTE_LOG(INFO, L3FWD_POWER, "Index given is not a valid number.");
- return -1;
- }
- int freq_array_len = rte_power_uncore_get_num_freqs(pkg, die);
- if (frequency_index > freq_array_len - 1) {
- RTE_LOG(INFO, L3FWD_POWER,
- "Frequency index given out of range, please choose a value from 0 to %d.\n",
- freq_array_len);
- return -1;
- }
- ret = rte_power_set_uncore_freq(pkg, die, frequency_index);
- if (ret == -1) {
- RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set specified frequency index for pkg %02u die %02u\n",
- pkg, die);
- return ret;
- }
- } else {
- RTE_LOG(INFO, L3FWD_POWER, "Uncore choice provided invalid\n");
- return -1;
- }
- }
- }
-
- RTE_LOG(INFO, L3FWD_POWER, "Successfully set max/min/index uncore frequency.\n");
- return ret;
-}
-
static int
parse_portmask(const char *portmask)
{
@@ -1793,25 +1723,21 @@ parse_args(int argc, char **argv)
promiscuous_on = 1;
break;
case 'u':
- enabled_uncore = parse_uncore_options(UNCORE_MIN, NULL);
- if (enabled_uncore < 0) {
- print_usage(prgname);
- return -1;
- }
+ enabled_uncore = 0;
+ g_uncore_cfg.uncore_choice = UNCORE_MIN;
break;
case 'U':
- enabled_uncore = parse_uncore_options(UNCORE_MAX, NULL);
- if (enabled_uncore < 0) {
- print_usage(prgname);
- return -1;
- }
+ enabled_uncore = 0;
+ g_uncore_cfg.uncore_choice = UNCORE_MAX;
break;
case 'i':
- enabled_uncore = parse_uncore_options(UNCORE_IDX, optarg);
- if (enabled_uncore < 0) {
+ enabled_uncore = 0;
+ if (parse_uint(optarg, UINT32_MAX, &g_uncore_cfg.freq_idx) != 0) {
+ RTE_LOG(INFO, L3FWD_POWER, "Index given is not a valid number.");
print_usage(prgname);
return -1;
}
+ g_uncore_cfg.uncore_choice = UNCORE_IDX;
break;
/* long options */
case 0:
@@ -2264,6 +2190,78 @@ static int check_ptype(uint16_t portid)
}
+static int
+power_uncore_init(void)
+{
+ unsigned int die, pkg, max_pkg, max_die;
+ int ret;
+
+ if (enabled_uncore == -1)
+ return 0;
+
+ ret = rte_power_set_uncore_env(RTE_UNCORE_PM_ENV_AUTO_DETECT);
+ if (ret < 0) {
+ RTE_LOG(INFO, L3FWD_POWER, "Failed to set uncore env\n");
+ return ret;
+ }
+
+ max_pkg = rte_power_uncore_get_num_pkgs();
+ if (max_pkg == 0)
+ return -1;
+
+ for (pkg = 0; pkg < max_pkg; pkg++) {
+ max_die = rte_power_uncore_get_num_dies(pkg);
+ if (max_die == 0)
+ return -1;
+ for (die = 0; die < max_die; die++) {
+ ret = rte_power_uncore_init(pkg, die);
+ if (ret == -1) {
+ RTE_LOG(INFO, L3FWD_POWER, "Unable to initialize uncore for pkg %02u die %02u\n"
+ , pkg, die);
+ return ret;
+ }
+ if (g_uncore_cfg.uncore_choice == UNCORE_MIN) {
+ ret = rte_power_uncore_freq_min(pkg, die);
+ if (ret == -1) {
+ RTE_LOG(INFO, L3FWD_POWER,
+ "Unable to set the uncore frequency to minimum value for pkg %02u die %02u\n"
+ , pkg, die);
+ return ret;
+ }
+ } else if (g_uncore_cfg.uncore_choice == UNCORE_MAX) {
+ ret = rte_power_uncore_freq_max(pkg, die);
+ if (ret == -1) {
+ RTE_LOG(INFO, L3FWD_POWER,
+ "Unable to set uncore frequency to maximum value for pkg %02u die %02u\n"
+ , pkg, die);
+ return ret;
+ }
+ } else if (g_uncore_cfg.uncore_choice == UNCORE_IDX) {
+ int freq_array_len = rte_power_uncore_get_num_freqs(pkg, die);
+ if (freq_array_len <= 0) {
+ RTE_LOG(INFO, L3FWD_POWER, "Get uncore frequency number failed.\n");
+ return -1;
+ }
+ if (g_uncore_cfg.freq_idx > (uint32_t)(freq_array_len - 1)) {
+ RTE_LOG(INFO, L3FWD_POWER,
+ "Frequency index given out of range, please choose a value from 0 to %d.\n",
+ freq_array_len);
+ return -1;
+ }
+ ret = rte_power_set_uncore_freq(pkg, die, g_uncore_cfg.freq_idx);
+ if (ret == -1) {
+ RTE_LOG(INFO, L3FWD_POWER,
+ "Unable to set specified frequency index for pkg %02u die %02u\n",
+ pkg, die);
+ return ret;
+ }
+ }
+ }
+ }
+
+ return 0;
+}
+
static int
init_power_library(void)
{
@@ -2295,6 +2293,10 @@ init_power_library(void)
}
}
+ ret = power_uncore_init();
+ if (ret != 0)
+ return ret;
+
if (cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
/* Back old CPU resume latency. */
--
2.33.0
^ permalink raw reply related
* [PATCH v1 3/7] examples/l3fwd-power: fix uncore help and log info
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
The '-i' parameter is to set any uncore frequency rather than
min/max value. And the error logs are not clear enough, fix it
by the way.
Fixes: 10db2a5b8724 ("examples/l3fwd-power: add options for uncore frequency")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
doc/guides/sample_app_ug/l3_forward_power_man.rst | 2 +-
examples/l3fwd-power/main.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/doc/guides/sample_app_ug/l3_forward_power_man.rst b/doc/guides/sample_app_ug/l3_forward_power_man.rst
index 3271bc2154..dc882c5d77 100644
--- a/doc/guides/sample_app_ug/l3_forward_power_man.rst
+++ b/doc/guides/sample_app_ug/l3_forward_power_man.rst
@@ -106,7 +106,7 @@ where,
* -U: optional, sets uncore min/max frequency to maximum value.
-* -i (frequency index): optional, sets uncore frequency to frequency index value, by setting min and max values to be the same.
+* -i (frequency index): set target frequency for uncore by specified frequency index.
* --config (port,queue,lcore)[,(port,queue,lcore)]: determines which queues from which ports are mapped to which cores.
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index ba2be5bf32..45b6697c85 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -1498,7 +1498,7 @@ print_usage(const char *prgname)
" -P: enable promiscuous mode\n"
" -u: set min/max frequency for uncore to minimum value\n"
" -U: set min/max frequency for uncore to maximum value\n"
- " -i (frequency index): set min/max frequency for uncore to specified frequency index\n"
+ " -i (frequency index): set target frequency for uncore by specified frequency index\n"
" --config (port,queue,lcore): rx queues configuration\n"
" --eth-link-speed: force link speed\n"
" --cpu-resume-latency LATENCY: set CPU resume latency to control C-state selection,"
@@ -1582,7 +1582,7 @@ parse_uncore_options(enum uncore_choice choice, const char *argument)
ret = rte_power_uncore_freq_min(pkg, die);
if (ret == -1) {
RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set the uncore min/max to minimum uncore frequency value for pkg %02u die %02u\n"
+ "Unable to set the uncore frequency to minimum value for pkg %02u die %02u\n"
, pkg, die);
return ret;
}
@@ -1590,7 +1590,7 @@ parse_uncore_options(enum uncore_choice choice, const char *argument)
ret = rte_power_uncore_freq_max(pkg, die);
if (ret == -1) {
RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set uncore min/max to maximum uncore frequency value for pkg %02u die %02u\n"
+ "Unable to set uncore frequency to maximum value for pkg %02u die %02u\n"
, pkg, die);
return ret;
}
@@ -1611,7 +1611,7 @@ parse_uncore_options(enum uncore_choice choice, const char *argument)
ret = rte_power_set_uncore_freq(pkg, die, frequency_index);
if (ret == -1) {
RTE_LOG(INFO, L3FWD_POWER,
- "Unable to set min/max uncore index value for pkg %02u die %02u\n",
+ "Unable to set specified frequency index for pkg %02u die %02u\n",
pkg, die);
return ret;
}
--
2.33.0
^ permalink raw reply related
* [PATCH v1 5/7] power: support automatic detection of uncore driver
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
Currently, uncore driver library just supports intel_uncore driver
as default driver when user use AUTO_DETECT, which is not good to
application. So it is necessary to support probing for multi-uncore
drivers.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
lib/power/rte_power_uncore.c | 44 +++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/lib/power/rte_power_uncore.c b/lib/power/rte_power_uncore.c
index e26919acce..a5d080e09e 100644
--- a/lib/power/rte_power_uncore.c
+++ b/lib/power/rte_power_uncore.c
@@ -2,6 +2,7 @@
* Copyright(c) 2010-2014 Intel Corporation
* Copyright(c) 2023 AMD Corporation
*/
+#include <errno.h>
#include <eal_export.h>
#include <rte_spinlock.h>
@@ -47,6 +48,37 @@ rte_power_register_uncore_ops(struct rte_power_uncore_ops *driver_ops)
return 0;
}
+static uint32_t rte_power_uncore_driver_name2env(char *name)
+{
+ for (uint32_t i = 0; i < RTE_DIM(uncore_env_str); i++) {
+ if (!strcmp(name, uncore_env_str[i]))
+ return i;
+ }
+
+ return UINT32_MAX;
+}
+
+static int rte_power_probe_uncore_driver(void)
+{
+ struct rte_power_uncore_ops *ops;
+ int ret;
+
+ global_uncore_ops = NULL;
+ /* Use package-0 and die-0 to probe uncore driver. */
+ RTE_TAILQ_FOREACH(ops, &uncore_ops_list, next) {
+ ret = ops->init(0, 0);
+ if (!ret) {
+ global_uncore_env =
+ rte_power_uncore_driver_name2env(ops->name);
+ global_uncore_ops = ops;
+ ops->exit(0, 0);
+ break;
+ }
+ }
+
+ return global_uncore_ops ? 0 : -ENODEV;
+}
+
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_power_set_uncore_env, 23.11)
int
rte_power_set_uncore_env(enum rte_uncore_power_mgmt_env env)
@@ -61,12 +93,12 @@ rte_power_set_uncore_env(enum rte_uncore_power_mgmt_env env)
goto out;
}
- if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT)
- /* Currently only intel_uncore is supported.
- * This will be extended with auto-detection support
- * for multiple uncore implementations.
- */
- env = RTE_UNCORE_PM_ENV_INTEL_UNCORE;
+ if (env == RTE_UNCORE_PM_ENV_AUTO_DETECT) {
+ ret = rte_power_probe_uncore_driver();
+ if (ret)
+ POWER_LOG(ERR, "Probe uncore driver failed, ret = %d.", ret);
+ goto out;
+ }
if (env <= RTE_DIM(uncore_env_str)) {
RTE_TAILQ_FOREACH(ops, &uncore_ops_list, next)
--
2.33.0
^ permalink raw reply related
* [PATCH v1 1/7] examples/l3fwd-power: fix uncore deinit for non-legacy
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
Uncore resources were not being deinitialized in non-legacy modes (such
as pmd-mgmt), causing the uncore frequency not to return to its original
value after the application exited.
The root cause is that uncore initialization can be performed for all
modes, whereas the deinitialization logic is incorrectly restricted
to legacy mode only. So do the deinitialization of uncore on all app
modes.
Fixes: 10db2a5b8724 ("examples/l3fwd-power: add options for uncore frequency")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
examples/l3fwd-power/main.c | 66 ++++++++++++++++++++-----------------
1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 02ec17d799..1122aeb930 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2271,28 +2271,31 @@ init_power_library(void)
unsigned int lcore_id;
int ret = 0;
- RTE_LCORE_FOREACH(lcore_id) {
- /* init power management library */
- ret = rte_power_init(lcore_id);
- if (ret) {
- RTE_LOG(ERR, L3FWD_POWER,
- "Library initialization failed on core %u\n",
- lcore_id);
- return ret;
- }
- /* we're not supporting the VM channel mode */
- env = rte_power_get_env();
- if (env != PM_ENV_ACPI_CPUFREQ &&
- env != PM_ENV_PSTATE_CPUFREQ &&
- env != PM_ENV_AMD_PSTATE_CPUFREQ &&
- env != PM_ENV_CPPC_CPUFREQ) {
- RTE_LOG(ERR, L3FWD_POWER,
- "Only ACPI and PSTATE mode are supported\n");
- return -1;
+ /* only legacy mode relies on the initialization of cpufreq library */
+ if (app_mode == APP_MODE_LEGACY) {
+ RTE_LCORE_FOREACH(lcore_id) {
+ /* init power management library */
+ ret = rte_power_init(lcore_id);
+ if (ret) {
+ RTE_LOG(ERR, L3FWD_POWER,
+ "Library initialization failed on core %u\n",
+ lcore_id);
+ return ret;
+ }
+ /* we're not supporting the VM channel mode */
+ env = rte_power_get_env();
+ if (env != PM_ENV_ACPI_CPUFREQ &&
+ env != PM_ENV_PSTATE_CPUFREQ &&
+ env != PM_ENV_AMD_PSTATE_CPUFREQ &&
+ env != PM_ENV_CPPC_CPUFREQ) {
+ RTE_LOG(ERR, L3FWD_POWER,
+ "Only ACPI and PSTATE mode are supported\n");
+ return -1;
+ }
}
}
- if (cpu_resume_latency != -1) {
+ if (app_mode == APP_MODE_LEGACY && cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
/* Back old CPU resume latency. */
ret = rte_power_qos_get_cpu_resume_latency(lcore_id);
@@ -2329,14 +2332,16 @@ deinit_power_library(void)
unsigned int lcore_id, max_pkg, max_die, die, pkg;
int ret = 0;
- RTE_LCORE_FOREACH(lcore_id) {
- /* deinit power management library */
- ret = rte_power_exit(lcore_id);
- if (ret) {
- RTE_LOG(ERR, L3FWD_POWER,
- "Library deinitialization failed on core %u\n",
- lcore_id);
- return ret;
+ if (app_mode == APP_MODE_LEGACY) {
+ RTE_LCORE_FOREACH(lcore_id) {
+ /* deinit power management library */
+ ret = rte_power_exit(lcore_id);
+ if (ret) {
+ RTE_LOG(ERR, L3FWD_POWER,
+ "Library deinitialization failed on core %u\n",
+ lcore_id);
+ return ret;
+ }
}
}
@@ -2360,7 +2365,7 @@ deinit_power_library(void)
}
}
- if (cpu_resume_latency != -1) {
+ if (app_mode == APP_MODE_LEGACY && cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
/* Restore the original value. */
rte_power_qos_set_cpu_resume_latency(lcore_id,
@@ -2602,8 +2607,7 @@ main(int argc, char **argv)
RTE_LOG(INFO, L3FWD_POWER, "Selected operation mode: %s\n",
mode_to_str(app_mode));
- /* only legacy mode relies on power library */
- if ((app_mode == APP_MODE_LEGACY) && init_power_library())
+ if (init_power_library())
rte_exit(EXIT_FAILURE, "init_power_library failed\n");
if (update_lcore_params() < 0)
@@ -2975,7 +2979,7 @@ main(int argc, char **argv)
rte_eth_dev_close(portid);
}
- if ((app_mode == APP_MODE_LEGACY) && deinit_power_library())
+ if (deinit_power_library())
rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
if (rte_eal_cleanup() < 0)
--
2.33.0
^ permalink raw reply related
* [PATCH v1 2/7] examples/l3fwd-power: support to set power QoS on any mode
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
In-Reply-To: <20260512023513.460169-1-lihuisong@huawei.com>
Power QoS feature doesn't depend on app mode, here open it for all.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
examples/l3fwd-power/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 1122aeb930..ba2be5bf32 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2295,7 +2295,7 @@ init_power_library(void)
}
}
- if (app_mode == APP_MODE_LEGACY && cpu_resume_latency != -1) {
+ if (cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
/* Back old CPU resume latency. */
ret = rte_power_qos_get_cpu_resume_latency(lcore_id);
@@ -2365,7 +2365,7 @@ deinit_power_library(void)
}
}
- if (app_mode == APP_MODE_LEGACY && cpu_resume_latency != -1) {
+ if (cpu_resume_latency != -1) {
RTE_LCORE_FOREACH(lcore_id) {
/* Restore the original value. */
rte_power_qos_set_cpu_resume_latency(lcore_id,
--
2.33.0
^ permalink raw reply related
* [PATCH v1 0/7] power: simplify the use of uncore library
From: Huisong Li @ 2026-05-12 2:35 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
Currently, the uncore power library requires user to manually set
environment variables and call rte_power_uncore_init() one by one
in their applications, which is not good to use. Since these steps
are essentially part of the driver initialization, they can be
encapsulated to one interface in uncore core. The uncore core should
be responsiable for detecting and initializing the uncore driver on
the platform.
In order to reach above goal, this series has made the following
changes:
1. Some cleancode and rewrok uncore in l3fwd-power.
2. Add automatic detection of uncore driver to support more uncore
drivers when application use RTE_UNCORE_PM_ENV_AUTO_DETECT.
3. Introduce new uncore init and deinit API in uncore core.
Note:
1. The patch 5/7 use package-0 and die-0 to probe uncore driver is
just avoided to break ABI. The good way is to add new probe
callback in each uncore driver to tell uncore core.
2. We plan that the new uncore init and deinit API will replace the
rte_power_set/get_uncore_env() and remove them in 26.11 version.
Huisong Li (7):
examples/l3fwd-power: fix uncore deinit for non-legacy
examples/l3fwd-power: support to set power QoS on any mode
examples/l3fwd-power: fix uncore help and log info
examples/l3fwd-power: relocate uncore initialization
power: support automatic detection of uncore driver
power: add global uncore init and deinit interface
examples/l3fwd-power: switch to new init/deinit uncore API
.../sample_app_ug/l3_forward_power_man.rst | 2 +-
examples/l3fwd-power/main.c | 272 +++++++++---------
lib/power/rte_power_uncore.c | 132 ++++++++-
lib/power/rte_power_uncore.h | 20 ++
4 files changed, 275 insertions(+), 151 deletions(-)
--
2.33.0
^ permalink raw reply
* [PATCH] power/intel_uncore: reduce log level for dependency missing
From: Huisong Li @ 2026-05-12 1:30 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
When run dpdk-l3fwd with '-u' on non-X86 platform, user would
happen a noisy print as the following:
"POWER: Uncore frequency management not supported/enabled on this
kernel. Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on Intel
x86 with linux kernel >= 5.6".
The root cause is that intel_uncore driver's .init() will be called
on any platform when use automatic detection mode. The function in
intel_uncore driver will print above log on non-X86 platform.
But the existing uncore core cannot solve this problem unless break
ABI to add new callback. So reduce its log level to avoid this
incorrect prompt.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/power/intel_uncore/intel_uncore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/power/intel_uncore/intel_uncore.c b/drivers/power/intel_uncore/intel_uncore.c
index 6759ea1445..ebca395075 100644
--- a/drivers/power/intel_uncore/intel_uncore.c
+++ b/drivers/power/intel_uncore/intel_uncore.c
@@ -419,7 +419,7 @@ power_intel_uncore_get_num_pkgs(void)
d = opendir(INTEL_UNCORE_FREQUENCY_DIR);
if (d == NULL) {
- POWER_LOG(ERR,
+ POWER_LOG(DEBUG,
"Uncore frequency management not supported/enabled on this kernel. "
"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on Intel x86 with linux kernel"
" >= 5.6");
@@ -457,7 +457,7 @@ power_intel_uncore_get_num_dies(unsigned int pkg)
d = opendir(INTEL_UNCORE_FREQUENCY_DIR);
if (d == NULL) {
- POWER_LOG(ERR,
+ POWER_LOG(DEBUG,
"Uncore frequency management not supported/enabled on this kernel. "
"Please enable CONFIG_INTEL_UNCORE_FREQ_CONTROL if on Intel x86 with linux kernel"
" >= 5.6");
--
2.33.0
^ permalink raw reply related
* [PATCH] examples/l3fwd-power: fix missing return on latency get
From: Huisong Li @ 2026-05-12 1:09 UTC (permalink / raw)
To: anatoly.burakov, sivaprasad.tummala
Cc: dev, thomas, stephen, fengchengwen, yangxingui, zhanjie9,
lihuisong
Add return value when get cpu_resume_latency failed.
Fixes: 4d23d39fd06e ("examples/l3fwd-power: add PM QoS configuration")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
examples/l3fwd-power/main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 02ec17d799..a7a1f675d3 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2300,6 +2300,7 @@ init_power_library(void)
RTE_LOG(ERR, L3FWD_POWER,
"Failed to get cpu resume latency on lcore-%u, ret=%d.\n",
lcore_id, ret);
+ return ret;
}
resume_latency_bk[lcore_id] = ret;
--
2.33.0
^ permalink raw reply related
* [PATCH 6/6] net/gve: reconstruct HW timestamps from DQO
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
A full 64-bit NIC timestamp is periodically synced via an AdminQ
command and cached in the driver. In the RX datapath, this cached
value is used as a base to expand the 32-bit hardware timestamp into
a full 64-bit value, which is then stored in the mbuf's dynamic
timestamp field.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
doc/guides/nics/features/gve.ini | 1 +
doc/guides/nics/gve.rst | 18 ++++++++++++++++++
doc/guides/rel_notes/release_26_07.rst | 3 +++
drivers/net/gve/base/gve_desc_dqo.h | 8 ++++++--
drivers/net/gve/gve_ethdev.c | 14 +++++++++++++-
drivers/net/gve/gve_ethdev.h | 25 +++++++++++++++++++++++++
drivers/net/gve/gve_rx_dqo.c | 26 ++++++++++++++++++++++++++
7 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/doc/guides/nics/features/gve.ini b/doc/guides/nics/features/gve.ini
index 89c97fd27a..117ad4fc65 100644
--- a/doc/guides/nics/features/gve.ini
+++ b/doc/guides/nics/features/gve.ini
@@ -13,6 +13,7 @@ RSS hash = Y
RSS key update = Y
RSS reta update = Y
L4 checksum offload = Y
+Timestamp offload = Y
Basic stats = Y
FreeBSD = Y
Linux = Y
diff --git a/doc/guides/nics/gve.rst b/doc/guides/nics/gve.rst
index 62648c47ed..44aedc9311 100644
--- a/doc/guides/nics/gve.rst
+++ b/doc/guides/nics/gve.rst
@@ -72,6 +72,7 @@ Supported features of the GVE PMD are:
- Tx UDP/TCP/SCTP Checksum
- RSS hash configuration
- RSS redirection table query and update
+- Timestamp offload
Currently, only GQI_QPL and GQI_RDA queue format are supported in PMD.
Jumbo Frame is not supported in PMD for now.
@@ -132,6 +133,23 @@ Security Protocols
- Flow priorities are not supported (must be 0).
- Masking is limited to full matches i.e. ``0x00...0`` or ``0xFF...F``.
+Timestamp Offload
+^^^^^^^^^^^^^^^^^
+
+The driver supports hardware-based packet timestamping on supported
+devices via the standard ``RTE_ETH_RX_OFFLOAD_TIMESTAMP`` offload capability.
+
+**Limitations**
+
+- If the driver fails to fetch the NIC hardware clock for 7 consecutive periods,
+ the cached timestamp is marked as stale,
+ and the reconstructed timestamps are no longer propagated to the mbuf.
+- The timestamp reconstruction is only accurate
+ if the time between a packet's reception
+ and the last hardware clock sync is less than approximately 2 seconds.
+ The driver's internal clock sync period is set to respect this limitation.
+
+
Device Reset
^^^^^^^^^^^^
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 1b012c4776..db886f19cf 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -62,6 +62,9 @@ New Features
* ``-A`` or ``--no-auto-probing`` disable the initial bus probing: no device is probed during
``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.
+* **Updated Google GVE net driver.**
+
+ * Added hardware timestamping support on DQO queues.
* **Updated PCAP ethernet driver.**
diff --git a/drivers/net/gve/base/gve_desc_dqo.h b/drivers/net/gve/base/gve_desc_dqo.h
index 71d9d60bb9..c1534959c2 100644
--- a/drivers/net/gve/base/gve_desc_dqo.h
+++ b/drivers/net/gve/base/gve_desc_dqo.h
@@ -226,7 +226,8 @@ struct gve_rx_compl_desc_dqo {
u8 status_error1;
- __le16 reserved5;
+ u8 reserved5;
+ u8 ts_sub_nsecs_low;
__le16 buf_id; /* Buffer ID which was sent on the buffer queue. */
union {
@@ -237,9 +238,12 @@ struct gve_rx_compl_desc_dqo {
};
__le32 hash;
__le32 reserved6;
- __le64 reserved7;
+ __le32 reserved7;
+ __le32 ts; /* timestamp in nanosecs */
} __packed;
+#define GVE_DQO_RX_HWTSTAMP_VALID 0x1
+
GVE_CHECK_STRUCT_LEN(32, gve_rx_compl_desc_dqo);
/* Ringing the doorbell too often can hurt performance.
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index e1f2585ede..fa26c2bdb4 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -214,6 +214,7 @@ static int
gve_dev_configure(struct rte_eth_dev *dev)
{
struct gve_priv *priv = dev->data->dev_private;
+ int err;
if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
dev->data->dev_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
@@ -223,13 +224,22 @@ gve_dev_configure(struct rte_eth_dev *dev)
if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TCP_LRO)
priv->enable_rsc = 1;
+ if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
+ err = rte_mbuf_dyn_rx_timestamp_register(&priv->mbuf_timestamp_offset,
+ &priv->mbuf_timestamp_mask);
+ if (err < 0) {
+ PMD_DRV_LOG(ERR, "Failed to register dynamic timestamp field");
+ return err;
+ }
+ }
+
/* Reset RSS RETA in case number of queues changed. */
if (priv->rss_config.indir) {
struct gve_rss_config update_reta_config;
gve_init_rss_config_from_priv(priv, &update_reta_config);
gve_generate_rss_reta(dev, &update_reta_config);
- int err = gve_adminq_configure_rss(priv, &update_reta_config);
+ err = gve_adminq_configure_rss(priv, &update_reta_config);
if (err)
PMD_DRV_LOG(ERR,
"Could not reconfigure RSS redirection table.");
@@ -817,6 +827,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
dev_info->min_mtu = RTE_ETHER_MIN_MTU;
dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_RSS_HASH;
+ if (priv->nic_timestamp_supported)
+ dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TIMESTAMP;
dev_info->tx_offload_capa =
RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 7e6f24e910..35d532284e 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -260,6 +260,7 @@ struct gve_rx_queue {
struct rte_mbuf **refill_bufs;
uint8_t is_gqi_qpl;
+ bool timestamp_enabled;
};
struct gve_flow {
@@ -368,8 +369,32 @@ struct gve_priv {
RTE_ATOMIC(uint64_t) last_read_nic_timestamp;
RTE_ATOMIC(uint32_t) nic_ts_read_fails;
RTE_ATOMIC(uint8_t) nic_ts_stale;
+
+ int mbuf_timestamp_offset;
+ uint64_t mbuf_timestamp_mask;
};
+/* Expand the hardware timestamp to the full 64 bits of width.
+ *
+ * This algorithm works by using the passed hardware timestamp to generate a
+ * diff relative to the last read of the nic clock. This diff can be positive or
+ * negative, as it is possible that we have read the clock more recently than
+ * the hardware has received this packet. To detect this, we use the high bit of
+ * the diff, and assume that the read is more recent if the high bit is set. In
+ * this case we invert the process.
+ *
+ * Note that this means if the time delta between packet reception and the last
+ * clock read is greater than ~2 seconds, this will provide invalid results.
+ */
+static inline uint64_t
+gve_reconstruct_ts(uint64_t last_sync, uint32_t ts)
+{
+ uint32_t low = (uint32_t)last_sync;
+ int32_t diff = (int32_t)(ts - low);
+
+ return last_sync + diff;
+}
+
static inline bool
gve_is_gqi(struct gve_priv *priv)
{
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 8035aee572..cc343f3fd8 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -160,6 +160,8 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
{
volatile struct gve_rx_compl_desc_dqo *rx_desc;
struct gve_rx_queue *rxq;
+ uint64_t last_sync = 0;
+ struct gve_priv *priv;
struct rte_mbuf *rxm;
uint16_t rx_buf_id;
uint16_t pkt_len;
@@ -171,6 +173,15 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
nb_rx = 0;
rxq = rx_queue;
rx_id = rxq->rx_tail;
+ priv = rxq->hw;
+
+ if (rxq->timestamp_enabled &&
+ !rte_atomic_load_explicit(&priv->nic_ts_stale,
+ rte_memory_order_acquire)) {
+ last_sync =
+ rte_atomic_load_explicit(&priv->last_read_nic_timestamp,
+ rte_memory_order_relaxed);
+ }
while (nb_rx < nb_pkts) {
rx_desc = &rxq->compl_ring[rx_id];
@@ -208,6 +219,16 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
gve_parse_csum_ol_flags(rxm, rx_desc);
rxm->hash.rss = rte_le_to_cpu_32(rx_desc->hash);
+ if (last_sync != 0 &&
+ (rx_desc->ts_sub_nsecs_low & GVE_DQO_RX_HWTSTAMP_VALID) &&
+ priv->mbuf_timestamp_offset >= 0) {
+ uint32_t ts = rte_le_to_cpu_32(rx_desc->ts);
+ uint64_t full_ts = gve_reconstruct_ts(last_sync, ts);
+
+ *RTE_MBUF_DYNFIELD(rxm, priv->mbuf_timestamp_offset, uint64_t *) = full_ts;
+ rxm->ol_flags |= priv->mbuf_timestamp_mask;
+ }
+
rx_pkts[nb_rx++] = rxm;
bytes += pkt_len;
}
@@ -320,6 +341,11 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
return -ENOMEM;
}
+ /* Setup hardware timestamping if enabled */
+ if ((conf->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) ||
+ (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP))
+ rxq->timestamp_enabled = true;
+
/* check free_thresh here */
free_thresh = conf->rx_free_thresh ?
conf->rx_free_thresh : GVE_DEFAULT_RX_FREE_THRESH;
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 5/6] net/gve: support read clock ethdev op
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
Implement the read_clock operation in eth_dev_ops. The function calls
the AdminQ command to fetch the current NIC timestamp synchronously,
updates the cached timestamp used for reconstruction, and returns the
full 64-bit value.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
drivers/net/gve/gve_ethdev.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index b36bc7266e..e1f2585ede 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1271,6 +1271,34 @@ gve_flow_ops_get(struct rte_eth_dev *dev, const struct rte_flow_ops **ops)
return 0;
}
+static int
+gve_read_clock(struct rte_eth_dev *dev, uint64_t *clock)
+{
+ struct gve_priv *priv = dev->data->dev_private;
+ uint64_t ts;
+ int err;
+
+ if (!priv->nic_timestamp_supported)
+ return -EOPNOTSUPP;
+
+ if (!priv->nic_ts_report_mz)
+ return -EIO;
+
+ err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+ if (err != 0)
+ return err;
+
+ ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
+ *clock = ts;
+
+ /* Update the cached value */
+ rte_atomic_store_explicit(&priv->last_read_nic_timestamp, ts, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 0, rte_memory_order_release);
+
+ return 0;
+}
+
static const struct eth_dev_ops gve_eth_dev_ops = {
.dev_configure = gve_dev_configure,
.dev_start = gve_dev_start,
@@ -1297,6 +1325,7 @@ static const struct eth_dev_ops gve_eth_dev_ops = {
.rss_hash_conf_get = gve_rss_hash_conf_get,
.reta_update = gve_rss_reta_update,
.reta_query = gve_rss_reta_query,
+ .read_clock = gve_read_clock,
};
static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
@@ -1325,6 +1354,7 @@ static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
.rss_hash_conf_get = gve_rss_hash_conf_get,
.reta_update = gve_rss_reta_update,
.reta_query = gve_rss_reta_query,
+ .read_clock = gve_read_clock,
};
static int
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 4/6] net/gve: add periodic NIC clock synchronization
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
Introduce a mechanism to periodically fetch the NIC hardware timestamp
using the GVE_ADMINQ_REPORT_NIC_TIMESTAMP AdminQ command. The
synchronization runs every 250ms using rte_alarm. If the read fails,
the alarm is still rescheduled. After 7 consecutive failures, the
timestamp is marked as stale, indicating to the RX path that
reconstructed timestamps may be unreliable.
Atomics exist because of the potential for async callers (introduced
here) and async callers (introduced later in the RX datapath) accessing
the cached state.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
drivers/net/gve/gve_ethdev.c | 104 +++++++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 9 +++
2 files changed, 113 insertions(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index a9e2063dda..b36bc7266e 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -452,6 +452,86 @@ gve_dev_start(struct rte_eth_dev *dev)
return 0;
}
+static void
+gve_read_nic_clock(void *arg)
+{
+ struct gve_priv *priv = (struct gve_priv *)arg;
+ uint32_t fails;
+ uint64_t ts;
+ int err;
+
+ if (!priv || !priv->nic_ts_report_mz)
+ return;
+
+ memset(priv->nic_ts_report, 0, sizeof(struct gve_nic_ts_report));
+
+ err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+ if (err == 0) {
+ ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
+ rte_atomic_store_explicit(&priv->last_read_nic_timestamp, ts,
+ rte_memory_order_relaxed);
+ PMD_DRV_LOG(DEBUG, "Fetched NIC Timestamp: %" PRIu64, ts);
+ rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0,
+ rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 0,
+ rte_memory_order_release);
+ } else {
+ PMD_DRV_LOG(ERR, "Failed to read NIC clock, AQ err: %d", err);
+ fails = rte_atomic_fetch_add_explicit(&priv->nic_ts_read_fails, 1,
+ rte_memory_order_relaxed) + 1;
+ if (fails >= GVE_NIC_CLOCK_READ_MAX_FAILS) {
+ if (!rte_atomic_load_explicit(&priv->nic_ts_stale,
+ rte_memory_order_relaxed))
+ PMD_DRV_LOG(ERR,
+ "NIC timestamping marked as stale after %u consecutive failures",
+ GVE_NIC_CLOCK_READ_MAX_FAILS);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 1,
+ rte_memory_order_release);
+ }
+ }
+
+ /* Reschedule the alarm for the next interval */
+ if (priv->nic_ts_report_mz) {
+ err = rte_eal_alarm_set(GVE_NIC_CLOCK_READ_PERIOD_MS * 1000,
+ gve_read_nic_clock, priv);
+ if (err < 0)
+ PMD_DRV_LOG(ERR, "Failed to reschedule NIC clock read alarm, ret=%d", err);
+ }
+}
+
+static int
+gve_alloc_nic_ts_report(struct gve_priv *priv)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+
+ if (!priv->nic_timestamp_supported)
+ return -EOPNOTSUPP;
+
+ snprintf(z_name, sizeof(z_name), "gve_%s_nic_ts_report",
+ priv->pci_dev->device.name);
+ priv->nic_ts_report_mz = rte_memzone_reserve_aligned(z_name,
+ sizeof(struct gve_nic_ts_report), rte_socket_id(),
+ RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
+
+ if (!priv->nic_ts_report_mz) {
+ PMD_DRV_LOG(ERR, "Failed to allocate memzone for NIC TS report");
+ return -ENOMEM;
+ }
+ priv->nic_ts_report = (struct gve_nic_ts_report *)priv->nic_ts_report_mz->addr;
+ rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
+ return 0;
+}
+
+static void
+gve_free_nic_ts_report(struct gve_priv *priv)
+{
+ if (priv->nic_ts_report_mz) {
+ rte_memzone_free(priv->nic_ts_report_mz);
+ priv->nic_ts_report_mz = NULL;
+ priv->nic_ts_report = NULL;
+ }
+}
+
static int
gve_dev_stop(struct rte_eth_dev *dev)
{
@@ -576,6 +656,7 @@ static void
gve_teardown_device_resources(struct gve_priv *priv)
{
int err;
+ int ret;
/* Tell device its resources are being freed */
if (gve_get_device_resources_ok(priv)) {
@@ -586,6 +667,13 @@ gve_teardown_device_resources(struct gve_priv *priv)
err);
}
+ if (priv->nic_ts_report_mz) {
+ ret = rte_eal_alarm_cancel(gve_read_nic_clock, priv);
+ if (ret < 0)
+ PMD_DRV_LOG(ERR, "Failed to cancel NIC clock sync alarm, ret=%d", ret);
+ gve_free_nic_ts_report(priv);
+ }
+
gve_free_ptype_lut_dqo(priv);
gve_free_counter_array(priv);
gve_free_irq_db(priv);
@@ -1252,6 +1340,21 @@ pci_dev_msix_vec_count(struct rte_pci_device *pdev)
return 0;
}
+static void
+gve_setup_nic_timestamp(struct gve_priv *priv)
+{
+ int err;
+
+ if (!priv->nic_timestamp_supported)
+ return;
+
+ rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 1, rte_memory_order_relaxed);
+ err = gve_alloc_nic_ts_report(priv);
+ if (err == 0)
+ gve_read_nic_clock(priv);
+}
+
static int
gve_setup_device_resources(struct gve_priv *priv)
{
@@ -1307,6 +1410,7 @@ gve_setup_device_resources(struct gve_priv *priv)
goto free_ptype_lut;
}
}
+ gve_setup_nic_timestamp(priv);
gve_set_device_resources_ok(priv);
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index b67f82c263..7e6f24e910 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -12,6 +12,7 @@
#include <rte_pci.h>
#include <pthread.h>
#include <rte_bitmap.h>
+#include <rte_memzone.h>
#include "base/gve.h"
@@ -39,6 +40,9 @@
#define GVE_RSS_HASH_KEY_SIZE 40
#define GVE_RSS_INDIR_SIZE 128
+#define GVE_NIC_CLOCK_READ_PERIOD_MS 250
+#define GVE_NIC_CLOCK_READ_MAX_FAILS 7
+
#define GVE_TX_CKSUM_OFFLOAD_MASK ( \
RTE_MBUF_F_TX_L4_MASK | \
RTE_MBUF_F_TX_TCP_SEG)
@@ -359,6 +363,11 @@ struct gve_priv {
/* HW Timestamping Fields */
bool nic_timestamp_supported;
+ const struct rte_memzone *nic_ts_report_mz;
+ struct gve_nic_ts_report *nic_ts_report;
+ RTE_ATOMIC(uint64_t) last_read_nic_timestamp;
+ RTE_ATOMIC(uint32_t) nic_ts_read_fails;
+ RTE_ATOMIC(uint8_t) nic_ts_stale;
};
static inline bool
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 3/6] net/gve: add AdminQ command for NIC timestamps
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
Introduce the necessary definitions and functions for the
GVE_ADMINQ_REPORT_NIC_TIMESTAMP AdminQ command.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
drivers/net/gve/base/gve_adminq.c | 18 ++++++++++++++++++
drivers/net/gve/base/gve_adminq.h | 20 ++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 1 +
3 files changed, 39 insertions(+)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index c9095fd165..e700262d7f 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -523,6 +523,10 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
case GVE_ADMINQ_CONFIGURE_FLOW_RULE:
priv->adminq_cfg_flow_rule_cnt++;
break;
+ case GVE_ADMINQ_REPORT_NIC_TIMESTAMP:
+ priv->adminq_report_nic_timestamp_cnt++;
+ break;
+
default:
PMD_DRV_LOG(ERR, "unknown AQ command opcode %d", opcode);
}
@@ -637,6 +641,20 @@ int gve_adminq_reset_flow_rules(struct gve_priv *priv)
return gve_adminq_configure_flow_rule(priv, &flow_rule_cmd);
}
+int gve_adminq_report_nic_timestamp(struct gve_priv *priv, dma_addr_t nic_ts_report_addr)
+{
+ union gve_adminq_command cmd;
+
+ memset(&cmd, 0, sizeof(cmd));
+ cmd.opcode = cpu_to_be32(GVE_ADMINQ_REPORT_NIC_TIMESTAMP);
+ cmd.report_nic_timestamp = (struct gve_adminq_report_nic_timestamp) {
+ .nic_ts_report_len = cpu_to_be64(sizeof(struct gve_nic_ts_report)),
+ .nic_timestamp_addr = cpu_to_be64(nic_ts_report_addr),
+ };
+
+ return gve_adminq_execute_cmd(priv, &cmd);
+}
+
/* The device specifies that the management vector can either be the first irq
* or the last irq. ntfy_blk_msix_base_idx indicates the first irq assigned to
* the ntfy blks. It if is 0 then the management vector is last, if it is 1 then
diff --git a/drivers/net/gve/base/gve_adminq.h b/drivers/net/gve/base/gve_adminq.h
index eaee5649f2..954be39fbf 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -26,6 +26,7 @@ enum gve_adminq_opcodes {
GVE_ADMINQ_REPORT_LINK_SPEED = 0xD,
GVE_ADMINQ_GET_PTYPE_MAP = 0xE,
GVE_ADMINQ_VERIFY_DRIVER_COMPATIBILITY = 0xF,
+ GVE_ADMINQ_REPORT_NIC_TIMESTAMP = 0x11,
/* For commands that are larger than 56 bytes */
GVE_ADMINQ_EXTENDED_COMMAND = 0xFF,
};
@@ -373,6 +374,23 @@ struct gve_stats_report {
GVE_CHECK_STRUCT_LEN(8, gve_stats_report);
+struct gve_adminq_report_nic_timestamp {
+ __be64 nic_ts_report_len;
+ __be64 nic_timestamp_addr;
+};
+
+GVE_CHECK_STRUCT_LEN(16, gve_adminq_report_nic_timestamp);
+
+struct gve_nic_ts_report {
+ __be64 nic_timestamp; /* NIC clock in nanoseconds */
+ __be64 pre_cycles; /* System cycle counter before NIC clock read */
+ __be64 post_cycles; /* System cycle counter after NIC clock read */
+ __be64 reserved3;
+ __be64 reserved4;
+};
+
+GVE_CHECK_STRUCT_LEN(40, gve_nic_ts_report);
+
/* Numbers of gve tx/rx stats in stats report. */
#define GVE_TX_STATS_REPORT_NUM 6
#define GVE_RX_STATS_REPORT_NUM 2
@@ -490,6 +508,7 @@ union gve_adminq_command {
struct gve_adminq_verify_driver_compatibility
verify_driver_compatibility;
struct gve_adminq_extended_command extended_command;
+ struct gve_adminq_report_nic_timestamp report_nic_timestamp;
};
};
u8 reserved[64];
@@ -537,5 +556,6 @@ int gve_adminq_add_flow_rule(struct gve_priv *priv,
struct gve_flow_rule_params *rule, u32 loc);
int gve_adminq_del_flow_rule(struct gve_priv *priv, u32 loc);
int gve_adminq_reset_flow_rules(struct gve_priv *priv);
+int gve_adminq_report_nic_timestamp(struct gve_priv *priv, dma_addr_t nic_ts_report_addr);
#endif /* _GVE_ADMINQ_H */
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index b9b4688367..b67f82c263 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -328,6 +328,7 @@ struct gve_priv {
uint32_t adminq_get_ptype_map_cnt;
uint32_t adminq_verify_driver_compatibility_cnt;
uint32_t adminq_cfg_flow_rule_cnt;
+ uint32_t adminq_report_nic_timestamp_cnt;
volatile uint32_t state_flags;
/* Gvnic device link speed from hypervisor. */
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 2/6] net/gve: add device option support for HW timestamps
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
Introduce the necessary definitions and functions for the device
option flag (GVE_DEV_OPT_ID_NIC_TIMESTAMP) to detect hardware
timestamping support in the gvnic device.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
drivers/net/gve/base/gve_adminq.c | 41 ++++++++++++++++++++++++++-----
drivers/net/gve/base/gve_adminq.h | 9 +++++++
drivers/net/gve/gve_ethdev.h | 3 +++
3 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 28661fb6cd..c9095fd165 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -38,7 +38,8 @@ void gve_parse_device_option(struct gve_priv *priv,
struct gve_device_option_dqo_rda **dev_op_dqo_rda,
struct gve_device_option_flow_steering **dev_op_flow_steering,
struct gve_device_option_modify_ring **dev_op_modify_ring,
- struct gve_device_option_jumbo_frames **dev_op_jumbo_frames)
+ struct gve_device_option_jumbo_frames **dev_op_jumbo_frames,
+ struct gve_device_option_nic_timestamp **dev_op_nic_timestamp)
{
u32 req_feat_mask = be32_to_cpu(option->required_features_mask);
u16 option_length = be16_to_cpu(option->option_length);
@@ -168,6 +169,24 @@ void gve_parse_device_option(struct gve_priv *priv,
}
*dev_op_jumbo_frames = RTE_PTR_ADD(option, sizeof(*option));
break;
+ case GVE_DEV_OPT_ID_NIC_TIMESTAMP:
+ if (option_length < sizeof(**dev_op_nic_timestamp) ||
+ req_feat_mask != GVE_DEV_OPT_REQ_FEAT_MASK_NIC_TIMESTAMP) {
+ PMD_DRV_LOG(WARNING, GVE_DEVICE_OPTION_ERROR_FMT,
+ "Nic Timestamp",
+ (int)sizeof(**dev_op_nic_timestamp),
+ GVE_DEV_OPT_REQ_FEAT_MASK_NIC_TIMESTAMP,
+ option_length, req_feat_mask);
+ break;
+ }
+
+ if (option_length > sizeof(**dev_op_nic_timestamp)) {
+ PMD_DRV_LOG(WARNING,
+ GVE_DEVICE_OPTION_TOO_BIG_FMT,
+ "Nic Timestamp");
+ }
+ *dev_op_nic_timestamp = RTE_PTR_ADD(option, sizeof(*option));
+ break;
default:
/* If we don't recognize the option just continue
* without doing anything.
@@ -186,7 +205,8 @@ gve_process_device_options(struct gve_priv *priv,
struct gve_device_option_dqo_rda **dev_op_dqo_rda,
struct gve_device_option_flow_steering **dev_op_flow_steering,
struct gve_device_option_modify_ring **dev_op_modify_ring,
- struct gve_device_option_jumbo_frames **dev_op_jumbo_frames)
+ struct gve_device_option_jumbo_frames **dev_op_jumbo_frames,
+ struct gve_device_option_nic_timestamp **dev_op_nic_timestamp)
{
const int num_options = be16_to_cpu(descriptor->num_device_options);
struct gve_device_option *dev_opt;
@@ -207,7 +227,8 @@ gve_process_device_options(struct gve_priv *priv,
gve_parse_device_option(priv, dev_opt,
dev_op_gqi_rda, dev_op_gqi_qpl,
dev_op_dqo_rda, dev_op_flow_steering,
- dev_op_modify_ring, dev_op_jumbo_frames);
+ dev_op_modify_ring, dev_op_jumbo_frames,
+ dev_op_nic_timestamp);
dev_opt = next_opt;
}
@@ -921,7 +942,8 @@ static void gve_enable_supported_features(struct gve_priv *priv,
u32 supported_features_mask,
const struct gve_device_option_flow_steering *dev_op_flow_steering,
const struct gve_device_option_modify_ring *dev_op_modify_ring,
- const struct gve_device_option_jumbo_frames *dev_op_jumbo_frames)
+ const struct gve_device_option_jumbo_frames *dev_op_jumbo_frames,
+ const struct gve_device_option_nic_timestamp *dev_op_nic_timestamp)
{
if (dev_op_flow_steering &&
(supported_features_mask & GVE_SUP_FLOW_STEERING_MASK) &&
@@ -948,6 +970,11 @@ static void gve_enable_supported_features(struct gve_priv *priv,
PMD_DRV_LOG(INFO, "JUMBO FRAMES device option enabled.");
priv->max_mtu = be16_to_cpu(dev_op_jumbo_frames->max_mtu);
}
+ if (dev_op_nic_timestamp &&
+ (supported_features_mask & GVE_SUP_NIC_TIMESTAMP_MASK)) {
+ PMD_DRV_LOG(INFO, "NIC TIMESTAMP device option enabled.");
+ priv->nic_timestamp_supported = true;
+ }
}
int gve_adminq_describe_device(struct gve_priv *priv)
@@ -955,6 +982,7 @@ int gve_adminq_describe_device(struct gve_priv *priv)
struct gve_device_option_jumbo_frames *dev_op_jumbo_frames = NULL;
struct gve_device_option_modify_ring *dev_op_modify_ring = NULL;
struct gve_device_option_flow_steering *dev_op_flow_steering = NULL;
+ struct gve_device_option_nic_timestamp *dev_op_nic_timestamp = NULL;
struct gve_device_option_gqi_rda *dev_op_gqi_rda = NULL;
struct gve_device_option_gqi_qpl *dev_op_gqi_qpl = NULL;
struct gve_device_option_dqo_rda *dev_op_dqo_rda = NULL;
@@ -984,7 +1012,8 @@ int gve_adminq_describe_device(struct gve_priv *priv)
&dev_op_gqi_qpl, &dev_op_dqo_rda,
&dev_op_flow_steering,
&dev_op_modify_ring,
- &dev_op_jumbo_frames);
+ &dev_op_jumbo_frames,
+ &dev_op_nic_timestamp);
if (err)
goto free_device_descriptor;
@@ -1039,7 +1068,7 @@ int gve_adminq_describe_device(struct gve_priv *priv)
gve_enable_supported_features(priv, supported_features_mask,
dev_op_flow_steering, dev_op_modify_ring,
- dev_op_jumbo_frames);
+ dev_op_jumbo_frames, dev_op_nic_timestamp);
free_device_descriptor:
gve_free_dma_mem(&descriptor_dma_mem);
diff --git a/drivers/net/gve/base/gve_adminq.h b/drivers/net/gve/base/gve_adminq.h
index d8e5e6a352..eaee5649f2 100644
--- a/drivers/net/gve/base/gve_adminq.h
+++ b/drivers/net/gve/base/gve_adminq.h
@@ -153,6 +153,12 @@ struct gve_device_option_jumbo_frames {
GVE_CHECK_STRUCT_LEN(8, gve_device_option_jumbo_frames);
+struct gve_device_option_nic_timestamp {
+ __be32 supported_features_mask;
+};
+
+GVE_CHECK_STRUCT_LEN(4, gve_device_option_nic_timestamp);
+
/* Terminology:
*
* RDA - Raw DMA Addressing - Buffers associated with SKBs are directly DMA
@@ -169,6 +175,7 @@ enum gve_dev_opt_id {
GVE_DEV_OPT_ID_MODIFY_RING = 0x6,
GVE_DEV_OPT_ID_JUMBO_FRAMES = 0x8,
GVE_DEV_OPT_ID_FLOW_STEERING = 0xb,
+ GVE_DEV_OPT_ID_NIC_TIMESTAMP = 0xd,
};
enum gve_dev_opt_req_feat_mask {
@@ -179,12 +186,14 @@ enum gve_dev_opt_req_feat_mask {
GVE_DEV_OPT_REQ_FEAT_MASK_FLOW_STEERING = 0x0,
GVE_DEV_OPT_REQ_FEAT_MASK_MODIFY_RING = 0x0,
GVE_DEV_OPT_REQ_FEAT_MASK_JUMBO_FRAMES = 0x0,
+ GVE_DEV_OPT_REQ_FEAT_MASK_NIC_TIMESTAMP = 0x0,
};
enum gve_sup_feature_mask {
GVE_SUP_MODIFY_RING_MASK = 1 << 0,
GVE_SUP_JUMBO_FRAMES_MASK = 1 << 2,
GVE_SUP_FLOW_STEERING_MASK = 1 << 5,
+ GVE_SUP_NIC_TIMESTAMP_MASK = 1 << 8,
};
#define GVE_DEV_OPT_LEN_GQI_RAW_ADDRESSING 0x0
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 524e48e723..b9b4688367 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -355,6 +355,9 @@ struct gve_priv {
void *avail_flow_rule_bmp_mem; /* Backing memory for the bitmap */
pthread_mutex_t flow_rule_lock; /* Lock for bitmap and tailq access */
TAILQ_HEAD(, gve_flow) active_flows;
+
+ /* HW Timestamping Fields */
+ bool nic_timestamp_supported;
};
static inline bool
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 1/6] net/gve: add thread safety to admin queue
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005404.946979-1-blasko@google.com>
Introduce a pthread_mutex to protect the admin queue operations.
Locking was added around gve_adminq_execute_cmd and the batch
queue creation/destruction functions.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
.mailmap | 1 +
drivers/net/gve/base/gve_adminq.c | 68 +++++++++++++++++++++++++------
drivers/net/gve/gve_ethdev.h | 1 +
3 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/.mailmap b/.mailmap
index 3ab7364668..7f7590866b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1009,6 +1009,7 @@ Mario Carrillo <mario.alfredo.c.arevalo@intel.com>
Mário Kuka <kuka@cesnet.cz>
Mariusz Drost <mariuszx.drost@intel.com>
Mark Asselstine <mark.asselstine@windriver.com>
+Mark Blasko <blasko@google.com>
Mark Bloch <mbloch@nvidia.com> <markb@mellanox.com>
Mark Gillott <mgillott@vyatta.att-mail.com>
Mark Kavanagh <mark.b.kavanagh@intel.com>
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 9c5316fb00..28661fb6cd 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -216,6 +216,7 @@ gve_process_device_options(struct gve_priv *priv,
int gve_adminq_alloc(struct gve_priv *priv)
{
+ pthread_mutexattr_t mutexattr;
uint8_t pci_rev_id;
priv->adminq = gve_alloc_dma_mem(&priv->adminq_dma_mem, PAGE_SIZE);
@@ -241,6 +242,12 @@ int gve_adminq_alloc(struct gve_priv *priv)
priv->adminq_get_ptype_map_cnt = 0;
priv->adminq_cfg_flow_rule_cnt = 0;
+ pthread_mutexattr_init(&mutexattr);
+ pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
+ pthread_mutexattr_setrobust(&mutexattr, PTHREAD_MUTEX_ROBUST);
+ pthread_mutex_init(&priv->adminq_lock, &mutexattr);
+ pthread_mutexattr_destroy(&mutexattr);
+
/* Setup Admin queue with the device */
rte_pci_read_config(priv->pci_dev, &pci_rev_id, sizeof(pci_rev_id),
RTE_PCI_REVISION_ID);
@@ -304,6 +311,7 @@ void gve_adminq_free(struct gve_priv *priv)
return;
gve_adminq_release(priv);
gve_free_dma_mem(&priv->adminq_dma_mem);
+ pthread_mutex_destroy(&priv->adminq_lock);
gve_clear_admin_queue_ok(priv);
}
@@ -418,7 +426,10 @@ static int gve_adminq_issue_cmd(struct gve_priv *priv,
(tail & priv->adminq_mask)) {
int err;
- /* Flush existing commands to make room. */
+ /* Flush existing commands to make room.
+ * Note: This kicks the doorbell for all staged commands.
+ * Any failure here means we failed after attempting to kick.
+ */
err = gve_adminq_kick_and_wait(priv);
if (err)
return err;
@@ -509,17 +520,24 @@ static int gve_adminq_execute_cmd(struct gve_priv *priv,
u32 tail, head;
int err;
+ pthread_mutex_lock(&priv->adminq_lock);
tail = ioread32be(&priv->reg_bar0->adminq_event_counter);
head = priv->adminq_prod_cnt;
- if (tail != head)
+ if (tail != head) {
/* This is not a valid path */
- return -EINVAL;
+ err = -EINVAL;
+ goto unlock_and_return;
+ }
err = gve_adminq_issue_cmd(priv, cmd_orig);
if (err)
- return err;
+ goto unlock_and_return;
- return gve_adminq_kick_and_wait(priv);
+ err = gve_adminq_kick_and_wait(priv);
+
+unlock_and_return:
+ pthread_mutex_unlock(&priv->adminq_lock);
+ return err;
}
static int gve_adminq_execute_extended_cmd(struct gve_priv *priv, u32 opcode,
@@ -693,13 +711,19 @@ int gve_adminq_create_tx_queues(struct gve_priv *priv, u32 num_queues)
int err;
u32 i;
+ pthread_mutex_lock(&priv->adminq_lock);
+
for (i = 0; i < num_queues; i++) {
err = gve_adminq_create_tx_queue(priv, i);
if (err)
- return err;
+ goto unlock_and_return;
}
- return gve_adminq_kick_and_wait(priv);
+ err = gve_adminq_kick_and_wait(priv);
+
+unlock_and_return:
+ pthread_mutex_unlock(&priv->adminq_lock);
+ return err;
}
static int gve_adminq_create_rx_queue(struct gve_priv *priv, u32 queue_index)
@@ -747,13 +771,19 @@ int gve_adminq_create_rx_queues(struct gve_priv *priv, u32 num_queues)
int err;
u32 i;
+ pthread_mutex_lock(&priv->adminq_lock);
+
for (i = 0; i < num_queues; i++) {
err = gve_adminq_create_rx_queue(priv, i);
if (err)
- return err;
+ goto unlock_and_return;
}
- return gve_adminq_kick_and_wait(priv);
+ err = gve_adminq_kick_and_wait(priv);
+
+unlock_and_return:
+ pthread_mutex_unlock(&priv->adminq_lock);
+ return err;
}
static int gve_adminq_destroy_tx_queue(struct gve_priv *priv, u32 queue_index)
@@ -779,13 +809,19 @@ int gve_adminq_destroy_tx_queues(struct gve_priv *priv, u32 num_queues)
int err;
u32 i;
+ pthread_mutex_lock(&priv->adminq_lock);
+
for (i = 0; i < num_queues; i++) {
err = gve_adminq_destroy_tx_queue(priv, i);
if (err)
- return err;
+ goto unlock_and_return;
}
- return gve_adminq_kick_and_wait(priv);
+ err = gve_adminq_kick_and_wait(priv);
+
+unlock_and_return:
+ pthread_mutex_unlock(&priv->adminq_lock);
+ return err;
}
static int gve_adminq_destroy_rx_queue(struct gve_priv *priv, u32 queue_index)
@@ -811,13 +847,19 @@ int gve_adminq_destroy_rx_queues(struct gve_priv *priv, u32 num_queues)
int err;
u32 i;
+ pthread_mutex_lock(&priv->adminq_lock);
+
for (i = 0; i < num_queues; i++) {
err = gve_adminq_destroy_rx_queue(priv, i);
if (err)
- return err;
+ goto unlock_and_return;
}
- return gve_adminq_kick_and_wait(priv);
+ err = gve_adminq_kick_and_wait(priv);
+
+unlock_and_return:
+ pthread_mutex_unlock(&priv->adminq_lock);
+ return err;
}
static int gve_set_desc_cnt(struct gve_priv *priv,
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 0577f03974..524e48e723 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -339,6 +339,7 @@ struct gve_priv {
struct gve_tx_queue **txqs;
struct gve_rx_queue **rxqs;
+ pthread_mutex_t adminq_lock; /* Protects AdminQ command execution */
uint32_t stats_report_len;
const struct rte_memzone *stats_report_mem;
uint16_t stats_start_idx; /* start index of array of stats written by NIC */
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 0/6] net/gve: add hardware timestamping support
From: Mark Blasko @ 2026-05-12 0:53 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko
This patch series introduces support for GVE hardware timestamping on DQO
queues. To support concurrent access, a mutex lock is introduced to protect
admin queue operations. A mechanism is then added to periodically synchronize
the NIC clock via AdminQ, and support is introduced for the read_clock ethdev
operation. Finally, the RX datapath is updated to reconstruct full 64-bit
timestamps from the 32-bit values in DQO descriptors.
Mark Blasko (6):
net/gve: add thread safety to admin queue
net/gve: add device option support for HW timestamps
net/gve: add AdminQ command for NIC timestamps
net/gve: add periodic NIC clock synchronization
net/gve: support read clock ethdev op
net/gve: reconstruct HW timestamps from DQO
.mailmap | 1 +
doc/guides/nics/features/gve.ini | 1 +
doc/guides/nics/gve.rst | 18 +++
doc/guides/rel_notes/release_26_07.rst | 3 +
drivers/net/gve/base/gve_adminq.c | 127 +++++++++++++++++----
drivers/net/gve/base/gve_adminq.h | 29 +++++
drivers/net/gve/base/gve_desc_dqo.h | 8 +-
drivers/net/gve/gve_ethdev.c | 148 ++++++++++++++++++++++++-
drivers/net/gve/gve_ethdev.h | 39 +++++++
drivers/net/gve/gve_rx_dqo.c | 26 +++++
10 files changed, 378 insertions(+), 22 deletions(-)
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply
* [PATCH 6/6] net/gve: reconstruct HW timestamps from DQO
From: mark-blasko @ 2026-05-12 0:50 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005057.944672-1-blasko@google.com>
From: Mark Blasko <blasko@google.com>
A full 64-bit NIC timestamp is periodically synced via an AdminQ
command and cached in the driver. In the RX datapath, this cached
value is used as a base to expand the 32-bit hardware timestamp into
a full 64-bit value, which is then stored in the mbuf's dynamic
timestamp field.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
doc/guides/nics/features/gve.ini | 1 +
doc/guides/nics/gve.rst | 18 ++++++++++++++++++
doc/guides/rel_notes/release_26_07.rst | 3 +++
drivers/net/gve/base/gve_desc_dqo.h | 8 ++++++--
drivers/net/gve/gve_ethdev.c | 14 +++++++++++++-
drivers/net/gve/gve_ethdev.h | 25 +++++++++++++++++++++++++
drivers/net/gve/gve_rx_dqo.c | 26 ++++++++++++++++++++++++++
7 files changed, 92 insertions(+), 3 deletions(-)
diff --git a/doc/guides/nics/features/gve.ini b/doc/guides/nics/features/gve.ini
index 89c97fd27a..117ad4fc65 100644
--- a/doc/guides/nics/features/gve.ini
+++ b/doc/guides/nics/features/gve.ini
@@ -13,6 +13,7 @@ RSS hash = Y
RSS key update = Y
RSS reta update = Y
L4 checksum offload = Y
+Timestamp offload = Y
Basic stats = Y
FreeBSD = Y
Linux = Y
diff --git a/doc/guides/nics/gve.rst b/doc/guides/nics/gve.rst
index 62648c47ed..44aedc9311 100644
--- a/doc/guides/nics/gve.rst
+++ b/doc/guides/nics/gve.rst
@@ -72,6 +72,7 @@ Supported features of the GVE PMD are:
- Tx UDP/TCP/SCTP Checksum
- RSS hash configuration
- RSS redirection table query and update
+- Timestamp offload
Currently, only GQI_QPL and GQI_RDA queue format are supported in PMD.
Jumbo Frame is not supported in PMD for now.
@@ -132,6 +133,23 @@ Security Protocols
- Flow priorities are not supported (must be 0).
- Masking is limited to full matches i.e. ``0x00...0`` or ``0xFF...F``.
+Timestamp Offload
+^^^^^^^^^^^^^^^^^
+
+The driver supports hardware-based packet timestamping on supported
+devices via the standard ``RTE_ETH_RX_OFFLOAD_TIMESTAMP`` offload capability.
+
+**Limitations**
+
+- If the driver fails to fetch the NIC hardware clock for 7 consecutive periods,
+ the cached timestamp is marked as stale,
+ and the reconstructed timestamps are no longer propagated to the mbuf.
+- The timestamp reconstruction is only accurate
+ if the time between a packet's reception
+ and the last hardware clock sync is less than approximately 2 seconds.
+ The driver's internal clock sync period is set to respect this limitation.
+
+
Device Reset
^^^^^^^^^^^^
diff --git a/doc/guides/rel_notes/release_26_07.rst b/doc/guides/rel_notes/release_26_07.rst
index 1b012c4776..db886f19cf 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -62,6 +62,9 @@ New Features
* ``-A`` or ``--no-auto-probing`` disable the initial bus probing: no device is probed during
``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.
+* **Updated Google GVE net driver.**
+
+ * Added hardware timestamping support on DQO queues.
* **Updated PCAP ethernet driver.**
diff --git a/drivers/net/gve/base/gve_desc_dqo.h b/drivers/net/gve/base/gve_desc_dqo.h
index 71d9d60bb9..c1534959c2 100644
--- a/drivers/net/gve/base/gve_desc_dqo.h
+++ b/drivers/net/gve/base/gve_desc_dqo.h
@@ -226,7 +226,8 @@ struct gve_rx_compl_desc_dqo {
u8 status_error1;
- __le16 reserved5;
+ u8 reserved5;
+ u8 ts_sub_nsecs_low;
__le16 buf_id; /* Buffer ID which was sent on the buffer queue. */
union {
@@ -237,9 +238,12 @@ struct gve_rx_compl_desc_dqo {
};
__le32 hash;
__le32 reserved6;
- __le64 reserved7;
+ __le32 reserved7;
+ __le32 ts; /* timestamp in nanosecs */
} __packed;
+#define GVE_DQO_RX_HWTSTAMP_VALID 0x1
+
GVE_CHECK_STRUCT_LEN(32, gve_rx_compl_desc_dqo);
/* Ringing the doorbell too often can hurt performance.
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index e1f2585ede..fa26c2bdb4 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -214,6 +214,7 @@ static int
gve_dev_configure(struct rte_eth_dev *dev)
{
struct gve_priv *priv = dev->data->dev_private;
+ int err;
if (dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) {
dev->data->dev_conf.rxmode.offloads |= RTE_ETH_RX_OFFLOAD_RSS_HASH;
@@ -223,13 +224,22 @@ gve_dev_configure(struct rte_eth_dev *dev)
if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TCP_LRO)
priv->enable_rsc = 1;
+ if (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) {
+ err = rte_mbuf_dyn_rx_timestamp_register(&priv->mbuf_timestamp_offset,
+ &priv->mbuf_timestamp_mask);
+ if (err < 0) {
+ PMD_DRV_LOG(ERR, "Failed to register dynamic timestamp field");
+ return err;
+ }
+ }
+
/* Reset RSS RETA in case number of queues changed. */
if (priv->rss_config.indir) {
struct gve_rss_config update_reta_config;
gve_init_rss_config_from_priv(priv, &update_reta_config);
gve_generate_rss_reta(dev, &update_reta_config);
- int err = gve_adminq_configure_rss(priv, &update_reta_config);
+ err = gve_adminq_configure_rss(priv, &update_reta_config);
if (err)
PMD_DRV_LOG(ERR,
"Could not reconfigure RSS redirection table.");
@@ -817,6 +827,8 @@ gve_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
dev_info->min_mtu = RTE_ETHER_MIN_MTU;
dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_RSS_HASH;
+ if (priv->nic_timestamp_supported)
+ dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TIMESTAMP;
dev_info->tx_offload_capa =
RTE_ETH_TX_OFFLOAD_MULTI_SEGS |
RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 7e6f24e910..35d532284e 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -260,6 +260,7 @@ struct gve_rx_queue {
struct rte_mbuf **refill_bufs;
uint8_t is_gqi_qpl;
+ bool timestamp_enabled;
};
struct gve_flow {
@@ -368,8 +369,32 @@ struct gve_priv {
RTE_ATOMIC(uint64_t) last_read_nic_timestamp;
RTE_ATOMIC(uint32_t) nic_ts_read_fails;
RTE_ATOMIC(uint8_t) nic_ts_stale;
+
+ int mbuf_timestamp_offset;
+ uint64_t mbuf_timestamp_mask;
};
+/* Expand the hardware timestamp to the full 64 bits of width.
+ *
+ * This algorithm works by using the passed hardware timestamp to generate a
+ * diff relative to the last read of the nic clock. This diff can be positive or
+ * negative, as it is possible that we have read the clock more recently than
+ * the hardware has received this packet. To detect this, we use the high bit of
+ * the diff, and assume that the read is more recent if the high bit is set. In
+ * this case we invert the process.
+ *
+ * Note that this means if the time delta between packet reception and the last
+ * clock read is greater than ~2 seconds, this will provide invalid results.
+ */
+static inline uint64_t
+gve_reconstruct_ts(uint64_t last_sync, uint32_t ts)
+{
+ uint32_t low = (uint32_t)last_sync;
+ int32_t diff = (int32_t)(ts - low);
+
+ return last_sync + diff;
+}
+
static inline bool
gve_is_gqi(struct gve_priv *priv)
{
diff --git a/drivers/net/gve/gve_rx_dqo.c b/drivers/net/gve/gve_rx_dqo.c
index 8035aee572..cc343f3fd8 100644
--- a/drivers/net/gve/gve_rx_dqo.c
+++ b/drivers/net/gve/gve_rx_dqo.c
@@ -160,6 +160,8 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
{
volatile struct gve_rx_compl_desc_dqo *rx_desc;
struct gve_rx_queue *rxq;
+ uint64_t last_sync = 0;
+ struct gve_priv *priv;
struct rte_mbuf *rxm;
uint16_t rx_buf_id;
uint16_t pkt_len;
@@ -171,6 +173,15 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
nb_rx = 0;
rxq = rx_queue;
rx_id = rxq->rx_tail;
+ priv = rxq->hw;
+
+ if (rxq->timestamp_enabled &&
+ !rte_atomic_load_explicit(&priv->nic_ts_stale,
+ rte_memory_order_acquire)) {
+ last_sync =
+ rte_atomic_load_explicit(&priv->last_read_nic_timestamp,
+ rte_memory_order_relaxed);
+ }
while (nb_rx < nb_pkts) {
rx_desc = &rxq->compl_ring[rx_id];
@@ -208,6 +219,16 @@ gve_rx_burst_dqo(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
gve_parse_csum_ol_flags(rxm, rx_desc);
rxm->hash.rss = rte_le_to_cpu_32(rx_desc->hash);
+ if (last_sync != 0 &&
+ (rx_desc->ts_sub_nsecs_low & GVE_DQO_RX_HWTSTAMP_VALID) &&
+ priv->mbuf_timestamp_offset >= 0) {
+ uint32_t ts = rte_le_to_cpu_32(rx_desc->ts);
+ uint64_t full_ts = gve_reconstruct_ts(last_sync, ts);
+
+ *RTE_MBUF_DYNFIELD(rxm, priv->mbuf_timestamp_offset, uint64_t *) = full_ts;
+ rxm->ol_flags |= priv->mbuf_timestamp_mask;
+ }
+
rx_pkts[nb_rx++] = rxm;
bytes += pkt_len;
}
@@ -320,6 +341,11 @@ gve_rx_queue_setup_dqo(struct rte_eth_dev *dev, uint16_t queue_id,
return -ENOMEM;
}
+ /* Setup hardware timestamping if enabled */
+ if ((conf->offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP) ||
+ (dev->data->dev_conf.rxmode.offloads & RTE_ETH_RX_OFFLOAD_TIMESTAMP))
+ rxq->timestamp_enabled = true;
+
/* check free_thresh here */
free_thresh = conf->rx_free_thresh ?
conf->rx_free_thresh : GVE_DEFAULT_RX_FREE_THRESH;
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* [PATCH 5/6] net/gve: support read clock ethdev op
From: mark-blasko @ 2026-05-12 0:50 UTC (permalink / raw)
To: stephen; +Cc: dev, Mark Blasko, Joshua Washington, Jasper Tran O'Leary
In-Reply-To: <20260512005057.944672-1-blasko@google.com>
From: Mark Blasko <blasko@google.com>
Implement the read_clock operation in eth_dev_ops. The function calls
the AdminQ command to fetch the current NIC timestamp synchronously,
updates the cached timestamp used for reconstruction, and returns the
full 64-bit value.
Signed-off-by: Mark Blasko <blasko@google.com>
Reviewed-by: Joshua Washington <joshwash@google.com>
Reviewed-by: Jasper Tran O'Leary <jtranoleary@google.com>
---
drivers/net/gve/gve_ethdev.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index b36bc7266e..e1f2585ede 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -1271,6 +1271,34 @@ gve_flow_ops_get(struct rte_eth_dev *dev, const struct rte_flow_ops **ops)
return 0;
}
+static int
+gve_read_clock(struct rte_eth_dev *dev, uint64_t *clock)
+{
+ struct gve_priv *priv = dev->data->dev_private;
+ uint64_t ts;
+ int err;
+
+ if (!priv->nic_timestamp_supported)
+ return -EOPNOTSUPP;
+
+ if (!priv->nic_ts_report_mz)
+ return -EIO;
+
+ err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+ if (err != 0)
+ return err;
+
+ ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
+ *clock = ts;
+
+ /* Update the cached value */
+ rte_atomic_store_explicit(&priv->last_read_nic_timestamp, ts, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_read_fails, 0, rte_memory_order_relaxed);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 0, rte_memory_order_release);
+
+ return 0;
+}
+
static const struct eth_dev_ops gve_eth_dev_ops = {
.dev_configure = gve_dev_configure,
.dev_start = gve_dev_start,
@@ -1297,6 +1325,7 @@ static const struct eth_dev_ops gve_eth_dev_ops = {
.rss_hash_conf_get = gve_rss_hash_conf_get,
.reta_update = gve_rss_reta_update,
.reta_query = gve_rss_reta_query,
+ .read_clock = gve_read_clock,
};
static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
@@ -1325,6 +1354,7 @@ static const struct eth_dev_ops gve_eth_dev_ops_dqo = {
.rss_hash_conf_get = gve_rss_hash_conf_get,
.reta_update = gve_rss_reta_update,
.reta_query = gve_rss_reta_query,
+ .read_clock = gve_read_clock,
};
static int
--
2.54.0.563.g4f69b47b94-goog
^ 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