[dpdk-dev] [PATCH v9 0/7] export PMD infos

[Neil Horman]

On Mon, Jul 04, 2016 at 03:13:58AM +0200, Thomas Monjalon wrote:
> This is a respin of the series from Neil.
> It was planned to be integrated in 16.07-rc1 but the discovered issues
> make a new revision needed.
> There are still few things which could be improved but it is not mandatory
> to fix them for an integration in 16.07-rc2:
>   - fix make clean after pmdinfogen
Clean works just fine, whats wrong with it?
>   - build installable pmdinfogen for target
Again, this seems to be working just fine for me, what do you see as wrong with
it?
>   - convert pmdinfo.py to Python 3
This was discussed weeks ago earlier in this thread, since there is no
requirement for python3 we decided not to use it.  Why renege on that now?
>   - document dependency pyelftools
In what capacity beyond simply noting the fact that it imports that package?

> 
> Changes done in this v9:
>   - fix build dependency of drivers on pmdinfogen
In what way is it broken?  The drivers directory is dependent on building
pmdinfogen, what more do you want?

>   - fix build of mlx4, mlx5, aesni
They weren't broken as of V8, not sure what you're getting at here

>   - fix new drivers bnxt, thunderx, kasumi
>   - fix MAINTAINERS file
>   - fix coding style in pmdinfogen
>   - add compiler checks for pmdinfogen
>   - remove useless functions in pmdinfogen
>   - fail build if pmdinfogen fails (set -e)
>   - fix verbose pmdinfogen run
>   - build pmdinfogen in buildtools directory (was app)
Are you kidding me!?  We just went 10 rounds over where to build this stupid
application, and after I finally gave in to your demands, you change it one
last time and call it a fix?  Thats a real slap in the face, thanks for that.

>   - install pmdinfogen in sdk package (was runtime)
>   - fix CamelCase in pmdinfo.py
>   - prefix executables with dpdk-
Again, seriously?  Are you just trying to assert some sort of not invented here
mentality?  We had this conversation over and over again weeks ago, but you
couldn't possibly chime in then?  And you have to slip it in now, because this
is the way you want it, as though I did it wrong previously?  

>   - rename PMD_REGISTER_DRIVER -> RTE_REGISTER_DRIVER
Annnd...one more, of course.  There simply no way you could leave this alone,
and modify subsequent pending patches to match the existing macro format, is
there? 

>   - separate commit for hostapp.mk refresh
>   - remove useless hostlib.mk
What does this have to do with 'fixes' for this series?

>   - spread doc in appropriate patches
> 
Again, why?  Whats wrong with adding documentation in its own patch?  To modify
my work to fit your sensibilities and call it a fix is really quite an insult.

Honestly, at this point, I'm done.  Do what you want to it, I don't plan on
making any more changes.

