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

[Pavan Nikhilesh <pbhagavatula@caviumnetworks.com>]

Hi Shahaf,

On Tue, Jun 26, 2018 at 12:48:49PM +0000, 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
>
> > ---
> >  v2 Changes:
> >  - Document the option usage in eal_common_usage.
> >  - Update commit log to be more informative.
> >
> >  lib/librte_eal/common/eal_common_devargs.c  | 3 +++
> > lib/librte_eal/common/eal_common_options.c  | 9 +++++++++
> >  lib/librte_eal/common/eal_options.h         | 2 ++
> >  lib/librte_eal/common/include/rte_dev.h     | 1 +
> >  lib/librte_eal/common/include/rte_devargs.h | 1 +
> >  5 files changed, 16 insertions(+)
>
> Should we also update the manual of testpmd (doc/guides/testpmd_app_ug/run_app.rst ) for the new eal arg?

I was wondering where exactly this has to be documented, Thanks for pointing it
out I will add it in the next version.

>
> >
> >  };
> > diff --git a/lib/librte_eal/common/include/rte_devargs.h
> > b/lib/librte_eal/common/include/rte_devargs.h
> > index 58fbd90a2..78c600bf2 100644
> > --- a/lib/librte_eal/common/include/rte_devargs.h
> > +++ b/lib/librte_eal/common/include/rte_devargs.h
> > @@ -29,6 +29,7 @@ extern "C" {
> >   * Type of generic device
> >   */
> >  enum rte_devtype {
> > +     RTE_DEVTYPE_NORMAL, /* Normal dev with special pci args */
>
> What is "Normal" device? Can we find a better name?

Maybe something like RTE_DEVTYPE_PCI would fit in?. Let me know if you have any
suggestions.

>
> >       RTE_DEVTYPE_WHITELISTED_PCI,
> >       RTE_DEVTYPE_BLACKLISTED_PCI,
> >       RTE_DEVTYPE_VIRTUAL,
> > --
> > 2.17.1
>

Thanks,
Pavan.