From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id CCF752956 for ; Wed, 31 Aug 2016 15:39:43 +0200 (CEST) Received: from lfbn-1-8252-96.w81-254.abo.wanadoo.fr ([81.254.151.96] helo=[192.168.1.13]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1bf5ml-0002jv-0W; Wed, 31 Aug 2016 15:42:37 +0200 To: Neil Horman 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> Cc: dev@dpdk.org, thomas.monjalon@6wind.com From: Olivier Matz Message-ID: <54a0164e-b242-b930-ec91-60f91b700119@6wind.com> Date: Wed, 31 Aug 2016 15:39:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160831132709.GA32000@hmsreliant.think-freely.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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: Wed, 31 Aug 2016 13:39:44 -0000 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