[v6] eventdev/eth_rx: fix memory leak when token parsing finished
Checks
Commit Message
The memory get from strdup should be freed when parameter parsing
finished, and also should be freed when error occurs.
Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
Signed-off-by: Weiguo Li <liwg06@foxmail.com>
Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
---
v6:
* fix double free problem in v5 as Jay noted
v5:
* fix according to Jay's comment.
* fix anothor bypassing the freeing of memory problem
v4:
* fix compilation issue
v3:
* validate "eth_dev_id" by rte_eth_dev_is_valid_port()
as Ganapati's comment.
* validate "token" by macros to reduce code redundancy.
* use new macros to avoid bypassing the freeing of memory problem.
v2:
* add memory check after strdup
---
lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
1 file changed, 72 insertions(+), 24 deletions(-)
Comments
Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
-Jay
> -----Original Message-----
> From: Weiguo Li <liwg06@foxmail.com>
> Sent: Friday, February 4, 2022 1:51 PM
> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> stephen@networkplumber.org; dev@dpdk.org
> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
>
> The memory get from strdup should be freed when parameter parsing
> finished, and also should be freed when error occurs.
>
> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>
> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>
> ---
> v6:
> * fix double free problem in v5 as Jay noted
> v5:
> * fix according to Jay's comment.
> * fix anothor bypassing the freeing of memory problem
> v4:
> * fix compilation issue
> v3:
> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> as Ganapati's comment.
> * validate "token" by macros to reduce code redundancy.
> * use new macros to avoid bypassing the freeing of memory problem.
> v2:
> * add memory check after strdup
> ---
> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> 1 file changed, 72 insertions(+), 24 deletions(-)
>
> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> index ae1e260c08..4da6f1ff90 100644
> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> } \
> } 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\n", id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} 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)) { \
> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> + ret = retval; \
> + goto error; \
> + } \
> +} while (0)
> +
> static inline int
> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> {
> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>
> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + 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_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* 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);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &queue_conf)) {
> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> RXA_ADD_DICT(queue_conf.ev, flow_id);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>
> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + 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_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* 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);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> rx_queue_id, &q_stats)) {
> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> RXA_ADD_DICT(q_stats, rx_dropped);
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> static int
> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> {
> uint8_t rx_adapter_id;
> uint16_t rx_queue_id;
> - int eth_dev_id;
> + int eth_dev_id, ret = -1;
> char *token, *l_params;
>
> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>
> /* Get Rx adapter ID from parameter string */
> l_params = strdup(params);
> + if (l_params == NULL)
> + return -ENOMEM;
> token = strtok(l_params, ",");
> + 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_ERR_RET(rx_adapter_id, -EINVAL);
> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>
> token = strtok(NULL, ",");
> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> - return -1;
> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>
> /* 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);
> - return -EINVAL;
> + ret = -EINVAL;
> + goto error;
> }
>
> token = strtok(NULL, "\0");
> if (token != NULL)
> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> " telemetry command, ignoring");
> + /* Parsing parameter finished */
> + free(l_params);
>
> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> eth_dev_id,
> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> }
>
> return 0;
> +
> +error:
> + free(l_params);
> + return ret;
> }
>
> RTE_INIT(rxa_init_telemetry)
> --
> 2.25.1
On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
<jay.jayatheerthan@intel.com> wrote:
>
> Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
>
> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
Applied to dpdk-next-net-eventdev/for-main. Thanks
>
> -Jay
>
>
>
> > -----Original Message-----
> > From: Weiguo Li <liwg06@foxmail.com>
> > Sent: Friday, February 4, 2022 1:51 PM
> > To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> > Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
> > stephen@networkplumber.org; dev@dpdk.org
> > Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
> >
> > The memory get from strdup should be freed when parameter parsing
> > finished, and also should be freed when error occurs.
> >
> > Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> > Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >
> > Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> > Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >
> > ---
> > v6:
> > * fix double free problem in v5 as Jay noted
> > v5:
> > * fix according to Jay's comment.
> > * fix anothor bypassing the freeing of memory problem
> > v4:
> > * fix compilation issue
> > v3:
> > * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> > as Ganapati's comment.
> > * validate "token" by macros to reduce code redundancy.
> > * use new macros to avoid bypassing the freeing of memory problem.
> > v2:
> > * add memory check after strdup
> > ---
> > lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> > 1 file changed, 72 insertions(+), 24 deletions(-)
> >
> > diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
> > index ae1e260c08..4da6f1ff90 100644
> > --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> > +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> > @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
> > } \
> > } 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\n", id); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} 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)) { \
> > + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} while (0)
> > +
> > +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
> > + if (!rte_eth_dev_is_valid_port(port_id)) { \
> > + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> > + ret = retval; \
> > + goto error; \
> > + } \
> > +} while (0)
> > +
> > static inline int
> > rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> > {
> > @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> > struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> >
> > @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + 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_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* 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);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
> > rx_queue_id, &queue_conf)) {
> > @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> > RXA_ADD_DICT(queue_conf.ev, flow_id);
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > static int
> > @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> > struct rte_event_eth_rx_adapter_queue_stats q_stats;
> >
> > @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + 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_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* 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);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
> > rx_queue_id, &q_stats)) {
> > @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
> > RXA_ADD_DICT(q_stats, rx_dropped);
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > static int
> > @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> > {
> > uint8_t rx_adapter_id;
> > uint16_t rx_queue_id;
> > - int eth_dev_id;
> > + int eth_dev_id, ret = -1;
> > char *token, *l_params;
> >
> > if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> > @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> >
> > /* Get Rx adapter ID from parameter string */
> > l_params = strdup(params);
> > + if (l_params == NULL)
> > + return -ENOMEM;
> > token = strtok(l_params, ",");
> > + 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_ERR_RET(rx_adapter_id, -EINVAL);
> > + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> > + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >
> > token = strtok(NULL, ",");
> > - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> > - return -1;
> > + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
> >
> > /* 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);
> > - return -EINVAL;
> > + ret = -EINVAL;
> > + goto error;
> > }
> >
> > token = strtok(NULL, "\0");
> > if (token != NULL)
> > RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> > " telemetry command, ignoring");
> > + /* Parsing parameter finished */
> > + free(l_params);
> >
> > if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> > eth_dev_id,
> > @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> > }
> >
> > return 0;
> > +
> > +error:
> > + free(l_params);
> > + return ret;
> > }
> >
> > RTE_INIT(rxa_init_telemetry)
> > --
> > 2.25.1
>
On 18/02/2022 09:39, Jerin Jacob wrote:
> On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com> wrote:
>>
>> Looks good. Thanks Weiguo for posting this patch and addressing all the comments!
>>
>> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
>
> Applied to dpdk-next-net-eventdev/for-main. Thanks
>
>
>>
>> -Jay
>>
>>
>>
>>> -----Original Message-----
>>> From: Weiguo Li <liwg06@foxmail.com>
>>> Sent: Friday, February 4, 2022 1:51 PM
>>> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
>>> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish K, S V <s.v.naga.harish.k@intel.com>;
>>> stephen@networkplumber.org; dev@dpdk.org
>>> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing finished
>>>
>>> The memory get from strdup should be freed when parameter parsing
>>> finished, and also should be freed when error occurs.
>>>
>>> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
>>> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
>>>
No stable tag - is it wanted for relevant stable branches?
>>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
>>> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
>>>
>>> ---
>>> v6:
>>> * fix double free problem in v5 as Jay noted
>>> v5:
>>> * fix according to Jay's comment.
>>> * fix anothor bypassing the freeing of memory problem
>>> v4:
>>> * fix compilation issue
>>> v3:
>>> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
>>> as Ganapati's comment.
>>> * validate "token" by macros to reduce code redundancy.
>>> * use new macros to avoid bypassing the freeing of memory problem.
>>> v2:
>>> * add memory check after strdup
>>> ---
>>> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
>>> 1 file changed, 72 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> index ae1e260c08..4da6f1ff90 100644
>>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
>>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
>>> } \
>>> } 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\n", id); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} 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)) { \
>>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} while (0)
>>> +
>>> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
>>> + if (!rte_eth_dev_is_valid_port(port_id)) { \
>>> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
>>> + ret = retval; \
>>> + goto error; \
>>> + } \
>>> +} while (0)
>>> +
>>> static inline int
>>> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
>>> {
>>> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
>>>
>>> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + 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_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* 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);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
>>> rx_queue_id, &queue_conf)) {
>>> @@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
>>> RXA_ADD_DICT(queue_conf.ev, flow_id);
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> static int
>>> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>> struct rte_event_eth_rx_adapter_queue_stats q_stats;
>>>
>>> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + 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_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* 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);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
>>> rx_queue_id, &q_stats)) {
>>> @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
>>> RXA_ADD_DICT(q_stats, rx_dropped);
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> static int
>>> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>> {
>>> uint8_t rx_adapter_id;
>>> uint16_t rx_queue_id;
>>> - int eth_dev_id;
>>> + int eth_dev_id, ret = -1;
>>> char *token, *l_params;
>>>
>>> if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>> @@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>>
>>> /* Get Rx adapter ID from parameter string */
>>> l_params = strdup(params);
>>> + if (l_params == NULL)
>>> + return -ENOMEM;
>>> token = strtok(l_params, ",");
>>> + 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_ERR_RET(rx_adapter_id, -EINVAL);
>>> + RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
>>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
>>>
>>> token = strtok(NULL, ",");
>>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
>>> - return -1;
>>> + RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
>>>
>>> /* 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);
>>> - return -EINVAL;
>>> + ret = -EINVAL;
>>> + goto error;
>>> }
>>>
>>> token = strtok(NULL, "\0");
>>> if (token != NULL)
>>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
>>> " telemetry command, ignoring");
>>> + /* Parsing parameter finished */
>>> + free(l_params);
>>>
>>> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
>>> eth_dev_id,
>>> @@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
>>> }
>>>
>>> return 0;
>>> +
>>> +error:
>>> + free(l_params);
>>> + return ret;
>>> }
>>>
>>> RTE_INIT(rxa_init_telemetry)
>>> --
>>> 2.25.1
>>
>
Hi Kevin,
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, February 22, 2022 3:39 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Jayatheerthan, Jay
> <jay.jayatheerthan@intel.com>
> Cc: Weiguo Li <liwg06@foxmail.com>; Kundapura, Ganapati
> <Ganapati.Kundapura@intel.com>; Naga Harish K, S V
> <s.v.naga.harish.k@intel.com>; stephen@networkplumber.org; dev@dpdk.org
> Subject: Re: [PATCH v6] eventdev/eth_rx: fix memory leak when token parsing
> finished
>
> On 18/02/2022 09:39, Jerin Jacob wrote:
> > On Fri, Feb 4, 2022 at 3:17 PM Jayatheerthan, Jay
> > <jay.jayatheerthan@intel.com> wrote:
> >>
> >> Looks good. Thanks Weiguo for posting this patch and addressing all the
> comments!
> >>
> >> Acked-by: Jay Jayatheerthan < jay.jayatheerthan@intel.com>
> >
> > Applied to dpdk-next-net-eventdev/for-main. Thanks
> >
> >
> >>
> >> -Jay
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Weiguo Li <liwg06@foxmail.com>
> >>> Sent: Friday, February 4, 2022 1:51 PM
> >>> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
> >>> Cc: Kundapura, Ganapati <ganapati.kundapura@intel.com>; Naga Harish
> >>> K, S V <s.v.naga.harish.k@intel.com>; stephen@networkplumber.org;
> >>> dev@dpdk.org
> >>> Subject: [PATCH v6] eventdev/eth_rx: fix memory leak when token
> >>> parsing finished
> >>>
> >>> The memory get from strdup should be freed when parameter parsing
> >>> finished, and also should be freed when error occurs.
> >>>
> >>> Fixes: 814d01709328 ("eventdev/eth_rx: support telemetry")
> >>> Fixes: 9e583185318f ("eventdev/eth_rx: support telemetry")
> >>>
>
> No stable tag - is it wanted for relevant stable branches?
>
Yes, this patch need to be included in relevant stable releases.
> >>> Signed-off-by: Weiguo Li <liwg06@foxmail.com>
> >>> Acked-by: Ganapati Kundapura <ganapati.kundapura@intel.com>
> >>>
> >>> ---
> >>> v6:
> >>> * fix double free problem in v5 as Jay noted
> >>> v5:
> >>> * fix according to Jay's comment.
> >>> * fix anothor bypassing the freeing of memory problem
> >>> v4:
> >>> * fix compilation issue
> >>> v3:
> >>> * validate "eth_dev_id" by rte_eth_dev_is_valid_port()
> >>> as Ganapati's comment.
> >>> * validate "token" by macros to reduce code redundancy.
> >>> * use new macros to avoid bypassing the freeing of memory problem.
> >>> v2:
> >>> * add memory check after strdup
> >>> ---
> >>> lib/eventdev/rte_event_eth_rx_adapter.c | 96 ++++++++++++++++++-------
> >>> 1 file changed, 72 insertions(+), 24 deletions(-)
> >>>
> >>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> index ae1e260c08..4da6f1ff90 100644
> >>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
> >>> @@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter
> *rx_adapter, uint16_t eth_dev_id,
> >>> } \
> >>> } 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\n", id); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} 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)) { \
> >>> + RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} while (0)
> >>> +
> >>> +#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do
> { \
> >>> + if (!rte_eth_dev_is_valid_port(port_id)) { \
> >>> + RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
> >>> + ret = retval; \
> >>> + goto error; \
> >>> + } \
> >>> +} while (0)
> >>> +
> >>> static inline int
> >>> rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
> >>> {
> >>> @@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>> struct rte_event_eth_rx_adapter_queue_conf queue_conf;
> >>>
> >>> @@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd
> >>> __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> 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_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* 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);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id,
> eth_dev_id,
> >>> rx_queue_id,
> >>> &queue_conf)) { @@ -3378,6 +3406,10 @@
> handle_rxa_get_queue_conf(const char *cmd __rte_unused,
> >>> RXA_ADD_DICT(queue_conf.ev, flow_id);
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> static int
> >>> @@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>> struct rte_event_eth_rx_adapter_queue_stats q_stats;
> >>>
> >>> @@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd
> >>> __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> 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_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* 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);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id,
> eth_dev_id,
> >>> rx_queue_id,
> >>> &q_stats)) { @@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const
> char *cmd __rte_unused,
> >>> RXA_ADD_DICT(q_stats, rx_dropped);
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> static int
> >>> @@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> >>> {
> >>> uint8_t rx_adapter_id;
> >>> uint16_t rx_queue_id;
> >>> - int eth_dev_id;
> >>> + int eth_dev_id, ret = -1;
> >>> char *token, *l_params;
> >>>
> >>> if (params == NULL || strlen(params) == 0 ||
> >>> !isdigit(*params)) @@ -3458,33 +3498,37 @@
> >>> handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
> >>>
> >>> /* Get Rx adapter ID from parameter string */
> >>> l_params = strdup(params);
> >>> + if (l_params == NULL)
> >>> + return -ENOMEM;
> >>> token = strtok(l_params, ",");
> >>> +
> 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_ERR_RET(rx_adapter_id,
> -EINVAL);
> >>> +
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id,
> >>> + -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
> >>> + RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
> >>>
> >>> token = strtok(NULL, ",");
> >>> - if (token == NULL || strlen(token) == 0 || !isdigit(*token))
> >>> - return -1;
> >>> +
> RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token,
> >>> + -1);
> >>>
> >>> /* 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);
> >>> - return -EINVAL;
> >>> + ret = -EINVAL;
> >>> + goto error;
> >>> }
> >>>
> >>> token = strtok(NULL, "\0");
> >>> if (token != NULL)
> >>> RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
> >>> " telemetry command, ignoring");
> >>> + /* Parsing parameter finished */
> >>> + free(l_params);
> >>>
> >>> if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
> >>> eth_dev_id, @@
> >>> -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd
> __rte_unused,
> >>> }
> >>>
> >>> return 0;
> >>> +
> >>> +error:
> >>> + free(l_params);
> >>> + return ret;
> >>> }
> >>>
> >>> RTE_INIT(rxa_init_telemetry)
> >>> --
> >>> 2.25.1
> >>
> >
>
@@ -293,6 +293,30 @@ rxa_event_buf_get(struct event_eth_rx_adapter *rx_adapter, uint16_t eth_dev_id,
} \
} 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\n", id); \
+ ret = retval; \
+ goto error; \
+ } \
+} 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)) { \
+ RTE_EDEV_LOG_ERR("Invalid eth Rx adapter token\n"); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
+#define RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_ETHDEV_LOG(ERR, "Invalid port_id=%u\n", port_id); \
+ ret = retval; \
+ goto error; \
+ } \
+} while (0)
+
static inline int
rxa_sw_adapter_queue_count(struct event_eth_rx_adapter *rx_adapter)
{
@@ -3323,7 +3347,7 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_conf queue_conf;
@@ -3332,33 +3356,37 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ 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_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* 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);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_conf_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &queue_conf)) {
@@ -3378,6 +3406,10 @@ handle_rxa_get_queue_conf(const char *cmd __rte_unused,
RXA_ADD_DICT(queue_conf.ev, flow_id);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3387,7 +3419,7 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
struct rte_event_eth_rx_adapter_queue_stats q_stats;
@@ -3396,33 +3428,37 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ 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_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* 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);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_get(rx_adapter_id, eth_dev_id,
rx_queue_id, &q_stats)) {
@@ -3441,6 +3477,10 @@ handle_rxa_get_queue_stats(const char *cmd __rte_unused,
RXA_ADD_DICT(q_stats, rx_dropped);
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
static int
@@ -3450,7 +3490,7 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
{
uint8_t rx_adapter_id;
uint16_t rx_queue_id;
- int eth_dev_id;
+ int eth_dev_id, ret = -1;
char *token, *l_params;
if (params == NULL || strlen(params) == 0 || !isdigit(*params))
@@ -3458,33 +3498,37 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
/* Get Rx adapter ID from parameter string */
l_params = strdup(params);
+ if (l_params == NULL)
+ return -ENOMEM;
token = strtok(l_params, ",");
+ 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_ERR_RET(rx_adapter_id, -EINVAL);
+ RTE_EVENT_ETH_RX_ADAPTER_ID_VALID_OR_GOTO_ERR_RET(rx_adapter_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ 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_EVENTDEV_VALID_DEVID_OR_ERR_RET(eth_dev_id, -EINVAL);
+ RTE_ETH_VALID_PORTID_OR_GOTO_ERR_RET(eth_dev_id, -EINVAL);
token = strtok(NULL, ",");
- if (token == NULL || strlen(token) == 0 || !isdigit(*token))
- return -1;
+ RTE_EVENT_ETH_RX_ADAPTER_TOKEN_VALID_OR_GOTO_ERR_RET(token, -1);
/* 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);
- return -EINVAL;
+ ret = -EINVAL;
+ goto error;
}
token = strtok(NULL, "\0");
if (token != NULL)
RTE_EDEV_LOG_ERR("Extra parameters passed to eventdev"
" telemetry command, ignoring");
+ /* Parsing parameter finished */
+ free(l_params);
if (rte_event_eth_rx_adapter_queue_stats_reset(rx_adapter_id,
eth_dev_id,
@@ -3494,6 +3538,10 @@ handle_rxa_queue_stats_reset(const char *cmd __rte_unused,
}
return 0;
+
+error:
+ free(l_params);
+ return ret;
}
RTE_INIT(rxa_init_telemetry)