From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id E317B438E6; Wed, 17 Jan 2024 09:47:03 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 62CA8402B0; Wed, 17 Jan 2024 09:47:03 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 0E068402A6 for ; Wed, 17 Jan 2024 09:47:02 +0100 (CET) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 4398A59; Wed, 17 Jan 2024 11:47:01 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 4398A59 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1705481221; bh=GwCC6YgGb+lAK/neGKTbsOkldk3h3ioPWz3uyd6hksU=; h=Date:Subject:To:References:From:In-Reply-To:From; b=kWkDwjolxqACWBL46VvWOKAd8RGsmMswxLyeKmoCIDi3gdWEUcChIwXBfW8klbpgf JqR1/MMc3Tf98kaK2qYQ8VFPvUpy68wOCxng4hwtUV0yu3oZWzZwX3p2BGuXWvufcf Wzp+67QbDGmY9RqGM+evzaTBsKvBydfg7j7j2vgY= Message-ID: Date: Wed, 17 Jan 2024 11:47:00 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/1] ethdev: parsing multiple representor devargs string Content-Language: en-US To: Harman Kalra , dev@dpdk.org, Thomas Monjalon , Ferruh Yigit , Ajit Khaparde , Somnath Kotur , John Daley , Hyong Youb Kim , Yuying Zhang , Beilei Xing , Qiming Yang , Qi Zhang , Wenjun Wu , Dariusz Sosnowski , Viacheslav Ovsiienko , Ori Kam , Suanming Mou , Matan Azrad , Chaoyong He References: <20240111064432.193119-1-hkalra@marvell.com> <20240116095533.40337-1-hkalra@marvell.com> <20240116095533.40337-2-hkalra@marvell.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20240116095533.40337-2-hkalra@marvell.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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: > ,representor=[pf[0-1],pf2vf[1,2-3],[4-5]] > > Signed-off-by: Harman Kalra 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 > #include > #include > > @@ -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]