Re: [dpdk-dev] [EXT] Re: [PATCH v1 1/1] bus/pci: probe PCI devices in whitelisted order

[Vamsi Krishna Attunuru <vattunuru@marvell.com>]

> -----Original Message-----
> From: Gaëtan Rivet <gaetan.rivet@6wind.com>
> Sent: Thursday, September 26, 2019 1:34 PM
> To: Vamsi Krishna Attunuru <vattunuru@marvell.com>
> Cc: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org;
> ferruh.yigit@intel.com; anatoly.burakov@intel.com; Thomas Monjalon
> <thomas@monjalon.net>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/1] bus/pci: probe PCI devices in
> whitelisted order
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Thu, Sep 26, 2019 at 04:15:49AM +0000, Vamsi Krishna Attunuru wrote:
> >
> >
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Gaëtan Rivet
> > Sent: Wednesday, September 25, 2019 2:37 PM
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>
> > Cc: Vamsi Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org;
> > ferruh.yigit@intel.com; anatoly.burakov@intel.com; Thomas Monjalon
> > <thomas@monjalon.net>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > Subject: Re: [dpdk-dev] [PATCH v1 1/1] bus/pci: probe PCI devices in
> > whitelisted order
> >
> > On Wed, Sep 25, 2019 at 06:41:36AM +0000, Slava Ovsiienko wrote:
> > > > -----Original Message-----
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of
> > > > vattunuru@marvell.com
> > > > Sent: Monday, September 23, 2019 14:57
> > > > To: dev@dpdk.org
> > > > Cc: gaetan.rivet@6wind.com; ferruh.yigit@intel.com;
> > > > anatoly.burakov@intel.com; Thomas Monjalon <thomas@monjalon.net>;
> > > > jerinj@marvell.com; Vamsi Attunuru <vattunuru@marvell.com>
> > > > Subject: [dpdk-dev] [PATCH v1 1/1] bus/pci: probe PCI devices in
> > > > whitelisted order
> > > >
> > > > From: Vamsi Attunuru <vattunuru@marvell.com>
> > > >
> > > > Current pci bus driver scans pci devices in the order that it read from sysfs.
> > > > Accordingly all or whitelisted devices are getting probed.
> > > >
> > > > Patch modifies the probing order of whitelisted pci devices in a
> > > > sequence the devices are whitelisted(using EAL flags).
> > >
> > > Thanks, it would be nice to have opportunity to control probing
> > > order, it might be useful for bonded devices and representors either.
> > >
> > > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > >
> > > >
> > > > It ensures the eth devices that application uses are probed in
> > > > device whitelisted sequence, in turn it facilitates the packet
> > > > forwarding applications to work without any packet loss or
> > > > performance drop when the underneath network ports have different
> > > > bandwidths. By altering the whitelist order applications like
> > > > testpmd, l2fwd can forward the ingress traffic to egress port that has of
> equivalent bandwidth.
> > > >
> > > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> >
> > Hello Vamsi, Viacheslav,
> >
> > This is a nice patch. I agree that port dependency could be better handled. The
> port-mapping part however should be managed at the app level.
> >
> > Vamsi, you gave the example of l2fwd and testpmd, being able to properly
> configure forwarding directions implicitly. I think the better approach here is to
> add these configurations items within the apps directly. Configuring the mapping
> at the port level is not precise enough. The proper control is about cores, port
> and queues, not only ports.
> > This patch only solves a limited part of this issue with testpmd.
> >
> > I wrote a command to do this, that collided with some stream rework from
> Intel at the time (3, 4 years back?), so I did not take the time to force it through.
> If there is a need we could discuss about adding this back. I had needed it to
> write a PMD, that could be useful to others.
> >
> > As you say Viacheslav, there are use-cases that will rely on fine-grained probe
> order. However, this patch solves this issue only regarding PCI devices,
> depending on other PCI devices. We have in EAL an improper hack about it,
> forcing the vdev probe last, because usually ports depending on others are
> virtual ones. As this patch shows, the hack is not sufficient, and as the hack
> shows, this patch does not cover everything.
> >
> > A solution, would be an EAL parameter (I propose --no-dev), that disable
> probing for all buses. Applications and devices requiring a fine-grained probe
> order, are then free to start in this mode (and maybe force it through EAL conf),
> then hotplug ports as they see fit.
> >
> > This will keep the existing behavior stable for current apps, while allowing
> flexibility for the more advanced ones.
> >
> >
> > Hi Gaetan,
> >
> > Thanks, vdev part was not taken care in this patch. Rather than imposing
> hotplug for every application which requires port mapping, If vdev probing order
> is also handled same as pdevs(in whitelist order),  existing whitelisting feature
> will serve the port mapping requirement, right. Also the existing applications get
> benefited instead of overloading them with more configuration options.  If
> these probing order is not needed by default, it can be triggered using an EAL
> parameter(not added yet).
> >
> > Regards,
> > A Vamsi
> 
> Hi,
> 
> The way buses are written right now, they will each do a whole scan, then they
> each probe all their devices.
> 
> You cannot intersperse probes across several buses, i.e. probe a PCI device, then
> a vdev, then another PCI device.
> 
> Changing this structure could be difficult. A possible way to do what you want
> without breaking everything would be to do what the app would have done in
> my solution above, but from within the EAL: block all probes, then go over a
> mixed list of (-w) and (--vdev) parameters and hotplug them in order. This would
> require the --no-dev (or --no-probe, or --no-auto-probe) flag anyway (or as a
> conf item, or something at least telling the EAL to behave this way).
> 
> Would this way of doing it work for you?
Yes, above approach sounds fine to me and it works without breaking everything.

> 
> In any case, controlling the probe order should be fixed properly for all buses
> and the general use-case if possible, instead of limiting the patch to the PCI bus.

 Ack
> 
> Kind regards,
> --
> Gaëtan Rivet
> 6WIND