[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Olivier Matz
Hi,

Thank you everyone for all these comments. I'll try to summarize them here:

a- the kmod information should be per-device instead of per-driver
(modalias-like)
b- there is no need to specify kmod dependencies (i.e. "uio_pci_generic"
is enough, not "uio & uio_pci_generic") since it is available via modinfo
c- the syntax with commas and colons is not ideal, we have no better
consensus for now, but maybe "&" and "|" are better.
d- the information provided is not complete:
   - some drivers requires module parameters or writing is /sys
   - choosing between one kmod or another is a system choice
  -> this info could be optional for this reason
e- for usual drivers having a "" tag instead of a kmod list
may be more simple (I'm not sure of it though)


So we could either:

- drop this RFC, and like today everyone shoule handle the loading of
kmods on its own

- keep the approach of this RFC, enhance it with a- b- c- e-

   Examples of string to manage specify the devices wildcards:
 /* kmod list for all pci devices */
 "pci:v*:d*:sv*:sd*:vfio|uio_pci_generic|igb_uio"
 /* if we need device-specific modules: */
 "pci:v8086:d1234:sv*:sd*:some_kmod"
 "pci:v8086:d4321:sv*:sd*:some_other_kmod"

   this is not a perfect solution, but I think it would make the
   development of an kmod autoload script easier.


Any other idea?

Thanks,
Olivier


[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Trahe, Fiona


> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Friday, September 2, 2016 2:33 PM
> To: Trahe, Fiona 
> Cc: Stephen Hemminger ; dev at dpdk.org;
> Olivier Matz ; Thomas Monjalon
> 
> Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> in pmdinfo
> 
> On Fri, Sep 02, 2016 at 09:19:26AM +, Trahe, Fiona wrote:
> >
> >
> > > -Original Message-
> > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > Sent: Thursday, September 1, 2016 8:16 PM
> > > To: Stephen Hemminger 
> > > Cc: Trahe, Fiona ; dev at dpdk.org; Olivier Matz
> > > ; Thomas Monjalon
> > > 
> > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod
> > > dependencies in pmdinfo
> > >
> > > On Thu, Sep 01, 2016 at 10:41:22AM -0700, Stephen Hemminger wrote:
> > > > On Thu, 1 Sep 2016 13:35:19 -0400
> > > > Neil Horman  wrote:
> > > >
> > > > > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > > > > Hi Neil and Olivier,
> > > > > >
> > > > > > > -Original Message-
> > > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier
> > > > > > > Matz
> > > > > > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > > > > > To: Neil Horman 
> > > > > > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > > > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise
> > > > > > > kmod dependencies in pmdinfo
> > > > > > >
> > > > > > > Hi Neil,
> > > > > > >
> > > > > > > On 08/31/2016 03:27 PM, Neil Horman wrote:
> > > > > > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> > > > > > > >> Hi Neil,
> > > > > > > >>
> > > > > > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > > > > > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> > > > > > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows
> > > > > > > >>>> a driver to declare the list of kernel modules required to 
> > > > > > > >>>> run
> properly.
> > > > > > > >>>>
> > > > > > > >>>> Today, most PCI drivers require uio/vfio.
> > > > > > > >>>>
> > > > > > > >>>> Signed-off-by: Olivier Matz 
> > > > > > > >>>>
> > > > > > > >>>> ---
> > > > > > > >>>> In this RFC, I supposed that all PCI drivers require a
> > > > > > > >>>> the loading of a uio/vfio module (except mlx*), this may be
> wrong.
> > > > > > > >>>> Comments are welcome!
> > > > > > > >>>>
> > > > > > > >>>>
> > > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > > > > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > > > > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > > > > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > > > > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > > > > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > > > > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > > > > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > > > > > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > > > > > > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > > > > > > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > > > > > > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > > > > > > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > > > > > > >>>>  drivers/net/mlx4/mlx4.c  

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Thomas Monjalon
2016-09-01 10:41, Stephen Hemminger:
> Neil Horman  wrote:
> > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Matz
> > > > On 08/31/2016 03:27 PM, Neil Horman wrote:  
> > > > > Oh, I see, so your list is a colon delimited list of module load 
> > > > > sets, where at
> > > > > least one set must succeed by loading all modules in its set, but the 
> > > > > failure of
> > > > > any one set isn't fatal to the process?  e.g. a string like this:
> > > > >
> > > > > uio,igb_uio:vfio,vfio-pci
> > > > >
> > > > > could be interpreted to mean "I must load (uio AND igb_uio) OR (vfio 
> > > > > AND
> > > > > vfio-pci).  If the evaluation of that statement results in false, 
> > > > > then the
> > > > > operation fails, otherwise it succedes.
> > > > >
> > > > > If thats the case, then, apologies, we're on the same page, and this 
> > > > > will work
> > > > > just fine.  
> > > > 
> > > > Yep, that's the idea.
> > > > 
> > > > Colon and commas are the best separators I've thought about, but any
> > > > idea to make the syntax clearer is welcome ;)
> > > > 
> > > > Maybe a syntax like is clearer:
> > > >   "(mod1 & mod2)|(mod3 & mod4)" ?
> > > > But it would let the user think that more complex expressions are valid,
> > > > like "(mod1 & (mod2 | mod3)) | mod4", which is probably overkill.
> > > 
> > > This RFC seems like a good idea - and something the Intel QuickAssist PMD 
> > > could benefit from.
> > > However the (mod1 & mod2) can handle the QAT case better in my opinion.
> > > i.e.
> > > as well as needing one of 
> > > * uio igb_uio
> > > * uio uio_pci_generic
> > > * vfio vfio-pci
> > > QAT PMD also needs one of (depending on which physical device is plugged)
> > >  * qat_dh895xcc
> > >  * qat_c62x
> > >  * qat_c3xxx
> > > 
> > > So the original syntax would result in a very long list of possible 
> > > variations.
> > > What really reflects the dependencies would be 
> > > ((uio & igb_uio) | (uio & uio_pci_generic) | (vfio & vfio_pci)) & 
> > > (qat_dh895xcc | qat_c62x | qat_c3xxx)
> > >   
> > Ah, I didn't consider that hardware specifics might create a use case where 
> > a
> > pmd must have one or more kernel modules available for hw support.  Perhaps 
> > it
> > is worthwhile to automate hardware support - that is to say, any module 
> > loading
> > script should automatically look at the pci table exported from a pmd, and, 
> > if
> > found, load any modules that claim support for that device:vendor tuple?  
> > Though
> > that might break in the case of uio, if there are separate driver modules 
> > that
> > support native hardware and uio access.
> 
> I ended up writing a script that went the other way.
> First look at the hardware and load VFIO if IOMMU is available.
> Then look for special driver needed for Xen and HyperV
> Lastly fallback to loading igb_uio if no VFIO and PCI device present.
> 
> In other words it is a system not driver issue.

That's partly right, yes.
But you need some information which are specific to the drivers and
we should try to embed them for three usages:
- give more info the user (without digging in the doc)
- replace info in some external system scripts harder to maintain
- prepare for hotplug

Some PMDs do not use UIO or VFIO at all,
However, I agree that the requirement
(uio & igb_uio) | (uio & uio_pci_generic) | (vfio & vfio_pci)
- and even the VFIO noiommu case - could be translated into a simple
flag, let's say "generic_device_mapping"
(unfortunately "queue_mapping" doesn't exist).

The other interesting point from Fiona is to show that this information is
device-related (not general for the whole driver). So we should add a device
parameter in the macro with the ability to set a wildcard.


[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Neil Horman
On Fri, Sep 02, 2016 at 01:52:46PM +, Trahe, Fiona wrote:
> 
> 
> > -Original Message-
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Friday, September 2, 2016 2:33 PM
> > To: Trahe, Fiona 
> > Cc: Stephen Hemminger ; dev at dpdk.org;
> > Olivier Matz ; Thomas Monjalon
> > 
> > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> > in pmdinfo
> > 
> > On Fri, Sep 02, 2016 at 09:19:26AM +, Trahe, Fiona wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > > > Sent: Thursday, September 1, 2016 8:16 PM
> > > > To: Stephen Hemminger 
> > > > Cc: Trahe, Fiona ; dev at dpdk.org; Olivier 
> > > > Matz
> > > > ; Thomas Monjalon
> > > > 
> > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod
> > > > dependencies in pmdinfo
> > > >
> > > > On Thu, Sep 01, 2016 at 10:41:22AM -0700, Stephen Hemminger wrote:
> > > > > On Thu, 1 Sep 2016 13:35:19 -0400
> > > > > Neil Horman  wrote:
> > > > >
> > > > > > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > > > > > Hi Neil and Olivier,
> > > > > > >
> > > > > > > > -Original Message-
> > > > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier
> > > > > > > > Matz
> > > > > > > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > > > > > > To: Neil Horman 
> > > > > > > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > > > > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise
> > > > > > > > kmod dependencies in pmdinfo
> > > > > > > >
> > > > > > > > Hi Neil,
> > > > > > > >
> > > > > > > > On 08/31/2016 03:27 PM, Neil Horman wrote:
> > > > > > > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> > > > > > > > >> Hi Neil,
> > > > > > > > >>
> > > > > > > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > > > > > > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz 
> > > > > > > > >>> wrote:
> > > > > > > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows
> > > > > > > > >>>> a driver to declare the list of kernel modules required to 
> > > > > > > > >>>> run
> > properly.
> > > > > > > > >>>>
> > > > > > > > >>>> Today, most PCI drivers require uio/vfio.
> > > > > > > > >>>>
> > > > > > > > >>>> Signed-off-by: Olivier Matz 
> > > > > > > > >>>>
> > > > > > > > >>>> ---
> > > > > > > > >>>> In this RFC, I supposed that all PCI drivers require a
> > > > > > > > >>>> the loading of a uio/vfio module (except mlx*), this may be
> > wrong.
> > > > > > > > >>>> Comments are welcome!
> > > > > > > > >>>>
> > > > > > > > >>>>
> > > > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > > > > > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > > > > > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > > > > > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > > > > > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > > > > > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > > > > > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > > > > > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > > > > > > >>>&g

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Neil Horman
On Fri, Sep 02, 2016 at 09:19:26AM +, Trahe, Fiona wrote:
> 
> 
> > -Original Message-
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Thursday, September 1, 2016 8:16 PM
> > To: Stephen Hemminger 
> > Cc: Trahe, Fiona ; dev at dpdk.org; Olivier Matz
> > ; Thomas Monjalon
> > 
> > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> > in pmdinfo
> > 
> > On Thu, Sep 01, 2016 at 10:41:22AM -0700, Stephen Hemminger wrote:
> > > On Thu, 1 Sep 2016 13:35:19 -0400
> > > Neil Horman  wrote:
> > >
> > > > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > > > Hi Neil and Olivier,
> > > > >
> > > > > > -Original Message-
> > > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier
> > > > > > Matz
> > > > > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > > > > To: Neil Horman 
> > > > > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod
> > > > > > dependencies in pmdinfo
> > > > > >
> > > > > > Hi Neil,
> > > > > >
> > > > > > On 08/31/2016 03:27 PM, Neil Horman wrote:
> > > > > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> > > > > > >> Hi Neil,
> > > > > > >>
> > > > > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > > > > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> > > > > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a
> > > > > > >>>> driver to declare the list of kernel modules required to run 
> > > > > > >>>> properly.
> > > > > > >>>>
> > > > > > >>>> Today, most PCI drivers require uio/vfio.
> > > > > > >>>>
> > > > > > >>>> Signed-off-by: Olivier Matz 
> > > > > > >>>>
> > > > > > >>>> ---
> > > > > > >>>> In this RFC, I supposed that all PCI drivers require a the
> > > > > > >>>> loading of a uio/vfio module (except mlx*), this may be wrong.
> > > > > > >>>> Comments are welcome!
> > > > > > >>>>
> > > > > > >>>>
> > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > > > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > > > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > > > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > > > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > > > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > > > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > > > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > > > > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > > > > > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > > > > > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > > > > > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > > > > > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > > > > > >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> > > > > > >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> > > > > > >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> > > > > > >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> > > > > > >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > > > > >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> > > > > > >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> > > > > > >>>>  drivers

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-02 Thread Trahe, Fiona


> -Original Message-
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Thursday, September 1, 2016 8:16 PM
> To: Stephen Hemminger 
> Cc: Trahe, Fiona ; dev at dpdk.org; Olivier Matz
> ; Thomas Monjalon
> 
> Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> in pmdinfo
> 
> On Thu, Sep 01, 2016 at 10:41:22AM -0700, Stephen Hemminger wrote:
> > On Thu, 1 Sep 2016 13:35:19 -0400
> > Neil Horman  wrote:
> >
> > > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > > Hi Neil and Olivier,
> > > >
> > > > > -Original Message-
> > > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier
> > > > > Matz
> > > > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > > > To: Neil Horman 
> > > > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod
> > > > > dependencies in pmdinfo
> > > > >
> > > > > Hi Neil,
> > > > >
> > > > > On 08/31/2016 03:27 PM, Neil Horman wrote:
> > > > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> > > > > >> Hi Neil,
> > > > > >>
> > > > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > > > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> > > > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a
> > > > > >>>> driver to declare the list of kernel modules required to run 
> > > > > >>>> properly.
> > > > > >>>>
> > > > > >>>> Today, most PCI drivers require uio/vfio.
> > > > > >>>>
> > > > > >>>> Signed-off-by: Olivier Matz 
> > > > > >>>>
> > > > > >>>> ---
> > > > > >>>> In this RFC, I supposed that all PCI drivers require a the
> > > > > >>>> loading of a uio/vfio module (except mlx*), this may be wrong.
> > > > > >>>> Comments are welcome!
> > > > > >>>>
> > > > > >>>>
> > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > > > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > > > > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > > > > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > > > > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > > > > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > > > > >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> > > > > >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> > > > > >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> > > > > >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> > > > > >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > > > >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> > > > > >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> > > > > >>>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> > > > > >>>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> > > > > >>>>  tools/dpdk-pmdinfo.py   |  5 -
> > > > > >>>>  24 files changed, 69 insertions(+), 1 deletion(-)
> > > > > >>>>
> > > > > >>>
> > > > > >>&g

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-01 Thread Neil Horman
On Thu, Sep 01, 2016 at 10:41:22AM -0700, Stephen Hemminger wrote:
> On Thu, 1 Sep 2016 13:35:19 -0400
> Neil Horman  wrote:
> 
> > On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > > Hi Neil and Olivier,
> > >   
> > > > -Original Message-
> > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Matz
> > > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > > To: Neil Horman 
> > > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod 
> > > > dependencies
> > > > in pmdinfo
> > > > 
> > > > Hi Neil,
> > > > 
> > > > On 08/31/2016 03:27 PM, Neil Horman wrote:  
> > > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:  
> > > > >> Hi Neil,
> > > > >>
> > > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:  
> > > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:  
> > > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> > > > >>>> declare the list of kernel modules required to run properly.
> > > > >>>>
> > > > >>>> Today, most PCI drivers require uio/vfio.
> > > > >>>>
> > > > >>>> Signed-off-by: Olivier Matz 
> > > > >>>>
> > > > >>>> ---
> > > > >>>> In this RFC, I supposed that all PCI drivers require a the loading 
> > > > >>>> of a
> > > > >>>> uio/vfio module (except mlx*), this may be wrong.
> > > > >>>> Comments are welcome!
> > > > >>>>
> > > > >>>>
> > > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > > > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > > > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > > > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > > > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > > > >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> > > > >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> > > > >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> > > > >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> > > > >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > > >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> > > > >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> > > > >>>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> > > > >>>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> > > > >>>>  tools/dpdk-pmdinfo.py   |  5 -
> > > > >>>>  24 files changed, 69 insertions(+), 1 deletion(-)
> > > > >>>>  
> > > > >>>
> > > > >>> Generally speaking, I like the idea, it makes sense to me in terms 
> > > > >>> of using
> > > > >>> pmdinfo to export this information
> > > > >>>
> > > > >>> That said, This may need to be a set of macros.  By that I mean 
> > > > >>> (and correct  
> > > > me  
> > > > >>> if I'm wrong here), but the relationship between pmd's and kernel 
> > > > >>> modules  
> > > > is in  
> > > > >>> some cases, more complex than a 'requires' or 'depends'

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-01 Thread Neil Horman
On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> Hi Neil and Olivier,
> 
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Matz
> > Sent: Wednesday, August 31, 2016 2:40 PM
> > To: Neil Horman 
> > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> > in pmdinfo
> > 
> > Hi Neil,
> > 
> > On 08/31/2016 03:27 PM, Neil Horman wrote:
> > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> > >> Hi Neil,
> > >>
> > >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> > >>>> declare the list of kernel modules required to run properly.
> > >>>>
> > >>>> Today, most PCI drivers require uio/vfio.
> > >>>>
> > >>>> Signed-off-by: Olivier Matz 
> > >>>>
> > >>>> ---
> > >>>> In this RFC, I supposed that all PCI drivers require a the loading of a
> > >>>> uio/vfio module (except mlx*), this may be wrong.
> > >>>> Comments are welcome!
> > >>>>
> > >>>>
> > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> > >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> > >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> > >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> > >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> > >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> > >>>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> > >>>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> > >>>>  tools/dpdk-pmdinfo.py   |  5 -
> > >>>>  24 files changed, 69 insertions(+), 1 deletion(-)
> > >>>>
> > >>>
> > >>> Generally speaking, I like the idea, it makes sense to me in terms of 
> > >>> using
> > >>> pmdinfo to export this information
> > >>>
> > >>> That said, This may need to be a set of macros.  By that I mean (and 
> > >>> correct
> > me
> > >>> if I'm wrong here), but the relationship between pmd's and kernel 
> > >>> modules
> > is in
> > >>> some cases, more complex than a 'requires' or 'depends' relationship.  
> > >>> That
> > is
> > >>> to say, some pmd may need user space hardware access, but can use either
> > uio OR
> > >>> vfio, but doesn't need both, and can continue to function if only one is
> > >>> available.  Other PMD's may be able to use vfio or uio, but can still 
> > >>> function
> > >>> without either.  And some, as your patch implements, simply require one 
> > >>> or
> > the
> > >>> other to function.  As such it seems like you may want a few macros, in 
> > >>> the
> > form
> > >>> of:
> > >>>
> > >>> DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading,
> > ignore any
> > >>> failures
> > >>> DRIVER_REGISTER_KMOD_REQUI

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-01 Thread Trahe, Fiona
Hi Neil and Olivier,

> -Original Message-
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Matz
> Sent: Wednesday, August 31, 2016 2:40 PM
> To: Neil Horman 
> Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies
> in pmdinfo
> 
> Hi Neil,
> 
> On 08/31/2016 03:27 PM, Neil Horman wrote:
> > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> >> Hi Neil,
> >>
> >> On 08/30/2016 03:23 PM, Neil Horman wrote:
> >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> >>>> declare the list of kernel modules required to run properly.
> >>>>
> >>>> Today, most PCI drivers require uio/vfio.
> >>>>
> >>>> Signed-off-by: Olivier Matz 
> >>>>
> >>>> ---
> >>>> In this RFC, I supposed that all PCI drivers require a the loading of a
> >>>> uio/vfio module (except mlx*), this may be wrong.
> >>>> Comments are welcome!
> >>>>
> >>>>
> >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> >>>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> >>>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> >>>>  tools/dpdk-pmdinfo.py   |  5 -
> >>>>  24 files changed, 69 insertions(+), 1 deletion(-)
> >>>>
> >>>
> >>> Generally speaking, I like the idea, it makes sense to me in terms of 
> >>> using
> >>> pmdinfo to export this information
> >>>
> >>> That said, This may need to be a set of macros.  By that I mean (and 
> >>> correct
> me
> >>> if I'm wrong here), but the relationship between pmd's and kernel modules
> is in
> >>> some cases, more complex than a 'requires' or 'depends' relationship.  
> >>> That
> is
> >>> to say, some pmd may need user space hardware access, but can use either
> uio OR
> >>> vfio, but doesn't need both, and can continue to function if only one is
> >>> available.  Other PMD's may be able to use vfio or uio, but can still 
> >>> function
> >>> without either.  And some, as your patch implements, simply require one or
> the
> >>> other to function.  As such it seems like you may want a few macros, in 
> >>> the
> form
> >>> of:
> >>>
> >>> DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading,
> ignore any
> >>> failures
> >>> DRIVER_REGISTER_KMOD_REQUIRE - List of modules required to be
> loaded after
> >>> request macro completes, fail if any are not loaded
> >>>
> >>> Thats just spitballing, mind you, theres probably a better way to do it, 
> >>> but
> the
> >>> idea is to list a set of modules you would like to have, and then create a
> >>> parsable syntax to describe the modules that need to be loaded after the
> request
> >>> is complete so that you can accurately codify the sit

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-09-01 Thread Stephen Hemminger
On Thu, 1 Sep 2016 13:35:19 -0400
Neil Horman  wrote:

> On Thu, Sep 01, 2016 at 12:55:27PM +, Trahe, Fiona wrote:
> > Hi Neil and Olivier,
> >   
> > > -Original Message-
> > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier Matz
> > > Sent: Wednesday, August 31, 2016 2:40 PM
> > > To: Neil Horman 
> > > Cc: dev at dpdk.org; thomas.monjalon at 6wind.com
> > > Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod 
> > > dependencies
> > > in pmdinfo
> > > 
> > > Hi Neil,
> > > 
> > > On 08/31/2016 03:27 PM, Neil Horman wrote:  
> > > > On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:  
> > > >> Hi Neil,
> > > >>
> > > >> On 08/30/2016 03:23 PM, Neil Horman wrote:  
> > > >>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:  
> > > >>>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> > > >>>> declare the list of kernel modules required to run properly.
> > > >>>>
> > > >>>> Today, most PCI drivers require uio/vfio.
> > > >>>>
> > > >>>> Signed-off-by: Olivier Matz 
> > > >>>>
> > > >>>> ---
> > > >>>> In this RFC, I supposed that all PCI drivers require a the loading 
> > > >>>> of a
> > > >>>> uio/vfio module (except mlx*), this may be wrong.
> > > >>>> Comments are welcome!
> > > >>>>
> > > >>>>
> > > >>>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> > > >>>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> > > >>>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> > > >>>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> > > >>>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> > > >>>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> > > >>>>  drivers/net/e1000/em_ethdev.c   |  2 ++
> > > >>>>  drivers/net/e1000/igb_ethdev.c  |  4 
> > > >>>>  drivers/net/ena/ena_ethdev.c|  2 ++
> > > >>>>  drivers/net/enic/enic_ethdev.c  |  2 ++
> > > >>>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> > > >>>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> > > >>>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> > > >>>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> > > >>>>  drivers/net/mlx4/mlx4.c |  2 ++
> > > >>>>  drivers/net/mlx5/mlx5.c |  3 +++
> > > >>>>  drivers/net/nfp/nfp_net.c   |  2 ++
> > > >>>>  drivers/net/qede/qede_ethdev.c  |  4 
> > > >>>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > >>>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> > > >>>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> > > >>>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> > > >>>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> > > >>>>  tools/dpdk-pmdinfo.py   |  5 -
> > > >>>>  24 files changed, 69 insertions(+), 1 deletion(-)
> > > >>>>  
> > > >>>
> > > >>> Generally speaking, I like the idea, it makes sense to me in terms of 
> > > >>> using
> > > >>> pmdinfo to export this information
> > > >>>
> > > >>> That said, This may need to be a set of macros.  By that I mean (and 
> > > >>> correct  
> > > me  
> > > >>> if I'm wrong here), but the relationship between pmd's and kernel 
> > > >>> modules  
> > > is in  
> > > >>> some cases, more complex than a 'requires' or 'depends' relationship. 
> > > >>>  That  
> > > is  
> > > >>> to say, some pmd may need user space hardware access, but can use 
> > > >>> either  
> > > uio OR  
> > > >>> vfio, but doesn't need both, and can continue to function if only one 
> > > >>> is
> > > >>> available.  Other PMD's may be able to us

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-08-31 Thread Olivier Matz
Hi Neil,

On 08/31/2016 03:27 PM, Neil Horman wrote:
> On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
>> Hi Neil,
>>
>> On 08/30/2016 03:23 PM, Neil Horman wrote:
>>> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
 Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
 declare the list of kernel modules required to run properly.

 Today, most PCI drivers require uio/vfio.

 Signed-off-by: Olivier Matz 

 ---
 In this RFC, I supposed that all PCI drivers require a the loading of a
 uio/vfio module (except mlx*), this may be wrong.
 Comments are welcome!


  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
  drivers/net/e1000/em_ethdev.c   |  2 ++
  drivers/net/e1000/igb_ethdev.c  |  4 
  drivers/net/ena/ena_ethdev.c|  2 ++
  drivers/net/enic/enic_ethdev.c  |  2 ++
  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
  drivers/net/i40e/i40e_ethdev.c  |  2 ++
  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
  drivers/net/mlx4/mlx4.c |  2 ++
  drivers/net/mlx5/mlx5.c |  3 +++
  drivers/net/nfp/nfp_net.c   |  2 ++
  drivers/net/qede/qede_ethdev.c  |  4 
  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
  drivers/net/virtio/virtio_ethdev.c  |  2 ++
  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
  lib/librte_eal/common/include/rte_dev.h | 14 ++
  tools/dpdk-pmdinfo.py   |  5 -
  24 files changed, 69 insertions(+), 1 deletion(-)

>>>
>>> Generally speaking, I like the idea, it makes sense to me in terms of using
>>> pmdinfo to export this information
>>>
>>> That said, This may need to be a set of macros.  By that I mean (and 
>>> correct me
>>> if I'm wrong here), but the relationship between pmd's and kernel modules 
>>> is in
>>> some cases, more complex than a 'requires' or 'depends' relationship.  That 
>>> is
>>> to say, some pmd may need user space hardware access, but can use either 
>>> uio OR
>>> vfio, but doesn't need both, and can continue to function if only one is
>>> available.  Other PMD's may be able to use vfio or uio, but can still 
>>> function
>>> without either.  And some, as your patch implements, simply require one or 
>>> the
>>> other to function.  As such it seems like you may want a few macros, in the 
>>> form
>>> of:
>>>
>>> DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading, ignore 
>>> any
>>> failures 
>>> DRIVER_REGISTER_KMOD_REQUIRE - List of modules required to be loaded after
>>> request macro completes, fail if any are not loaded
>>>
>>> Thats just spitballing, mind you, theres probably a better way to do it, 
>>> but the
>>> idea is to list a set of modules you would like to have, and then create a
>>> parsable syntax to describe the modules that need to be loaded after the 
>>> request
>>> is complete so that you can accurately codify the situations I described 
>>> above.
>>
>> Thank you for your feedback.
>> However, I'm not sure I'm perfectly getting what you suggest.
>>
>> Do you think some PMDs could request a kernel module without really
>> requiring it? Do you have an example in mind?
>>
> Yes, thats precisely it.  The most clear example I could think of (though I'm
> not sure if any pmd currently supports this), is a pmd that supports both UIO
> and VFIO communication with the kernel.  Such a PMD requires that one of those
> two modules be loaded, but only one (i.e. both are not required), so if only 
> the
> uio kernel module loads is a success case, likewise if only the vfio module
> loads can be treated as success.  Both loading are clearly successful.  Only 
> if
> neither load do we have a failure case.  I'm suggesting that the grammer that
> your exports define should take those cases into account.  Its not always as
> simple as "I must have the following modules"
> 
>> The syntax I've submitted lets you define several lists of modules, so
>> that the user or the script that starts the application can decide which
>> kmod list is better according to the environment.
>>
> If you have a human intervening in the module load process, sure, then its 
> fine.
> But it seems that this particular feature that you're implemnting might have
> automated uses.  That is to say the dpdk core library might be interested in
> parsing this particular information to direct module autoloading, and if thats
> desireable then you need to define these lists such that you can codify 
> failu

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-08-31 Thread Olivier Matz
Hi Neil,

On 08/30/2016 03:23 PM, Neil Horman wrote:
> On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
>> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
>> declare the list of kernel modules required to run properly.
>>
>> Today, most PCI drivers require uio/vfio.
>>
>> Signed-off-by: Olivier Matz 
>>
>> ---
>> In this RFC, I supposed that all PCI drivers require a the loading of a
>> uio/vfio module (except mlx*), this may be wrong.
>> Comments are welcome!
>>
>>
>>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
>>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
>>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
>>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
>>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
>>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
>>  drivers/net/e1000/em_ethdev.c   |  2 ++
>>  drivers/net/e1000/igb_ethdev.c  |  4 
>>  drivers/net/ena/ena_ethdev.c|  2 ++
>>  drivers/net/enic/enic_ethdev.c  |  2 ++
>>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
>>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
>>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
>>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
>>  drivers/net/mlx4/mlx4.c |  2 ++
>>  drivers/net/mlx5/mlx5.c |  3 +++
>>  drivers/net/nfp/nfp_net.c   |  2 ++
>>  drivers/net/qede/qede_ethdev.c  |  4 
>>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
>>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
>>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
>>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
>>  lib/librte_eal/common/include/rte_dev.h | 14 ++
>>  tools/dpdk-pmdinfo.py   |  5 -
>>  24 files changed, 69 insertions(+), 1 deletion(-)
>>
> 
> Generally speaking, I like the idea, it makes sense to me in terms of using
> pmdinfo to export this information
> 
> That said, This may need to be a set of macros.  By that I mean (and correct 
> me
> if I'm wrong here), but the relationship between pmd's and kernel modules is 
> in
> some cases, more complex than a 'requires' or 'depends' relationship.  That is
> to say, some pmd may need user space hardware access, but can use either uio 
> OR
> vfio, but doesn't need both, and can continue to function if only one is
> available.  Other PMD's may be able to use vfio or uio, but can still function
> without either.  And some, as your patch implements, simply require one or the
> other to function.  As such it seems like you may want a few macros, in the 
> form
> of:
> 
> DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading, ignore any
> failures 
> DRIVER_REGISTER_KMOD_REQUIRE - List of modules required to be loaded after
> request macro completes, fail if any are not loaded
> 
> Thats just spitballing, mind you, theres probably a better way to do it, but 
> the
> idea is to list a set of modules you would like to have, and then create a
> parsable syntax to describe the modules that need to be loaded after the 
> request
> is complete so that you can accurately codify the situations I described 
> above.

Thank you for your feedback.
However, I'm not sure I'm perfectly getting what you suggest.

Do you think some PMDs could request a kernel module without really
requiring it? Do you have an example in mind?

The syntax I've submitted lets you define several lists of modules, so
that the user or the script that starts the application can decide which
kmod list is better according to the environment.

For example, most drivers will advertise
"uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci", and the user or script
will have to choose between loading:
- uio igb_uio
- uio uio_pci_generic
- vfio vfio-pci


Olivier


[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-08-31 Thread Neil Horman
On Wed, Aug 31, 2016 at 11:21:18AM +0200, Olivier Matz wrote:
> Hi Neil,
> 
> On 08/30/2016 03:23 PM, Neil Horman wrote:
> > On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> >> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> >> declare the list of kernel modules required to run properly.
> >>
> >> Today, most PCI drivers require uio/vfio.
> >>
> >> Signed-off-by: Olivier Matz 
> >>
> >> ---
> >> In this RFC, I supposed that all PCI drivers require a the loading of a
> >> uio/vfio module (except mlx*), this may be wrong.
> >> Comments are welcome!
> >>
> >>
> >>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
> >>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
> >>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
> >>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
> >>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
> >>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
> >>  drivers/net/e1000/em_ethdev.c   |  2 ++
> >>  drivers/net/e1000/igb_ethdev.c  |  4 
> >>  drivers/net/ena/ena_ethdev.c|  2 ++
> >>  drivers/net/enic/enic_ethdev.c  |  2 ++
> >>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
> >>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
> >>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
> >>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
> >>  drivers/net/mlx4/mlx4.c |  2 ++
> >>  drivers/net/mlx5/mlx5.c |  3 +++
> >>  drivers/net/nfp/nfp_net.c   |  2 ++
> >>  drivers/net/qede/qede_ethdev.c  |  4 
> >>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> >>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
> >>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
> >>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
> >>  lib/librte_eal/common/include/rte_dev.h | 14 ++
> >>  tools/dpdk-pmdinfo.py   |  5 -
> >>  24 files changed, 69 insertions(+), 1 deletion(-)
> >>
> > 
> > Generally speaking, I like the idea, it makes sense to me in terms of using
> > pmdinfo to export this information
> > 
> > That said, This may need to be a set of macros.  By that I mean (and 
> > correct me
> > if I'm wrong here), but the relationship between pmd's and kernel modules 
> > is in
> > some cases, more complex than a 'requires' or 'depends' relationship.  That 
> > is
> > to say, some pmd may need user space hardware access, but can use either 
> > uio OR
> > vfio, but doesn't need both, and can continue to function if only one is
> > available.  Other PMD's may be able to use vfio or uio, but can still 
> > function
> > without either.  And some, as your patch implements, simply require one or 
> > the
> > other to function.  As such it seems like you may want a few macros, in the 
> > form
> > of:
> > 
> > DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading, ignore 
> > any
> > failures 
> > DRIVER_REGISTER_KMOD_REQUIRE - List of modules required to be loaded after
> > request macro completes, fail if any are not loaded
> > 
> > Thats just spitballing, mind you, theres probably a better way to do it, 
> > but the
> > idea is to list a set of modules you would like to have, and then create a
> > parsable syntax to describe the modules that need to be loaded after the 
> > request
> > is complete so that you can accurately codify the situations I described 
> > above.
> 
> Thank you for your feedback.
> However, I'm not sure I'm perfectly getting what you suggest.
> 
> Do you think some PMDs could request a kernel module without really
> requiring it? Do you have an example in mind?
> 
Yes, thats precisely it.  The most clear example I could think of (though I'm
not sure if any pmd currently supports this), is a pmd that supports both UIO
and VFIO communication with the kernel.  Such a PMD requires that one of those
two modules be loaded, but only one (i.e. both are not required), so if only the
uio kernel module loads is a success case, likewise if only the vfio module
loads can be treated as success.  Both loading are clearly successful.  Only if
neither load do we have a failure case.  I'm suggesting that the grammer that
your exports define should take those cases into account.  Its not always as
simple as "I must have the following modules"

> The syntax I've submitted lets you define several lists of modules, so
> that the user or the script that starts the application can decide which
> kmod list is better according to the environment.
> 
If you have a human intervening in the module load process, sure, then its fine.
But it seems that this particular feature that you're implemnting might have
automated uses.  That is to say the dpdk core library might be interested in
parsing this particular information to direct module autoloading, and if thats
desireable then you need to define these lists such that you can codify failure
and success conditions.

> For example, most drivers will advertise
> "uio,igb_uio:uio,uio_pci_gene

[dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo

2016-08-30 Thread Neil Horman
On Fri, Aug 26, 2016 at 03:20:46PM +0200, Olivier Matz wrote:
> Add a new macro DRIVER_REGISTER_KMOD_DEP() that allows a driver to
> declare the list of kernel modules required to run properly.
> 
> Today, most PCI drivers require uio/vfio.
> 
> Signed-off-by: Olivier Matz 
> 
> ---
> In this RFC, I supposed that all PCI drivers require a the loading of a
> uio/vfio module (except mlx*), this may be wrong.
> Comments are welcome!
> 
> 
>  buildtools/pmdinfogen/pmdinfogen.c  |  1 +
>  buildtools/pmdinfogen/pmdinfogen.h  |  1 +
>  drivers/crypto/qat/rte_qat_cryptodev.c  |  2 ++
>  drivers/net/bnx2x/bnx2x_ethdev.c|  4 
>  drivers/net/bnxt/bnxt_ethdev.c  |  2 ++
>  drivers/net/cxgbe/cxgbe_ethdev.c|  2 ++
>  drivers/net/e1000/em_ethdev.c   |  2 ++
>  drivers/net/e1000/igb_ethdev.c  |  4 
>  drivers/net/ena/ena_ethdev.c|  2 ++
>  drivers/net/enic/enic_ethdev.c  |  2 ++
>  drivers/net/fm10k/fm10k_ethdev.c|  2 ++
>  drivers/net/i40e/i40e_ethdev.c  |  2 ++
>  drivers/net/i40e/i40e_ethdev_vf.c   |  2 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c|  4 
>  drivers/net/mlx4/mlx4.c |  2 ++
>  drivers/net/mlx5/mlx5.c |  3 +++
>  drivers/net/nfp/nfp_net.c   |  2 ++
>  drivers/net/qede/qede_ethdev.c  |  4 
>  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
>  drivers/net/thunderx/nicvf_ethdev.c |  2 ++
>  drivers/net/virtio/virtio_ethdev.c  |  2 ++
>  drivers/net/vmxnet3/vmxnet3_ethdev.c|  2 ++
>  lib/librte_eal/common/include/rte_dev.h | 14 ++
>  tools/dpdk-pmdinfo.py   |  5 -
>  24 files changed, 69 insertions(+), 1 deletion(-)
> 

Generally speaking, I like the idea, it makes sense to me in terms of using
pmdinfo to export this information

That said, This may need to be a set of macros.  By that I mean (and correct me
if I'm wrong here), but the relationship between pmd's and kernel modules is in
some cases, more complex than a 'requires' or 'depends' relationship.  That is
to say, some pmd may need user space hardware access, but can use either uio OR
vfio, but doesn't need both, and can continue to function if only one is
available.  Other PMD's may be able to use vfio or uio, but can still function
without either.  And some, as your patch implements, simply require one or the
other to function.  As such it seems like you may want a few macros, in the form
of:

DRIVER_REGISTER_KMOD_REQUEST - List of modules to attempt loading, ignore any
failures 
DRIVER_REGISTER_KMOD_REQUIRE - List of modules required to be loaded after
request macro completes, fail if any are not loaded

Thats just spitballing, mind you, theres probably a better way to do it, but the
idea is to list a set of modules you would like to have, and then create a
parsable syntax to describe the modules that need to be loaded after the request
is complete so that you can accurately codify the situations I described above.

Neil

> diff --git a/buildtools/pmdinfogen/pmdinfogen.c 
> b/buildtools/pmdinfogen/pmdinfogen.c
> index e1bf2e4..1e5b6f3 100644
> --- a/buildtools/pmdinfogen/pmdinfogen.c
> +++ b/buildtools/pmdinfogen/pmdinfogen.c
> @@ -269,6 +269,7 @@ struct opt_tag {
>  
>  static const struct opt_tag opt_tags[] = {
>   {"_param_string_export", "params"},
> + {"_kmod_dep_export", "kmod"},
>  };
>  
>  static int complete_pmd_entry(struct elf_info *info, struct pmd_driver *drv)
> diff --git a/buildtools/pmdinfogen/pmdinfogen.h 
> b/buildtools/pmdinfogen/pmdinfogen.h
> index 1da2966..2fab2aa 100644
> --- a/buildtools/pmdinfogen/pmdinfogen.h
> +++ b/buildtools/pmdinfogen/pmdinfogen.h
> @@ -85,6 +85,7 @@ else \
>  
>  enum opt_params {
>   PMD_PARAM_STRING = 0,
> + PMD_KMOD_DEP,
>   PMD_OPT_MAX
>  };
>  
> diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c 
> b/drivers/crypto/qat/rte_qat_cryptodev.c
> index 82ab047..fc62be9 100644
> --- a/drivers/crypto/qat/rte_qat_cryptodev.c
> +++ b/drivers/crypto/qat/rte_qat_cryptodev.c
> @@ -135,4 +135,6 @@ static struct rte_driver pmd_qat_drv = {
>  
>  PMD_REGISTER_DRIVER(pmd_qat_drv, CRYPTODEV_NAME_QAT_SYM_PMD);
>  DRIVER_REGISTER_PCI_TABLE(CRYPTODEV_NAME_QAT_SYM_PMD, pci_id_qat_map);
> +DRIVER_REGISTER_KMOD_DEP(CRYPTODEV_NAME_QAT_SYM_PMD,
> + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci");
>  
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c 
> b/drivers/net/bnx2x/bnx2x_ethdev.c
> index f3ab355..ba8831a 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -667,5 +667,9 @@ static struct rte_driver rte_bnx2xvf_driver = {
>  
>  PMD_REGISTER_DRIVER(rte_bnx2x_driver, bnx2x);
>  DRIVER_REGISTER_PCI_TABLE(bnx2x, pci_id_bnx2x_map);
> +DRIVER_REGISTER_KMOD_DEP(bnx2x,
> + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci");
>  PMD_REGISTER_DRIVER(rte_bnx2xvf_driver, bnx2xvf);
>  DRIVER_REGISTER_PCI_TABLE(bnx2xvf, pci_id_bnx2xvf_map);
> +DRIVER_REGISTER_KMOD_DE