28/06/2017 15:09, Jan Blunck: > On Wed, Jun 28, 2017 at 2:11 PM, Thomas Monjalon <tho...@monjalon.net> wrote: > > 28/06/2017 13:58, Jan Blunck: > >> On Wed, Jun 28, 2017 at 1:44 PM, Thomas Monjalon <tho...@monjalon.net> > >> wrote: > >> > 27/06/2017 21:03, Jan Blunck: > >> >> On Tue, Jun 27, 2017 at 6:11 PM, Gaetan Rivet <gaetan.ri...@6wind.com> > >> >> 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. > >> 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.
