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 6B6694389F; Fri, 12 Jan 2024 08:25:35 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E49904028C; Fri, 12 Jan 2024 08:25:34 +0100 (CET) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 30A2F4025E for ; Fri, 12 Jan 2024 08:25:33 +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 32B3250; Fri, 12 Jan 2024 10:25:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 32B3250 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1705044332; bh=EmhIrfDo02FzKxtksMUdnSfHf1v7hYYtLTddNKF7Zuw=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=AMP3ukZHNqUN2jlqg8Dmo/XaMMvNKozgi0N5oOlfF6RZLtdqm8W7HyoQck8K9peGi StHoMdgHwHtjxbqf7NfyrjqT+HXXW3YlKc7CwZzJKTJu2Da+bwBp0WG9YuV4sBFrq4 iS/zq4F+nAt91qu0rUamYLGdsGshZ4f3hs5zFj44= Message-ID: <31463941-bfaf-4170-9889-90a860cea4ba@oktetlabs.ru> Date: Fri, 12 Jan 2024 10:25:31 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] ethdev: parsing multiple representor devargs string Content-Language: en-US To: Harman Kalra , 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 , Thomas Monjalon , Ferruh Yigit Cc: dev@dpdk.org References: <20240111064432.193119-1-hkalra@marvell.com> <20240111064432.193119-2-hkalra@marvell.com> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20240111064432.193119-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/11/24 09:44, 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],representor=pf2vf[1,2-3],representor=[4-5] > > Signed-off-by: Harman Kalra [snip] > diff --git a/lib/ethdev/ethdev_driver.c b/lib/ethdev/ethdev_driver.c > index bd917a15fc..62a06b75a2 100644 > --- a/lib/ethdev/ethdev_driver.c > +++ b/lib/ethdev/ethdev_driver.c > @@ -470,15 +470,14 @@ eth_dev_devargs_tokenise(struct rte_kvargs *arglist, const char *str_in) > } > > 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) > { > - struct rte_kvargs args; > + struct rte_eth_devargs *eth_da; > struct rte_kvargs_pair *pair; > - unsigned int i; > + struct rte_kvargs args; > + unsigned int i, j = 0; Please, avoid multiple variable in one line declaration with initialization. It is too error prone. Also j is a bad name here. It is not a loop counter or seomthing like this. Maybe 'parsed' or 'devargs_parsed'? > int result = 0; > > - memset(eth_da, 0, sizeof(*eth_da)); > - > result = eth_dev_devargs_tokenise(&args, dargs); > if (result < 0) > goto parse_cleanup; > @@ -486,18 +485,16 @@ rte_eth_devargs_parse(const char *dargs, struct rte_eth_devargs *eth_da) > for (i = 0; i < args.count; i++) { > pair = &args.pairs[i]; > if (strcmp("representor", pair->key) == 0) { > - if (eth_da->type != RTE_ETH_REPRESENTOR_NONE) { > - RTE_ETHDEV_LOG_LINE(ERR, "duplicated representor key: %s", > - dargs); > - result = -1; > - goto parse_cleanup; > - } > + eth_da = ð_devargs[j]; > + memset(eth_da, 0, sizeof(*eth_da)); > result = rte_eth_devargs_parse_representor_ports( > pair->value, eth_da); > if (result < 0) > goto parse_cleanup; > + j++; > } > } > + result = (int)j; > > parse_cleanup: > free(args.str); > diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h > index b482cd12bb..6f4f1a1606 100644 > --- a/lib/ethdev/ethdev_driver.h > +++ b/lib/ethdev/ethdev_driver.h > @@ -1800,10 +1800,10 @@ rte_eth_representor_id_get(uint16_t port_id, > * @param devargs > * device arguments > * @param eth_devargs > - * parsed ethdev specific arguments. > + * contiguous memory populated with parsed ethdev specific arguments. Caller must provide information about array size. Right now you simply introduce out-of-bound array access. All drivers just provide one entry and if I pass many-many devargs I can write far beyond that location and corrupt stack. > * > * @return > - * Negative errno value on error, 0 on success. > + * Negative errno value on error, no of devargs parsed on success. > */ > __rte_internal > int