From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id 3BD302C13 for ; Wed, 28 Jun 2017 17:11:48 +0200 (CEST) Received: by mail-wm0-f65.google.com with SMTP id p204so7264812wmg.1 for ; Wed, 28 Jun 2017 08:11:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=hm46Se2L9Iq89O2Edju6m2RULOSuOl8g9UDvp8extiM=; b=BkIvIpU3FbPDNKfn+xww48L62VPRPPTsqwATsczfaEbqBDddsbGTR3ZE0AdHE1y26O QXsDq4SOG491a/lEabDcseMh/TLf6gIhD7ZMOUoy6P8Er6SkAbIa15vdtJSI6/qZkAtf 0nbib00N6TMJ7VEdtDF7X+LUx3/mVnrpQUoKqofbfEpqbQhxqs3uakG06TX09YQGcaeB 3j2hhnke8tB1QsHLzokyNLTH+dqGaPeqlZ+sBovTQAzLg5YuYkicAB/On60ivLb6yY8N wi8kwtYNNIs6gG/KZZ1vJYCGwHpx++lZvzIo9eh+tbp0OWpOOck5/ptR14xK3PdnA+nX In1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=hm46Se2L9Iq89O2Edju6m2RULOSuOl8g9UDvp8extiM=; b=qyVUwLG3YyLE107fc//SgI4GP3lq3Pel6DQNbl2G37AxejG+K/8PvU2qnFm4U3KGfi OgGBS4xQ7p7KrQFZgBYamRR0o4Ycibu+GkIc8/H19Bpsr1z+ZQV+hZy6rnQoCQMUTqpe j5tEFWcVipVYi9xvCB9J0NbEqy3jWexrVodTYsD3LxaGZX1WNr+xSIYpXPZ5rhU9F/Ax w6hNdMVdJdAymjD8Emss3LzjSJtXrozDHCFi6JhBwfdZS+DfUYPzSkYciWPlKhTKuADN 6BfgdqbV7SzCkPGJ0Q7Mqa9lKkrGLzYTjq22tMPIrJCGaBafWRp3viqH7otM5fQZdi6O jr1g== X-Gm-Message-State: AKS2vOxB9NCDEH/easTUVa2cyBzjBDPFAk7XfCtZ3WBU5yho5YHDUwyG rgxIXGMZVR77hmHi8NM6tKCqikqXig== X-Received: by 10.28.134.79 with SMTP id i76mr7581091wmd.108.1498662707801; Wed, 28 Jun 2017 08:11:47 -0700 (PDT) MIME-Version: 1.0 Sender: jblunck@gmail.com Received: by 10.28.5.136 with HTTP; Wed, 28 Jun 2017 08:11:46 -0700 (PDT) In-Reply-To: <1765263.t0XI3ojoND@xps> References: <14287370.n4WKZa6dWl@xps> <1765263.t0XI3ojoND@xps> From: Jan Blunck Date: Wed, 28 Jun 2017 17:11:46 +0200 X-Google-Sender-Auth: P22cFjS3BRWix5zrRfnam8ECzkI Message-ID: To: Thomas Monjalon Cc: dev , Gaetan Rivet Content-Type: text/plain; charset="UTF-8" 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: Wed, 28 Jun 2017 15:11:48 -0000 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". >> >> When we get notified about a hotplug event we already know which bus >> >> this event belongs to: >> >> >> >> 1. scan the bus for incoming devices >> > >> > No need to scan every devices here. >> >> This is a readdir followed by open+read+close for any new device. This >> code belongs here anyway. Its lightweight if nothing changed. The scan >> itself should be idempotent anyway. >> >> >> 2. plug single device with devargs and probe for drivers >> >> >> >> Makes sense? >> > >> > I want to make sure there is no misunderstanding first :) >> >> Which makes sense. That is probably my fault due to being too >> distracted with other things and not communicating well enough while >> Gaetan consumed my code. > > Your ideas are probably interesting, and I want to understand them. > In the meantime, we need to progress on 17.08-rc1 which must be done > in following days. Please let's separate the ideas which are not > yet implemented from what we are already able to deliver. Sorry, if you got the impression that this is taken from thin air but this is how my original patch series looked like and I'm just providing the rational behind it.