* [PATCH 2/2] common/cnxk: fix thread-unsafe NIX telemetry parsing
From: Stephen Hemminger @ 2026-06-05 22:44 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Nithin Dabilpuram, Kiran Kumar K,
Sunil Kumar Kori, Satha Rao, Harman Kalra,
Gowrishankar Muthukrishnan, Jerin Jacob
In-Reply-To: <20260605224514.651081-1-stephen@networkplumber.org>
cnxk_nix_tel_handle_info_x() backs the /cnxk/nix/{rq,cq,sq}/{info,ctx}
telemetry commands and parsed its "<pcidev>,<queue_id>" parameter with
strtok(), which keeps non-reentrant state and races when telemetry
callbacks run on per-connection threads.
Split the parameter with strchr() and parse the queue id with strtoul().
While here, copy the full parameter (the length was capped at
PCI_PRI_STR_SIZE + 1, truncating the id for longer device addresses) and
reject non-numeric or out-of-range ids instead of letting strtol() alias
them to queue 0.
Fixes: af75aac78978 ("common/cnxk: support telemetry for NIX")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/common/cnxk/cnxk_telemetry_nix.c | 80 +++++++++---------------
1 file changed, 30 insertions(+), 50 deletions(-)
diff --git a/drivers/common/cnxk/cnxk_telemetry_nix.c b/drivers/common/cnxk/cnxk_telemetry_nix.c
index abeefafe1e..82a146c139 100644
--- a/drivers/common/cnxk/cnxk_telemetry_nix.c
+++ b/drivers/common/cnxk/cnxk_telemetry_nix.c
@@ -1015,76 +1015,56 @@ cnxk_nix_tel_handle_info_x(const char *cmd, const char *params,
struct plt_tel_data *d)
{
struct nix_tel_node *node;
- char *name, *param;
char buf[1024];
+ char *comma, *end;
+ unsigned long qid;
int rc = -1;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
- goto exit;
+ if (params == NULL || !isdigit((unsigned char)params[0]))
+ return -1;
- plt_strlcpy(buf, params, PCI_PRI_STR_SIZE + 1);
- name = strtok(buf, ",");
- if (name == NULL)
- goto exit;
+ plt_strlcpy(buf, params, sizeof(buf)); /* was PCI_PRI_STR_SIZE + 1 */
- param = strtok(NULL, "\0");
+ /* params is "<pcidev_name>,<queue_id>" */
+ comma = strchr(buf, ',');
+ if (comma == NULL || !isdigit((unsigned char)comma[1]))
+ return -1;
+ *comma = '\0';
- node = nix_tel_node_get_by_pcidev_name(name);
- if (!node)
- goto exit;
+ errno = 0;
+ qid = strtoul(comma + 1, &end, 10);
+ if (errno != 0 || (*end != '\0' && *end != ','))
+ return -1;
+
+ node = nix_tel_node_get_by_pcidev_name(buf);
+ if (node == NULL)
+ return -1;
plt_tel_data_start_dict(d);
if (strstr(cmd, "rq")) {
- char *tok = strtok(param, ",");
- int rq;
-
- if (!tok)
- goto exit;
-
- rq = strtol(tok, NULL, 10);
- if ((node->n_rq <= rq) || (rq < 0))
- goto exit;
-
+ if (qid >= node->n_rq)
+ return -1;
if (strstr(cmd, "ctx"))
- rc = cnxk_tel_nix_rq_ctx(node->nix, rq, d);
+ rc = cnxk_tel_nix_rq_ctx(node->nix, qid, d);
else
- rc = cnxk_tel_nix_rq(node->rqs[rq], d);
-
+ rc = cnxk_tel_nix_rq(node->rqs[qid], d);
} else if (strstr(cmd, "cq")) {
- char *tok = strtok(param, ",");
- int cq;
-
- if (!tok)
- goto exit;
-
- cq = strtol(tok, NULL, 10);
- if ((node->n_cq <= cq) || (cq < 0))
- goto exit;
-
+ if (qid >= node->n_cq)
+ return -1;
if (strstr(cmd, "ctx"))
- rc = cnxk_tel_nix_cq_ctx(node->nix, cq, d);
+ rc = cnxk_tel_nix_cq_ctx(node->nix, qid, d);
else
- rc = cnxk_tel_nix_cq(node->cqs[cq], d);
-
+ rc = cnxk_tel_nix_cq(node->cqs[qid], d);
} else if (strstr(cmd, "sq")) {
- char *tok = strtok(param, ",");
- int sq;
-
- if (!tok)
- goto exit;
-
- sq = strtol(tok, NULL, 10);
- if ((node->n_sq <= sq) || (sq < 0))
- goto exit;
-
+ if (qid >= node->n_sq)
+ return -1;
if (strstr(cmd, "ctx"))
- rc = cnxk_tel_nix_sq_ctx(node->nix, sq, d);
+ rc = cnxk_tel_nix_sq_ctx(node->nix, qid, d);
else
- rc = cnxk_tel_nix_sq(node->sqs[sq], d);
+ rc = cnxk_tel_nix_sq(node->sqs[qid], d);
}
-exit:
return rc;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 1/2] net/cnxk: fix telemetry SA info parameter parsing
From: Stephen Hemminger @ 2026-06-05 22:44 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Nithin Dabilpuram, Kiran Kumar K,
Sunil Kumar Kori, Satha Rao, Harman Kalra, Rakesh Kudurumalla
In-Reply-To: <20260605224514.651081-1-stephen@networkplumber.org>
The /cnxk/ipsec/sa_info handler would silently wrap 32 bit value
to 16 bit port id.
An out-of-range port such as 65536 narrowed to a valid port
for the check and then read past the array.
Reject port ids >= RTE_MAX_ETHPORTS before the lookup.
The /cnxk/ipsec/info handler has similar issue with
strtoul().
Rework parse_params() to walk the string with strtoul()/endptr
rather than strtok(), which is not thread safe and races when the
telemetry callbacks run on per-connection threads. This drops the
strdup()/free(), range checks each value against UINT32_MAX, and
passes an unsigned char to isdigit().
Fixes: d74ed1628f7e ("net/cnxk: add SA info telemetry")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c | 50 ++++++++++----------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c b/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
index 86c2453c09..0c1533e3d7 100644
--- a/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
+++ b/drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c
@@ -211,33 +211,30 @@ copy_inb_sa_10k(struct rte_tel_data *d, uint32_t i, void *sa)
static int
parse_params(const char *params, uint32_t *vals, size_t n_vals)
{
- char dlim[2] = ",";
- char *params_args;
size_t count = 0;
- char *token;
- if (vals == NULL || params == NULL || strlen(params) == 0)
+ if (params == NULL || !isdigit((unsigned char)params[0]))
return -1;
- /* strtok expects char * and param is const char *. Hence on using
- * params as "const char *" compiler throws warning.
- */
- params_args = strdup(params);
- if (params_args == NULL)
- return -1;
+ while (count < n_vals) {
+ char *end;
+ unsigned long v;
- token = strtok(params_args, dlim);
- while (token && isdigit(*token) && count < n_vals) {
- vals[count++] = strtoul(token, NULL, 10);
- token = strtok(NULL, dlim);
- }
+ errno = 0;
+ v = strtoul(params, &end, 10);
+ if (errno != 0 || v > UINT32_MAX)
+ return -EINVAL;
+ vals[count++] = v;
- free(params_args);
+ if (*end == '\0')
+ break;
- if (count < n_vals)
- return -1;
+ if (*end != ',' || !isdigit((unsigned char)end[1]))
+ return -EINVAL;
+ params = end + 1;
+ }
- return 0;
+ return count == n_vals ? 0 : -EINVAL;
}
static int
@@ -252,13 +249,13 @@ ethdev_sec_tel_handle_sa_info(const char *cmd __rte_unused, const char *params,
uint32_t i;
int ret;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
- return -EINVAL;
-
if (parse_params(params, vals, RTE_DIM(vals)) < 0)
return -EINVAL;
port_id = vals[0];
+ if (port_id >= RTE_MAX_ETHPORTS)
+ return -EINVAL;
+
sa_idx = vals[1];
if (!rte_eth_dev_is_valid_port(port_id)) {
@@ -320,12 +317,13 @@ ethdev_sec_tel_handle_info(const char *cmd __rte_unused, const char *params,
struct cnxk_eth_sec_sess *eth_sec, *tvar;
struct rte_eth_dev *eth_dev;
struct cnxk_eth_dev *dev;
- uint16_t port_id;
+ unsigned long port_id;
char *end_p;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || !isdigit((unsigned char)*params))
return -EINVAL;
+ errno = 0;
port_id = strtoul(params, &end_p, 0);
if (errno != 0)
return -EINVAL;
@@ -333,8 +331,8 @@ ethdev_sec_tel_handle_info(const char *cmd __rte_unused, const char *params,
if (*end_p != '\0')
plt_err("Extra parameters passed to telemetry, ignoring it");
- if (!rte_eth_dev_is_valid_port(port_id)) {
- plt_err("Invalid port id %u", port_id);
+ if (port_id >= RTE_MAX_ETHPORTS || !rte_eth_dev_is_valid_port(port_id)) {
+ plt_err("Invalid port id %lu", port_id);
return -EINVAL;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 0/2] net/cnxk: harden telemetry parameter parsing
From: Stephen Hemminger @ 2026-06-05 22:44 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
The cnxk telemetry handlers parse their command parameters with strtok(),
which keeps non-reentrant internal state and races when telemetry callbacks
run on per-connection threads. Both handlers also trust the parsed integers
more than they should: values are narrowed to the destination width or
aliased to a valid index before any range check, so an out-of-range port or
queue id can slip through and read past the backing array.
These two patches replace the strtok() walks with strtoul()/endptr parsing,
range-check each value before it is used, and drop the strdup()/free() that
the old SA-info path needed. The NIX handler additionally copies the full
parameter string rather than capping it at PCI_PRI_STR_SIZE + 1, which had
been truncating the queue id for longer device addresses.
Stephen Hemminger (2):
net/cnxk: fix telemetry SA info parameter parsing
common/cnxk: fix thread-unsafe NIX telemetry parsing
drivers/common/cnxk/cnxk_telemetry_nix.c | 80 ++++++++------------
drivers/net/cnxk/cnxk_ethdev_sec_telemetry.c | 50 ++++++------
2 files changed, 54 insertions(+), 76 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH v3 6/6] net/gve: reconstruct HW timestamps from DQO
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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>
---
v2:
- Scoped timestamp offload capability advertisement strictly
to DQO queues.
- Predicated capability advertisement directly on memzone
allocation.
- Initialized mbuf_timestamp_offset to -1.
- Added blank line separating release notes.
---
doc/guides/nics/features/gve.ini | 1 +
doc/guides/nics/gve.rst | 20 ++++++++++++++++++++
doc/guides/rel_notes/release_26_07.rst | 4 ++++
drivers/net/gve/base/gve_desc_dqo.h | 8 ++++++--
drivers/net/gve/gve_ethdev.c | 15 ++++++++++++++-
drivers/net/gve/gve_ethdev.h | 25 +++++++++++++++++++++++++
drivers/net/gve/gve_rx_dqo.c | 26 ++++++++++++++++++++++++++
7 files changed, 96 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 be855b645d..b0a02f29bd 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,25 @@ 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.
+While the ethdev ``.read_clock`` operation works regardless of queue format,
+per-packet RX timestamp offloading requires the DQO queue format.
+
+**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 2e449d3ee8..4bd4b9ad93 100644
--- a/doc/guides/rel_notes/release_26_07.rst
+++ b/doc/guides/rel_notes/release_26_07.rst
@@ -69,6 +69,10 @@ New Features
``rte_eal_init`` and the application is responsible for probing each device,
* ``--auto-probing`` enables the initial bus probing, which is the current default behavior.
+* **Updated Google GVE net driver.**
+
+ * Added hardware timestamping support on DQO queues.
+
* **Added RISC-V vector paths.**
* Increased the default SIMD bitwidth to allow using the vector extension.
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 ec9f511a00..e7f4860d2d 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.");
@@ -821,6 +831,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 (!gve_is_gqi(priv) && priv->nic_ts_report_mz)
+ 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 |
@@ -1661,6 +1673,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
priv->max_nb_txq = max_tx_queues;
priv->max_nb_rxq = max_rx_queues;
+ priv->mbuf_timestamp_offset = -1;
err = gve_init_priv(priv, false);
if (err)
return err;
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 114531a481..16ba6aa40a 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 {
@@ -369,8 +370,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.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 5/6] net/gve: support read clock ethdev op
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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>
---
v3:
- Add mutex lock to protect shared NIC timestamp memzone access.
- Fix missing read_clock assignment to DQO queue ops table
(accidental omission in v2).
v2:
- Scoped read_clock ethdev operation strictly to DQO queues.
---
drivers/net/gve/gve_ethdev.c | 38 ++++++++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 1 +
2 files changed, 39 insertions(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 5d4f1e4ae8..ec9f511a00 100644
--- a/drivers/net/gve/gve_ethdev.c
+++ b/drivers/net/gve/gve_ethdev.c
@@ -463,11 +463,13 @@ gve_read_nic_clock(void *arg)
if (!priv || !priv->nic_ts_report_mz)
return;
+ pthread_mutex_lock(&priv->nic_ts_lock);
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);
+ pthread_mutex_unlock(&priv->nic_ts_lock);
rte_atomic_store_explicit(&priv->last_read_nic_timestamp, ts,
rte_memory_order_relaxed);
PMD_DRV_LOG(DEBUG, "Fetched NIC Timestamp: %" PRIu64, ts);
@@ -476,6 +478,7 @@ gve_read_nic_clock(void *arg)
rte_atomic_store_explicit(&priv->nic_ts_stale, 0,
rte_memory_order_release);
} else {
+ pthread_mutex_unlock(&priv->nic_ts_lock);
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;
@@ -699,6 +702,7 @@ gve_dev_close(struct rte_eth_dev *dev)
gve_teardown_flow_subsystem(priv);
pthread_mutex_destroy(&priv->flow_rule_lock);
+ pthread_mutex_destroy(&priv->nic_ts_lock);
gve_free_queues(dev);
gve_teardown_device_resources(priv);
@@ -1271,6 +1275,38 @@ 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;
+
+ pthread_mutex_lock(&priv->nic_ts_lock);
+ err = gve_adminq_report_nic_timestamp(priv, priv->nic_ts_report_mz->iova);
+ if (err != 0) {
+ pthread_mutex_unlock(&priv->nic_ts_lock);
+ return err;
+ }
+
+ ts = be64_to_cpu(priv->nic_ts_report->nic_timestamp);
+ pthread_mutex_unlock(&priv->nic_ts_lock);
+ *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,
@@ -1325,6 +1361,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
@@ -1643,6 +1680,7 @@ gve_dev_init(struct rte_eth_dev *eth_dev)
pthread_mutexattr_init(&mutexattr);
pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
pthread_mutex_init(&priv->flow_rule_lock, &mutexattr);
+ pthread_mutex_init(&priv->nic_ts_lock, &mutexattr);
pthread_mutexattr_destroy(&mutexattr);
return 0;
diff --git a/drivers/net/gve/gve_ethdev.h b/drivers/net/gve/gve_ethdev.h
index 7e6f24e910..114531a481 100644
--- a/drivers/net/gve/gve_ethdev.h
+++ b/drivers/net/gve/gve_ethdev.h
@@ -365,6 +365,7 @@ struct gve_priv {
bool nic_timestamp_supported;
const struct rte_memzone *nic_ts_report_mz;
struct gve_nic_ts_report *nic_ts_report;
+ pthread_mutex_t nic_ts_lock;
RTE_ATOMIC(uint64_t) last_read_nic_timestamp;
RTE_ATOMIC(uint32_t) nic_ts_read_fails;
RTE_ATOMIC(uint8_t) nic_ts_stale;
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 4/6] net/gve: add periodic NIC clock synchronization
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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>
---
v2:
- Removed redundant void* casts.
- Handled alarm reschedule failures by marking timestamp stale.
- Added transient error logging on memzone allocation failure.
---
drivers/net/gve/gve_ethdev.c | 106 +++++++++++++++++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 9 +++
2 files changed, 115 insertions(+)
diff --git a/drivers/net/gve/gve_ethdev.c b/drivers/net/gve/gve_ethdev.c
index 476b2c311f..5d4f1e4ae8 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 = 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);
+ rte_atomic_store_explicit(&priv->nic_ts_stale, 1,
+ rte_memory_order_release);
+ }
+ }
+}
+
+static int
+gve_alloc_nic_ts_report(struct gve_priv *priv)
+{
+ char z_name[RTE_MEMZONE_NAMESIZE];
+
+ 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 = 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,23 @@ 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);
+ else
+ PMD_DRV_LOG(ERR, "Failed to allocate memory for NIC timestamping subsystem. Please reset device to retry.");
+}
+
static int
gve_setup_device_resources(struct gve_priv *priv)
{
@@ -1307,6 +1412,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.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 3/6] net/gve: add AdminQ command for NIC timestamps
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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>
---
v2:
- Added adminq timestamp counter reset to gve_adminq_alloc.
---
drivers/net/gve/base/gve_adminq.c | 20 ++++++++++++++++++++
drivers/net/gve/base/gve_adminq.h | 20 ++++++++++++++++++++
drivers/net/gve/gve_ethdev.h | 1 +
3 files changed, 41 insertions(+)
diff --git a/drivers/net/gve/base/gve_adminq.c b/drivers/net/gve/base/gve_adminq.c
index 1ced1e442e..2b25c7f390 100644
--- a/drivers/net/gve/base/gve_adminq.c
+++ b/drivers/net/gve/base/gve_adminq.c
@@ -263,6 +263,8 @@ int gve_adminq_alloc(struct gve_priv *priv)
priv->adminq_get_ptype_map_cnt = 0;
priv->adminq_cfg_flow_rule_cnt = 0;
+ priv->adminq_report_nic_timestamp_cnt = 0;
+
pthread_mutexattr_init(&mutexattr);
pthread_mutexattr_setpshared(&mutexattr, PTHREAD_PROCESS_SHARED);
pthread_mutex_init(&priv->adminq_lock, &mutexattr);
@@ -522,6 +524,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);
}
@@ -636,6 +642,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.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 2/6] net/gve: add device option support for HW timestamps
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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 743ab8e7ae..1ced1e442e 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;
}
@@ -920,7 +941,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) &&
@@ -947,6 +969,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)
@@ -954,6 +981,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;
@@ -983,7 +1011,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;
@@ -1038,7 +1067,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.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 1/6] net/gve: add thread safety to admin queue
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260605213022.2770893-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>
---
v2:
- Dropped ROBUST mutex attribute.
---
.mailmap | 1 +
drivers/net/gve/base/gve_adminq.c | 67 +++++++++++++++++++++++++------
drivers/net/gve/gve_ethdev.h | 1 +
3 files changed, 56 insertions(+), 13 deletions(-)
diff --git a/.mailmap b/.mailmap
index e052b85213..1b10cfca35 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1016,6 +1016,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..743ab8e7ae 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,11 @@ 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_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 +310,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 +425,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 +519,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 +710,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 +770,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 +808,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 +846,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.1032.g2f8565e1d1-goog
^ permalink raw reply related
* [PATCH v3 0/6] net/gve: add hardware timestamping support
From: Mark Blasko @ 2026-06-05 21:29 UTC (permalink / raw)
To: stephen; +Cc: dev, joshwash, jtranoleary, blasko
In-Reply-To: <20260515231936.3296603-1-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.
---
v3:
- Patch 5:
- Add mutex lock to protect shared NIC timestamp memzone access.
- Fix missing read_clock assignment to DQO queue ops table
(accidental omission in v2).
v2:
- Patch 1: Dropped ROBUST mutex attribute.
- Patch 3: Added adminq timestamp counter reset to gve_adminq_alloc.
- Patch 4:
- Removed redundant void* casts.
- Handled alarm reschedule failures by marking timestamp stale.
- Added transient error logging on memzone allocation failure.
- Patch 5: Scoped read_clock ethdev operation strictly to DQO queues.
- Patch 6:
- Scoped timestamp offload capability advertisement strictly to
DQO queues.
- Predicated capability advertisement directly on memzone
allocation.
- Initialized mbuf_timestamp_offset to -1.
- Added blank line separating release notes.
---
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 | 20 ++++
doc/guides/rel_notes/release_26_07.rst | 4 +
drivers/net/gve/base/gve_adminq.c | 128 +++++++++++++++++---
drivers/net/gve/base/gve_adminq.h | 29 +++++
drivers/net/gve/base/gve_desc_dqo.h | 8 +-
drivers/net/gve/gve_ethdev.c | 159 ++++++++++++++++++++++++-
drivers/net/gve/gve_ethdev.h | 40 +++++++
drivers/net/gve/gve_rx_dqo.c | 26 ++++
10 files changed, 394 insertions(+), 22 deletions(-)
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply
* Re: [PATCH v8 9/9] dts: add selective Rx tests
From: Stephen Hemminger @ 2026-06-05 21:28 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev, Luca Vizzarro, Patrick Robb
In-Reply-To: <20260604193324.1996141-10-thomas@monjalon.net>
On Thu, 4 Jun 2026 21:31:01 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:
> Add TestSuite_rx_split with 7 test cases:
> - 3 positive: headers only, payload only, two non-contiguous segments
> - 4 negative: missing offload flag, out-of-range, overlap, all-discard
>
> Add selective Rx capability detection via testpmd "show port info".
>
> The test suite could be completed later for the basic buffer split
> configuration based on offsets or protocols.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
AI review found:
Patch 9 (dts: add selective Rx tests)
selective_rx_out_of_range expects a rejection that never happens, so the
negative test will fail. It configures a real segment plus an oversized
discard segment:
rx_segments_length=[ETHER_IP_HDR_LEN, 20000],
mbuf_size=[256, 0],
and expects start_all_ports() to fail. But an over-range length on a discard
segment is not rejected anywhere: rte_eth_rx_queue_check_split() does
"continue" for mp == NULL segments, so it never length-checks them, and
mlx5_rxq_new() clamps it:
if (seg_len > tail_len)
seg_len = qs_seg->mp != NULL ? buf_len - offset : tail_len;
The discard seg_len becomes the remaining frame length, the queue is built,
the port starts, and the test hits its fail().
Clamping an over-long discard to "the rest" is harmless (the bytes are
discarded anyway), so the cleanest fix is probably to drop or rework this
test rather than add a rejection path. If rejection is the intended
contract, it would have to be added for discard segments in patch 2 or
patch 6 -- a behavior choice, not a correctness requirement.
Minor: expressing a leading discard as --mbuf-size=0,... puts 0 at index 0,
and testpmd treats mbuf_data_size[0] as the primary pool size elsewhere (the
max_rx_pkt_len > mbuf_data_size[0] check, the default mbuf_pool_find(socket,
0)). Only bites an unusual config, but it is a latent foot-gun.
^ permalink raw reply
* [PATCH 8/8] eventdev/timer: reject out-of-range ID
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Erik Gabriel Carrillo, Jerin Jacob,
Ankur Dwivedi
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The eventdev timer adapter code was using atoi() to parse numeric
parameters which does not handle out-of-range or extra garbage
on input. Tighten the code to only accept valid numbers.
Fixes: 791dfec24d00 ("eventdev/timer: add telemetry")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eventdev/rte_event_timer_adapter.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/lib/eventdev/rte_event_timer_adapter.c b/lib/eventdev/rte_event_timer_adapter.c
index af98b1d9f6..e3640a3bf8 100644
--- a/lib/eventdev/rte_event_timer_adapter.c
+++ b/lib/eventdev/rte_event_timer_adapter.c
@@ -1402,16 +1402,15 @@ handle_ta_info(const char *cmd __rte_unused, const char *params,
{
struct rte_event_timer_adapter_info adapter_info;
struct rte_event_timer_adapter *adapter;
- uint16_t adapter_id;
+ unsigned long adapter_id;
int ret;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
- adapter_id = atoi(params);
-
+ adapter_id = strtoul(params, NULL, 10);
if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
- EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
+ EVTIM_LOG_ERR("Invalid timer adapter id %lu", adapter_id);
return -EINVAL;
}
@@ -1419,7 +1418,7 @@ handle_ta_info(const char *cmd __rte_unused, const char *params,
ret = rte_event_timer_adapter_get_info(adapter, &adapter_info);
if (ret < 0) {
- EVTIM_LOG_ERR("Failed to get info for timer adapter id %u", adapter_id);
+ EVTIM_LOG_ERR("Failed to get info for timer adapter id %lu", adapter_id);
return ret;
}
@@ -1448,16 +1447,15 @@ handle_ta_stats(const char *cmd __rte_unused, const char *params,
{
struct rte_event_timer_adapter_stats stats;
struct rte_event_timer_adapter *adapter;
- uint16_t adapter_id;
+ unsigned long adapter_id;
int ret;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
- adapter_id = atoi(params);
-
+ adapter_id = strtoul(params, NULL, 10);
if (adapters == NULL || adapter_id >= RTE_EVENT_TIMER_ADAPTER_NUM_MAX) {
- EVTIM_LOG_ERR("Invalid timer adapter id %u", adapter_id);
+ EVTIM_LOG_ERR("Invalid timer adapter id %lu", adapter_id);
return -EINVAL;
}
@@ -1465,7 +1463,7 @@ handle_ta_stats(const char *cmd __rte_unused, const char *params,
ret = rte_event_timer_adapter_stats_get(adapter, &stats);
if (ret < 0) {
- EVTIM_LOG_ERR("Failed to get stats for timer adapter id %u", adapter_id);
+ EVTIM_LOG_ERR("Failed to get stats for timer adapter id %lu", adapter_id);
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 7/8] eventdev/eth_rx: reject out-of-range telemetry adapter ID
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Naga Harish K S V, Jerin Jacob,
Ganapati Kundapura, Jay Jayatheerthan
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The eventdev rx adapter code was using atoi() to parse numeric
parameters which does not handle out-of-range or extra garbage
on input. Tighten the code to only accept valid numbers.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eventdev/rte_event_eth_rx_adapter.c | 45 +++++++++++--------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 96a4a0d926..635bd6014b 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -270,8 +270,8 @@ rxa_timestamp_dynfield(struct rte_mbuf *mbuf)
event_eth_rx_timestamp_dynfield_offset, rte_mbuf_timestamp_t *);
}
-static inline int
-rxa_validate_id(uint8_t id)
+static inline bool
+rxa_validate_id(unsigned long id)
{
return id < RTE_EVENT_ETH_RX_ADAPTER_MAX_INSTANCE;
}
@@ -294,14 +294,14 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(id, retval) do { \
if (!rxa_validate_id(id)) { \
- RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d", id); \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %lu", (unsigned long)id); \
return retval; \
} \
} while (0)
#define RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(id, retval) do { \
if (!rxa_validate_id(id)) { \
- RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %d", id); \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter id = %lu", (unsigned long)id); \
ret = retval; \
goto error; \
} \
@@ -316,8 +316,8 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} while (0)
#define RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(port_id, retval) do { \
- if (!rte_eth_dev_is_valid_port(port_id)) { \
- RTE_EDEV_LOG_ERR("Invalid port_id=%u", port_id); \
+ if (port_id >= RTE_MAX_ETHPORTS || !rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_EDEV_LOG_ERR("Invalid port_id=%lu", (unsigned long)port_id); \
ret = retval; \
goto error; \
} \
@@ -3761,14 +3761,14 @@ handle_rxa_stats(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- uint8_t rx_adapter_id;
+ unsigned long rx_adapter_id;
struct rte_event_eth_rx_adapter_stats rx_adptr_stats;
if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
- rx_adapter_id = atoi(params);
+ rx_adapter_id = strtoul(params, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
/* Get Rx adapter stats */
@@ -3802,13 +3802,13 @@ handle_rxa_stats_reset(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d __rte_unused)
{
- uint8_t rx_adapter_id;
+ unsigned long rx_adapter_id;
if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
- rx_adapter_id = atoi(params);
+ rx_adapter_id = strtoul(params, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_ERR_RET(rx_adapter_id, -EINVAL);
/* Reset Rx adapter stats */
@@ -3825,9 +3825,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- uint8_t rx_adapter_id;
- uint16_t rx_queue_id;
- uint16_t eth_dev_id;
+ unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
int ret = -1;
char *token, *l_params, *saveptr = NULL;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3857,7 +3855,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
- RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
ret = -EINVAL;
goto error;
}
@@ -3898,9 +3896,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- uint8_t rx_adapter_id;
- uint16_t rx_queue_id;
- uint16_t eth_dev_id;
+ unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
int ret = -1;
char *token, *l_params, *saveptr = NULL;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3930,7 +3926,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
- RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
ret = -EINVAL;
goto error;
}
@@ -3970,9 +3966,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d __rte_unused)
{
- uint8_t rx_adapter_id;
- uint16_t rx_queue_id;
- uint16_t eth_dev_id;
+ unsigned long rx_adapter_id, rx_queue_id, eth_dev_id;
int ret = -1;
char *token, *l_params, *saveptr = NULL;
@@ -4001,7 +3995,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
- RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
ret = -EINVAL;
goto error;
}
@@ -4033,8 +4027,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
struct rte_tel_data *d)
{
uint8_t instance_id;
- uint16_t rx_queue_id;
- uint16_t eth_dev_id;
+ unsigned long rx_queue_id, eth_dev_id;
int ret = -1;
char *token, *l_params, *saveptr = NULL;
@@ -4057,7 +4050,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
/* Get Rx queue ID from parameter string */
rx_queue_id = strtoul(token, NULL, 10);
if (rx_queue_id >= rte_eth_devices[eth_dev_id].data->nb_rx_queues) {
- RTE_EDEV_LOG_ERR("Invalid rx queue_id %u", rx_queue_id);
+ RTE_EDEV_LOG_ERR("Invalid rx queue_id %lu", rx_queue_id);
ret = -EINVAL;
goto error;
}
@@ -4074,7 +4067,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
rx_queue_id,
&instance_id)) {
RTE_EDEV_LOG_ERR("Failed to get RX adapter instance ID "
- " for rx_queue_id = %d", rx_queue_id);
+ " for rx_queue_id = %lu", rx_queue_id);
return -1;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 6/8] eventdev/eth_rx: fix thread-unsafe telemetry parsing
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Naga Harish K S V, Jerin Jacob,
Ganapati Kundapura, Jay Jayatheerthan
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The eth Rx adapter telemetry handlers parse multi-field parameter
strings with a strtok()/strtok(NULL, ...) continuation chain.
Since strtok() holds its parser state in a process-global static,
and telemetry callbacks run in a separate thread per client connection.
With concurrent clients the continuation calls read another thread's
state - and since each handler strdup()s and frees its own buffer,
a stale continuation can dereference freed memory.
Thread the parse through a local save pointer with strtok_r().
Also passing was passing a char to isdigit(), which is undefined
in C standard for high-bit input.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eventdev/rte_event_eth_rx_adapter.c | 52 ++++++++++++-------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
index 2183adce6f..96a4a0d926 100644
--- a/lib/eventdev/rte_event_eth_rx_adapter.c
+++ b/lib/eventdev/rte_event_eth_rx_adapter.c
@@ -308,7 +308,7 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} while (0)
#define RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, retval) do { \
- if ((token) == NULL || strlen(token) == 0 || !isdigit(*token)) { \
+ if ((token) == NULL || strlen(token) == 0 || !isdigit((unsigned char)*token)) { \
RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token"); \
ret = retval; \
goto error; \
@@ -3764,7 +3764,7 @@ handle_rxa_stats(const char *cmd __rte_unused,
uint8_t rx_adapter_id;
struct rte_event_eth_rx_adapter_stats rx_adptr_stats;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
@@ -3804,7 +3804,7 @@ handle_rxa_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
@@ -3829,29 +3829,29 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
uint16_t rx_queue_id;
uint16_t eth_dev_id;
int ret = -1;
- char *token, *l_params;
+ char *token, *l_params, *saveptr = NULL;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
if (l_params == NULL)
return -ENOMEM;
- token = strtok(l_params, ",");
+ token = strtok_r(l_params, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
@@ -3862,7 +3862,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
goto error;
}
- token = strtok(NULL, "\0");
+ token = strtok_r(NULL, "\0", &saveptr);
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
@@ -3902,29 +3902,29 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
uint16_t rx_queue_id;
uint16_t eth_dev_id;
int ret = -1;
- char *token, *l_params;
+ char *token, *l_params, *saveptr = NULL;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
if (l_params == NULL)
return -ENOMEM;
- token = strtok(l_params, ",");
+ token = strtok_r(l_params, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
@@ -3935,7 +3935,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
goto error;
}
- token = strtok(NULL, "\0");
+ token = strtok_r(NULL, "\0", &saveptr);
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
@@ -3974,28 +3974,28 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
uint16_t rx_queue_id;
uint16_t eth_dev_id;
int ret = -1;
- char *token, *l_params;
+ char *token, *l_params, *saveptr = NULL;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
if (l_params == NULL)
return -ENOMEM;
- token = strtok(l_params, ",");
+ token = strtok_r(l_params, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
rx_adapter_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
@@ -4006,7 +4006,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
goto error;
}
- token = strtok(NULL, "\0");
+ token = strtok_r(NULL, "\0", &saveptr);
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
@@ -4036,22 +4036,22 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
uint16_t rx_queue_id;
uint16_t eth_dev_id;
int ret = -1;
- char *token, *l_params;
+ char *token, *l_params, *saveptr = NULL;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (params == NULL || strlen(params) == 0 || !isdigit((unsigned char)*params))
return -1;
l_params = strdup(params);
if (l_params == NULL)
return -ENOMEM;
- token = strtok(l_params, ",");
+ token = strtok_r(l_params, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get device ID from parameter string */
eth_dev_id = strtoul(token, NULL, 10);
RTE_EVENT_ETH_RX_ADAPTER_PORTID_VALID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
- token = strtok(NULL, ",");
+ token = strtok_r(NULL, ",", &saveptr);
RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* Get Rx queue ID from parameter string */
@@ -4062,7 +4062,7 @@ handle_rxa_instance_get(const char *cmd __rte_unused,
goto error;
}
- token = strtok(NULL, "\0");
+ token = strtok_r(NULL, "\0", &saveptr);
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
--
2.53.0
^ permalink raw reply related
* [PATCH 5/8] eventdev: remove strtok from telemetry handlers
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, stable, Jerin Jacob, Bruce Richardson
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The eventdev telemetry command handlers parsed parameters with strtok()
which is not thread safe. The code also has silent truncation
issues since it assigns result of strtoul() to smaller types.
Introduce common helper that checks parameter string
and gets a device id. Make sure and range check values
to the appropriate upper bound for that parameter.
Fixes: feaea0573429 ("eventdev: add device info telemetry command")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/eventdev/rte_eventdev.c | 136 +++++++++++++++++-------------------
1 file changed, 64 insertions(+), 72 deletions(-)
diff --git a/lib/eventdev/rte_eventdev.c b/lib/eventdev/rte_eventdev.c
index 572cd5bd7d..a6f6b6879e 100644
--- a/lib/eventdev/rte_eventdev.c
+++ b/lib/eventdev/rte_eventdev.c
@@ -1790,6 +1790,34 @@ handle_dev_list(const char *cmd __rte_unused,
return 0;
}
+/* Get dev ID from parameter string */
+static int
+parse_dev_id(const char *params, uint8_t *dev_id, char **next_param)
+{
+ char *end_param;
+ unsigned long id;
+
+ if (params == NULL || *params == '\0' ||
+ !isdigit((unsigned char)*params))
+ return -1;
+
+ id = strtoul(params, &end_param, 10);
+ if (next_param != NULL) {
+ *next_param = end_param;
+ } else if (*end_param != '\0') {
+ RTE_EDEV_LOG_DEBUG(
+ "Extra parameters passed to eventdev telemetry command, ignoring");
+ }
+
+ if (id >= RTE_EVENT_MAX_DEVS) {
+ RTE_EDEV_LOG_DEBUG(
+ "Invalid device id out of range %lu", id);
+ return -1;
+ }
+ *dev_id = id;
+ return 0;
+}
+
static int
handle_dev_info(const char *cmd __rte_unused,
const char *params,
@@ -1797,16 +1825,10 @@ handle_dev_info(const char *cmd __rte_unused,
{
uint8_t dev_id;
struct rte_eventdev *dev;
- char *end_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, NULL) < 0)
return -1;
- dev_id = strtoul(params, &end_param, 10);
- if (*end_param != '\0')
- RTE_EDEV_LOG_DEBUG(
- "Extra parameters passed to eventdev telemetry command, ignoring");
-
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
dev = &rte_eventdevs[dev_id];
@@ -1833,16 +1855,10 @@ handle_port_list(const char *cmd __rte_unused,
int i;
uint8_t dev_id;
struct rte_eventdev *dev;
- char *end_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, NULL) < 0)
return -1;
- dev_id = strtoul(params, &end_param, 10);
- if (*end_param != '\0')
- RTE_EDEV_LOG_DEBUG(
- "Extra parameters passed to eventdev telemetry command, ignoring");
-
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
dev = &rte_eventdevs[dev_id];
@@ -1861,16 +1877,10 @@ handle_queue_list(const char *cmd __rte_unused,
int i;
uint8_t dev_id;
struct rte_eventdev *dev;
- char *end_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, NULL) != 0)
return -1;
- dev_id = strtoul(params, &end_param, 10);
- if (*end_param != '\0')
- RTE_EDEV_LOG_DEBUG(
- "Extra parameters passed to eventdev telemetry command, ignoring");
-
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
dev = &rte_eventdevs[dev_id];
@@ -1886,27 +1896,28 @@ handle_queue_links(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- int i, ret, port_id = 0;
+ int i, ret;
char *end_param;
uint8_t dev_id;
uint8_t queues[RTE_EVENT_MAX_QUEUES_PER_DEV];
uint8_t priorities[RTE_EVENT_MAX_QUEUES_PER_DEV];
- const char *p_param;
+ unsigned long port_id;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, &end_param) != 0)
return -1;
- /* Get dev ID from parameter string */
- dev_id = strtoul(params, &end_param, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
- p_param = strtok(end_param, ",");
- if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+ if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
return -1;
- port_id = strtoul(p_param, &end_param, 10);
- p_param = strtok(NULL, "\0");
- if (p_param != NULL)
+ port_id = strtoul(end_param + 1, &end_param, 10);
+ if (port_id >= RTE_EVENT_MAX_PORTS_PER_DEV) {
+ RTE_EDEV_LOG_DEBUG("Invalid port id out of range %lu", port_id);
+ return -1;
+ }
+
+ if (*end_param != '\0')
RTE_EDEV_LOG_DEBUG(
"Extra parameters passed to eventdev telemetry command, ignoring");
@@ -1998,19 +2009,12 @@ handle_dev_xstats(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- int dev_id;
+ uint8_t dev_id;
enum rte_event_dev_xstats_mode mode;
- char *end_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, NULL) != 0)
return -1;
- /* Get dev ID from parameter string */
- dev_id = strtoul(params, &end_param, 10);
- if (*end_param != '\0')
- RTE_EDEV_LOG_DEBUG(
- "Extra parameters passed to eventdev telemetry command, ignoring");
-
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
mode = RTE_EVENT_DEV_XSTATS_DEVICE;
@@ -2022,29 +2026,25 @@ handle_port_xstats(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- int dev_id;
- int port_queue_id = 0;
+ uint8_t dev_id;
enum rte_event_dev_xstats_mode mode;
+ unsigned long port_queue_id;
char *end_param;
- const char *p_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, &end_param) != 0)
return -1;
-
- /* Get dev ID from parameter string */
- dev_id = strtoul(params, &end_param, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
- p_param = strtok(end_param, ",");
mode = RTE_EVENT_DEV_XSTATS_PORT;
- if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+ if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
return -1;
- port_queue_id = strtoul(p_param, &end_param, 10);
+ port_queue_id = strtoul(end_param + 1, &end_param, 10);
+ if (port_queue_id >= RTE_EVENT_MAX_PORTS_PER_DEV)
+ return -1;
- p_param = strtok(NULL, "\0");
- if (p_param != NULL)
+ if (*end_param != '\0')
RTE_EDEV_LOG_DEBUG(
"Extra parameters passed to eventdev telemetry command, ignoring");
@@ -2056,29 +2056,26 @@ handle_queue_xstats(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- int dev_id;
- int port_queue_id = 0;
+ uint8_t dev_id;
+ unsigned long port_queue_id;
enum rte_event_dev_xstats_mode mode;
char *end_param;
- const char *p_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, &end_param) != 0)
return -1;
- /* Get dev ID from parameter string */
- dev_id = strtoul(params, &end_param, 10);
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
- p_param = strtok(end_param, ",");
mode = RTE_EVENT_DEV_XSTATS_QUEUE;
- if (p_param == NULL || strlen(p_param) == 0 || !isdigit(*p_param))
+ if (*end_param != ',' || !isdigit((unsigned char)end_param[1]))
return -1;
- port_queue_id = strtoul(p_param, &end_param, 10);
+ port_queue_id = strtoul(end_param + 1, &end_param, 10);
+ if (port_queue_id >= RTE_EVENT_MAX_QUEUES_PER_DEV)
+ return -1;
- p_param = strtok(NULL, "\0");
- if (p_param != NULL)
+ if (*end_param != '\0')
RTE_EDEV_LOG_DEBUG(
"Extra parameters passed to eventdev telemetry command, ignoring");
@@ -2090,19 +2087,14 @@ handle_dev_dump(const char *cmd __rte_unused,
const char *params,
struct rte_tel_data *d)
{
- char *buf, *end_param;
- int dev_id, ret;
+ char *buf;
+ uint8_t dev_id;
+ int ret;
FILE *f;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ if (parse_dev_id(params, &dev_id, NULL) != 0)
return -1;
- /* Get dev ID from parameter string */
- dev_id = strtoul(params, &end_param, 10);
- if (*end_param != '\0')
- RTE_EDEV_LOG_DEBUG(
- "Extra parameters passed to eventdev telemetry command, ignoring");
-
RTE_EVENTDEV_VALID_DEVID_OR_ERR_RET(dev_id, -EINVAL);
buf = calloc(RTE_TEL_MAX_SINGLE_STRING_LEN, sizeof(char));
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] security: harden telemetry parameter parsing
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Akhil Goyal, Anoob Joseph,
Gowrishankar Muthukrishnan
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The cryptodev security telemetry handlers parsed dev_id/capa_id with
strtoul() and no overflow or range check, so an out-of-range dev_id
(e.g. 256) silently truncated to a valid device in
rte_cryptodev_is_valid_dev(). isdigit() was also called on a plain
(signed) char, which is undefined for high-bit input.
The parser was also using strtok() which is not thread safe.
Use a validated parse helper and reject malformed input rather than
logging and continuing. This also drops the thread-unsafe strtok() in
the crypto_caps handler.
Fixes: 259ca6d1617f ("security: add telemetry endpoint for capabilities")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/security/rte_security.c | 41 ++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index c47fe44da0..0d89f8af3f 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -7,6 +7,8 @@
#include <stdalign.h>
#include <ctype.h>
#include <stdlib.h>
+#include <errno.h>
+#include <limits.h>
#include <eal_export.h>
#include <rte_cryptodev.h>
@@ -474,6 +476,25 @@ security_capabilities_from_dev_id(int dev_id, const void **caps)
return 0;
}
+/* Parse an unsigned integer parameter, returning the value or -EINVAL.
+ * 'max' must be <= INT_MAX.
+ */
+static int
+telemetry_parse_uint(const char *str, char **end, unsigned long max)
+{
+ unsigned long val;
+
+ if (str == NULL || !isdigit((unsigned char)*str))
+ return -EINVAL;
+
+ errno = 0;
+ val = strtoul(str, end, 0);
+ if (errno != 0 || val > max)
+ return -EINVAL;
+
+ return (int)val;
+}
+
static int
security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *params,
struct rte_tel_data *d)
@@ -485,13 +506,10 @@ security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *par
int dev_id;
int rc;
- if (!params || strlen(params) == 0 || !isdigit(*params))
+ dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+ if (dev_id < 0 || *end_param != '\0')
return -EINVAL;
- dev_id = strtoul(params, &end_param, 0);
- if (*end_param != '\0')
- CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
if (rc < 0)
return rc;
@@ -513,24 +531,19 @@ security_handle_cryptodev_crypto_caps(const char *cmd __rte_unused, const char *
{
const struct rte_security_capability *capabilities;
struct rte_tel_data *crypto_caps;
- const char *capa_param;
int dev_id, capa_id;
int crypto_caps_n;
char *end_param;
int rc;
- if (!params || strlen(params) == 0 || !isdigit(*params))
+ dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+ if (dev_id < 0 || *end_param != ',')
return -EINVAL;
- dev_id = strtoul(params, &end_param, 0);
- capa_param = strtok(end_param, ",");
- if (!capa_param || strlen(capa_param) == 0 || !isdigit(*capa_param))
+ capa_id = telemetry_parse_uint(end_param + 1, &end_param, INT_MAX);
+ if (capa_id < 0 || *end_param != '\0')
return -EINVAL;
- capa_id = strtoul(capa_param, &end_param, 0);
- if (*end_param != '\0')
- CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
if (rc < 0)
return rc;
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] dmadev: validate telemetry parameters
From: Stephen Hemminger @ 2026-06-05 20:51 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Chengwen Feng, Kevin Laatz,
Bruce Richardson, Conor Walsh, Sean Morrissey
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
Tighten parsing of the dmadev telemetry device and vchan parameters:
reject non-numeric and out-of-range ids through a bounded helper rather
than narrowing strtoul()'s result to int and leaning on the downstream
int16_t/uint16_t API to revalidate. This also drops the thread-unsafe
strtok() in the stats handler.
Fixes: 39b5ab60df30 ("dmadev: add telemetry")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/dmadev/rte_dmadev.c | 44 ++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 14 deletions(-)
diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index b75b4f9bd1..822bb7c89f 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -4,6 +4,7 @@
*/
#include <ctype.h>
+#include <errno.h>
#include <inttypes.h>
#include <stdlib.h>
@@ -1157,6 +1158,25 @@ dmadev_handle_dev_list(const char *cmd __rte_unused,
return 0;
}
+/* Parse an unsigned integer telemetry parameter, returning the value or
+ * -EINVAL. 'max' must be <= INT_MAX.
+ */
+static int
+dmadev_parse_uint(const char *str, char **end, unsigned long max)
+{
+ unsigned long val;
+
+ if (str == NULL || !isdigit((unsigned char)*str))
+ return -EINVAL;
+
+ errno = 0;
+ val = strtoul(str, end, 0);
+ if (errno != 0 || val > max)
+ return -EINVAL;
+
+ return (int)val;
+}
+
#define ADD_CAPA(td, dc, c) rte_tel_data_add_dict_int(td, dma_capability_name(c), !!(dc & c))
static int
@@ -1169,10 +1189,9 @@ dmadev_handle_dev_info(const char *cmd __rte_unused,
uint64_t dev_capa;
char *end_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+ if (dev_id < 0)
return -EINVAL;
-
- dev_id = strtoul(params, &end_param, 0);
if (*end_param != '\0')
RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
@@ -1227,13 +1246,11 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
struct rte_dma_stats dma_stats;
int dev_id, ret, vchan_id;
char *end_param;
- const char *vchan_param;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+ if (dev_id < 0)
return -EINVAL;
- dev_id = strtoul(params, &end_param, 0);
-
/* Function info_get validates dev_id so we don't need to. */
ret = rte_dma_info_get(dev_id, &dma_info);
if (ret < 0)
@@ -1245,11 +1262,11 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
if (dma_info.nb_vchans == 1 && *end_param == '\0')
vchan_id = 0;
else {
- vchan_param = strtok(end_param, ",");
- if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+ if (*end_param != ',')
+ return -EINVAL;
+ vchan_id = dmadev_parse_uint(end_param + 1, &end_param, UINT16_MAX);
+ if (vchan_id < 0)
return -EINVAL;
-
- vchan_id = strtoul(vchan_param, &end_param, 0);
}
if (*end_param != '\0')
RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
@@ -1276,10 +1293,9 @@ dmadev_handle_dev_dump(const char *cmd __rte_unused,
int dev_id, ret;
FILE *f;
- if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+ dev_id = dmadev_parse_uint(params, &end_param, INT16_MAX);
+ if (dev_id < 0)
return -EINVAL;
-
- dev_id = strtoul(params, &end_param, 0);
if (*end_param != '\0')
RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] ethdev: make telemetry parameter parsing thread-safe
From: Stephen Hemminger @ 2026-06-05 20:50 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Thomas Monjalon, Andrew Rybchenko,
Ferruh Yigit, Jie Hai
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The ethdev telemetry handlers run in a per-connection thread.
Two of the parameter parsers used strtok(), which keeps its state in a
process-global static shared across threads.
Use strtok_r() with a local save pointer.
Also pass an unsigned char to isdigit(),
which is undefined for characters with high-bit set.
Fixes: 9e7533aeb80a ("ethdev: add telemetry command for TM level capabilities")
Fixes: f38f62650f7b ("ethdev: add Rx queue telemetry query")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/ethdev/rte_ethdev_telemetry.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/ethdev/rte_ethdev_telemetry.c b/lib/ethdev/rte_ethdev_telemetry.c
index a910864bc5..ca7f4681c9 100644
--- a/lib/ethdev/rte_ethdev_telemetry.c
+++ b/lib/ethdev/rte_ethdev_telemetry.c
@@ -32,7 +32,7 @@ eth_dev_parse_port_params(const char *params, uint16_t *port_id,
uint64_t pi;
if (params == NULL || strlen(params) == 0 ||
- !isdigit(*params) || port_id == NULL)
+ !isdigit((unsigned char)*params) || port_id == NULL)
return -EINVAL;
pi = strtoul(params, end_param, 0);
@@ -459,6 +459,7 @@ ethdev_parse_queue_params(const char *params, bool is_rx,
const char *qid_param;
uint16_t nb_queues;
char *end_param;
+ char *saveptr = NULL;
uint64_t qid;
int ret;
@@ -471,8 +472,8 @@ ethdev_parse_queue_params(const char *params, bool is_rx,
if (nb_queues == 1 && *end_param == '\0')
qid = 0;
else {
- qid_param = strtok(end_param, ",");
- if (!qid_param || strlen(qid_param) == 0 || !isdigit(*qid_param))
+ qid_param = strtok_r(end_param, ",", &saveptr);
+ if (!qid_param || strlen(qid_param) == 0 || !isdigit((unsigned char)*qid_param))
return -EINVAL;
qid = strtoul(qid_param, &end_param, 0);
@@ -1207,10 +1208,11 @@ static int
eth_dev_parse_tm_params(char *params, uint32_t *result)
{
const char *splited_param;
+ char *saveptr = NULL;
uint64_t ret;
- splited_param = strtok(params, ",");
- if (!splited_param || strlen(splited_param) == 0 || !isdigit(*splited_param))
+ splited_param = strtok_r(params, ",", &saveptr);
+ if (!splited_param || strlen(splited_param) == 0 || !isdigit((unsigned char)*splited_param))
return -EINVAL;
ret = strtoul(splited_param, ¶ms, 0);
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] telemetry: fix thread-unsafe command parsing
From: Stephen Hemminger @ 2026-06-05 20:50 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger, stable, Bruce Richardson, Ciara Power,
Keith Wiles
In-Reply-To: <20260605205253.520196-1-stephen@networkplumber.org>
The telemetry client_handler() runs in a detached thread per connection,
and up to MAX_CONNECTIONS instances can run concurrently.
The function strtok() keeps parser state in a static variable
shared across all threads, so concurrent clients corrupt each other's
command parsing. Use strtok_r() with a local saveptr.
Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/telemetry/telemetry.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index b109d076d4..e591c1e283 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -415,8 +415,9 @@ client_handler(void *sock_id)
int bytes = read(s, buffer, sizeof(buffer) - 1);
while (bytes > 0) {
buffer[bytes] = 0;
- const char *cmd = strtok(buffer, ",");
- const char *param = strtok(NULL, "\0");
+ char *saveptr = NULL;
+ const char *cmd = strtok_r(buffer, ",", &saveptr);
+ const char *param = strtok_r(NULL, "\0", &saveptr);
struct cmd_callback cb = {.fn = unknown_command};
int i;
--
2.53.0
^ permalink raw reply related
* [PATCH 0/8] telemetry: thread-safe and bounded parameter parsing
From: Stephen Hemminger @ 2026-06-05 20:50 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
While looking into extending telemetry for other uses, I noticed a
pattern of unsafe string handling in the command handlers. They run one
thread per client connection but parse parameters with non-reentrant
strtok(), and convert ids with atoi()/unchecked strtoul() that silently
truncate or alias out-of-range values; in eth_rx the strtok()
continuation chain can also dereference freed memory.
This series covers the library code (telemetry, ethdev, dmadev, security,
eventdev, eth_rx, timer). A follow-up is needed for the same strtok()
use in drivers.
They are marked for stable: the races and the use-after-free are real and
the changes are low-risk to backport. But severity is low since telemetry is
not a remote interface, but these are the kind of issues likely to
be found by AI security scanning tools.
In future, atoi() and strtok() look worth adding to the forbidden
tokens list in devtools/checkpatches.sh.
Stephen Hemminger (8):
telemetry: fix thread-unsafe command parsing
ethdev: make telemetry parameter parsing thread-safe
dmadev: validate telemetry parameters
security: harden telemetry parameter parsing
eventdev: remove strtok from telemetry handlers
eventdev/eth_rx: fix thread-unsafe telemetry parsing
eventdev/eth_rx: reject out-of-range telemetry adapter ID
eventdev/timer: reject out-of-range ID
lib/dmadev/rte_dmadev.c | 44 +++++---
lib/ethdev/rte_ethdev_telemetry.c | 12 ++-
lib/eventdev/rte_event_eth_rx_adapter.c | 97 ++++++++---------
lib/eventdev/rte_event_timer_adapter.c | 22 ++--
lib/eventdev/rte_eventdev.c | 136 +++++++++++-------------
lib/security/rte_security.c | 41 ++++---
lib/telemetry/telemetry.c | 5 +-
7 files changed, 186 insertions(+), 171 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH] net/iavf: fix duplicate VF reset during PF reset recovery
From: Anurag Mandal @ 2026-06-05 20:29 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, vladimir.medvedkin, Anurag Mandal, stable
During PF initiated reset recovery, iavf_dev_close() sending
an extra VIRTCHNL_OP_RESET_VF while recovery is already in progress.
That second reset can leave PF/VF virtchnl state inconsistent and
cause VIRTCHNL_OP_CONFIG_VSI_QUEUES to fail with ERR_PARAM after
ToR link flap, leaving the VF unable to recover.
This results in connection loss.
Skipped close-time VF reset and related close-time virtchnl
operations when PF triggered reset recovery is set. This is
done to avoid a duplicate VF reset, and keep normal behavior
for application-driven close.
Handled link-change events through a common static function that
reads the correct advanced & legacy link fields properly and
updates no-poll/watchdog/LSC state consistently.
Also added IAVF_ERR_ADMIN_QUEUE_NO_WORK in virtchnl message
drain as a normal empty-queue condition and avoid logging it as
an misleading AQ failure.
Fixes: 675a104e2e94 ("net/iavf: fix abnormal disable HW interrupt")
Fixes: b34fe66ea893 ("net/iavf: delay VF reset command")
Fixes: 5e03e316c753 ("net/iavf: handle virtchnl event message without interrupt")
Fixes: 5c8ca9f13c78 ("net/iavf: fix no polling mode switching")
Fixes: 48de41ca11f0 ("net/avf: enable link status update")
Fixes: 02d212ca3125 ("net/iavf: rename remaining avf strings")
Cc: stable@dpdk.org
Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
drivers/net/intel/iavf/iavf_ethdev.c | 36 ++++----
drivers/net/intel/iavf/iavf_vchnl.c | 130 +++++++++++++++------------
2 files changed, 95 insertions(+), 71 deletions(-)
diff --git a/drivers/net/intel/iavf/iavf_ethdev.c b/drivers/net/intel/iavf/iavf_ethdev.c
index bdf650b822..a936748397 100644
--- a/drivers/net/intel/iavf/iavf_ethdev.c
+++ b/drivers/net/intel/iavf/iavf_ethdev.c
@@ -3166,24 +3166,26 @@ iavf_dev_close(struct rte_eth_dev *dev)
ret = iavf_dev_stop(dev);
- /*
- * Release redundant queue resource when close the dev
- * so that other vfs can re-use the queues.
- */
- if (vf->lv_enabled) {
- ret = iavf_request_queues(dev, IAVF_MAX_NUM_QUEUES_DFLT);
- if (ret)
- PMD_DRV_LOG(ERR, "Reset the num of queues failed");
+ /* Skip RESET_VF on a PF-initiated reset */
+ if (!vf->in_reset_recovery) {
+ /*
+ * Release redundant queue resource when close the dev
+ * so that other vfs can re-use the queues.
+ */
+ if (vf->lv_enabled) {
+ ret = iavf_request_queues(dev, IAVF_MAX_NUM_QUEUES_DFLT);
+ if (ret)
+ PMD_DRV_LOG(ERR, "Reset the num of queues failed");
+ vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
+ }
- vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
+ /* Disable promiscuous mode before resetting the VF. This is to avoid
+ * potential issues when the PF is bound to the kernel driver.
+ */
+ if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled)
+ iavf_config_promisc(adapter, false, false);
}
- /* Disable promiscuous mode before resetting the VF. This is to avoid
- * potential issues when the PF is bound to the kernel driver.
- */
- if (vf->promisc_unicast_enabled || vf->promisc_multicast_enabled)
- iavf_config_promisc(adapter, false, false);
-
adapter->closed = true;
/* free iAVF security device context all related resources */
@@ -3195,7 +3197,9 @@ iavf_dev_close(struct rte_eth_dev *dev)
iavf_flow_flush(dev, NULL);
iavf_flow_uninit(adapter);
- iavf_vf_reset(hw);
+ /* Skip RESET_VF on a PF-initiated reset */
+ if (!vf->in_reset_recovery)
+ iavf_vf_reset(hw);
vf->aq_intr_enabled = false;
iavf_shutdown_adminq(hw);
if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_WB_ON_ITR) {
diff --git a/drivers/net/intel/iavf/iavf_vchnl.c b/drivers/net/intel/iavf/iavf_vchnl.c
index 94ccfb5d6e..fd22973a68 100644
--- a/drivers/net/intel/iavf/iavf_vchnl.c
+++ b/drivers/net/intel/iavf/iavf_vchnl.c
@@ -216,6 +216,63 @@ iavf_convert_link_speed(enum virtchnl_link_speed virt_link_speed)
return speed;
}
+/*
+ * iavf_handle_link_change_event: common handler for VIRTCHNL link change events
+ *
+ * @dev: pointer to rte_eth_dev for this VF
+ * @vpe: pointer to the virtchnl_pf_event payload received from the PF
+ *
+ * Handle PF link-change event: decode adv/legacy link info, update VF
+ * link state, sync no-poll/watchdog behavior & notify app via LSC event.
+ */
+static void
+iavf_handle_link_change_event(struct rte_eth_dev *dev,
+ struct virtchnl_pf_event *vpe)
+{
+ struct iavf_adapter *adapter =
+ IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct iavf_info *vf = &adapter->vf;
+ bool adv_link_speed;
+
+ adv_link_speed = (vf->vf_res != NULL) &&
+ (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_ADV_LINK_SPEED);
+
+ if (adv_link_speed) {
+ vf->link_up = vpe->event_data.link_event_adv.link_status;
+ vf->link_speed = vpe->event_data.link_event_adv.link_speed;
+ } else {
+ enum virtchnl_link_speed speed;
+
+ vf->link_up = vpe->event_data.link_event.link_status;
+ speed = vpe->event_data.link_event.link_speed;
+ vf->link_speed = iavf_convert_link_speed(speed);
+ }
+
+ iavf_dev_link_update(dev, 0);
+
+ /*
+ * Update watchdog/no_poll state BEFORE notifying the application via
+ * the LSC event. Otherwise the application's link-up callback could
+ * race with stale (link-down) no_poll/watchdog state and either
+ * continue to drop traffic or trigger a spurious reset detection.
+ */
+ if (vf->link_up && !vf->vf_reset)
+ iavf_dev_watchdog_disable(adapter);
+ else if (!vf->link_up)
+ iavf_dev_watchdog_enable(adapter);
+
+ if (adapter->devargs.no_poll_on_link_down) {
+ iavf_set_no_poll(adapter, true);
+ PMD_DRV_LOG(DEBUG, "VF no poll turned %s",
+ adapter->no_poll ? "on" : "off");
+ }
+
+ iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
+
+ PMD_DRV_LOG(INFO, "Link status update:%s",
+ vf->link_up ? "up" : "down");
+}
+
/* Read data in admin queue to get msg from pf driver */
static enum iavf_aq_result
iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
@@ -253,38 +310,17 @@ iavf_read_msg_from_pf(struct iavf_adapter *adapter, uint16_t buf_len,
result = IAVF_MSG_SYS;
switch (vpe->event) {
case VIRTCHNL_EVENT_LINK_CHANGE:
- vf->link_up =
- vpe->event_data.link_event.link_status;
- if (vf->vf_res != NULL &&
- vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
- vf->link_speed =
- vpe->event_data.link_event_adv.link_speed;
- } else {
- enum virtchnl_link_speed speed;
- speed = vpe->event_data.link_event.link_speed;
- vf->link_speed = iavf_convert_link_speed(speed);
- }
- iavf_dev_link_update(vf->eth_dev, 0);
- iavf_dev_event_post(vf->eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
- if (vf->link_up && !vf->vf_reset) {
- iavf_dev_watchdog_disable(adapter);
- } else {
- if (!vf->link_up)
- iavf_dev_watchdog_enable(adapter);
- }
- if (adapter->devargs.no_poll_on_link_down) {
- iavf_set_no_poll(adapter, true);
- if (adapter->no_poll)
- PMD_DRV_LOG(DEBUG, "VF no poll turned on");
- else
- PMD_DRV_LOG(DEBUG, "VF no poll turned off");
- }
- PMD_DRV_LOG(INFO, "Link status update:%s",
- vf->link_up ? "up" : "down");
+ iavf_handle_link_change_event(vf->eth_dev, vpe);
break;
case VIRTCHNL_EVENT_RESET_IMPENDING:
- vf->vf_reset = true;
- iavf_set_no_poll(adapter, false);
+ vf->link_up = false;
+ if (!vf->vf_reset) {
+ vf->vf_reset = true;
+ iavf_set_no_poll(adapter, false);
+ iavf_dev_event_post(vf->eth_dev,
+ RTE_ETH_EVENT_INTR_RESET,
+ NULL, 0);
+ }
PMD_DRV_LOG(INFO, "VF is resetting");
break;
case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
@@ -518,30 +554,7 @@ iavf_handle_pf_event_msg(struct rte_eth_dev *dev, uint8_t *msg,
break;
case VIRTCHNL_EVENT_LINK_CHANGE:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event");
- vf->link_up = pf_msg->event_data.link_event.link_status;
- if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
- vf->link_speed =
- pf_msg->event_data.link_event_adv.link_speed;
- } else {
- enum virtchnl_link_speed speed;
- speed = pf_msg->event_data.link_event.link_speed;
- vf->link_speed = iavf_convert_link_speed(speed);
- }
- iavf_dev_link_update(dev, 0);
- if (vf->link_up && !vf->vf_reset) {
- iavf_dev_watchdog_disable(adapter);
- } else {
- if (!vf->link_up)
- iavf_dev_watchdog_enable(adapter);
- }
- if (adapter->devargs.no_poll_on_link_down) {
- iavf_set_no_poll(adapter, true);
- if (adapter->no_poll)
- PMD_DRV_LOG(DEBUG, "VF no poll turned on");
- else
- PMD_DRV_LOG(DEBUG, "VF no poll turned off");
- }
- iavf_dev_event_post(dev, RTE_ETH_EVENT_INTR_LSC, NULL, 0);
+ iavf_handle_link_change_event(dev, pf_msg);
break;
case VIRTCHNL_EVENT_PF_DRIVER_CLOSE:
PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_PF_DRIVER_CLOSE event");
@@ -570,7 +583,14 @@ iavf_handle_virtchnl_msg(struct rte_eth_dev *dev)
while (pending) {
ret = iavf_clean_arq_element(hw, &info, &pending);
- if (ret != IAVF_SUCCESS) {
+ /* IAVF_ERR_ADMIN_QUEUE_NO_WORK (-57) means AQ is empty
+ * and is a normal way to terminate the drain loop.
+ * Log error only for genuine other failure codes.
+ * Incorrect logging like this during VF resets might
+ * mislead into chasing a non-existent AQ failure.
+ */
+ if (ret != IAVF_SUCCESS &&
+ ret != IAVF_ERR_ADMIN_QUEUE_NO_WORK) {
PMD_DRV_LOG(INFO, "Failed to read msg from AdminQ,"
"ret: %d", ret);
break;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 2/5] eal: fix async IPC callback not fired when no peers
From: Stephen Hemminger @ 2026-06-05 18:15 UTC (permalink / raw)
To: Anatoly Burakov; +Cc: dev, Jianfeng Tan
In-Reply-To: <843e56829da93b5d7c917e61118acd525196dc7d.1780590727.git.anatoly.burakov@intel.com>
On Thu, 4 Jun 2026 17:32:16 +0100
Anatoly Burakov <anatoly.burakov@intel.com> wrote:
> Currently, when rte_mp_request_async() is called and no peer processes
> are connected (nb_sent == 0), the user callback is never invoked.
>
> The original implementation used a dedicated background thread and
> pthread_cond_signal() to wake it after queuing the dummy request. When
> that thread was replaced with per-message alarms, no alarm was set for
> the dummy request, silently breaking the nb_sent == 0 path.
>
> This was not noticed because async requests are used while handling
> secondary process requests, where peers are typically already present.
>
> Fix it by setting a 1us alarm on the dummy request, so the callback path
> immediately triggers and processes it.
>
> Fixes: daf9bfca717e ("ipc: remove thread for async requests")
> Cc: stable@dpdk.org
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> lib/eal/common/eal_common_proc.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_proc.c b/lib/eal/common/eal_common_proc.c
> index 799c6e81b0..5cc15a0f78 100644
> --- a/lib/eal/common/eal_common_proc.c
> +++ b/lib/eal/common/eal_common_proc.c
> @@ -1187,11 +1187,21 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
> if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
> ret = mp_request_async(eal_mp_socket_path(), copy, param, ts);
>
> - /* if we didn't send anything, put dummy request on the queue */
> + /* if we didn't send anything, put dummy request on the queue
> + * and set a minimum-delay alarm so the callback fires immediately.
> + */
> if (ret == 0 && reply->nb_sent == 0) {
> TAILQ_INSERT_TAIL(&pending_requests.requests, dummy,
> next);
> dummy_used = true;
> +
> + if (rte_eal_alarm_set(1, async_reply_handle, dummy) < 0) {
> + EAL_LOG(ERR, "Fail to set alarm for dummy request");
> + /* roll back the changes */
> + TAILQ_REMOVE(&pending_requests.requests, dummy, next);
> + dummy_used = false;
> + ret = -1;
> + }
> }
>
> pthread_mutex_unlock(&pending_requests.lock);
> @@ -1232,10 +1242,14 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,
> } else if (mp_request_async(path, copy, param, ts))
> ret = -1;
> }
> - /* if we didn't send anything, put dummy request on the queue */
> + /* if we didn't send anything, put dummy request on the queue
> + * and set a minimum-delay alarm so the callback fires immediately.
> + */
> if (ret == 0 && reply->nb_sent == 0) {
> TAILQ_INSERT_HEAD(&pending_requests.requests, dummy, next);
> dummy_used = true;
> + if (rte_eal_alarm_set(1, async_reply_handle, dummy) < 0)
> + EAL_LOG(ERR, "Fail to set alarm for dummy request");
> }
>
> /* finally, unlock the queue */
AI spotted potential issue:
The bug in 2/5: in the primary-process path, if rte_eal_alarm_set() fails for the dummy request, the code only logs it.
The dummy stays on the queue with no alarm, the function returns 0 (success),
the callback never fires, and dummy/copy/param leak.
The secondary path right above it handles this correctly (rolls back, returns -1).
Fix is to make the primary path do the same. This corner is never fixed by the later patches.
^ permalink raw reply
* Re: [PATCH] eal: fix function versioning with LTO
From: Stephen Hemminger @ 2026-06-05 18:12 UTC (permalink / raw)
To: David Marchand; +Cc: dev, stable, Thomas Monjalon
In-Reply-To: <CAJFAV8zEg7YNuoO-U-fwnWCJ4MWk9Zm0Zp9pwoirRRn9LHFKrQ@mail.gmail.com>
On Thu, 4 Jun 2026 09:50:25 +0200
David Marchand <david.marchand@redhat.com> wrote:
> On Wed, 3 Jun 2026 at 17:56, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Wed, 3 Jun 2026 12:01:48 +0200
> > David Marchand <david.marchand@redhat.com> wrote:
> >
> > > Hello,
> > >
> > > On Wed, 3 Jun 2026 at 00:57, Stephen Hemminger
> > > <stephen@networkplumber.org> wrote:
> > > >
> > > > When using function versioning and building with Link Time Optimization,
> > > > the compiler does not see the __asm__ annotation of symbols and
> > > > therefore thinks there are two versions of the same symbol.
> > > >
> > > > The fix is to use compiler symver attribute on the function which
> > > > was added in GCC 10. Keep the older method for backward compatibility
> > > > with older compilers.
> > > >
> > > > Bugzilla ID: 1949
> > > > Fixes: e30e194c4d06 ("eal: rework function versioning macros")
> > >
> > > We never used the symver stuff, so it seems unlikely the issue was
> > > introduced with this rework.
> > >
> > > The fact that clang does not support this attribute is a concern.
> >
> > Clang doesn't have this problem. It works as is.
>
> The Fixes: tag is wrong regardless.
> The issue is probably present since introduction of the versioning
> macros, or introduction of LTO in DPDK (not sure which came first).
Right the Fixes is wrong, will resend without
>
>
> > > > Cc: stable@dpdk.org
> > >
> > > Why do we need to backport?
> >
> > Well LTO has worked for a long time, it is not experimental just
> > not commonly done since it takes so long to build.
> >
> > We were doing it years ago at MSFT.
>
> Well, sorry, but every time I enable LTO, I end up with some warnings somewhere.
> I don't think I am doing stuff really exotic though.
Using LTO has always been extra effort. It is worth it for largish
legacy code because the compiler can crunch things down.
>
> Looking at bugzilla, we had various fixes for LTO over the years.
> We still have one open bz btw: https://bugs.dpdk.org/show_bug.cgi?id=1709
That bug was fixed a while back. It required addition of hints (__rte_assume)
>
> Hence my feeling this feature is not something used by many people around.
> And without a CI, we will keep on having to fix bugs/issues.
>
>
> > > LTO is kind of experimental, so it seems good enough to reply "not
> > > expected to work in older LTS" if someone reported an issue.
> > >
> > > And in practice, no LTS release call the versioning macros, since a
> > > LTS drops all compatibility.
>
> Just to be clear, we don't need fixing the macros in LTS: every time
> we prepare a LTS rc0, we drop any kind of symbol compat.
Agree, this is not LTS related
> > >
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > >
> > > I would like to reproduce, but I can't build main with LTO.
> > > What patches did you apply locally to avoid warnings on the hash library?
> > I use Debian testing and GCC 15 but shows up on older versions as well
> > I get no warnings building main
>
> Building from scratch, I do avoid the warnings I hit yesterday.
>
> The fix looks correct, my problem is with the form.
> Fixes: tags accuracy is important.
> And I prefer we stick to "It is not broken, don't fix it".
Could argue it is a GCC bug, and applying their desired workaround :-)
^ permalink raw reply
* [PATCH] security: harden telemetry parameter parsing
From: Stephen Hemminger @ 2026-06-05 17:46 UTC (permalink / raw)
To: dev
Cc: Stephen Hemminger, stable, Akhil Goyal, Anoob Joseph,
Gowrishankar Muthukrishnan
The cryptodev security telemetry handlers parsed dev_id/capa_id with
strtoul() and no overflow or range check, so an out-of-range dev_id
(e.g. 256) silently truncated to a valid device in
rte_cryptodev_is_valid_dev(). isdigit() was also called on a plain
(signed) char, which is undefined for high-bit input.
The parser was also using strtok() which is not thread safe.
Use a validated parse helper and reject malformed input rather than
logging and continuing. This also drops the thread-unsafe strtok() in
the crypto_caps handler.
Fixes: 259ca6d1617f ("security: add telemetry endpoint for capabilities")
Cc: stable@dpdk.org
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/security/rte_security.c | 41 ++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/lib/security/rte_security.c b/lib/security/rte_security.c
index c47fe44da0..0d89f8af3f 100644
--- a/lib/security/rte_security.c
+++ b/lib/security/rte_security.c
@@ -7,6 +7,8 @@
#include <stdalign.h>
#include <ctype.h>
#include <stdlib.h>
+#include <errno.h>
+#include <limits.h>
#include <eal_export.h>
#include <rte_cryptodev.h>
@@ -474,6 +476,25 @@ security_capabilities_from_dev_id(int dev_id, const void **caps)
return 0;
}
+/* Parse an unsigned integer parameter, returning the value or -EINVAL.
+ * 'max' must be <= INT_MAX.
+ */
+static int
+telemetry_parse_uint(const char *str, char **end, unsigned long max)
+{
+ unsigned long val;
+
+ if (str == NULL || !isdigit((unsigned char)*str))
+ return -EINVAL;
+
+ errno = 0;
+ val = strtoul(str, end, 0);
+ if (errno != 0 || val > max)
+ return -EINVAL;
+
+ return (int)val;
+}
+
static int
security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *params,
struct rte_tel_data *d)
@@ -485,13 +506,10 @@ security_handle_cryptodev_sec_caps(const char *cmd __rte_unused, const char *par
int dev_id;
int rc;
- if (!params || strlen(params) == 0 || !isdigit(*params))
+ dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+ if (dev_id < 0 || *end_param != '\0')
return -EINVAL;
- dev_id = strtoul(params, &end_param, 0);
- if (*end_param != '\0')
- CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
if (rc < 0)
return rc;
@@ -513,24 +531,19 @@ security_handle_cryptodev_crypto_caps(const char *cmd __rte_unused, const char *
{
const struct rte_security_capability *capabilities;
struct rte_tel_data *crypto_caps;
- const char *capa_param;
int dev_id, capa_id;
int crypto_caps_n;
char *end_param;
int rc;
- if (!params || strlen(params) == 0 || !isdigit(*params))
+ dev_id = telemetry_parse_uint(params, &end_param, RTE_CRYPTO_MAX_DEVS - 1);
+ if (dev_id < 0 || *end_param != ',')
return -EINVAL;
- dev_id = strtoul(params, &end_param, 0);
- capa_param = strtok(end_param, ",");
- if (!capa_param || strlen(capa_param) == 0 || !isdigit(*capa_param))
+ capa_id = telemetry_parse_uint(end_param + 1, &end_param, INT_MAX);
+ if (capa_id < 0 || *end_param != '\0')
return -EINVAL;
- capa_id = strtoul(capa_param, &end_param, 0);
- if (*end_param != '\0')
- CDEV_LOG_ERR("Extra parameters passed to command, ignoring");
-
rc = security_capabilities_from_dev_id(dev_id, (void *)&capabilities);
if (rc < 0)
return rc;
--
2.53.0
^ permalink raw reply related
* [PATCH] fib6: fix error code propagation on next hop update
From: Vladimir Medvedkin @ 2026-06-05 16:47 UTC (permalink / raw)
To: dev; +Cc: stable
When updating the next hop of an existing prefix, trie_modify() ignored
the return value of modify_dp() and always returned 0. An out-of-range
next hop is rejected by modify_dp() with -EINVAL but was reported to
the caller as success. Return the actual result.
Fixes: c3e12e0f0354 ("fib: add dataplane algorithm for IPv6")
Cc: stable@dpdk.org
Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
lib/fib/trie.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/fib/trie.c b/lib/fib/trie.c
index fa5d9ec6b0..99272f45bd 100644
--- a/lib/fib/trie.c
+++ b/lib/fib/trie.c
@@ -600,7 +600,8 @@ trie_modify(struct rte_fib6 *fib, const struct rte_ipv6_addr *ip,
ret = modify_dp(dp, rib, &ip_masked, depth, next_hop);
if (ret == 0)
rte_rib6_set_nh(node, next_hop);
- return 0;
+
+ return ret;
}
if ((depth > 24) && (dp->rsvd_tbl8s + depth_diff > dp->number_tbl8s))
--
2.43.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox