* 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
* Re: [PATCH v15 0/5] Support add/remove memory region and get-max-slots
From: Stephen Hemminger @ 2026-06-05 16:45 UTC (permalink / raw)
To: pravin.bathija; +Cc: dev, fengchengwen, maxime.coquelin, thomas
In-Reply-To: <20260604235723.1046607-1-pravin.bathija@dell.com>
On Thu, 4 Jun 2026 23:57:18 +0000
<pravin.bathija@dell.com> wrote:
> From: Pravin M Bathija <pravin.bathija@dell.com>
>
> This is version v15 of the patchset and it incorporates the
> recommendations made by Maxime Coquelin.
>
> Patch 4/5
> - Changed VHOST_USER_REM_MEM_REG handler declaration from
> accepts_fd=true to accepts_fd=false, as the remove request does not
> expect FDs in ancillary data.
> - Removed all close_msg_fds(ctx) calls from vhost_user_rem_mem_reg(), no
> longer needed since the handler is declared as not accepting FDs.
> - Removed validate_msg_fds(dev, ctx, 0) check from
> vhost_user_rem_mem_reg(), as FD validation is now handled generically
> by the framework.
> - Added targeted IOTLB cache invalidation in vhost_user_rem_mem_reg()
> using vhost_user_iotlb_cache_remove() for the removed region's GPA
> range, instead of the nuclear iotlb_flush_all() used by set_mem_table.
>
> This implementation has been extensively tested by doing Read/Write I/O
> from multiple instances of fio + libblkio (front-end) talking to
> spdk/dpdk (back-end) based drives. Tested with qemu front-end talking to
> dpdk testpmd (back-end) performing add/removal of memory regions. Also
> tested post-copy live migration after doing add_memory_region.
>
> Version Log:
> Version v15 (Current version): Incorporate code review suggestions from
> Maxime Coquelin as described above.
>
> Version v14: Incorporate code review suggestions from Stephen Hemminger
> and Fengcheng Wen.
> Changes from Fengcheng Wen review:
> Patch 3/5
> - Moved free_all_mem_regions() call sites in vhost_user_set_mem_table()
> from patch 4/5 to patch 3/5 so each commit compiles independently
> Patch 4/5
> - Renamed _dev_invalidate_vrings() to vhost_user_invalidate_vrings() to
> follow vhost naming convention
> - Added comment explaining *pdev propagation through
> translate_ring_addresses / numa_realloc()
> - Reordered local variables in vhost_user_add_mem_reg() and
> vhost_user_rem_mem_reg() by descending line length
> - Shortened overlap check variable names (current_region_guest_start/end
> --> cur_start/end, proposed_region_guest_start/end -> new_start/end)
> - Fixed DMA error path in vhost_user_add_mem_reg(): added
> free_new_region_no_dma label so async_dma_map_region(false) is not
> called when the map itself failed.
> Changes from Stephen Hemminger review:
> Patch 4/5
> - vhost_user_add_mem_reg() now constructs a reply with the back-end's
> host mapping address in userspace_addr and returns
> RTE_VHOST_MSG_RESULT_REPLY per the vhost-user spec
> - Added validate_msg_fds(dev, ctx, 0) in vhost_user_rem_mem_reg() to
> reject malformed messages with unexpected file descriptors
> - Dropped unnecessary (uint64_t) cast in vhost_user_get_max_mem_slots()
>
> Version v13: Incorporate code review suggestions from Fengcheng Wen
> Patch 2/5
> Renamed VhostUserSingleMemReg to VhostUserMemRegMsg and memory_single
> to memreg
> Patches 3/5 and 4/5
> Relocated function remove_guest_pages from patch 3/5 to 4/5
>
> Version v12: Incorporate code review suggestions from Maxime Coquelin
> and ai-code-review.
> Patch 3/5
> Refactored async_dma_map() to delegate to async_dma_map_region(),
> eliminating code duplication between the two functions.
> Restored original comments in async_dma_map_region() explaining why
> ENODEV and EINVAL errors are ignored (these were stripped in v10)
> Reverted unnecessary changes to vhost_user_postcopy_register() --
> removed the host_user_addr == 0 checks and reg_msg_index indirection
> that were added in v10, since this function is only called from
> vhost_user_set_mem_table() where regions are always contiguous.
>
> Version v11: Incorporate code review suggestions from Stephen Hemminger.
> Patch 4/5
> Fix incomplete cleanup in vhost_user_add_mem_reg() when
> vhost_user_mmap_region() fails after the mmap succeeds (e.g.
> add_guest_pages() realloc failure) realloc failure). The error path now
> calls remove_guest_pages() and free_mem_region() to undo the mapping
> and stale guest-page entries, preventing a leaked mmap and slot reuse
> corruption. The plain close(fd) path is kept for pre-mmap failures.
>
> Version v10: Incorporate code review suggestions from Stephen Hemminger.
> Patch 4/5
> Moved dev_invalidate_vrings after free_mem_region, array compaction, and
> nregions decrement. This ensures translate_ring_addresses only sees
> surviving memory regions, preventing vring pointers from resolving into
> a region that is about to be unmapped.
>
> Version v9: Incorporate code review suggestions from Stephen Hemminger.
> Patch 3/5
> Restored max_guest_pages initial value to hardcoded 8 instead of
> VHOST_MEMORY_MAX_NREGIONS, matching upstream semantics.
> Patch 4/5
> Added close(reg->fd) and reg->fd = -1 before goto close_msg_fds in the
> mmap failure path to fix fd leak after fd was moved from ctx->fds[0].
> Converted dev_invalidate_vrings from a plain function to a macro +
> implementation function pair, accepting message ID as a parameter so
> the static_assert reports the correct handler at each call site.
> Updated dev_invalidate_vrings call in add_mem_reg to pass
> VHOST_USER_ADD_MEM_REG as message ID.
> Updated dev_invalidate_vrings call in rem_mem_reg to pass
> VHOST_USER_REM_MEM_REG as message ID.
>
> Version v8: Incorporate code review suggestions from Stephen Hemminger.
> rewrite async_dma_map_region function to iterate guest pages by host
> address range matching
> change function dev_invalidate_vrings to accept a double pointer to
> propagate pointer updates
> new function remove_guest_pages was added
> add_mem_reg error path was narrowed to only clean up the single failed
> region instead of destroting all existing regions
>
> Version v7: Incorporate code review suggestions from Maxime Coquelin.
> Add debug messages to vhost_postcopy_register function.
>
> Version v6: Added the enablement of this feature as a final patch in
> this patch-set and other code optimizations as suggested by Maxime
> Coquelin.
>
> Version v5: removed the patch that increased the number of memory regions
> from 8 to 128. This will be submitted as a separate feature at a later
> point after incorporating additional optimizations. Also includes code
> optimizations as suggested by Feng Cheng Wen.
>
> Version v4: code optimizations as suggested by Feng Cheng Wen.
>
> Version v3: code optimizations as suggested by Maxime Coquelin
> and Thomas Monjalon.
>
> Version v2: code optimizations as suggested by Maxime Coquelin.
>
> Version v1: Initial patch set.
>
> Pravin M Bathija (5):
> vhost: add user to mailmap and define to vhost hdr
> vhost: header defines for add/rem mem region
> vhost: refactor memory helper functions
> vhost: add mem region add/remove handlers
> vhost: enable configure memory slots
>
> .mailmap | 1 +
> lib/vhost/rte_vhost.h | 4 +
> lib/vhost/vhost_user.c | 425 +++++++++++++++++++++++++++++++++++------
> lib/vhost/vhost_user.h | 10 +
> 4 files changed, 378 insertions(+), 62 deletions(-)
>
I don't think this is ready to merge based on AI review.
Did AI review with Opus 4.8 on a chat which has past context.
Summary of v15 findings
New in v15 (both patch 4/5, both errors):
Use-after-free on the reply path: reg points into dev->mem->regions[], but dev_invalidate_vrings() -> translate_ring_addresses() -> numa_realloc() can relocate dev->mem. dev is refreshed via *pdev, reg is not, then reg->host_user_addr is read for the reply. Re-derive reg (or capture host_user_addr) after dev = *pdev.
ADD_MEM_REG reply sent unconditionally: handler always returns RESULT_REPLY, but the spec makes the mapping-address reply postcopy- only. In non-postcopy mode this desyncs the channel (no REPLY_ACK: the front-end never reads it; with REPLY_ACK: it expects a u64 ack, not a memreg). Gate the reply on dev->postcopy_listening, else return RESULT_OK -- same as SET_MEM_TABLE.
Carried over from v13 (now in a different form):
The v13 Warning (missing postcopy mapping-address reply) is addressed but mis-gated; correct fix is the conditional reply above. Until then postcopy correctness still isn't right.
^ permalink raw reply
* Re: [PATCH 0/7] net/ice: L2TPv2 flow rule fixes
From: Bruce Richardson @ 2026-06-05 15:20 UTC (permalink / raw)
To: Shaiq Wani; +Cc: dev, aman.deep.singh
In-Reply-To: <20260427023115.1225843-1-shaiq.wani@intel.com>
On Mon, Apr 27, 2026 at 08:01:08AM +0530, Shaiq Wani wrote:
> The original L2TPv2 flow support (733640dae75e) mapped every PPP tunnel
> variant to a single generic PTYPE and programmed both segments with
> identical headers. This caused several interrelated problems:
> cross-protocol matches, silent inner-field drops, rule deletion
> failures, and unintended side-effects on GTP-U flows.
>
> This series addresses each issue:
>
> 1/7 Use the 30 granular HW PTYPEs (396-425) defined by the DDP
> package instead of the generic ICE_MAC_IPV4_L2TPV2, and
> extend the training-packet switch to cover the new flow types.
>
> 2/7 Add the 8 missing tunnel inset-to-flow-field mappings so
> inner IP/L4 fields are no longer silently dropped during
> field parsing.
>
> 3/7 Pass a segment index to ice_fdir_input_set_hdrs() and expand
> each L2TPv2/PPP ptype into its own case with distinct outer
> and inner header sets. Also always program inner-segment
> headers for tunnel profiles, even when no inner fields are
> extracted, so ptype-only narrowing works.
>
> 4/7 Fix deletion of bare L2TPv2 rules (no PPP) by switching to a
> single-segment profile, and normalize the L2TPv2 flags in the
> SW hash key to prevent lookup mismatches.
>
> 5/7 Stop L2TPv2 tunnel detection from overwriting the GTP-U
> tunnel profile, which caused GTP-U flow rules to fail.
>
> 6/7 Invalidate stale HW profiles when the L2TPv2 subtype changes
> between rule creations.
>
> 7/7 Pin the outer Ethertype (0x0800 / 0x86DD) in L2TPv2 rules
> so IPv4 and IPv6 flows are not cross-matched.
>
> Shaiq Wani (7):
> net/ice: use granular PTYPEs for L2TPv2 PPP
> net/ice: add tunnel inset bits to flow input set map
> net/ice: fix L2TPv2 inner segment header setup
> net/ice: fix bare L2TPv2 flow rule deletion
> net/ice: fix GTP-U failure due to wrong tunnel profile
> net/ice: fix stale profile after L2TPv2 subtype change
> net/ice: pin outer Ethertype for L2TPv2 flow rules
>
Series applied to dpdk-next-net-intel.
Thanks,
/Bruce
^ permalink raw reply
* Re: [PATCH v7 00/27] Add common flow attr/action parsing infrastructure to Intel PMD's
From: Bruce Richardson @ 2026-06-05 14:58 UTC (permalink / raw)
To: Anatoly Burakov; +Cc: dev
In-Reply-To: <cover.1780589913.git.anatoly.burakov@intel.com>
On Thu, Jun 04, 2026 at 05:19:04PM +0100, Anatoly Burakov wrote:
> This patchset introduces common flow attr/action checking infrastructure to
> some Intel PMD's (IXGBE, I40E, IAVF, and ICE). The aim is to reduce code
> duplication, simplify implementation of new parsers/verification of existing
> ones, and make action/attr handling more consistent across drivers.
>
> v7:
> - Fail on empty action list before calling into user callbacks
> - Remove empty code
>
Series applied to dpdk-next-net-intel.
Thanks,
/Bruce
^ permalink raw reply
* Re: [PATCH 7/7] net/ice: pin outer Ethertype for L2TPv2 flow rules
From: Burakov, Anatoly @ 2026-06-05 14:54 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-8-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> L2TPv2 flow rules do not include the outer Ethertype in the input set,
> so a rule created for outer IPv6 also matches packets with outer IPv4
> (and vice versa). The hardware profile cannot distinguish the two
> families because nothing in the match key differentiates them.
>
> Add ICE_INSET_ETHERTYPE to input_set_o and set ext_data.ether_type to
> 0x0800 or 0x86DD based on the outer L3 item, so the profile includes
> the Ethertype field and rejects cross-family mismatches.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 6/7] net/ice: fix stale profile after L2TPv2 subtype change
From: Burakov, Anatoly @ 2026-06-05 14:52 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-7-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> After a rule is destroyed, the HW profile for that ptype persists.
> If a new rule with the same ptype but a different L2TPv2 subtype is
> created (e.g. data then data_l), the old profile is reused via -EEXIST
> with stale field extraction offsets. Since data and data_l have
> session_id at different byte offsets, the NIC matches on the wrong
> field and packets with mismatched session_id incorrectly hit the rule.
>
> Remove the HW profile in ice_fdir_destroy_filter when the last
> filter for a given ptype is destroyed so the next rule creates a fresh
> profile with the correct offsets.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 5/7] net/ice: fix GTP-U failure due to wrong tunnel profile
From: Burakov, Anatoly @ 2026-06-05 14:51 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-6-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> Remove GTP-U tunnel types from ice_fdir_is_tunnel_profile().
> GTP-U uses a non-tunnel profile path and should not be marked
> as tunnel, which causes training packet generation to fail.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 4/7] net/ice: fix bare L2TPv2 flow rule deletion
From: Burakov, Anatoly @ 2026-06-05 14:50 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-5-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> Bare L2TPv2 flow rules (no inner PPP) fail on deletion because
> ice_fdir_is_tunnel_profile() forces a 2-segment profile with an empty
> inner segment the NIC cannot remove. Reset tunnel_type to NONE when
> no inner fields are present so a single-segment profile is used.
>
> Also normalize L2TPv2 flags_version in the SW hash key to only the
> CTRL/LEN/VER bits, preventing delete lookup mismatches when S/O/P
> bits differ from the create path.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Needs rebase on account of memcpy changes, but easy enough to apply
manually.
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 3/7] net/ice: fix L2TPv2 inner segment header setup
From: Burakov, Anatoly @ 2026-06-05 14:47 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-4-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> ice_fdir_input_set_hdrs() sets identical headers on both outer and
> inner segments and groups all PPP variants into a single fall-through,
> so the HW cannot distinguish inner IPv4 from IPv6 or TCP from UDP.
>
> Add a seg_idx parameter and expand each L2TPv2/PPP ptype into its own
> case with per-segment header selection. Also ensure the inner segment
> headers are always programmed even when no inner fields are extracted,
> so ptype-only narrowing works correctly.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Nitpick: TCP cases can be reordered to slightly reduce code churn.
Otherwise,
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 2/7] net/ice: add tunnel inset bits to flow input set map
From: Burakov, Anatoly @ 2026-06-05 14:45 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-3-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> Tunnel inset bits (ICE_INSET_TUN_IPV4_SRC, ICE_INSET_TUN_IPV4_DST,
> ICE_INSET_TUN_IPV6_SRC/DST, ICE_INSET_TUN_TCP/UDP_SRC/DST_PORT) are
> absent from ice_inset_map[], so inner IP/L4 fields for L2TPv2/PPP
> tunnel flow rules are silently ignored during field parsing.
>
> Add the 8 missing tunnel inset-to-flow-field mappings.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
* Re: [PATCH 1/7] net/ice: use granular PTYPEs for L2TPv2 PPP
From: Burakov, Anatoly @ 2026-06-05 14:44 UTC (permalink / raw)
To: Shaiq Wani, dev, bruce.richardson, aman.deep.singh
In-Reply-To: <20260427023115.1225843-2-shaiq.wani@intel.com>
On 4/27/2026 4:31 AM, Shaiq Wani wrote:
> All L2TPv2 PPP variants map to ICE_MAC_IPV4_L2TPV2 (398), so inner
> protocol type is not differentiated, allowing cross-protocol flow
> matches (e.g. a PPP/IPv4 rule hitting a PPP/IPv6 packet).
>
> Fix ice_ptype_map[] to use the 30 granular L2TPv2 PTYPEs (396-425)
> defined by the DDP package. Also add PPP inner protocol flow types to
> ice_fdir_gen_l2tpv2_pkt() so training packets get dynamically built
> L2TPv2 headers instead of static templates.
>
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Fixes: bf662653976e ("net/ice/base: support L2TPv2 flow rule")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply
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