Re: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev args

[Thomas Monjalon <thomas@monjalon.net>]

10/07/2018 12:19, Pavan Nikhilesh:
> Hi Gaëtan,Ferruh,
> 
> On Wed, Jun 27, 2018 at 11:57:36AM +0200, Gaëtan Rivet wrote:
> > On Wed, Jun 27, 2018 at 02:25:30PM +0530, Pavan Nikhilesh wrote:
> > > Hi Gaëtan,
> > >
> > > On Wed, Jun 27, 2018 at 10:39:59AM +0200, Gaëtan Rivet wrote:
> > > > Hi Ferruh, Pavan,
> > > >
> > > > sorry for the delay,
> > > >
> > > > On Tue, Jun 26, 2018 at 04:40:21PM +0100, Ferruh Yigit wrote:
> > > > > On 6/26/2018 1:48 PM, Shahaf Shuler wrote:
> > > > > > Hi Pavan,
> > > > > >
> > > > > > Friday, June 15, 2018 7:44 AM, Pavan Nikhilesh:
> > > > > >> Subject: [dpdk-dev] [PATCH v2] eal/devargs: add option to supply PCI dev
> > > > > >> args
> > > > > >>
> > > > > >> Currently, the only way of supplying device argument to a pci device is to
> > > > > >> whitelist it i.e. -w 000X:00:0X.0,self_test=1. This is not a very feasible method
> > > > > >> as whitelisting a device has its own side effects i.e only the whitelisted pci
> > > > > >> devices are probed.
> > > > > >>
> > > > > >> Add a new eal command line option --pci-args to pass device args without the
> > > > > >> need to whitelist the devices.
> > > > > >>            --pci-args 000X:00:0X.0,self_test=1
> > > > > >>
> > > > > >> Signed-off-by: Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>
> > > > > >
> > > > > > Tested-by: Shahaf Shuler <shahafs@mellanox.com>
> > > > > >
> > > > > > It seems to work.
> > > > > > Please see small comments below
> > > > >
> > > > > Isn't this conflict with Gaetan's devarg work which has wider scope?
> > > > >
> > > >
> > > > Indeed it does.
> > > >
> > > > Pavan, I have submitted a new version of a series adding generic kvargs
> > > > to several layers (bus, class, driver).
> > > >
> > > > It does cover this exact use-case.
> > > >
> > > > However, while writing it, I wasn't able to find PCI bus specific
> > > > parameters, that could showcase the functionality.
> > >
> > > The idea of the patch is to avoid whitelising a device when we want to
> > > supply kvargs to it, I tried mapping it to devargs rework patchset but couldn't
> > > do it at a glance. For example, the following patch[1] reads kvargs through
> > > whitelisting which should be avoided.
> > >
> > > [1]http://patches.dpdk.org/patch/41223/
> > >
> >
> > I see.
> >
> > Actually, your use-case won't be covered by the devargs rework.
> >
> > I am still dumbfounded by how this blacklist/whitelist mode stuff is
> > kept against all odds. But that's not the time to deal with it.
> >
> > The issue is that the two features "declaring a device" and
> > "configuring a bus" are currently awkwardly merged. You are piling stuff
> > on the "declaring a device" part to enhance the "configuring a bus"
> > feature.
> 
> The feature is very much needed to avoid polluting the cmdline args when we are
> trying to configure a device at probe (for now).
> 
> >
> > Instead of going this way, I would advise to separate the two features.
> >
> > If buses could be configured with a generic EAL option
> > "--blacklist=pci,vdev" for example, then you could provide devargs as
> > much as you want, the buses themselves would stay properly configured.
> >
> > This means removing devargs policy, device types and rewriting bus logic
> > about it.
> 
> I think this can be done as a future work and is not in the scope of this
> patch.
> 
> @Ferruh,
> As Gaëtan mentioned this patch is not related to devargs rework can we make
> some forward progress.

No, we should not add a new parameter just to fix one use case for one bus.
The work of Gaetan is opening the door to a generic syntax which can be
used for device matching (like for whitelisting), or for settings
(what you need) of any bus, any device class or any driver.
We can discuss about which option to add for generic device settings,
and whether or not it should be mixed with whitelisting,
but please let's work on a generic solution.