From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 5E8781E2B for ; Thu, 25 Oct 2018 08:48:22 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id D7A9822127; Thu, 25 Oct 2018 02:48:21 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 25 Oct 2018 02:48:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=So4MrwnROps8RwTC0F4KunVWJuDYfFnmye+WBVUhCpM=; b=s12t6LxWScA9 LK/ImuQ8LBA9dEdrVKoN5annmLhSJM5R7iEj4Q/pFn6ty7BDH0TVlOr5m9TqZHMA 6zbLFkd+JT1xo1aKGkfkZmmbDL047rf85H6Zusy1eNIufrHGtvtBdYjI/QK+vU/p fWz21rhec5Xnt6wlpGLQmgSvEpgKcew= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=So4MrwnROps8RwTC0F4KunVWJuDYfFnmye+WBVUhC pM=; b=BH2E9MCIrj/xPJPiX4ZiUe5dH0ExNGc6vTntUzlDrRcvB7vXDC7d3ocap kLbNT+OpVbTsV5EIVETZqzvy0E6wUwTwb553h0P+HFIPvxnRpPT6cnxMxljQUbNM 3SE/0dV2CAfAJjSdLKYW3llxNBkE9yOaAUO+udn2JuBzdeuTN5CKpOPFcTm5+exN qYRTCsLl4vno8xpIa4nZJ3e6TDIdicQtSxIpQSLVE701dmv9ByhHQlNIDr9Qi7zh 5hL0qMqY2yA5bw4bjIMR1insNKoBSduXnx70cL5oVx0WQ0Zsve1+hAKzt/nfjrfx vRTB6mXtoZd2CawpsiBQTLPPQ3wWQ== X-ME-Sender: X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id B0E1EE44D4; Thu, 25 Oct 2018 02:48:19 -0400 (EDT) From: Thomas Monjalon To: Hideyuki Yamashita Cc: dev@dpdk.org, Gaetan Rivet Date: Thu, 25 Oct 2018 08:48:22 +0200 Message-ID: <5338151.rt6oEsQEfv@xps> In-Reply-To: <201810250255.w9P2tC3e007738@ccmail04.silk.ntt-tx.co.jp> References: <201810230152.w9N1qgQD015354@ccmail04.silk.ntt-tx.co.jp> <3271179.oIOqrX44Jl@xps> <201810250255.w9P2tC3e007738@ccmail04.silk.ntt-tx.co.jp> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: Thu, 25 Oct 2018 06:48:22 -0000 25/10/2018 04:54, Hideyuki Yamashita: > Hi, > > > Yes it may work with most of the drivers. > Question for my understadnding. > You said that most of the drivers assign only one > port when hotplug_add is called, right? > Then what is the exception? > What kind of device/driver assign multiple ports? At least Mellanox and Chelsio, maybe more. > My program attach to > - vhost pmd > - pcap pmd > - null pmd > and I understand those device(?) assign only one port per attach. No problem with vdevs. > > Hi, > > > > 23/10/2018 03:52, Hideyuki Yamashita: > > > 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). > > > > Yes > > > > > 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) > > > > Yes it may work with most of the drivers. > > > > > 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? > > > > Better to wait 18.11. > > > > > 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. > > > > We do not remove a method without a replacement. > > You can use rte_eth_dev_attach() until 18.08, > > and switch to the new iterator with 18.11. > > > > > > > > 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! > > > > > > > > > > > >