From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id AADA4293C for ; Thu, 1 Sep 2016 14:55:30 +0200 (CEST) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP; 01 Sep 2016 05:55:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.30,267,1470726000"; d="scan'208";a="755934212" Received: from irsmsx103.ger.corp.intel.com ([163.33.3.157]) by FMSMGA003.fm.intel.com with ESMTP; 01 Sep 2016 05:55:28 -0700 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.183]) by IRSMSX103.ger.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Thu, 1 Sep 2016 13:55:27 +0100 From: "Trahe, Fiona" To: "dev@dpdk.org" CC: Olivier Matz , "nhorman@tuxdriver.com" , Thomas Monjalon , "Trahe, Fiona" Thread-Topic: [dpdk-dev] [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo Thread-Index: AQHSAsHVEd0mp5ZWP0K1DdXkNvLng6Biu8cAgABEsYCAAAN4AIABgaTg Date: Thu, 1 Sep 2016 12:55:27 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B4358909A43A@IRSMSX101.ger.corp.intel.com> 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> In-Reply-To: <54a0164e-b242-b930-ec91-60f91b700119@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTFlM2Y3YmYtMDRiOC00MGUxLTliYjUtZTMzYmY0NjY2MTg3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkpDSlJSaGlkUkNnejZlcU5UNG1YMmZSXC9WWHFVMWp3YlRzQnJWaUJsSDhvPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 12:55:31 -0000 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 dependenc= ies > in pmdinfo >=20 > Hi Neil, >=20 > 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 o= f 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 mod= ules > 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 eit= her > 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 o= ne 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 cre= ate 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 descri= bed > 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 (thou= gh I'm > > not sure if any pmd currently supports this), is a pmd that supports bo= th UIO > > and VFIO communication with the kernel. Such a PMD requires that one o= f > 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 mo= dule > > 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 gramme= r that > > your exports define should take those cases into account. Its not alwa= ys 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 whi= ch > >> 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 interest= ed in > > parsing this particular information to direct module autoloading, and i= f 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 scrip= t > >> 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 f= ailure 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 AN= D > > 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 wi= ll work > > just fine. >=20 > Yep, that's the idea. >=20 > Colon and commas are the best separators I've thought about, but any > idea to make the syntax clearer is welcome ;) >=20 > 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. >=20 > Regards, > Olivier This RFC seems like a good idea - and something the Intel QuickAssist PMD c= ould benefit from. However the (mod1 & mod2) can handle the QAT case better in my opinion. i.e. as well as needing one of=20 * 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 variati= ons. What really reflects the dependencies would be=20 ((uio & igb_uio) | (uio & uio_pci_generic) | (vfio & vfio_pci)) & (qat_dh89= 5xcc | qat_c62x | qat_c3xxx) Also the dependencies on a VM are different to a bare-metal installation, i= .e. the qat_xxxx driver just=20 needs to be loaded in the Host. So maybe this could be satisfied by a separ= ate list? DRIVER_REGISTER_KMOD_DEP() DRIVER_REGISTER_KMOD_VM_DEP() But maybe this is all too complex, and instead the feature should be consid= ered as optional and=20 not requiring all dependencies to be declared?=20 Regards, Fiona