Re: [PATCH v7 13/15] eal: add hotplug add/remove functions

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Jun 30, 2017 at 06:13:51PM +0200, Gaëtan Rivet wrote:
> On Fri, Jun 30, 2017 at 06:03:17PM +0200, Thomas Monjalon wrote:
> > 30/06/2017 17:44, Jan Blunck:
> > > On Fri, Jun 30, 2017 at 11:20 AM, Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > On Fri, Jun 30, 2017 at 11:11:42AM +0200, Gaėtan Rivet wrote:
> > > >> On Fri, Jun 30, 2017 at 11:06:03AM +0200, Thomas Monjalon wrote:
> > > >> > 29/06/2017 20:22, Jan Blunck:
> > > >> > >  /**
> > > >> > > + * Hotplug add a given device to a specific bus.
> > > >> > > + *
> > > >> > > + * @param busname
> > > >> > > + *   The bus name the device is added to.
> > > >> > > + * @param devname
> > > >> > > + *   The device name. Based on this device name, eal will identify a driver
> > > >> > > + *   capable of handling it and pass it to the driver probing function.
> > > >> > > + * @param devargs
> > > >> > > + *   Device arguments to be passed to the driver.
> > > >> > > + * @return
> > > >> > > + *   0 on success, negative on error.
> > > >> > > + */
> > > >> > > +int rte_eal_hotplug_add(const char *busname, const char *devname,
> > > >> > > +                 const char *devargs);
> > > >> >
> > > >> > After the hotplug, we may need to get the rte_device.
> > > >> > Should we add a struct **rte_device as parameter,
> > > >> > or should we add a helper function to get the rte_device
> > > >> > from busname and devname?
> > > >>
> > > >> Also possible: return a struct *rte_device and set rte_errno on error.
> > > >>
> > > > +1 for this option.
> > > 
> > > Given that the caller of this is usually something that injects events
> > > from the system I wonder what it is going to do with a rte_device
> > > reference. Additionally to what the caller knows anyway (name,
> > > numa_node, devargs) it can check if a driver got assigned. Sure the
> > > caller could upcast conditionally based on the busname ...
> > > 
> > > At this point I guess the control plane would anyway want to get
> > > access to a high-level object, e.g. the rte_ethdev. I believe it is
> > > better to decouple this through callbacks that can get registered with
> > > individual buses.
> > 
> > I think Gaetan has an example of use of rte_device after plugging
> > with the failsafe PMD (managing slaves).
> > Anyway, it can be discussed later with a real example of use if needed.
> > We have a couple of weeks before freezing the API.
> 
> The rte_device should be accessible from the rte_eth_dev anyway so it
> does not make much difference. As long as a handle on the device is
> available. It is of course possible to add yet another callback to
> search the device just plugged, but I don't see a reason here not to do
> it in one pass.
> 
At this point in the process we just need to get in what we can.

Given there is so much discussion I would suggest we apply what we have
now, but mark the new APIs as "experimental" at this point. That should
allow us to test them, and build upon them without holding us back if we
do need to change them in the next release. Once everyone is happy with
the final result, we can lock them down then. It seems premature
to do so now, with the current discussion, but on the other hand
it seems foolish to not put what work has been done thus far
into a release as a starting point.

my 2c.
/Bruce