From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Horman Subject: Re: [dpdk-dev, RFC] drivers: advertise kmod dependencies in pmdinfo Date: Tue, 30 Aug 2016 09:23:52 -0400 Message-ID: <20160830132352.GB30977@hmsreliant.think-freely.org> References: <1472217646-26219-1-git-send-email-olivier.matz@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: dev@dpdk.org, thomas.monjalon@6wind.com To: Olivier Matz Return-path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 0AB62CE7 for ; Tue, 30 Aug 2016 15:24:02 +0200 (CEST) Content-Disposition: inline In-Reply-To: <1472217646-26219-1-git-send-email-olivier.matz@6wind.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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_DEP(bnx2xvf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c > index 3795fac..5c6c7b5 100644 > --- a/drivers/net/bnxt/bnxt_ethdev.c > +++ b/drivers/net/bnxt/bnxt_ethdev.c > @@ -1068,3 +1068,5 @@ static struct rte_driver bnxt_pmd_drv = { > > PMD_REGISTER_DRIVER(bnxt_pmd_drv, bnxt); > DRIVER_REGISTER_PCI_TABLE(bnxt, bnxt_pci_id_map); > +DRIVER_REGISTER_KMOD_DEP(bnxt, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c > index 9208a61..cea2741 100644 > --- a/drivers/net/cxgbe/cxgbe_ethdev.c > +++ b/drivers/net/cxgbe/cxgbe_ethdev.c > @@ -1068,4 +1068,6 @@ static struct rte_driver rte_cxgbe_driver = { > > PMD_REGISTER_DRIVER(rte_cxgbe_driver, cxgb4); > DRIVER_REGISTER_PCI_TABLE(cxgb4, cxgb4_pci_tbl); > +DRIVER_REGISTER_KMOD_DEP(cxgb4, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > > diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c > index ad104ed..bd0d0ea 100644 > --- a/drivers/net/e1000/em_ethdev.c > +++ b/drivers/net/e1000/em_ethdev.c > @@ -1806,3 +1806,5 @@ struct rte_driver em_pmd_drv = { > > PMD_REGISTER_DRIVER(em_pmd_drv, em); > DRIVER_REGISTER_PCI_TABLE(em, pci_id_em_map); > +DRIVER_REGISTER_KMOD_DEP(em, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c > index 4e9e6a3..a3dfbfe 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -5257,5 +5257,9 @@ eth_igb_configure_msix_intr(struct rte_eth_dev *dev) > > PMD_REGISTER_DRIVER(pmd_igb_drv, igb); > DRIVER_REGISTER_PCI_TABLE(igb, pci_id_igb_map); > +DRIVER_REGISTER_KMOD_DEP(igb, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > PMD_REGISTER_DRIVER(pmd_igbvf_drv, igbvf); > DRIVER_REGISTER_PCI_TABLE(igbvf, pci_id_igbvf_map); > +DRIVER_REGISTER_KMOD_DEP(igbvf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c > index ac0803d..a45d60c 100644 > --- a/drivers/net/ena/ena_ethdev.c > +++ b/drivers/net/ena/ena_ethdev.c > @@ -1709,3 +1709,5 @@ struct rte_driver ena_pmd_drv = { > > PMD_REGISTER_DRIVER(ena_pmd_drv, ena); > DRIVER_REGISTER_PCI_TABLE(ena, pci_id_ena_map); > +DRIVER_REGISTER_KMOD_DEP(ena, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c > index 47b07c9..a1b8abc 100644 > --- a/drivers/net/enic/enic_ethdev.c > +++ b/drivers/net/enic/enic_ethdev.c > @@ -642,3 +642,5 @@ static struct rte_driver rte_enic_driver = { > > PMD_REGISTER_DRIVER(rte_enic_driver, enic); > DRIVER_REGISTER_PCI_TABLE(enic, pci_id_enic_map); > +DRIVER_REGISTER_KMOD_DEP(enic, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c > index 01f4a72..391ccd7 100644 > --- a/drivers/net/fm10k/fm10k_ethdev.c > +++ b/drivers/net/fm10k/fm10k_ethdev.c > @@ -3086,3 +3086,5 @@ static struct rte_driver rte_fm10k_driver = { > > PMD_REGISTER_DRIVER(rte_fm10k_driver, fm10k); > DRIVER_REGISTER_PCI_TABLE(fm10k, pci_id_fm10k_map); > +DRIVER_REGISTER_KMOD_DEP(fm10k, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c > index d0aeb70..a1466aa 100644 > --- a/drivers/net/i40e/i40e_ethdev.c > +++ b/drivers/net/i40e/i40e_ethdev.c > @@ -723,6 +723,8 @@ static struct rte_driver rte_i40e_driver = { > > PMD_REGISTER_DRIVER(rte_i40e_driver, i40e); > DRIVER_REGISTER_PCI_TABLE(i40e, pci_id_i40e_map); > +DRIVER_REGISTER_KMOD_DEP(i40e, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > > /* > * Initialize registers for flexible payload, which should be set by NVM. > diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c > index a616ae0..61be44a 100644 > --- a/drivers/net/i40e/i40e_ethdev_vf.c > +++ b/drivers/net/i40e/i40e_ethdev_vf.c > @@ -1586,6 +1586,8 @@ static struct rte_driver rte_i40evf_driver = { > > PMD_REGISTER_DRIVER(rte_i40evf_driver, i40evf); > DRIVER_REGISTER_PCI_TABLE(i40evf, pci_id_i40evf_map); > +DRIVER_REGISTER_KMOD_DEP(i40evf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > > static int > i40evf_dev_configure(struct rte_eth_dev *dev) > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index fb618ef..e353d7a 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -7421,5 +7421,9 @@ static struct rte_driver rte_ixgbevf_driver = { > > PMD_REGISTER_DRIVER(rte_ixgbe_driver, ixgbe); > DRIVER_REGISTER_PCI_TABLE(ixgbe, pci_id_ixgbe_map); > +DRIVER_REGISTER_KMOD_DEP(ixgbe, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > PMD_REGISTER_DRIVER(rte_ixgbevf_driver, ixgbevf); > DRIVER_REGISTER_PCI_TABLE(ixgbevf, pci_id_ixgbevf_map); > +DRIVER_REGISTER_KMOD_DEP(ixgbevf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > index 304c846..d8f6905 100644 > --- a/drivers/net/mlx4/mlx4.c > +++ b/drivers/net/mlx4/mlx4.c > @@ -5948,3 +5948,5 @@ static struct rte_driver rte_mlx4_driver = { > > PMD_REGISTER_DRIVER(rte_mlx4_driver, mlx4); > DRIVER_REGISTER_PCI_TABLE(mlx4, mlx4_pci_id_map); > +DRIVER_REGISTER_KMOD_DEP(mlx4, > + "ib_uverbs,mlx4_core,mlx4_en,mlx4_ib"); > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c > index d96a9af..29d7332 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -763,3 +763,6 @@ static struct rte_driver rte_mlx5_driver = { > > PMD_REGISTER_DRIVER(rte_mlx5_driver, mlx5); > DRIVER_REGISTER_PCI_TABLE(mlx5, mlx5_pci_id_map); > +DRIVER_REGISTER_KMOD_DEP(mlx5, > + "ptp,inet_lro,ib_sa,ib_mad,ib_netlink,ib_addr," > + "ib_core,ib_uverbs,mlx5_core,mlx5_ib"); > diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c > index 82e3e4e..f4c8a39 100644 > --- a/drivers/net/nfp/nfp_net.c > +++ b/drivers/net/nfp/nfp_net.c > @@ -2488,6 +2488,8 @@ static struct rte_driver rte_nfp_net_driver = { > > PMD_REGISTER_DRIVER(rte_nfp_net_driver, nfp); > DRIVER_REGISTER_PCI_TABLE(nfp, pci_id_nfp_net_map); > +DRIVER_REGISTER_KMOD_DEP(nfp, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > > /* > * Local variables: > diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c > index 82e44b8..a3c6b44 100644 > --- a/drivers/net/qede/qede_ethdev.c > +++ b/drivers/net/qede/qede_ethdev.c > @@ -1530,5 +1530,9 @@ static struct rte_driver rte_qede_driver = { > > PMD_REGISTER_DRIVER(rte_qede_driver, qede); > DRIVER_REGISTER_PCI_TABLE(qede, pci_id_qede_map); > +DRIVER_REGISTER_KMOD_DEP(qede, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > PMD_REGISTER_DRIVER(rte_qedevf_driver, qedevf); > DRIVER_REGISTER_PCI_TABLE(qedevf, pci_id_qedevf_map); > +DRIVER_REGISTER_KMOD_DEP(qedevf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c > index 483d789..409e71f 100644 > --- a/drivers/net/szedata2/rte_eth_szedata2.c > +++ b/drivers/net/szedata2/rte_eth_szedata2.c > @@ -1602,3 +1602,5 @@ static struct rte_driver rte_szedata2_driver = { > > PMD_REGISTER_DRIVER(rte_szedata2_driver, RTE_SZEDATA2_DRIVER_NAME); > DRIVER_REGISTER_PCI_TABLE(RTE_SZEDATA2_DRIVER_NAME, rte_szedata2_pci_id_table); > +DRIVER_REGISTER_KMOD_DEP(RTE_SZEDATA2_DRIVER_NAME, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c > index 4f875c0..8c33df2 100644 > --- a/drivers/net/thunderx/nicvf_ethdev.c > +++ b/drivers/net/thunderx/nicvf_ethdev.c > @@ -1785,3 +1785,5 @@ static struct rte_driver rte_nicvf_driver = { > > PMD_REGISTER_DRIVER(rte_nicvf_driver, thunderx_nicvf); > DRIVER_REGISTER_PCI_TABLE(thunderx_nicvf, pci_id_nicvf_map); > +DRIVER_REGISTER_KMOD_DEP(thunderx_nicvf, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c > index 07d6449..f65b9a4 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -1570,3 +1570,5 @@ static struct rte_driver rte_virtio_driver = { > > PMD_REGISTER_DRIVER(rte_virtio_driver, virtio_net); > DRIVER_REGISTER_PCI_TABLE(virtio_net, pci_id_virtio_map); > +DRIVER_REGISTER_KMOD_DEP(virtio_net, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c > index 5874215..d2d07ad 100644 > --- a/drivers/net/vmxnet3/vmxnet3_ethdev.c > +++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c > @@ -955,3 +955,5 @@ static struct rte_driver rte_vmxnet3_driver = { > > PMD_REGISTER_DRIVER(rte_vmxnet3_driver, vmxnet3); > DRIVER_REGISTER_PCI_TABLE(vmxnet3, pci_id_vmxnet3_map); > +DRIVER_REGISTER_KMOD_DEP(vmxnet3, > + "uio,igb_uio:uio,uio_pci_generic:vfio,vfio-pci"); > diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h > index 95789f9..b721dc3 100644 > --- a/lib/librte_eal/common/include/rte_dev.h > +++ b/lib/librte_eal/common/include/rte_dev.h > @@ -203,6 +203,20 @@ RTE_STR(table) > static const char DRV_EXP_TAG(name, param_string_export)[] \ > __attribute__((used)) = str > > +/** > + * Advertise the list of kernel modules required to run this driver > + * > + * This string list the name of kernel modules, separated by commas. The > + * order is important. If several modules lists are possible, they are > + * separated by colons. > + * > + * Example: "uio,igb_uio:uio,uio_pci_generic" means either "uio,igb_uio" > + * or "uio,uio_pci_generic". > + */ > +#define DRIVER_REGISTER_KMOD_DEP(name, str) \ > +static const char DRV_EXP_TAG(name, kmod_dep_export)[] \ > +__attribute__((used)) = str > + > #ifdef __cplusplus > } > #endif > diff --git a/tools/dpdk-pmdinfo.py b/tools/dpdk-pmdinfo.py > index 3db9819..17bfed4 100755 > --- a/tools/dpdk-pmdinfo.py > +++ b/tools/dpdk-pmdinfo.py > @@ -312,7 +312,10 @@ class ReadElf(object): > global raw_output > global pcidb > > - optional_pmd_info = [{'id': 'params', 'tag': 'PMD PARAMETERS'}] > + optional_pmd_info = [ > + {'id': 'params', 'tag': 'PMD PARAMETERS'}, > + {'id': 'kmod', 'tag': 'PMD KMOD DEPENDENCIES'} > + ] > > i = mystring.index("=") > mystring = mystring[i + 2:]