On Fri, 21 Aug 2026 11:26:43 +0530
Gagandeep Singh <[email protected]> wrote:
> @@ -71,7 +72,46 @@ parse_txq_prior(const char *key __rte_unused, const char
> *value, void *opaque)
>
> str = strtok(input_str, "|");
> while (str != NULL && i < hw->max_tx_queues) {
> - hw->txq_prior[i++] = (uint32_t)atoi(str);
> + hw->txq_prior[i++] = atoi(str) & ENETC_TBMR_PRIO_MASK;
> + str = strtok(NULL, "|");
> + }
> +
> + free(input_str);
> + return 0;
> +}
The arg parsing in this driver has lots of usage of functions that are on
the naughty list like: atoi, atof, and strtok.
Suggest reworking this to use strtok_r and strtoul and strtod.
Not a fan of so many nerd knobs either. These kind of queue configurations
really
need to be under ethdev but that is more work.
Something like this?
diff --git a/drivers/net/enetc/enetc4_ethdev.c
b/drivers/net/enetc/enetc4_ethdev.c
index bb579a0b90..3e13542302 100644
--- a/drivers/net/enetc/enetc4_ethdev.c
+++ b/drivers/net/enetc/enetc4_ethdev.c
@@ -2,7 +2,9 @@
* Copyright 2024-2026 NXP
*/
+#include <errno.h>
#include <stdbool.h>
+#include <stdlib.h>
#include <rte_kvargs.h>
#include <rte_random.h>
#include <dpaax_iova_table.h>
@@ -49,85 +51,172 @@ static uint64_t dev_tx_offloads_sup =
#define ENETC4_NC_MEMORY "nc"
+/* Tx ring WRR weight range; ENETC_TBMR_WRR() encodes it as (weight - 1). */
+#define ENETC4_TXQ_WRR_MIN 1
+#define ENETC4_TXQ_WRR_MAX 8
+
+static const char * const enetc4_valid_args[] = {
+ ENETC4_TXQ_PRIORITIES,
+ ENETC4_TXQ_WRR,
+ ENETC4_NC_MEMORY,
+ NULL,
+};
+
+/*
+ * Parse one unsigned decimal value, rejecting empty strings, signs, trailing
+ * garbage and values outside [min, max]. atoi() reports none of these: it
+ * returns 0 for any non-numeric string and is undefined on overflow.
+ */
static int
-parse_txq_prior(const char *key __rte_unused, const char *value, void *opaque)
+enetc4_parse_uint(const char *str, uint32_t min, uint32_t max, uint32_t *val)
{
- struct rte_eth_dev *dev = (struct rte_eth_dev *)opaque;
- struct enetc_eth_hw *hw =
- ENETC_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- char *input_str;
- char *str;
- uint32_t i = 0;
+ unsigned long res;
+ char *endptr;
+
+ if (str == NULL || *str == '\0')
+ return -EINVAL;
+
+ /* strtoul() silently wraps a leading '-', so reject signs up front. */
+ if (*str == '-' || *str == '+')
+ return -EINVAL;
+
+ errno = 0;
+ res = strtoul(str, &endptr, 10);
+ if (errno != 0 || endptr == str || *endptr != '\0')
+ return -EINVAL;
+
+ if (res < min || res > max)
+ return -ERANGE;
+
+ *val = (uint32_t)res;
+ return 0;
+}
+
+/*
+ * Parse a "v0|v1|..." list into vals[], validating every field against
+ * [min, max]. Returns the number of values parsed, or a negative errno.
+ */
+static int
+enetc4_parse_uint_list(const char *key, const char *value, uint32_t min,
+ uint32_t max, uint32_t *vals, uint32_t max_vals)
+{
+ char *input_str, *str, *saveptr;
+ uint32_t n = 0;
+ int ret;
+
+ if (value == NULL) {
+ ENETC_PMD_ERR("%s: missing value", key);
+ return -EINVAL;
+ }
input_str = strdup(value);
- if (!input_str)
+ if (input_str == NULL)
return -ENOMEM;
- rte_free(hw->txq_prior);
- hw->txq_prior = rte_zmalloc(NULL, hw->max_tx_queues * sizeof(uint32_t),
0);
- if (!hw->txq_prior) {
- free(input_str);
- return -ENOMEM;
+ for (str = strtok_r(input_str, "|", &saveptr); str != NULL;
+ str = strtok_r(NULL, "|", &saveptr)) {
+ if (n == max_vals) {
+ ENETC_PMD_ERR("%s: too many values, at most %u
supported",
+ key, max_vals);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = enetc4_parse_uint(str, min, max, &vals[n]);
+ if (ret != 0) {
+ ENETC_PMD_ERR("%s: invalid value '%s' at index %u,
expected %u..%u",
+ key, str, n, min, max);
+ goto out;
+ }
+ n++;
}
- str = strtok(input_str, "|");
- while (str != NULL && i < hw->max_tx_queues) {
- hw->txq_prior[i++] = atoi(str) & ENETC_TBMR_PRIO_MASK;
- str = strtok(NULL, "|");
+ if (n == 0) {
+ ENETC_PMD_ERR("%s: empty value list", key);
+ ret = -EINVAL;
+ goto out;
}
+ ret = n;
+out:
free(input_str);
+ return ret;
+}
+
+/* Parse enetc4_txq_prior="p0|p1|..." devarg; priority 0..7 per ring. */
+static int
+parse_txq_prior(const char *key, const char *value, void *opaque)
+{
+ struct rte_eth_dev *dev = opaque;
+ struct enetc_eth_hw *hw =
+ ENETC_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ uint32_t *prior;
+ int ret;
+
+ prior = rte_zmalloc(NULL, hw->max_tx_queues * sizeof(uint32_t), 0);
+ if (prior == NULL)
+ return -ENOMEM;
+
+ ret = enetc4_parse_uint_list(key, value, 0, ENETC_TBMR_PRIO_MASK,
+ prior, hw->max_tx_queues);
+ if (ret < 0) {
+ rte_free(prior);
+ return ret;
+ }
+
+ /* Only swap in the new table once the whole list is known good. */
+ rte_free(hw->txq_prior);
+ hw->txq_prior = prior;
+
return 0;
}
/* Parse enetc4_txq_wrr="w0|w1|..." devarg; weight 1..8 per ring. */
-static int parse_txq_wrr(const char *key __rte_unused, const char *value,
- void *opaque)
+static int
+parse_txq_wrr(const char *key, const char *value, void *opaque)
{
- struct rte_eth_dev *dev = (struct rte_eth_dev *)opaque;
+ struct rte_eth_dev *dev = opaque;
struct enetc_eth_hw *hw =
ENETC_DEV_PRIVATE_TO_HW(dev->data->dev_private);
- char *input_str;
- char *str;
- uint32_t i = 0;
- int w;
+ uint32_t *wrr;
+ int n, i;
- input_str = strdup(value);
- if (!input_str)
+ wrr = rte_zmalloc(NULL, hw->max_tx_queues * sizeof(uint32_t), 0);
+ if (wrr == NULL)
return -ENOMEM;
- rte_free(hw->txq_wrr);
- hw->txq_wrr = rte_zmalloc(NULL,
- hw->max_tx_queues * sizeof(uint32_t), 0);
- if (!hw->txq_wrr) {
- free(input_str);
- return -ENOMEM;
+ n = enetc4_parse_uint_list(key, value, ENETC4_TXQ_WRR_MIN,
+ ENETC4_TXQ_WRR_MAX, wrr, hw->max_tx_queues);
+ if (n < 0) {
+ rte_free(wrr);
+ return n;
}
- str = strtok(input_str, "|");
- while (str != NULL && i < hw->max_tx_queues) {
- w = atoi(str);
- if (w < 1)
- w = 1;
- if (w > 8)
- w = 8;
- hw->txq_wrr[i++] = ENETC_TBMR_WRR(w);
- str = strtok(NULL, "|");
- }
+ for (i = 0; i < n; i++)
+ wrr[i] = ENETC_TBMR_WRR(wrr[i]);
+
+ /* Only swap in the new table once the whole list is known good. */
+ rte_free(hw->txq_wrr);
+ hw->txq_wrr = wrr;
- free(input_str);
return 0;
}
static int
-parse_nc(const char *key __rte_unused, const char *value, void *extra_args)
+parse_nc(const char *key, const char *value, void *extra_args)
{
struct rte_eth_dev *dev = extra_args;
struct enetc_eth_hw *hw =
ENETC_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ uint32_t val;
+
+ if (enetc4_parse_uint(value, 0, 1, &val) != 0) {
+ ENETC_PMD_ERR("%s: invalid value '%s', expected 0 or 1",
+ key, value ? value : "(null)");
+ return -EINVAL;
+ }
- if (value && atoi(value) == 1)
- hw->nc_mode = 1;
+ hw->nc_mode = val;
return 0;
}
@@ -135,45 +224,41 @@ parse_nc(const char *key __rte_unused, const char *value,
void *extra_args)
static int
enetc4_get_devargs(struct rte_eth_dev *dev, const char *key)
{
+ static const struct {
+ const char *key;
+ arg_handler_t handler;
+ } handlers[] = {
+ { ENETC4_TXQ_PRIORITIES, parse_txq_prior },
+ { ENETC4_TXQ_WRR, parse_txq_wrr },
+ { ENETC4_NC_MEMORY, parse_nc },
+ };
struct rte_devargs *devargs = dev->device->devargs;
struct rte_kvargs *kvlist;
+ unsigned int i;
+ int ret = 0;
- if (!devargs)
+ if (devargs == NULL)
return 0;
- kvlist = rte_kvargs_parse(devargs->args, NULL);
- if (!kvlist)
- return 0;
-
- if (!rte_kvargs_count(kvlist, key)) {
- rte_kvargs_free(kvlist);
- return 0;
+ /* Passing the key list makes a mistyped devarg an error, not a no-op.
*/
+ kvlist = rte_kvargs_parse(devargs->args, enetc4_valid_args);
+ if (kvlist == NULL) {
+ ENETC_PMD_ERR("Invalid device arguments '%s'", devargs->args);
+ return -EINVAL;
}
- if (!strcmp(key, ENETC4_TXQ_PRIORITIES)) {
- if (rte_kvargs_process(kvlist, key,
- parse_txq_prior, (void *)dev) < 0) {
- rte_kvargs_free(kvlist);
- return 0;
- }
- }
- if (!strcmp(key, ENETC4_TXQ_WRR)) {
- if (rte_kvargs_process(kvlist, key,
- parse_txq_wrr, (void *)dev) < 0) {
- rte_kvargs_free(kvlist);
- return 0;
- }
- }
- if (!strcmp(key, ENETC4_NC_MEMORY)) {
- if (rte_kvargs_process(kvlist, key,
- parse_nc, (void *)dev) < 0) {
- rte_kvargs_free(kvlist);
- return 0;
- }
- }
+ if (rte_kvargs_count(kvlist, key) == 0)
+ goto out;
+ for (i = 0; i < RTE_DIM(handlers); i++) {
+ if (strcmp(key, handlers[i].key) != 0)
+ continue;
+ ret = rte_kvargs_process(kvlist, key, handlers[i].handler, dev);
+ break;
+ }
+out:
rte_kvargs_free(kvlist);
- return 0;
+ return ret;
}
static int
@@ -1129,9 +1214,13 @@ enetc4_dev_configure(struct rte_eth_dev *dev)
enetc4_txbdr_wr(enetc_hw, i, ENETC_TBMR, ENETC_BMR_RESET);
hw->nc_mode = 0;
- enetc4_get_devargs(dev, ENETC4_TXQ_PRIORITIES);
- enetc4_get_devargs(dev, ENETC4_TXQ_WRR);
- enetc4_get_devargs(dev, ENETC4_NC_MEMORY);
+ ret = enetc4_get_devargs(dev, ENETC4_TXQ_PRIORITIES);
+ if (ret == 0)
+ ret = enetc4_get_devargs(dev, ENETC4_TXQ_WRR);
+ if (ret == 0)
+ ret = enetc4_get_devargs(dev, ENETC4_NC_MEMORY);
+ if (ret != 0)
+ return ret;
if (dev->data->nb_rx_queues <= 1)
return 0;
@@ -1538,8 +1627,13 @@ enetc4_dev_init(struct rte_eth_dev *eth_dev)
hw->max_rx_queues = (si_cap >> 16) & ENETC_SICAPR0_BDR_MASK;
hw->nc_mode = 0;
- enetc4_get_devargs(eth_dev, ENETC4_TXQ_PRIORITIES);
- enetc4_get_devargs(eth_dev, ENETC4_NC_MEMORY);
+ error = enetc4_get_devargs(eth_dev, ENETC4_TXQ_PRIORITIES);
+ if (error == 0)
+ error = enetc4_get_devargs(eth_dev, ENETC4_NC_MEMORY);
+ if (error != 0) {
+ ENETC_PMD_ERR("Invalid device arguments");
+ return error;
+ }
if (hw->nc_mode) {
eth_dev->rx_pkt_burst = &enetc_recv_pkts_nc;
eth_dev->tx_pkt_burst = &enetc_xmit_pkts_nc;
