From: Neil Horman <nhorman@tuxdriver.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
Stephen Hemminger <stephen@networkplumber.org>,
Panu Matilainen <pmatilai@redhat.com>
Subject: Re: [dpdk-dev] [PATCHv4 1/5] pmdinfogen: Add buildtools and pmdinfogen utility
Date: Wed, 25 May 2016 13:22:27 -0400 [thread overview]
Message-ID: <20160525172227.GB14322@hmsreliant.think-freely.org> (raw)
In-Reply-To: <12522566.xGRn81iLRB@xps13>
On Wed, May 25, 2016 at 03:21:19PM +0200, Thomas Monjalon wrote:
> 2016-05-24 15:41, Neil Horman:
> > pmdinfogen is a tool used to parse object files and build json strings for use in
> > later determining hardware support in a dso or application binary. pmdinfo
> > looks for the non-exported symbol names this_pmd_name<n> and this_pmd_tbl<n>
> > (where n is a integer counter). It records the name of each of these tuples,
> > using the later to find the symbolic name of the pci_table for physical devices
> > that the object supports. With this information, it outputs a C file with a
> > single line of the form:
> >
> > static char *<pmd_name>_driver_info[] __attribute__((used)) = " \
> > PMD_DRIVER_INFO=<json string>";
> >
> > Where <pmd_name> is the arbitrary name of the pmd, and <json_string> is the json
> > encoded string that hold relevant pmd information, including the pmd name, type
> > and optional array of pci device/vendor ids that the driver supports.
> >
> > This c file is suitable for compiling to object code, then relocatably linking
> > into the parent file from which the C was generated. This creates an entry in
> > the string table of the object that can inform a later tool about hardware
> > support.
>
> This description is helpful and should be in the doc:
> doc/guides/prog_guide/dev_kit_build_system.rst
Yeah, ok I can add that.
>
> > --- a/GNUmakefile
> > +++ b/GNUmakefile
> > -ROOTDIRS-y := lib drivers app
> > +ROOTDIRS-y := buildtools lib drivers app
>
> Why a new directory?
> It is not a script nor an end-user tool, I guess.
Dependencies. This tool has to be built prior to the rest of the dpdk, but app
already relies on dpdk libraries to be built, so you get circular dependencies.
I could have put it in scripts I guess, but its not a script. Its own directory
seemed to make the most sense, given those two points
>
> I feel strange to build an app for the build system.
Why? I agree its not overly common, but theres lots of precident for it.
The linux and bsd kernels obviously do this for modules, and there are lots of
tools that convert generic descriptions in xml into platform native source code
prior to compilation.
> For information, do you know some Python lib to do this kind of tool?
>
No, if there was I would have used it, but this sort of thing is project
specific, theres no 'generic' symbol stringification solution available.
> > +++ b/buildtools/pmdinfogen/Makefile
> > +#CFLAGS += $(WERROR_FLAGS) -g
> > +CFLAGS += -g
>
> Please choose one line or the other or none of them.
>
Oh, thats a debug error, I can fix that.
> > +include $(RTE_SDK)/mk/rte.buildtools.mk
>
> Why a new Makefile? Can you use rte.hostapp.mk?
>
I don't know, maybe. Nothing else currently uses rte.hostapp.mk, so I missed
its existance. I make the argument that, that being the case, we should stick
with the Makefile I just tested with, and remove the rte.hostapp.mk file
> > +++ b/buildtools/pmdinfogen/pmdinfogen.c
> [...]
> > + /*
> > + * If this returns NULL, then this is a PMD_VDEV, because
> > + * it has no pci table reference
> > + */
>
> We can imagine physical PMD not using PCI.
> I think this comment should be removed.
We can, but currently its a true statement. we have two types of PMDs, a PDEV
and a VDEV, the former is a pci device, and the latter is a virtual device, so
you can imply the PDEV type from the presence of pci entries, and VDEV from the
alternative. If we were to do something, I would recommend adding a macro to
explicitly ennumerate each pmds type. I would prefer to wait until that was a
need however, as it can be done invisibly to the user.
>
> > + if (!tmpsym) {
> > + drv->pci_tbl = NULL;
> > + return 0;
> > + }
> [...]
> > +
> > +
> > + return 0;
> > +
> > +}
>
> That's a lot of blank lines ;)
>
My eyes were getting tired :)
> [...]
> > + fprintf(ofd,"\\\"type\\\" : \\\"%s\\\", ", drv->pci_tbl ? "PMD_PDEV" : "PMD_VDEV");
>
> Please forget the naming PDEV/VDEV.
>
I don't know what you mean here, you would rather they be named PCI and Virtual,
or something else?
> [...]
> > + if (info.drivers) {
> > + output_pmd_info_string(&info, argv[2]);
> > + rc = 0;
> > + } else {
> > + fprintf(stderr, "Hmm, Appears to be a driver but no drivers registered\n");
>
> Why it appears to be a driver?
> What means "no drivers registered" exactly?
>
It means that the tool has identified this file as a driver based on some
criteria (in this case the source code contained a use of the
PMD_REGISTER_DRIVER macro, but for whatever reason, when this tool scanned it,
it never located the pmd_driver_name<n> symbol. It should never happen, and
serves as a indicator to the developer that they need to investigate either the
construction of the driver or the use of this tool.
> > +++ b/buildtools/pmdinfogen/pmdinfogen.h
> [...]
> > +#define Elf_Ehdr Elf64_Ehdr
> > +#define Elf_Shdr Elf64_Shdr
> > +#define Elf_Sym Elf64_Sym
> > +#define Elf_Addr Elf64_Addr
> > +#define Elf_Sword Elf64_Sxword
> > +#define Elf_Section Elf64_Half
> > +#define ELF_ST_BIND ELF64_ST_BIND
> > +#define ELF_ST_TYPE ELF64_ST_TYPE
> > +
> > +#define Elf_Rel Elf64_Rel
> > +#define Elf_Rela Elf64_Rela
> > +#define ELF_R_SYM ELF64_R_SYM
> > +#define ELF_R_TYPE ELF64_R_TYPE
>
> Why these defines are needed?
>
Because I borrowed the code from modpost.c, which allows for both ELF32 and
ELF64 compilation. I wanted to keep it in place should DPDK ever target
different sized architectures.
> > +#define TO_NATIVE(x) (x)
>
> Nice :) Why?
>
Because there is more than intel in the world :). For alternate endian
machines, this can easily be redefined to accomodate that.
> > +struct rte_pci_id {
> > + uint16_t vendor_id; /**< Vendor ID or PCI_ANY_ID. */
> > + uint16_t device_id; /**< Device ID or PCI_ANY_ID. */
> > + uint16_t subsystem_vendor_id; /**< Subsystem vendor ID or PCI_ANY_ID. */
> > + uint16_t subsystem_device_id; /**< Subsystem device ID or PCI_ANY_ID. */
> > +};
> [...]
> > +struct pmd_driver {
> > + Elf_Sym *name_sym;
> > + const char *name;
> > + struct rte_pci_id *pci_tbl;
> > + struct pmd_driver *next;
> > +
> > + const char* opt_vals[PMD_OPT_MAX];
> > +};
>
> Are you duplicating some structures from EAL?
> It will be out of sync easily.
>
Only the rte_pci_id, which hasn't changed since the initial public release of
the DPDK. We can clean this up later if you like, but I'm really not too
worried about it.
> > +struct elf_info {
> > + unsigned long size;
> > + Elf_Ehdr *hdr;
> > + Elf_Shdr *sechdrs;
> > + Elf_Sym *symtab_start;
> > + Elf_Sym *symtab_stop;
> > + Elf_Section export_sec;
> > + Elf_Section export_unused_sec;
> > + Elf_Section export_gpl_sec;
> > + Elf_Section export_unused_gpl_sec;
> > + Elf_Section export_gpl_future_sec;
> > + char *strtab;
> > + char *modinfo;
> > + unsigned int modinfo_len;
>
> Why these fields?
>
Because thats how you parse an ELF file and look up the information you need to
extract the data this tool then exports. I don't mean to sound short, but your
question doesn't really make any sense. The members of this structure are
needed to extract the info from object files to build the export strings that
pmdinfo.py needs later.
> > +++ b/mk/rte.buildtools.mk
>
> This file must be removed I think.
> We are going to be sick after digesting so much makefiles ;)
>
See above, given that I just tested this, and rte.hostapp.mk isn't used, I'd
recommend deleting the latter, rather than deleting this one and moving to the
old one.
> Last comment,
> The MAINTAINERS file must be updated for this tool.
>
Yeah, I can do that.
> Thanks for taking care of tooling.
>
Yup
next prev parent reply other threads:[~2016-05-25 17:22 UTC|newest]
Thread overview: 166+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-16 20:41 [dpdk-dev] [PATCH 0/4] Implement pmd hardware support exports Neil Horman
2016-05-16 20:41 ` [dpdk-dev] [PATCH 1/4] pmdinfo: Add buildtools and pmdinfo utility Neil Horman
2016-05-16 20:41 ` [dpdk-dev] [PATCH 2/4] drivers: Update driver registration macro usage Neil Horman
2016-05-16 20:41 ` [dpdk-dev] [PATCH 3/4] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-16 20:41 ` [dpdk-dev] [PATCH 4/4] pmd_hw_support.py: Add tool to query binaries for hw support information Neil Horman
2016-05-18 11:48 ` Panu Matilainen
2016-05-18 12:03 ` Neil Horman
2016-05-18 12:48 ` Panu Matilainen
2016-05-18 13:48 ` Neil Horman
2016-05-19 6:08 ` Panu Matilainen
2016-05-19 13:26 ` Neil Horman
2016-05-20 7:30 ` Panu Matilainen
2016-05-20 14:06 ` Neil Horman
2016-05-23 11:56 ` Panu Matilainen
2016-05-23 13:55 ` Neil Horman
2016-05-24 6:15 ` Panu Matilainen
2016-05-24 14:55 ` Neil Horman
2016-05-18 12:38 ` Thomas Monjalon
2016-05-18 13:09 ` Panu Matilainen
2016-05-18 13:26 ` Thomas Monjalon
2016-05-18 13:54 ` Neil Horman
2016-05-18 21:08 ` [dpdk-dev] [PATCHv2 0/4] Implement pmd hardware support exports Neil Horman
2016-05-18 21:08 ` [dpdk-dev] [PATCHv2 1/4] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-05-19 7:51 ` Panu Matilainen
2016-05-19 12:00 ` Neil Horman
2016-05-18 21:08 ` [dpdk-dev] [PATCHv2 2/4] drivers: Update driver registration macro usage Neil Horman
2016-05-19 7:58 ` Panu Matilainen
2016-05-19 10:45 ` Neil Horman
2016-05-19 10:51 ` [dpdk-dev] [dpdk-dev, PATCHv2, " Jan Viktorin
[not found] ` <20160519124650.060aa09a@pcviktorin.fit.vutbr.cz>
2016-05-19 11:40 ` Neil Horman
2016-05-18 21:08 ` [dpdk-dev] [PATCHv2 3/4] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-18 21:08 ` [dpdk-dev] [PATCHv2 4/4] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-05-19 9:02 ` Panu Matilainen
2016-05-19 12:00 ` Neil Horman
2016-05-20 5:22 ` Panu Matilainen
2016-05-20 8:55 ` Thomas Monjalon
2016-05-20 9:12 ` Panu Matilainen
2016-05-20 14:22 ` Neil Horman
2016-05-20 14:20 ` Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 0/5] Implement pmd hardware support exports Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 1/5] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 2/5] drivers: Update driver registration macro usage Neil Horman
2016-05-24 6:57 ` Panu Matilainen
2016-05-24 13:21 ` Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 3/5] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 4/5] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-20 17:24 ` [dpdk-dev] [PATCHv3 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-05-24 7:41 ` Panu Matilainen
2016-05-24 15:17 ` Neil Horman
2016-05-24 8:34 ` Panu Matilainen
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 0/5] Implement pmd hardware support exports Neil Horman
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 1/5] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-05-25 13:21 ` Thomas Monjalon
2016-05-25 17:22 ` Neil Horman [this message]
2016-05-25 17:39 ` Thomas Monjalon
2016-05-25 19:13 ` Neil Horman
2016-05-25 19:39 ` Thomas Monjalon
2016-05-25 19:57 ` Neil Horman
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 2/5] drivers: Update driver registration macro usage Neil Horman
2016-05-25 16:20 ` Thomas Monjalon
2016-05-25 17:35 ` Neil Horman
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 3/5] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 4/5] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-25 17:08 ` Thomas Monjalon
2016-05-25 17:40 ` Neil Horman
2016-05-25 18:56 ` Thomas Monjalon
2016-05-25 19:43 ` Neil Horman
2016-05-25 20:04 ` Thomas Monjalon
2016-05-25 20:16 ` Neil Horman
2016-05-24 19:41 ` [dpdk-dev] [PATCHv4 5/5] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-05-25 17:22 ` Thomas Monjalon
2016-05-25 17:47 ` Neil Horman
2016-05-25 18:58 ` Thomas Monjalon
2016-05-27 9:16 ` Panu Matilainen
2016-05-27 11:35 ` Neil Horman
2016-05-27 12:46 ` Panu Matilainen
2016-05-25 8:32 ` [dpdk-dev] [PATCHv4 0/5] Implement pmd hardware support exports Panu Matilainen
2016-05-25 11:27 ` Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 0/6] " Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 1/6] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 2/6] drivers: Update driver registration macro usage Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 3/6] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 4/6] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-05-27 14:38 ` Mcnamara, John
2016-05-27 19:56 ` Neil Horman
2016-05-26 17:17 ` [dpdk-dev] [PATCHv5 6/6] remove rte.hostapp.mk Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 0/7] Implement pmd hardware support exports Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 1/7] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-06-07 9:57 ` Thomas Monjalon
2016-06-07 12:04 ` Neil Horman
2016-06-07 12:53 ` Thomas Monjalon
2016-06-07 13:03 ` Neil Horman
2016-06-07 13:24 ` Thomas Monjalon
2016-06-07 13:49 ` Neil Horman
2016-06-07 14:09 ` Thomas Monjalon
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 2/7] drivers: Update driver registration macro usage Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 3/7] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 4/7] Makefile: Do post processing on objects that register a driver Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 5/7] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 6/7] remove rte.hostapp.mk Neil Horman
2016-05-31 13:57 ` [dpdk-dev] [PATCHv6 7/7] doc: Add prog_guide section documenting pmdinfo script Neil Horman
2016-06-08 17:14 ` Mcnamara, John
2016-06-09 17:31 ` Neil Horman
2016-06-05 0:20 ` [dpdk-dev] [PATCHv6 0/7] Implement pmd hardware support exports Neil Horman
2016-06-07 9:34 ` Thomas Monjalon
2016-06-07 12:08 ` Neil Horman
2016-06-07 12:27 ` Thomas Monjalon
2016-06-09 17:46 ` [dpdk-dev] [PATCHv7 0/6] " Neil Horman
2016-06-09 17:46 ` [dpdk-dev] [PATCHv7 1/6] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-06-16 12:29 ` Panu Matilainen
2016-06-16 13:33 ` Neil Horman
2016-06-16 14:06 ` Panu Matilainen
2016-06-16 14:41 ` Neil Horman
2016-06-09 17:46 ` [dpdk-dev] [PATCHv7 2/6] drivers: Update driver registration macro usage Neil Horman
2016-06-09 17:46 ` [dpdk-dev] [PATCHv7 3/6] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-06-09 17:46 ` [dpdk-dev] [PATCHv7 4/6] Makefile: Do post processing on objects that register a driver Neil Horman
2016-06-09 17:47 ` [dpdk-dev] [PATCHv7 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-06-16 12:32 ` Panu Matilainen
2016-06-09 17:47 ` [dpdk-dev] [PATCHv7 6/6] doc: Add prog_guide section documenting pmdinfo script Neil Horman
2016-06-09 19:55 ` Mcnamara, John
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 0/6] Implement pmd hardware support exports Neil Horman
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 1/6] pmdinfogen: Add buildtools and pmdinfogen utility Neil Horman
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 2/6] drivers: Update driver registration macro usage Neil Horman
2016-07-07 11:00 ` De Lara Guarch, Pablo
2016-07-07 11:39 ` Neil Horman
2016-07-07 15:37 ` [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros Neil Horman
2016-07-08 9:09 ` De Lara Guarch, Pablo
2016-07-08 12:17 ` Neil Horman
2016-07-08 12:40 ` Thomas Monjalon
2016-07-08 13:42 ` Neil Horman
2016-07-08 19:03 ` Mcnamara, John
2016-07-09 13:34 ` Neil Horman
2016-07-09 16:04 ` Mcnamara, John
2016-07-08 14:00 ` De Lara Guarch, Pablo
2016-07-08 14:14 ` Neil Horman
2016-07-08 10:03 ` Thomas Monjalon
2016-07-08 16:34 ` [dpdk-dev] [PATCH v2] " Pablo de Lara
2016-07-08 16:46 ` [dpdk-dev] [PATCH v3] " Pablo de Lara
2016-07-08 17:14 ` Thomas Monjalon
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 3/6] eal: Add an export symbol to expose the autoload path to external tools Neil Horman
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 4/6] Makefile: Do post processing on objects that register a driver Neil Horman
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 5/6] pmdinfo.py: Add tool to query binaries for hw and other support information Neil Horman
2016-06-29 15:12 ` Remy Horton
2016-06-29 16:18 ` Neil Horman
2016-06-30 7:45 ` Remy Horton
2016-06-17 18:46 ` [dpdk-dev] [PATCHv8 6/6] doc: Add prog_guide section documenting pmdinfo script Neil Horman
2016-06-29 9:18 ` [dpdk-dev] [PATCHv8 0/6] Implement pmd hardware support exports Remy Horton
2016-06-29 11:31 ` Neil Horman
2016-06-30 7:45 ` Remy Horton
2016-07-06 21:21 ` Thomas Monjalon
2016-07-04 1:13 ` [dpdk-dev] [PATCH v9 0/7] export PMD infos Thomas Monjalon
2016-07-04 1:13 ` [dpdk-dev] [PATCH v9 1/7] drivers: export infos as string symbols Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 2/7] mk: remove recipe for tool library Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 3/7] mk: refresh recipe for any host application Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 4/7] pmdinfogen: parse driver to generate code to export Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 5/7] mk: link infos generated by pmdinfogen Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 6/7] eal: export default plugin path to external tools Thomas Monjalon
2016-07-04 1:14 ` [dpdk-dev] [PATCH v9 7/7] tools: query binaries for support information Thomas Monjalon
2016-07-04 12:34 ` [dpdk-dev] [PATCH v9 0/7] export PMD infos Neil Horman
2016-07-04 13:10 ` Thomas Monjalon
2016-07-04 16:41 ` Neil Horman
2016-07-04 20:10 ` Thomas Monjalon
2016-07-05 20:08 ` Neil Horman
2016-07-06 15:33 ` Thomas Monjalon
2016-07-04 15:22 ` Bruce Richardson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160525172227.GB14322@hmsreliant.think-freely.org \
--to=nhorman@tuxdriver.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=pmatilai@redhat.com \
--cc=stephen@networkplumber.org \
--cc=thomas.monjalon@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).