> Please review carefully.
> 
> Older changes:
> 
> v8)
>  * Modified symlink for pmdinfo to use dpdk_
>  * Added dpdk_pmdinfogen for pmdinfogen binary
> 
> v7)
>  * Fixed up copyright
>  * Switched buildtool makefile to use hostapp.mk
>  * Modified arch check to use RTE_ARCH_64
>  * Modifed hostapp.mk to drop output in build/app
>  * Additional testing on ppc64 to ensure big endian works
>  * Modified TO_NATIVE macro to use rte_byteorder inlines 
>    based on endianess of target ELF file
>  * Ran checkpatch on commits
>  * Fixed some typos in doc
> 
> v6)
>  * Added some programming guide documentation
>  * Reformatted python script with pep8
> 
> v5)
>  * Added a dpdk- prefix to pmdinfo symlink
>  * Renamed build announcement to PMDINFOGEN/BUILD
>  * Removed erroneous true statement from makefile
>  * Removed duplicate rte.hostapp.mk makefile
>  * Fixed some whitespace
>  * Whitespace fixups
>  * Fixed makefile if; then style
>  * Renamed module string C file
>  * Removed duplicate rte_pci_id definition
>  * Clarified macro names
>  * Removed PMD type attribute
>  * Fixed tools usage for 32 bit arches
>  * Removed some unused code
>  * Added a few comments
> 
> v4)
>  * Modified the operation of the -p option. As much as I don't like implying
> that autoloaded pmds are guaranteed to be there at run time, I'm having a hard
> time seeing how we can avoid specifying the application file to scan for the
> autoload directory.  Without it we can't determine which library the user means
> in a multiversion installation
>  * Cleaned up the help text
>  * Added a rule for an install target for pmdinfo
>  * Guarded against some tracebacks in pmdinfo
>  * Use DT_NEEDED entries to get versioned libraries in -p mode
>  * Fixed traceback that occurs on lack of input arguments
>  * Fixed some erroneous macro usage in drivers that aren't in the default build
> 
> v3)
>  * Made table mode printing the default mode
>  * Added a default path to the pci.ids file
>  * Modifed pmdinfo to use python rather than python3
>  * Cleaned up some changelog entries
>  * Added an export for RTE_EAL_PMD_PATH
>  * Added a plugin option to pmdinfo to scan for autoloaded DSO's
> 
> v2)
>  * Made the export macros a bit easier to expand
>  * Added a macro to optionally export command line strings
>  * Renamed utilties to be more appropriate
>    (pmdinfo -> pmdinfogen, pmd_hw_support.py -> pmdinfo.py)
>  * Added search capabilities to pmdinfo.py so that we search for libraries
>    linked using DT_NEEDINFO entries.  We search DT_RUNPATH, LD_LIBRARY_PATH,
>    /usr/lib and /lib
>  * Added serch abilities if the specified binary to pmdinfo isn't found, we
>    search LD_LIBRARY_PATH, /usr/lib and /lib
>  * Added an option to pmdinfo.py to pretty-print hardware support info using the
>    pci.ids database
> 
> ----------------
> Original summary
> ----------------
> 
>         So heres attempt number 2 at a method for exporting PMD hardware support
> information.  As we discussed previously, the consensus seems to be that pmd
> information should be:
> 
> 1) Able to be interrogated on any ELF binary (application binary or individual
> DSO)
> 2) Equally functional on statically linked applications or on DSO's
> 3) Resilient to symbol stripping
> 4) Script friendly
> 5) Show kernel dependencies
> 6) List driver options
> 7) Show driver name
> 8) Offer human readable output
> 9) Show DPDK version
> 10) Show driver version
> 11) Allow for expansion
> 12) Not place additional build environment dependencies on an application
> 
> I added item 12 myself, because I don't think its reasonable to require
> applications to use a specific linker script to get hardware support information
> (which precludes the use of a special .modinfo section like the kernel uses)
> 
> However, we still can use some tricks from the kernel to make this work.  In
> this approach, what I've done is the following:
> 
> A) Modified the driver registration macro to also define a variable:
> this_pmd_name<n>= "name"
> 
> Based on the unique name string pointed to by the above variable, we can
> query for an arbitrary number of other symbols following the pattern:
> <name>_<tag>
> 
> Where tag is some well known identifier for information we wish to export
> 
> B) Added a utility called pmdinfogen.  This utility is not meant for general use,
> but rather is used by the dpdk build environment itself when compiling pmds.
> for each object that uses the PMD_REGISTER_DRIVER macro, this utiity is run.  It
> searches for the symbols in (A), and using those, extracts the hardware support
> info, and module name from the object, using that to produce a new c file
> containing a single variable in the following format:
> 
> static const char <name>[] __attribute__((used)) = "PMD_DRIVER_INFO=<json>";
> 
> The <name> is arbitrary, as its static and not referenced.  The relevant bit is
> the string value assigned to it.  The <json> is a json encoded string of the
> extracted hardware support information pmdinfo found for the corresponding
> object.  This C file is suitable for compilation and relocatable linking back
> into the parent object file.  The result of this operation is that the object
> string table now contains a string that will not e removed by stripping, whos
> leading text (PMD_DRIVER_INFO) can be easily searched for at any time weather
> the symbol referring to it is stripped or not.
> 
> C) Added a utilty called pmdinfo.py.  This python script, searches the
> string table of the .rodata section of any provided ELF file looking for the
> PMD_DRIVER_INFO prefix on a string.  When found, it will interpret the remainder
> of the string as json, and output the hardware support for that ELF file (if
> any).
> 
> 
> This approach ticks most of the above boxes:
> 1) Impervious to stripping
> 2) Works on static and dynamic binaries
> 3) Human and script friendly
> 4) Allows for expansion
> 
> Because of (4) the other items should be pretty easy to implement, as its just a
> matter of modifying the macros to export the info, pmdinfo to encode it to json,
> and pmd_hw_support.py to read it.  I'm not adding them now, as theres alot of
> rote work to do to get some of them in place (e.g. DPDK has no current global
> VERSION macro, drivers don't have a consistent version scheme, command line
> strings have to be built for each driver, etc).  But once this is accepted,
> those items can be done as time allows and should be fairly easy to implement.
> 
> ----------------
> 
> Neil Horman (4):
>   drivers: export infos as string symbols
>   pmdinfogen: parse driver to generate code to export
>   eal: export default plugin path to external tools
>   tools: query binaries for support information
> 
> Thomas Monjalon (3):
>   mk: remove recipe for tool library
>   mk: refresh recipe for any host application
>   mk: link infos generated by pmdinfogen
> 
>  GNUmakefile                                    |   2 +-
>  MAINTAINERS                                    |   5 +
>  GNUmakefile => buildtools/Makefile             |  17 +-
>  GNUmakefile => buildtools/pmdinfogen/Makefile  |  21 +-
>  buildtools/pmdinfogen/pmdinfogen.c             | 451 ++++++++++++++++++
>  buildtools/pmdinfogen/pmdinfogen.h             |  99 ++++
>  doc/guides/freebsd_gsg/build_dpdk.rst          |   2 +-
>  doc/guides/linux_gsg/build_dpdk.rst            |   2 +-
>  doc/guides/prog_guide/dev_kit_build_system.rst |  38 +-
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c       |   4 +-
>  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c     |   4 +-
>  drivers/crypto/kasumi/rte_kasumi_pmd.c         |   4 +-
>  drivers/crypto/null/null_crypto_pmd.c          |   4 +-
>  drivers/crypto/qat/rte_qat_cryptodev.c         |   3 +-
>  drivers/crypto/snow3g/rte_snow3g_pmd.c         |   4 +-
>  drivers/net/af_packet/rte_eth_af_packet.c      |   4 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c               |   6 +-
>  drivers/net/bnxt/bnxt_ethdev.c                 |   3 +-
>  drivers/net/bonding/rte_eth_bond_pmd.c         |   5 +-
>  drivers/net/cxgbe/cxgbe_ethdev.c               |   3 +-
>  drivers/net/e1000/em_ethdev.c                  |   3 +-
>  drivers/net/e1000/igb_ethdev.c                 |   6 +-
>  drivers/net/ena/ena_ethdev.c                   |   3 +-
>  drivers/net/enic/enic_ethdev.c                 |   3 +-
>  drivers/net/fm10k/fm10k_ethdev.c               |   3 +-
>  drivers/net/i40e/i40e_ethdev.c                 |   3 +-
>  drivers/net/i40e/i40e_ethdev_vf.c              |   3 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c               |   6 +-
>  drivers/net/mlx4/mlx4.c                        |   3 +-
>  drivers/net/mlx5/mlx5.c                        |   3 +-
>  drivers/net/mpipe/mpipe_tilegx.c               |   4 +-
>  drivers/net/nfp/nfp_net.c                      |   3 +-
>  drivers/net/null/rte_eth_null.c                |   3 +-
>  drivers/net/pcap/rte_eth_pcap.c                |   4 +-
>  drivers/net/qede/qede_ethdev.c                 |   6 +-
>  drivers/net/ring/rte_eth_ring.c                |   3 +-
>  drivers/net/szedata2/rte_eth_szedata2.c        |   3 +-
>  drivers/net/thunderx/nicvf_ethdev.c            |   3 +-
>  drivers/net/vhost/rte_eth_vhost.c              |   3 +-
>  drivers/net/virtio/virtio_ethdev.c             |   3 +-
>  drivers/net/virtio/virtio_user_ethdev.c        |   2 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c           |   3 +-
>  drivers/net/xenvirt/rte_eth_xenvirt.c          |   2 +-
>  lib/librte_eal/common/eal_common_dev.c         |   2 +-
>  lib/librte_eal/common/eal_common_options.c     |   4 +
>  lib/librte_eal/common/include/rte_dev.h        |  30 +-
>  mk/internal/rte.compile-pre.mk                 |  12 +
>  mk/rte.hostapp.mk                              |  23 +-
>  mk/rte.hostlib.mk                              | 116 -----
>  mk/rte.sdkbuild.mk                             |   4 +-
>  mk/rte.sdkconfig.mk                            |   3 +-
>  mk/rte.sdkinstall.mk                           |   5 +
>  tools/dpdk-pmdinfo.py                          | 628 +++++++++++++++++++++++++
>  53 files changed, 1371 insertions(+), 215 deletions(-)
>  copy GNUmakefile => buildtools/Makefile (87%)
>  copy GNUmakefile => buildtools/pmdinfogen/Makefile (84%)
>  create mode 100644 buildtools/pmdinfogen/pmdinfogen.c
>  create mode 100644 buildtools/pmdinfogen/pmdinfogen.h
>  delete mode 100644 mk/rte.hostlib.mk
>  create mode 100755 tools/dpdk-pmdinfo.py
> 
> -- 
> 2.7.0
> 
>