From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 99AFF374 for ; Thu, 29 Jun 2017 14:59:49 +0200 (CEST) Received: by mail-wm0-f47.google.com with SMTP id b184so13733505wme.1 for ; Thu, 29 Jun 2017 05:59:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=OeYUR2Vl/eNUWBLQr7DcKDjqzCm/wC+2X+wy+rhVWG8=; b=CfH3MPKSYu2DYFJQbQki1fsorxmMVddEyeWsjkxcT8Vt6wXoucRSAWrwoWb2BA4WUZ Zn+e5eRh0jCkj/tzNgMEZoPJ3VGTfQjlMBGWJtHLIT912GDmHmCi0FnVcAWjnQmZLH7V hhXagLUbzSR65pAPSC5ybe6Jr/QK8+wJVjKunMtagLr6oUuFNOAEW+Yidk1zQ46hOtIC t1/t4mPhesIvZtmQzIbXUO/qSSSsAUBOEY9m5tt1s3ZrVC0XfdFz2FknUCLuPmyXeMue P48tI0B9jRrV1ySuK0Q1nCG+K3FBdwZwY+dlaOcPcUZQ9dqppPEVuqxHjw+kE/bJwguY GAEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=OeYUR2Vl/eNUWBLQr7DcKDjqzCm/wC+2X+wy+rhVWG8=; b=TgK0pHocFcq5liOVt/XWFwljsw7xr7ABsuULf8AHISwU6yvF0Z9yNr9eeblsY6d2uW fQQCKeeNk9xGf93v5DVdVp3TsQJMtkSPFSz0IcPDiJHd9r/VVjabON4wkO/5ZSwRZ95k zIG8ZtZqYPJf5W5/KVA4A8ec7wAUYCNco4PazcocUSCRlzpA2+bLhAFH67bk1Mo5p3Yp bFUqu0grJLT3amcdn+TOqqdpTsPw2JigUQH6IWtueRXpoT+Rnv43E/pYs+FSSi2G5eof 7Vh8inwHfmirmSXlLbAeo1EK588nqzkW7Se9wkmxIkwT8MKD2wxppNt5dL4p4Pv4mOEW BBWQ== X-Gm-Message-State: AKS2vOyPJwILu3vWdVB7w2q9nr+1ltYjfdlzsdpPOPj+MVRcBXsTzbe/ JtPVBe25venmyY3g X-Received: by 10.28.175.17 with SMTP id y17mr537922wme.100.1498741188924; Thu, 29 Jun 2017 05:59:48 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id a99sm6491622wrc.64.2017.06.29.05.59.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 29 Jun 2017 05:59:48 -0700 (PDT) Date: Thu, 29 Jun 2017 14:59:40 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Bruce Richardson Cc: Jan Blunck , Thomas Monjalon , dev Message-ID: <20170629125940.GL13355@bidouze.vm.6wind.com> References: <14287370.n4WKZa6dWl@xps> <1765263.t0XI3ojoND@xps> <20170628153320.GA14672@bricha3-MOBL3.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170628153320.GA14672@bricha3-MOBL3.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v6 05/11] bus: introduce hotplug functionality 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, 29 Jun 2017 12:59:49 -0000 On Wed, Jun 28, 2017 at 04:33:20PM +0100, Bruce Richardson wrote: > On Wed, Jun 28, 2017 at 05:11:46PM +0200, Jan Blunck wrote: > > On Wed, Jun 28, 2017 at 3:30 PM, Thomas Monjalon wrote: > > > 28/06/2017 15:09, Jan Blunck: > > >> On Wed, Jun 28, 2017 at 2:11 PM, Thomas Monjalon wrote: > > >> > 28/06/2017 13:58, Jan Blunck: > > >> >> On Wed, Jun 28, 2017 at 1:44 PM, Thomas Monjalon wrote: > > >> >> > 27/06/2017 21:03, Jan Blunck: > > >> >> >> On Tue, Jun 27, 2017 at 6:11 PM, Gaetan Rivet wrote: > > >> >> >> > --- a/lib/librte_eal/common/include/rte_bus.h > > >> >> >> > +++ b/lib/librte_eal/common/include/rte_bus.h > > >> >> >> > /** > > >> >> >> > + * Implementation specific probe function which is responsible for linking > > >> >> >> > + * devices on that bus with applicable drivers. > > >> >> >> > + * The plugged device might already have been used previously by the bus, > > >> >> >> > + * in which case some buses might prefer to detect and re-use the relevant > > >> >> >> > + * information pertaining to this device. > > >> >> >> > + * > > >> >> >> > + * @param da > > >> >> >> > + * Device declaration. > > >> >> >> > + * > > >> >> >> > + * @return > > >> >> >> > + * The pointer to a valid rte_device usable by the bus on success. > > >> >> >> > + * NULL on error. rte_errno is then set. > > >> >> >> > + */ > > >> >> >> > +typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da); > > >> >> >> > > >> >> >> Shouldn't this be orthogonal to unplug() and take a rte_device. You > > >> >> >> should only be able to plug devices that have been found by scan > > >> >> >> before. > > >> >> > > > >> >> > Plugging a device that has been scanned before is a special case. > > >> >> > In a true hotplug scenario, we could use this plug function passing > > >> >> > a devargs. > > >> >> > I don't see any issue passing rte_devargs to plug and rte_device to unplug. > > >> >> > > >> >> What do you mean by "true hotplug"? > > >> > > > >> > I mean a kernel notification of a new device. > > >> > > >> Does a "false hotplug" exist, too? :) > > > > > > The false hotplug was the original attach function which was just > > > adding a new ethdev interface. > > > > > >> >> The problem with this is that passing just rte_devargs to plug() > > >> >> requires the bus to parse and enrich the rte_devargs with bus > > >> >> specifics. From there it gets folded into the to-be-created bus > > >> >> specific rte_XXX_device. This makes it unnecessarily complicated and > > >> >> even worse it adds a second code path how devices come alive. > > >> > > > >> > Just after the notification, the rte_device does not exist yet. > > >> > So the plug function could share the same code as the scan function > > >> > to get the metadata and create the device instance. > > >> > > >> Exactly this is what I want to avoid. > > > > > > Why do you want to avoid that? > > > I think you mean it is not what you had in mind. > > > > > >> The plug() function would become a "scan-one and probe". > > > > > > Yes > > > > > >> From my point of view plug() and unplug() should be orthogonal. > > >> The plug() and unplug() should only be responsible for adding drivers > > >> with optional arguments. The EAL should allow the drivers to get > > >> unplugged/re-plugged at run-time. I want to be able to change arguments > > >> ... or even drivers :) > > > > > > It is a totally different thing. > > > We are talking about hotplug of a device, > > > and you are talking about changing drivers dynamically. > > > > > > So I still don't understand what is the issue with the plug/unplug > > > functions proposed here. > > > > > > > I don't agree with the notion that plug() means "scan-one and probe". > > What do you see as plug doing then? What do you see as the use-case and > then logic flow for hotplug? > Hi all, We are all for having "true" hotplug support in DPDK. By true hotplug, it means that at some point, a new device exists on the system, while the DPDK bus on which it should be probed does not yet have its metadata. Something needs to be done to retrieve these metadata, one way or another. What I see as a solution to this: - An interrupt framework integrated to rte_bus, allowing drivers to describe interrupt sources (Kernel UEVENT, custom FDs, ...), to their buses. - Applications should be able to pilot these interrupts for rte_bus (either in describing expected devices, or allowing actions to be taken upon device removal). - Buses could take the responsibility of plugging in and out their own devices, once properly configured. In this context, it is possible to imagine triggering a bus-rescan upon device ADD, iff we explicitly define scan() as being idempotent (a thing that is not part of its API yet and that we cannot expect from buses at this point). Then, we can limit bus->plug() to a probe, once we are sure that metadatas for the bus are (almost) always in sync with the system. Where we are: - Intel is proposing a UEVENT API[1]. It might be interesting to help them make it respect a generic framework. - A first plug / unplug implementation is being proposed. Plug() is currently effectively defined as (scan-one + probe). What can be done to go forward: - Define the API for rte_bus interrupt sources, configuration and triggers to allow the development of the needed subsystems. - Each device event sources should offer an event parser to transform them in device descriptions. Depending on the specifics of the source, different levels of info are available. - Redefine the requirements for bus->scan() to allow hotplugging. - Reduce the scope of plug() from (scan-one + probe) to (probe), as everything is now in place. - Further hotplugging developments are then possible: add INTR_ADD support, with flexible device definition for example... A thing that is not yet possible with the current architecture but seemed to interest a few people. If we can agree that this is a way forward, do you think Jan that having the current plug() API hinders this plan? We can reduce its scope later, without changing current implementations as, as you said, a proper scan should be idempotent. The future API as envisionned is already respected, but right now the hotplug support in buses is a little more involved. Applications that will start using plug() right now would not have to be rewritten, as the requirements for plugging would still be fullfilled. We can support already hotplugging in DPDK. We can refine this support later to make it more generic and easier to implement for PMD maintainers. But I do not see this as a reason to block this support from being integrated right now. [1]: http://dpdk.org/ml/archives/dev/2017-June/069057.html -- Gaëtan Rivet 6WIND