From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail04.ics.ntt-tx.co.jp (mail05.ics.ntt-tx.co.jp [210.232.35.69]) by dpdk.org (Postfix) with ESMTP id BDA0E1B1EA for ; Tue, 23 Oct 2018 03:53:10 +0200 (CEST) Received: from gwchk03.silk.ntt-tx.co.jp (gwchk03.silk.ntt-tx.co.jp [10.107.0.111]) by mail04.ics.ntt-tx.co.jp (unknown) with ESMTP id w9N1r7an005963; Tue, 23 Oct 2018 10:53:07 +0900 Received: (from root@localhost) by gwchk03.silk.ntt-tx.co.jp (unknown) id w9N1r7iE018069; Tue, 23 Oct 2018 10:53:07 +0900 Received: from gwchk.silk.ntt-tx.co.jp [10.107.0.110] by gwchk03.silk.ntt-tx.co.jp with ESMTP id LAA18068; Tue, 23 Oct 2018 10:53:07 +0900 Received: from imss06.silk.ntt-tx.co.jp (localhost [127.0.0.1]) by ccmail04.silk.ntt-tx.co.jp (unknown) with ESMTP id w9N1r6ug015592; Tue, 23 Oct 2018 10:53:06 +0900 Received: from imss06.silk.ntt-tx.co.jp (localhost [127.0.0.1]) by imss06.silk.ntt-tx.co.jp (unknown) with ESMTP id w9N1r6kx024300; Tue, 23 Oct 2018 10:53:06 +0900 Received: from ccmail04 (smtp03.silk.ntt-tx.co.jp [10.107.0.135]) by imss06.silk.ntt-tx.co.jp (unknown) with SMTP id w9N1r6Gu024297; Tue, 23 Oct 2018 10:53:06 +0900 Date: Tue, 23 Oct 2018 10:52:08 +0900 From: Hideyuki Yamashita In-Reply-To: <12632543.HjcP2Z8abA@xps> References: <201810221124.w9MBOrLn020296@ccmail04.silk.ntt-tx.co.jp> <12632543.HjcP2Z8abA@xps> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Mailer: Becky! ver. 2.74 [ja] X-CCMail7: CC-Mail-V7.0.2-Client-Relayed Message-Id: <201810230152.w9N1qgQD015354@ccmail04.silk.ntt-tx.co.jp> X-TM-AS-MML: No X-CC-Mail-RelayStamp: CC-Mail-V5.14-Server To: Thomas Monjalon Cc: dev@dpdk.org, Gaetan Rivet Subject: Re: [dpdk-dev] How to replace rte_eth_dev_attach with rte_eal_hotplug_add X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Oct 2018 01:53:11 -0000 Hi, Thanks for your guidance again. Q1. Is following my understanding correct? If a device has multiple port, then "rte_eth_dev_get_port_by_name" will NOT work becauase it uses strcmp and needs "exact match" of the device name. New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care of this issue and even if the name is "parially matched" with the given parameter(user provided devargs). Q2. If my program only handles devices which create only one port, then "rte_eth_dev_get_port_by_name" may work. (Though such a program does not have extensibility and only workable under certain limitations) Q3. When new iterator RTE_ETH_FOREACH_MATCHING_DEV will be available? Do I have to wait 18.11 release or can I get those in git before release? I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV is much better than using "rte_eth_dev_get_port_by_name" with regard to handle devices which create multiple port. But I need to replace existing deprecated attach/detach APIs to new codes to maintain continuity of my product. BR, Hideyuki Yamashita NTT TechnoCross > Hi, > > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons: > - it is a loop, so work if multiple ports are matching > - it uses devargs parameter, which is what the user requests > > Note: your code assumes that the ethdev name is devargs.name. > It can be true by chance, but nothing forces drivers to assign port names > this way. It will be wrong, for sure, if a device has multiple ports. > > > 22/10/2018 13:24, Hideyuki Yamashita: > > Hello Thomas, > > > > Thanks for your info. > > What is the difference between using > > rte_eth_dev_get_port_by_name and > > RTE_ETH_FOREACH_MATCHING_DEV? > > > > I think using rte_eth_dev_get_port_by_name is > > workable. > > (In fact I modified my code already and it worked with no problem) > > > > So my question is "what is the difference" and "which is better approach". > > > > Thanks and BR, > > Hideyuki Yamashita > > NTT TechnoCross > > > > > Hi, > > > > > > I am actively working on it. > > > Look how rte_eth_dev_attach is replaced in testpmd: > > > https://patches.dpdk.org/patch/47019/ > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV. > > > > > > > > > 22/10/2018 06:34, Hideyuki Yamashita: > > > > Dear Thomas and all, > > > > > > > > About a month ago, I posted the topic related with > > > > how to replace rte_eth_dev_attach. > > > > > > > > Following your advice, > > > > my code would be as below: > > > > (Old code using deprecated API is commented out) > > > > > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port > > > > after rte_eal_hotplug_add. > > > > Note that my application is just one of the dpdk applications(in the host) > > > > and within the process, only one thread handles device attatch/detach. > > > > (No race condition with regard to device hot_plug will > > > > not take place) > > > > ------------------------------------------------------------------- > > > > //ret = rte_eth_dev_attach(devargs, &vhost_port_id); > > > > //if (ret < 0) > > > > // return ret; > > > > > > > > struct rte_devargs da; > > > > > > > > memset(&da, 0, sizeof(da)); > > > > > > > > /* parse devargs */ > > > > if (rte_devargs_parse(&da, devargs)) > > > > return -1; > > > > ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args); > > > > if (ret < 0) { > > > > free(da.args); > > > > return ret; > > > > } > > > > free(da.args); > > > > ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id); > > > > if (ret < 0) > > > > return ret; > > > > ----------------------------------------------------------------------------- > > > > > > > > If you have any concerns/additional advices, please let me know. > > > > > > > > BR, > > > > Hideyuki Yamashita > > > > NTT TechnoCross > > > > > > > > > > > > > 27/09/2018 12:40, Hideyuki Yamashita: > > > > > > Dear Thomas, > > > > > > > > > > > > Thansk for your answer. > > > > > > Please see inline. > > > > > > > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita: > > > > > > > > Dear Thomas, > > > > > > > > > > > > > > > > Thanks for your answer. > > > > > > > > It took me a little time to digest answer. > > > > > > > > Please see inline. > > > > > > > > > > > > > > > > > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita: > > > > > > > > > > Dear Gaetan and Thomas, > > > > > > > > > > > > > > > > > > > > Thanks for your answer. > > > > > > > > > > Please see inline. > > > > > > > > > > > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet: > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote: > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add > > > > > > > > > > > > > and rte_eal_hotplug_remove. > > > > > > > > > > > > > > > > > > > > > > > > > > My program uses above mentioned deprecated APIs > > > > > > > > > > > > > and have to replace those. > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd. > > > > > > > > > > > > > > > > > > > > > > > > > > My question is whether it is correct to replace those as following: > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and > > > > > > > > > > > > > migrate those content into my program. > > > > > > > > > > > > > > > > > > > > > > > > > > e.g. > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach > > > > > > > > > > > > > > > > > > > > > > > > > > Your advice/guidance are much appreciated. > > > > > > > > > > > > > Thanks! > > > > > > > > > > > > > > > > > > > > > > > > Hello Hideyuki, > > > > > > > > > > > > > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug > > > > > > > > > > > > function would already return an error if the PMD was not able to create > > > > > > > > > > > > the necessary devices. > > > > > > > > > > > > > > > > > > > > > > > > The main issue might be to find the port_id of your new port. > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one > > > > > > > > > > > > matching your parameters (you might for example match the rte_device > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard > > > > > > > > > > > > naming scheme at the ethdev level). > > > > > > > > > > First of all, thank for your answering to my question. > > > > > > > > > > But I have questions. > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions) > > > > > > > > > > > > > > > > > > > > Q1. > > > > > > > > > > Why eth_dev_last_created_port can not be used? > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls > > > > > > > > > > > > > > > > > > > > *port_id = eth_dev_last_created_port; > > > > > > > > > > > > > > > > > > > > at the end of the function. > > > > > > > > > > > > > > > > > > You can have a race condition. > > > > > > > > Please elaborate me a bit more. > > > > > > > > > > > > > > > > Is it correct understanding that race condition > > > > > > > > includes > > > > > > > > - read information before port is available > > > > > > > > - other device may be plugged (or unplugged) > > > > > > > > and so using "eth_dev_last_created_port" is > > > > > > > > NOT reliable? > > > > > > > > > > > > > > I am thinking about the second one only. > > > > > > > > > > > > If we assume there is only one DPDK application > > > > > > inside the host and within the application, only one thread > > > > > > handles attach/detach of devices, then is it ok to use > > > > > > > > > > *port_id = eth_dev_last_created_port; > > > > > > because there seems no possiblity race condition > > > > > > takes place? > > > > > > > > > > If you are never probing a new port outside of this thread, > > > > > I guess it's OK. > > > > > Take care of not attaching from the interrupt thread too! > > >