From: "Trahe, Fiona" <fiona.trahe@intel.com>
To: "dev@dpdk.org" <dev@dpdk.org>
Cc: Olivier Matz <olivier.matz@6wind.com>,
"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
Thomas Monjalon <thomas.monjalon@6wind.com>,
"Trahe, Fiona" <fiona.trahe@intel.com>
Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo
Date: Thu, 1 Sep 2016 12:55:27 +0000 [thread overview]
Message-ID: <348A99DA5F5B7549AA880327E580B4358909A43A@IRSMSX101.ger.corp.intel.com> (raw)
In-Reply-To: <54a0164e-b242-b930-ec91-60f91b700119@6wind.com>
Hi Neil and Olivier,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier Matz
> Sent: Wednesday, August 31, 2016 2:40 PM
> To: Neil Horman <nhorman@tuxdriver.com>
> Cc: dev@dpdk.org; thomas.monjalon@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 <olivier.matz@6wind.com>
> >>>>
> >>>> ---
> >>>> 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_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
> >>
> > 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.
>
> Regards,
> Olivier
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)
Also the dependencies on a VM are different to a bare-metal installation, i.e. the qat_xxxx driver just
needs to be loaded in the Host. So maybe this could be satisfied by a separate list?
DRIVER_REGISTER_KMOD_DEP()
DRIVER_REGISTER_KMOD_VM_DEP()
But maybe this is all too complex, and instead the feature should be considered as optional and
not requiring all dependencies to be declared?
Regards,
Fiona
next prev parent reply other threads:[~2016-09-01 12:55 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-26 13:20 [dpdk-dev] [RFC] " Olivier Matz
2016-08-30 8:40 ` Matej Vido
2016-08-30 8:56 ` Olivier Matz
2016-08-30 13:23 ` [dpdk-dev] [dpdk-dev, RFC] " Neil Horman
2016-08-31 9:21 ` Olivier Matz
2016-08-31 13:27 ` Neil Horman
2016-08-31 13:39 ` Olivier Matz
2016-09-01 12:55 ` Trahe, Fiona [this message]
2016-09-01 17:35 ` Neil Horman
2016-09-01 17:41 ` Stephen Hemminger
2016-09-01 19:15 ` Neil Horman
2016-09-02 9:19 ` Trahe, Fiona
2016-09-02 13:33 ` Neil Horman
2016-09-02 13:52 ` Trahe, Fiona
2016-09-02 14:15 ` Neil Horman
2016-09-02 16:13 ` Olivier Matz
2016-09-02 10:55 ` Thomas Monjalon
2016-09-15 14:22 ` [dpdk-dev] [PATCH] " Olivier Matz
2016-09-16 9:49 ` Trahe, Fiona
2016-09-16 14:26 ` David Marchand
2016-09-19 8:17 ` Bruce Richardson
2016-09-19 12:25 ` Olivier Matz
2016-11-22 9:50 ` [dpdk-dev] [PATCH v2] " Olivier Matz
2016-11-22 10:27 ` Adrien Mazarguil
2016-11-22 10:31 ` Olivier Matz
2016-12-15 13:46 ` [dpdk-dev] [PATCH v3] " Olivier Matz
2016-12-15 14:52 ` Ferruh Yigit
2016-12-16 9:36 ` Olivier Matz
2016-12-19 13:30 ` Thomas Monjalon
2016-12-15 16:09 ` Neil Horman
2016-12-15 17:22 ` Stephen Hemminger
2016-12-16 9:22 ` Olivier Matz
2016-12-16 12:37 ` Neil Horman
2016-12-16 13:04 ` Bruce Richardson
2016-12-16 14:19 ` Ferruh Yigit
2016-12-19 12:42 ` Neil Horman
2016-12-19 14:12 ` Ferruh Yigit
2016-12-16 8:23 ` Adrien Mazarguil
2016-12-20 17:26 ` Thomas Monjalon
2016-12-21 9:21 ` Andrew Rybchenko
2016-12-21 11:37 ` Neil Horman
2016-12-21 11:40 ` Andrew Rybchenko
2016-12-22 11:04 ` Ferruh Yigit
2016-12-22 11:35 ` Andrew Rybchenko
2016-12-22 12:07 ` Ferruh Yigit
2016-12-22 12:08 ` Andrew Rybchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=348A99DA5F5B7549AA880327E580B4358909A43A@IRSMSX101.ger.corp.intel.com \
--to=fiona.trahe@intel.com \
--cc=dev@dpdk.org \
--cc=nhorman@tuxdriver.com \
--cc=olivier.matz@6wind.com \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).