[dpdk-dev] [PATCH v4 01/24] eal: introduce one device scan

Zhang, Qi Z qi.z.zhang at intel.com
Wed Jun 27 14:32:07 CEST 2018



> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet at 6wind.com]
> Sent: Wednesday, June 27, 2018 12:34 AM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov at intel.com>; thomas at monjalon.net;
> Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org;
> Richardson, Bruce <bruce.richardson at intel.com>; Yigit, Ferruh
> <ferruh.yigit at intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton at intel.com>; Vangati, Narender
> <narender.vangati at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v4 01/24] eal: introduce one device scan
> 
> On Tue, Jun 26, 2018 at 12:26:05PM +0000, Zhang, Qi Z wrote:
> >
> >
> > > -----Original Message-----
> > > From: Burakov, Anatoly
> > > Sent: Tuesday, June 26, 2018 7:48 PM
> > > To: Zhang, Qi Z <qi.z.zhang at intel.com>; thomas at monjalon.net
> > > Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>;
> > > dev at dpdk.org; Richardson, Bruce <bruce.richardson at intel.com>; Yigit,
> > > Ferruh <ferruh.yigit at intel.com>; Shelton, Benjamin H
> > > <benjamin.h.shelton at intel.com>; Vangati, Narender
> > > <narender.vangati at intel.com>
> > > Subject: Re: [PATCH v4 01/24] eal: introduce one device scan
> > >
> > > On 26-Jun-18 8:08 AM, Qi Zhang wrote:
> > > > When hot plug a new device, it is not necessary to scan everything
> > > > on the bus since the devname and devargs are already there. So new
> > > > rte_bus ops "scan_one" is introduced, bus driver can implement
> > > > this function to simplify the hotplug process.
> > > >
> > > > Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> > > > ---
> > > >
> > >
> > > <snip>
> > >
> > > > + *	NULL for unsuccessful scan
> > > > + */
> > > > +typedef struct rte_device *(*rte_bus_scan_one_t)(struct
> > > > +rte_devargs *devargs);
> > > > +
> > > > +/**
> > > >    * Implementation specific probe function which is responsible for
> linking
> > > >    * devices on that bus with applicable drivers.
> > > >    *
> > > > @@ -204,6 +219,7 @@ struct rte_bus {
> > > >   	TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> > > >   	const char *name;            /**< Name of the bus */
> > > >   	rte_bus_scan_t scan;         /**< Scan for devices attached to
> > > bus */
> > > > +	rte_bus_scan_one_t scan_one; /**< Scan one device using devargs
> > > > +*/
> > > >   	rte_bus_probe_t probe;       /**< Probe devices on bus */
> > > >   	rte_bus_find_device_t find_device; /**< Find a device on the bus */
> > > >   	rte_bus_plug_t plug;         /**< Probe single device for drivers
> > > */
> > > >
> > >
> > > Does changing this structure break ABI for bus drivers?
> >
> > For bus driver, I think yes, but I'm not sure what I should do for
> > this, since this is not for application
> >
> >
> 
> This should be appropriately announced in advance, in general.
> However, it seems there is some leeway if the new field will not move the
> others and not make the structure grow (i.e. replace a padding).
> 
> There is an ABI check script that can be used.
> 
> This however breaks the bus ABI, which breaks the EAL ABI.
> This is usually an issue.


OK, since we are able to invoke IPC request in a separate thread and also with below
fix, there is no issue on the vdev->scan during hotplug on secondary.
https://patches.dpdk.org/patch/41647/

ABI break is not necessary, I will withdraw patch 1 and 2. 

Thanks
Qi


> 
> More generally, I was in favor of changing the whole bus scan process to a
> per-device iteration. I was shut down on this when adding hotplug.
> As a result, bus->scan() process was made to require the operation to be
> idempotent.
> 
> Adding a new ops adds noise to the bus API. It should be kept as clean as
> possible. This new one seems unnecessary, now that all bus scans are
> idempotent (when supporting hotplug).
> 
> Regards,
> --
> Gaëtan Rivet
> 6WIND


More information about the dev mailing list