From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id DA9E41518 for ; Thu, 1 Sep 2016 19:35:56 +0200 (CEST) Received: from cpe-2606-a000-111b-40ed-7aac-c0ff-fec2-933b.dyn6.twc.com ([2606:a000:111b:40ed:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1bfVu3-0000o4-Na; Thu, 01 Sep 2016 13:35:54 -0400 Date: Thu, 1 Sep 2016 13:35:19 -0400 From: Neil Horman To: "Trahe, Fiona" Cc: "dev@dpdk.org" , Olivier Matz , Thomas Monjalon Message-ID: <20160901173519.GA11132@hmsreliant.think-freely.org> References: <1472217646-26219-1-git-send-email-olivier.matz@6wind.com> <20160830132352.GB30977@hmsreliant.think-freely.org> <48f9320b-9402-0ecd-8971-c3785778081a@6wind.com> <20160831132709.GA32000@hmsreliant.think-freely.org> <54a0164e-b242-b930-ec91-60f91b700119@6wind.com> <348A99DA5F5B7549AA880327E580B4358909A43A@IRSMSX101.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <348A99DA5F5B7549AA880327E580B4358909A43A@IRSMSX101.ger.corp.intel.com> User-Agent: Mutt/1.7.0 (2016-08-17) X-Spam-Score: -1.0 (-) X-Spam-Status: No Subject: Re: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Sep 2016 17:35:57 -0000 On Thu, Sep 01, 2016 at 12:55:27PM +0000, Trahe, Fiona wrote: > 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 > > 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 > > >>>> > > >>>> --- > > >>>> 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) > 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. > 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() > This makes me a bit nervous, Ideally, nothing should have to know if its running on bare metal or in a vm, we should try to avoid vm specific macros if possible. Not sure what the alternative is yet, though. > 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 > >