[PATCH v3 1/1] ethdev: parsing multiple representor devargs string
Andrew Rybchenko
andrew.rybchenko at oktetlabs.ru
Wed Jan 17 09:47:00 CET 2024
On 1/16/24 12:55, Harman Kalra wrote:
> Adding support for parsing multiple representor devargs strings
> passed to a PCI BDF. There may be scenario where port representors
> for various PFs or VFs under PFs are required and all these are
> representor ports shall be backed by single pci device. In such
> case port representors can be created using devargs string:
> <PCI BDF>,representor=[pf[0-1],pf2vf[1,2-3],[4-5]]
>
> Signed-off-by: Harman Kalra <hkalra at marvell.com>
see below
> diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c
> index bd917a15fc..8a49511516 100644
> --- a/lib/ethdev/ethdev_driver.c
> +++ b/lib/ethdev/ethdev_driver.c
> @@ -2,6 +2,7 @@
> * Copyright(c) 2022 Intel Corporation
> */
>
> +#include <ctype.h>
> #include <stdlib.h>
> #include <pthread.h>
>
> @@ -459,9 +460,23 @@ eth_dev_devargs_tokenise(struct rte_kvargs *arglist, const char *str_in)
> break;
>
> case 3: /* Parsing list */
> - if (*letter == ']')
> - state = 2;
> - else if (*letter == '\0')
> + if (*letter == ']') {
> + /* Multiple representor case has ']' dual meaning, first end of
> + * individual pfvf list and other end of consolidated list of
> + * representors.
> + * Complete multiple representors list to be considered as one
> + * pair value.
> + */
> + if ((strcmp("representor", pair->key) == 0) &&
> + ((*(letter + 2) == 'p' && *(letter + 3) == 'f') ||
Sorry, but it is unclear why it is not out-of-bound access.
> + (*(letter + 2) == 'v' && *(letter + 3) == 'f') ||
> + (*(letter + 2) == 's' && *(letter + 3) == 'f') ||
may be it is better to use strncmp() instead? IMHO it is a bit hard to
follow.
> + (*(letter + 2) == 'c' && isdigit(*(letter + 3))) ||
> + (*(letter + 2) == '[' && isdigit(*(letter + 3)))))
> + state = 3;
> + else
> + state = 2;
> + } else if (*letter == '\0')
> return -EINVAL;
> break;
> }
> @@ -469,16 +484,56 @@ eth_dev_devargs_tokenise(struct rte_kvargs *arglist, const char *str_in)
> }
> }
>
> +static int
> +eth_dev_tokenise_representor_list(char *p_val, struct rte_eth_devargs *eth_devargs,
> + uint8_t nb_da)
> +{
> + struct rte_eth_devargs *eth_da;
> + char da_val[BUFSIZ];
> + char delim[] = "]";
> + int devargs = 0;
> + int result = 0;
> + char *token;
> +
> + token = strtok(&p_val[1], delim);
since strtok() is MT-unsafe, I'd recommend to use strtok_r()
> + while (token != NULL) {
> + eth_da = ð_devargs[devargs];
> + memset(eth_da, 0, sizeof(*eth_da));
> + snprintf(da_val, BUFSIZ, "%s%c", (token[0] == ',') ? ++token : token, ']');
> + /* Parse the tokenised devarg value */
> + result = rte_eth_devargs_parse_representor_ports(da_val, eth_da);
> + if (result < 0)
> + goto parse_cleanup;
> + devargs++;
> + if (devargs > nb_da) {
> + RTE_ETHDEV_LOG_LINE(ERR,
> + "Devargs parsed %d > max array size %d",
> + devargs, nb_da);
> + result = -1;
> + goto parse_cleanup;
> + }
> + token = strtok(NULL, delim);
> + }
> +
> + result = devargs;
> +
> +parse_cleanup:
> + return result;
> +
> +}
> +
> int
> -rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da)
> +rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_devargs,
> + uint8_t nb_da)
I see no single reason to limit nb_da to uint8_t type. IMHO it should be
'unsigned int' as an unsigned number of default type.
'unsigned int' is used for number of stats and ptypes in array.
[snip]
More information about the dev
mailing list