[dpdk-dev,PATCHv4,1/5] pmdinfogen: Add buildtools and pmdinfogen utility

Message ID 1464118912-19658-2-git-send-email-nhorman@tuxdriver.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Neil Horman May 24, 2016, 7:41 p.m. UTC
  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.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Bruce Richardson <bruce.richardson@intel.com>
CC: Thomas Monjalon <thomas.monjalon@6wind.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: Panu Matilainen <pmatilai@redhat.com>
---
 GNUmakefile                        |   2 +-
 buildtools/Makefile                |  36 ++++
 buildtools/pmdinfogen/Makefile     |  48 +++++
 buildtools/pmdinfogen/pmdinfogen.c | 413 +++++++++++++++++++++++++++++++++++++
 buildtools/pmdinfogen/pmdinfogen.h |  83 ++++++++
 mk/rte.buildtools.mk               | 148 +++++++++++++
 mk/rte.sdkbuild.mk                 |   3 +-
 7 files changed, 731 insertions(+), 2 deletions(-)
 create mode 100644 buildtools/Makefile
 create mode 100644 buildtools/pmdinfogen/Makefile
 create mode 100644 buildtools/pmdinfogen/pmdinfogen.c
 create mode 100644 buildtools/pmdinfogen/pmdinfogen.h
 create mode 100644 mk/rte.buildtools.mk
  

Comments

Thomas Monjalon May 25, 2016, 1:21 p.m. UTC | #1
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

> --- 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.

I feel strange to build an app for the build system.
For information, do you know some Python lib to do this kind of tool?

> +++ b/buildtools/pmdinfogen/Makefile
> +#CFLAGS += $(WERROR_FLAGS) -g
> +CFLAGS += -g

Please choose one line or the other or none of them.

> +include $(RTE_SDK)/mk/rte.buildtools.mk

Why a new Makefile? Can you use rte.hostapp.mk?

> +++ 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.

> +	if (!tmpsym) {
> +		drv->pci_tbl = NULL;
> +		return 0;
> +	}
[...]
> +
> +
> +	return 0;
> +	
> +}

That's a lot of blank lines ;)

[...]
> +		fprintf(ofd,"\\\"type\\\" : \\\"%s\\\", ", drv->pci_tbl ? "PMD_PDEV" : "PMD_VDEV");

Please forget the naming PDEV/VDEV.

[...]
> +	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?

> +++ 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?

> +#define TO_NATIVE(x) (x)

Nice :) Why?

> +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.

> +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?

> +++ b/mk/rte.buildtools.mk

This file must be removed I think.
We are going to be sick after digesting so much makefiles ;)

Last comment,
The MAINTAINERS file must be updated for this tool.

Thanks for taking care of tooling.
  
Neil Horman May 25, 2016, 5:22 p.m. UTC | #2
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
  
Thomas Monjalon May 25, 2016, 5:39 p.m. UTC | #3
2016-05-25 13:22, Neil Horman:
> On Wed, May 25, 2016 at 03:21:19PM +0200, Thomas Monjalon wrote:
> > 2016-05-24 15:41, Neil Horman:
> > > --- 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

OK

> > > +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

No, rte.hostapp.mk has been used and tested in the history of the project.
Please try it.

> > > +++ 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.

We are removing the PMD types in the EAL rework.
So this comment will be outdated. Better to remove now.

> > [...]
> > > +		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?

Yes please.

> > [...]
> > > +	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.

OK

> > > +++ 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.

Maybe a comment is needed.
Is ELF32 used on 32-bit archs like i686 or ARMv7?

> > > +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.

I would prefer an include if possible.
rte_pci_id is changing in 16.07 ;)

> > > +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.

Sorry, I haven't parse the whole code but some fields are unused.
And modinfo looks wrong.

> > > +++ 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.

See above, I do not agree :)
  
Neil Horman May 25, 2016, 7:13 p.m. UTC | #4
On Wed, May 25, 2016 at 07:39:30PM +0200, Thomas Monjalon wrote:
> 2016-05-25 13:22, Neil Horman:
> > On Wed, May 25, 2016 at 03:21:19PM +0200, Thomas Monjalon wrote:
> > > 2016-05-24 15:41, Neil Horman:
> > > > --- 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
> 
> OK
> 
> > > > +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
> 
> No, rte.hostapp.mk has been used and tested in the history of the project.
> Please try it.
> 
It works, but its really ugly (as it means that the buildtools directory gets
install to the hostapp directory under the build).  I could move that of course,
but at this point, you are asking me to remove a working makefile to replace it
with another makefile that, by all rights should have been removed as part of
commit efa2084a840fb83fd9be83adca57e5f23d3fa9fe:
Author: Thomas Monjalon <thomas.monjalon@6wind.com>
Date:   Tue Mar 10 17:55:25 2015 +0100

    scripts: remove useless build tools
    
    test-framework.sh is an old script to check building of some dependencies.
    testhost is an old app used to check HOSTCC.
    
    Let's clean the scripts directory.

Here you removed the only user of rte.hostapp.mk, but neglected to remove
hostapp.mk itself.  I really fail to see why making me rework my current
makefile setup, that matches the purpose of the tool is a superior solution to
just getting rid of the unused makefile thats there right now.

> > > > +++ 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.
> 
> We are removing the PMD types in the EAL rework.
> So this comment will be outdated. Better to remove now.
> 
Then, I'm just not going to enumerate the type of driver at all, I'll remove
that attribute entirely.  But I really don't like to write code for things that
are 'predictive'.

> > > [...]
> > > > +		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?
> 
> Yes please.
> 
No, If you're removing the types, and you're sure of that, I'm just going to
remove the description entirely.  If you're unsure about exactly whats going to
happen, we should reflect the state of the build now, and make the appropriate
change when it lands.


> > > [...]
> > > > +	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.
> 
> OK
> 
> > > > +++ 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.
> 
> Maybe a comment is needed.
> Is ELF32 used on 32-bit archs like i686 or ARMv7?
It depends on exactly how its built, but that would be a common use, yes.

> 
> > > > +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.
> 
> I would prefer an include if possible.
> rte_pci_id is changing in 16.07 ;)
> 
So, we've had this discussion before :).  Its really not fair to ask anyone to
write code based on predictive changes.  If theres some patch out there thats
planning on making a change, we can't be expected to write with it in mind.  If
you want people to use it, then get it merged.  I understand thats not really
the issue here, and I'm making the change because you're right, we should avoid
duplicating the structures if we can, but please understand that its impossible
to write for change thats predicted to come at a later date.

> > > > +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.
> 
> Sorry, I haven't parse the whole code but some fields are unused.
> And modinfo looks wrong.
> 
Yup, your right, modifo did get orphaned when I was doing previous cleanup, and
can be removed.

> > > > +++ 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.
> 
> See above, I do not agree :)
> 
Then we're not going to agree about this :).  I'll re-iterate my stance.  Moving to
use rte.hotapp.mk, causes alot more work for me, makes the use of the tool
somewhat uglier, and by all rights shouldn't be there at all, due to your
previously mentioned commit. It just makes more sense to use the buildtools
makefile and remove the vesitgial rte.hostapp.mk makefile.

Neil

>
  
Thomas Monjalon May 25, 2016, 7:39 p.m. UTC | #5
2016-05-25 15:13, Neil Horman:
> On Wed, May 25, 2016 at 07:39:30PM +0200, Thomas Monjalon wrote:
> > 2016-05-25 13:22, Neil Horman:
> > > On Wed, May 25, 2016 at 03:21:19PM +0200, Thomas Monjalon wrote:
> > > > 2016-05-24 15:41, Neil Horman:
> > > > > +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
> > 
> > No, rte.hostapp.mk has been used and tested in the history of the project.
> > Please try it.
> > 
> It works, but its really ugly (as it means that the buildtools directory gets
> install to the hostapp directory under the build).  I could move that of course,
> but at this point, you are asking me to remove a working makefile to replace it
> with another makefile that, by all rights should have been removed as part of
> commit efa2084a840fb83fd9be83adca57e5f23d3fa9fe:
> Author: Thomas Monjalon <thomas.monjalon@6wind.com>
> Date:   Tue Mar 10 17:55:25 2015 +0100
> 
>     scripts: remove useless build tools
>     
>     test-framework.sh is an old script to check building of some dependencies.
>     testhost is an old app used to check HOSTCC.
>     
>     Let's clean the scripts directory.
> 
> Here you removed the only user of rte.hostapp.mk, but neglected to remove
> hostapp.mk itself.

Yes. I didn't really neglect to remove it. I thought it would be used later.

> I really fail to see why making me rework my current
> makefile setup, that matches the purpose of the tool is a superior solution to
> just getting rid of the unused makefile thats there right now.

I'm just trying to avoid creating a new makefile for each tool.
Is it possible to fix the directory in rte.hostapp.mk?
Every apps use the same makefile rte.app.mk. I think it should be the same
for host apps.

> > > > > +++ 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.
> > 
> > We are removing the PMD types in the EAL rework.
> > So this comment will be outdated. Better to remove now.
> > 
> Then, I'm just not going to enumerate the type of driver at all, I'll remove
> that attribute entirely.

OK

> But I really don't like to write code for things that are 'predictive'.

Not really predictive as it is an older patch.

> > > > [...]
> > > > > +		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?
> > 
> > Yes please.
> > 
> No, If you're removing the types, and you're sure of that, I'm just going to
> remove the description entirely.  If you're unsure about exactly whats going to
> happen, we should reflect the state of the build now, and make the appropriate
> change when it lands.

OK to remove the type description.

> > > > > +++ 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.
> > 
> > Maybe a comment is needed.
> > Is ELF32 used on 32-bit archs like i686 or ARMv7?
> It depends on exactly how its built, but that would be a common use, yes.

We have such 32-bit archs in DPDK. Is pmdinfogen working for them?

> > > > > +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.
> > 
> > I would prefer an include if possible.
> > rte_pci_id is changing in 16.07 ;)
> > 
> So, we've had this discussion before :).  Its really not fair to ask anyone to
> write code based on predictive changes.  If theres some patch out there thats
> planning on making a change, we can't be expected to write with it in mind.  If
> you want people to use it, then get it merged.  I understand thats not really
> the issue here, and I'm making the change because you're right, we should avoid
> duplicating the structures if we can, but please understand that its impossible
> to write for change thats predicted to come at a later date.

I understand your point.
The rte_pci_id change has been reviewed several times already and should be
applied very soon.

> > > > > +++ 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.
> > 
> > See above, I do not agree :)
> > 
> Then we're not going to agree about this :).  I'll re-iterate my stance.  Moving to
> use rte.hotapp.mk, causes alot more work for me, makes the use of the tool
> somewhat uglier, and by all rights shouldn't be there at all, due to your
> previously mentioned commit. It just makes more sense to use the buildtools
> makefile and remove the vesitgial rte.hostapp.mk makefile.
  
Neil Horman May 25, 2016, 7:57 p.m. UTC | #6
On Wed, May 25, 2016 at 09:39:43PM +0200, Thomas Monjalon wrote:
> 2016-05-25 15:13, Neil Horman:
> > On Wed, May 25, 2016 at 07:39:30PM +0200, Thomas Monjalon wrote:
> > > 2016-05-25 13:22, Neil Horman:
> > > > On Wed, May 25, 2016 at 03:21:19PM +0200, Thomas Monjalon wrote:
> > > > > 2016-05-24 15:41, Neil Horman:
> > > > > > +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
> > > 
> > > No, rte.hostapp.mk has been used and tested in the history of the project.
> > > Please try it.
> > > 
> > It works, but its really ugly (as it means that the buildtools directory gets
> > install to the hostapp directory under the build).  I could move that of course,
> > but at this point, you are asking me to remove a working makefile to replace it
> > with another makefile that, by all rights should have been removed as part of
> > commit efa2084a840fb83fd9be83adca57e5f23d3fa9fe:
> > Author: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Date:   Tue Mar 10 17:55:25 2015 +0100
> > 
> >     scripts: remove useless build tools
> >     
> >     test-framework.sh is an old script to check building of some dependencies.
> >     testhost is an old app used to check HOSTCC.
> >     
> >     Let's clean the scripts directory.
> > 
> > Here you removed the only user of rte.hostapp.mk, but neglected to remove
> > hostapp.mk itself.
> 
> Yes. I didn't really neglect to remove it. I thought it would be used later.
> 
Ok, thats fair.

> > I really fail to see why making me rework my current
> > makefile setup, that matches the purpose of the tool is a superior solution to
> > just getting rid of the unused makefile thats there right now.
> 
> I'm just trying to avoid creating a new makefile for each tool.
> Is it possible to fix the directory in rte.hostapp.mk?
> Every apps use the same makefile rte.app.mk. I think it should be the same
> for host apps.
> 
Yes, I could do that, I could fix up the directory path in rte.hostapp.mk so
that it installs to buildtools rather than hostapp, and that would be fine.  But
then if I were to additionally issue this command:
git mv mk/rte.hostapp.mk mk/rte.buildtools.mk

We would have exactly what I'm proposing anyway.  

I don't disagree that rte.buildtools.mk and rte.hostapp.mk are simmilar, they
are in fact almost identical, and I simply missed the latter because I didn't
see any uses of it.  What I am saying is that, due to their simmilarity, Its
pretty much an equivalent situation to use either makefile, and its less work
for me to remove hostapp.mk and just use what I have.

> > > > > > +++ 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.
> > > 
> > > We are removing the PMD types in the EAL rework.
> > > So this comment will be outdated. Better to remove now.
> > > 
> > Then, I'm just not going to enumerate the type of driver at all, I'll remove
> > that attribute entirely.
> 
> OK
> 
> > But I really don't like to write code for things that are 'predictive'.
> 
> Not really predictive as it is an older patch.
And how many older patches never get integrated?  Or languish for long periods
of time?  We've debated this before.

Its really not reasonable to expect developers (myself or others) to
go through the mailing list and create an ordinal list of patches to apply
before doing our development work.  If that were the case, then they should just
be applied immediately so the HEAD of the git tree is an accurate representation
of the development state of the tree.  But thats not the case, and patches don't
always get applied in the order that they are posted.  So, if Davids Patch
series goes in ahead of mine, I'll gladly rebase, but I don't want to create
some artificial ordinality just because we touch the same code, especially if
his patch series has to go back for further revision.

> 
> > > > > [...]
> > > > > > +		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?
> > > 
> > > Yes please.
> > > 
> > No, If you're removing the types, and you're sure of that, I'm just going to
> > remove the description entirely.  If you're unsure about exactly whats going to
> > happen, we should reflect the state of the build now, and make the appropriate
> > change when it lands.
> 
> OK to remove the type description.
> 
Ok, I'll do that.

> > > > > > +++ 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.
> > > 
> > > Maybe a comment is needed.
> > > Is ELF32 used on 32-bit archs like i686 or ARMv7?
> > It depends on exactly how its built, but that would be a common use, yes.
> 
> We have such 32-bit archs in DPDK. Is pmdinfogen working for them?
> 
It was a few revisions ago, but I've changed so much now, I should probably
check again.

> > > > > > +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.
> > > 
> > > I would prefer an include if possible.
> > > rte_pci_id is changing in 16.07 ;)
> > > 
> > So, we've had this discussion before :).  Its really not fair to ask anyone to
> > write code based on predictive changes.  If theres some patch out there thats
> > planning on making a change, we can't be expected to write with it in mind.  If
> > you want people to use it, then get it merged.  I understand thats not really
> > the issue here, and I'm making the change because you're right, we should avoid
> > duplicating the structures if we can, but please understand that its impossible
> > to write for change thats predicted to come at a later date.
> 
> I understand your point.
> The rte_pci_id change has been reviewed several times already and should be
> applied very soon.
> 
Ok, As I said in my prior note, I'm making this change, because its sane to do
in and of itself, but again, if its not in HEAD, it really doesn't exist yet
from a development standpoint.

> > > > > > +++ 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.
> > > 
> > > See above, I do not agree :)
> > > 
> > Then we're not going to agree about this :).  I'll re-iterate my stance.  Moving to
> > use rte.hotapp.mk, causes alot more work for me, makes the use of the tool
> > somewhat uglier, and by all rights shouldn't be there at all, due to your
> > previously mentioned commit. It just makes more sense to use the buildtools
> > makefile and remove the vesitgial rte.hostapp.mk makefile.
> 
>
  

Patch

diff --git a/GNUmakefile b/GNUmakefile
index b59e4b6..00fe0db 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -40,6 +40,6 @@  export RTE_SDK
 # directory list
 #
 
-ROOTDIRS-y := lib drivers app
+ROOTDIRS-y := buildtools lib drivers app
 
 include $(RTE_SDK)/mk/rte.sdkroot.mk
diff --git a/buildtools/Makefile b/buildtools/Makefile
new file mode 100644
index 0000000..eb565eb
--- /dev/null
+++ b/buildtools/Makefile
@@ -0,0 +1,36 @@ 
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+DIRS-y += pmdinfogen
+
+include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/buildtools/pmdinfogen/Makefile b/buildtools/pmdinfogen/Makefile
new file mode 100644
index 0000000..4de9506
--- /dev/null
+++ b/buildtools/pmdinfogen/Makefile
@@ -0,0 +1,48 @@ 
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+APP = pmdinfogen
+
+#
+# all sources are stored in SRCS-y
+#
+SRCS-y += pmdinfogen.c
+
+#CFLAGS += $(WERROR_FLAGS) -g
+CFLAGS += -g
+
+include $(RTE_SDK)/mk/rte.buildtools.mk
+
diff --git a/buildtools/pmdinfogen/pmdinfogen.c b/buildtools/pmdinfogen/pmdinfogen.c
new file mode 100644
index 0000000..af0b2df
--- /dev/null
+++ b/buildtools/pmdinfogen/pmdinfogen.c
@@ -0,0 +1,413 @@ 
+/* Postprocess pmd object files to export hw support 
+ *
+ * Copyright 2016 Neil Horman <nhorman@tuxdriver.com>
+ * Based in part on modpost.c from the linux kernel
+ *
+ * This software may be used and distributed according to the terms
+ * of the GNU General Public License V2, incorporated herein by reference.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <ctype.h>
+#include <string.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <errno.h>
+#include "pmdinfogen.h"
+
+
+static const char *sym_name(struct elf_info *elf, Elf_Sym *sym)
+{
+	if (sym)
+		return elf->strtab + sym->st_name;
+	else
+		return "(unknown)";
+}
+
+void *grab_file(const char *filename, unsigned long *size)
+{
+	struct stat st;
+	void *map = MAP_FAILED;
+	int fd;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+	if (fstat(fd, &st))
+		goto failed;
+
+	*size = st.st_size;
+	map = mmap(NULL, *size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);
+
+failed:
+	close(fd);
+	if (map == MAP_FAILED)
+		return NULL;
+	return map;
+}
+
+/**
+  * Return a copy of the next line in a mmap'ed file.
+  * spaces in the beginning of the line is trimmed away.
+  * Return a pointer to a static buffer.
+  **/
+void release_file(void *file, unsigned long size)
+{
+	munmap(file, size);
+}
+
+
+static void *get_sym_value(struct elf_info *info, const Elf_Sym *sym)
+{
+	void *ptr = (void *)info->hdr + info->sechdrs[sym->st_shndx].sh_offset;
+
+	return (void *)(ptr + sym->st_value);
+}
+
+static Elf_Sym *find_sym_in_symtab(struct elf_info *info, 
+				   const char *name, Elf_Sym *last)
+{
+	Elf_Sym *idx;
+	if (last)
+		idx = last+1;
+	else
+		idx = info->symtab_start;
+
+	for(; idx < info->symtab_stop; idx++) {
+		const char *n = sym_name(info, idx);
+		if (!strncmp(n, name, strlen(name)))
+			return idx;
+	}
+	return NULL;
+}
+
+static int parse_elf(struct elf_info *info, const char *filename)
+{
+	unsigned int i;
+	Elf_Ehdr *hdr;
+	Elf_Shdr *sechdrs;
+	Elf_Sym  *sym;
+	const char *secstrings;
+	unsigned int symtab_idx = ~0U, symtab_shndx_idx = ~0U;
+
+	hdr = grab_file(filename, &info->size);
+	if (!hdr) {
+		perror(filename);
+		exit(1);
+	}
+	info->hdr = hdr;
+	if (info->size < sizeof(*hdr)) {
+		/* file too small, assume this is an empty .o file */
+		return 0;
+	}
+	/* Is this a valid ELF file? */
+	if ((hdr->e_ident[EI_MAG0] != ELFMAG0) ||
+	    (hdr->e_ident[EI_MAG1] != ELFMAG1) ||
+	    (hdr->e_ident[EI_MAG2] != ELFMAG2) ||
+	    (hdr->e_ident[EI_MAG3] != ELFMAG3)) {
+		/* Not an ELF file - silently ignore it */
+		return 0;
+	}
+	/* Fix endianness in ELF header */
+	hdr->e_type      = TO_NATIVE(hdr->e_type);
+	hdr->e_machine   = TO_NATIVE(hdr->e_machine);
+	hdr->e_version   = TO_NATIVE(hdr->e_version);
+	hdr->e_entry     = TO_NATIVE(hdr->e_entry);
+	hdr->e_phoff     = TO_NATIVE(hdr->e_phoff);
+	hdr->e_shoff     = TO_NATIVE(hdr->e_shoff);
+	hdr->e_flags     = TO_NATIVE(hdr->e_flags);
+	hdr->e_ehsize    = TO_NATIVE(hdr->e_ehsize);
+	hdr->e_phentsize = TO_NATIVE(hdr->e_phentsize);
+	hdr->e_phnum     = TO_NATIVE(hdr->e_phnum);
+	hdr->e_shentsize = TO_NATIVE(hdr->e_shentsize);
+	hdr->e_shnum     = TO_NATIVE(hdr->e_shnum);
+	hdr->e_shstrndx  = TO_NATIVE(hdr->e_shstrndx);
+	sechdrs = (void *)hdr + hdr->e_shoff;
+	info->sechdrs = sechdrs;
+
+	/* Check if file offset is correct */
+	if (hdr->e_shoff > info->size) {
+		fprintf(stderr, "section header offset=%lu in file '%s' is bigger than "
+		      "filesize=%lu\n", (unsigned long)hdr->e_shoff,
+		      filename, info->size);
+		return 0;
+	}
+
+	if (hdr->e_shnum == SHN_UNDEF) {
+		/*
+		 * There are more than 64k sections,
+		 * read count from .sh_size.
+		 */
+		info->num_sections = TO_NATIVE(sechdrs[0].sh_size);
+	}
+	else {
+		info->num_sections = hdr->e_shnum;
+	}
+	if (hdr->e_shstrndx == SHN_XINDEX) {
+		info->secindex_strings = TO_NATIVE(sechdrs[0].sh_link);
+	}
+	else {
+		info->secindex_strings = hdr->e_shstrndx;
+	}
+
+	/* Fix endianness in section headers */
+	for (i = 0; i < info->num_sections; i++) {
+		sechdrs[i].sh_name      = TO_NATIVE(sechdrs[i].sh_name);
+		sechdrs[i].sh_type      = TO_NATIVE(sechdrs[i].sh_type);
+		sechdrs[i].sh_flags     = TO_NATIVE(sechdrs[i].sh_flags);
+		sechdrs[i].sh_addr      = TO_NATIVE(sechdrs[i].sh_addr);
+		sechdrs[i].sh_offset    = TO_NATIVE(sechdrs[i].sh_offset);
+		sechdrs[i].sh_size      = TO_NATIVE(sechdrs[i].sh_size);
+		sechdrs[i].sh_link      = TO_NATIVE(sechdrs[i].sh_link);
+		sechdrs[i].sh_info      = TO_NATIVE(sechdrs[i].sh_info);
+		sechdrs[i].sh_addralign = TO_NATIVE(sechdrs[i].sh_addralign);
+		sechdrs[i].sh_entsize   = TO_NATIVE(sechdrs[i].sh_entsize);
+	}
+	/* Find symbol table. */
+	secstrings = (void *)hdr + sechdrs[info->secindex_strings].sh_offset;
+	for (i = 1; i < info->num_sections; i++) {
+		int nobits = sechdrs[i].sh_type == SHT_NOBITS;
+
+		if (!nobits && sechdrs[i].sh_offset > info->size) {
+			fprintf(stderr, "%s is truncated. sechdrs[i].sh_offset=%lu > "
+			      "sizeof(*hrd)=%zu\n", filename,
+			      (unsigned long)sechdrs[i].sh_offset,
+			      sizeof(*hdr));
+			return 0;
+		}
+
+		if (sechdrs[i].sh_type == SHT_SYMTAB) {
+			unsigned int sh_link_idx;
+			symtab_idx = i;
+			info->symtab_start = (void *)hdr +
+			    sechdrs[i].sh_offset;
+			info->symtab_stop  = (void *)hdr +
+			    sechdrs[i].sh_offset + sechdrs[i].sh_size;
+			sh_link_idx = sechdrs[i].sh_link;
+			info->strtab       = (void *)hdr +
+			    sechdrs[sh_link_idx].sh_offset;
+		}
+
+		/* 32bit section no. table? ("more than 64k sections") */
+		if (sechdrs[i].sh_type == SHT_SYMTAB_SHNDX) {
+			symtab_shndx_idx = i;
+			info->symtab_shndx_start = (void *)hdr +
+			    sechdrs[i].sh_offset;
+			info->symtab_shndx_stop  = (void *)hdr +
+			    sechdrs[i].sh_offset + sechdrs[i].sh_size;
+		}
+	}
+	if (!info->symtab_start)
+		fprintf(stderr, "%s has no symtab?\n", filename);
+
+	/* Fix endianness in symbols */
+	for (sym = info->symtab_start; sym < info->symtab_stop; sym++) {
+		sym->st_shndx = TO_NATIVE(sym->st_shndx);
+		sym->st_name  = TO_NATIVE(sym->st_name);
+		sym->st_value = TO_NATIVE(sym->st_value);
+		sym->st_size  = TO_NATIVE(sym->st_size);
+	}
+
+	if (symtab_shndx_idx != ~0U) {
+		Elf32_Word *p;
+		if (symtab_idx != sechdrs[symtab_shndx_idx].sh_link)
+			fprintf(stderr, "%s: SYMTAB_SHNDX has bad sh_link: %u!=%u\n",
+			      filename, sechdrs[symtab_shndx_idx].sh_link,
+			      symtab_idx);
+		/* Fix endianness */
+		for (p = info->symtab_shndx_start; p < info->symtab_shndx_stop;
+		     p++)
+			*p = TO_NATIVE(*p);
+	}
+
+	return 1;
+}
+
+static void parse_elf_finish(struct elf_info *info)
+{
+	struct pmd_driver *tmp, *idx = info->drivers;
+	release_file(info->hdr, info->size);
+	while (idx) {
+		tmp = idx->next;
+		free(idx);
+		idx = tmp;
+	}
+}
+
+static const char *sec_name(struct elf_info *elf, int secindex)
+{
+	Elf_Shdr *sechdrs = elf->sechdrs;
+	return (void *)elf->hdr +
+		elf->sechdrs[elf->secindex_strings].sh_offset +
+		sechdrs[secindex].sh_name;
+}
+
+static int get_symbol_index(struct elf_info *info, Elf64_Sym *sym)
+{
+	const char *name =  sym_name(info, sym);
+	const char *idx;
+
+	idx = name;
+	while (idx) {
+		if (isdigit(*idx))
+			return atoi(idx);
+		idx++;
+	}
+	return -1;
+}
+
+struct opt_tag {
+	const char* suffix;
+	const char* json_id;
+};
+
+static const struct opt_tag opt_tags[] = {
+	{"_param_string_export", "params"},
+};
+
+static int complete_pmd_entry(struct elf_info *info, struct pmd_driver *drv)
+{
+	const char *tname;
+	int i;
+	char tmpsymname[128];
+	Elf_Sym *tmpsym;
+	
+
+	drv->name = get_sym_value(info, drv->name_sym);
+
+	for (i=0; i<PMD_OPT_MAX; i++) {
+		memset(tmpsymname, 0, 128);
+		sprintf(tmpsymname, "__%s%s", drv->name, opt_tags[i].suffix);
+		tmpsym = find_sym_in_symtab(info, tmpsymname, NULL);
+		if (!tmpsym)
+			continue;
+		drv->opt_vals[i] = get_sym_value(info, tmpsym);
+	}
+
+	memset(tmpsymname, 0, 128);
+	sprintf(tmpsymname, "__%s_pci_tbl_export", drv->name);
+
+	tmpsym = find_sym_in_symtab(info, tmpsymname, NULL);
+
+
+	/*
+ 	 * If this returns NULL, then this is a PMD_VDEV, because
+ 	 * it has no pci table reference
+ 	 */
+	if (!tmpsym) {
+		drv->pci_tbl = NULL;
+		return 0;
+	}
+
+	tname = get_sym_value(info, tmpsym);
+	tmpsym = find_sym_in_symtab(info, tname, NULL);
+	if (!tmpsym)
+		return -ENOENT;
+
+	drv->pci_tbl = (struct rte_pci_id *)get_sym_value(info, tmpsym);
+	if (!drv->pci_tbl)
+		return -ENOENT;
+
+
+	return 0;
+	
+}
+
+static int locate_pmd_entries(struct elf_info *info)
+{
+	Elf_Sym *last = NULL;
+	struct pmd_driver *new;
+
+	info->drivers = NULL;
+
+	do {
+		new = calloc(sizeof(struct pmd_driver), 1);
+		new->name_sym = find_sym_in_symtab(info, "this_pmd_name", last);
+		last = new->name_sym;
+		if (!new->name_sym)
+			free(new);
+		else {
+			if (complete_pmd_entry(info, new)) {
+				fprintf(stderr, "Failed to complete pmd entry\n");
+				free(new);
+			} else {
+				new->next = info->drivers;
+				info->drivers = new;
+			}
+		}
+	} while (last);
+}
+
+static void output_pmd_info_string(struct elf_info *info, char *outfile)
+{
+	FILE *ofd;
+	struct pmd_driver *drv;
+	struct rte_pci_id *pci_ids;
+	int idx = 0;
+
+	ofd = fopen(outfile, "w+");
+	if (!ofd) {
+		fprintf(stderr, "Unable to open output file\n");
+		return;
+	}
+
+	drv = info->drivers;
+
+	while (drv) {
+		fprintf(ofd, "const char %s_pmd_info[] __attribute__((used)) = \"PMD_INFO_STRING= {",
+			drv->name);
+		fprintf(ofd,"\\\"name\\\" : \\\"%s\\\", ", drv->name);
+		fprintf(ofd,"\\\"type\\\" : \\\"%s\\\", ", drv->pci_tbl ? "PMD_PDEV" : "PMD_VDEV");
+
+		for(idx=0; idx<PMD_OPT_MAX; idx++) {
+			if (drv->opt_vals[idx])
+				fprintf(ofd,"\\\"%s\\\" : \\\"%s\\\", ", opt_tags[idx].json_id,
+					drv->opt_vals[idx]);
+		}
+
+		pci_ids = drv->pci_tbl;
+		fprintf(ofd, "\\\"pci_ids\\\" : [");
+
+		while (pci_ids && pci_ids->device_id) {
+			fprintf(ofd, "[%d, %d, %d, %d]",
+				pci_ids->vendor_id, pci_ids->device_id,
+				pci_ids->subsystem_vendor_id,
+				pci_ids->subsystem_device_id);
+			pci_ids++;
+			if (pci_ids->device_id)
+				fprintf(ofd, ",");
+			else
+				fprintf(ofd, " ");
+		}
+		fprintf(ofd, "]}\";");
+		drv = drv->next;
+	}
+
+	fclose(ofd);
+}
+
+int main(int argc, char **argv)
+{
+	struct elf_info info;
+	int rc = 1;
+
+	if (argc < 3) {
+		fprintf(stderr, "usage: pmdinfo <object file> <c output file>\n");
+		exit(127);
+	}
+	parse_elf(&info, argv[1]);
+
+	locate_pmd_entries(&info);
+
+	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");
+	}
+
+	parse_elf_finish(&info);
+	exit(rc);
+}
diff --git a/buildtools/pmdinfogen/pmdinfogen.h b/buildtools/pmdinfogen/pmdinfogen.h
new file mode 100644
index 0000000..580ed9f
--- /dev/null
+++ b/buildtools/pmdinfogen/pmdinfogen.h
@@ -0,0 +1,83 @@ 
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <elf.h>
+
+
+/* On BSD-alike OSes elf.h defines these according to host's word size */
+#undef ELF_ST_BIND
+#undef ELF_ST_TYPE
+#undef ELF_R_SYM
+#undef ELF_R_TYPE
+
+#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
+
+#define TO_NATIVE(x) (x)
+
+
+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. */
+};
+
+enum opt_params {
+	PMD_PARAM_STRING = 0,
+	PMD_OPT_MAX
+};
+
+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];
+};
+
+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;
+
+	/* support for 32bit section numbers */
+
+	unsigned int num_sections; /* max_secindex + 1 */
+	unsigned int secindex_strings;
+	/* if Nth symbol table entry has .st_shndx = SHN_XINDEX,
+	 * take shndx from symtab_shndx_start[N] instead */
+	Elf32_Word   *symtab_shndx_start;
+	Elf32_Word   *symtab_shndx_stop;
+
+	struct pmd_driver *drivers;
+};
+
diff --git a/mk/rte.buildtools.mk b/mk/rte.buildtools.mk
new file mode 100644
index 0000000..e8bfcef
--- /dev/null
+++ b/mk/rte.buildtools.mk
@@ -0,0 +1,148 @@ 
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
+#   Copyright(c) 2014-2015 6WIND S.A.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/internal/rte.compile-pre.mk
+include $(RTE_SDK)/mk/internal/rte.install-pre.mk
+include $(RTE_SDK)/mk/internal/rte.clean-pre.mk
+include $(RTE_SDK)/mk/internal/rte.build-pre.mk
+include $(RTE_SDK)/mk/internal/rte.depdirs-pre.mk
+
+# VPATH contains at least SRCDIR
+VPATH += $(SRCDIR)
+
+_BUILD = $(APP)
+_INSTALL = $(INSTALL-FILES-y) $(SYMLINK-FILES-y)
+_INSTALL += $(RTE_OUTPUT)/buildtools/$(APP) $(RTE_OUTPUT)/buildtools/$(APP).map
+POSTINSTALL += target-appinstall
+_CLEAN = doclean
+POSTCLEAN += target-appclean
+
+.PHONY: all
+all: install
+
+.PHONY: install
+install: build _postinstall
+
+_postinstall: build
+
+.PHONY: build
+build: _postbuild
+
+exe2cmd = $(strip $(call dotfile,$(patsubst %,%.cmd,$(1))))
+
+ifeq ($(LINK_USING_CC),1)
+override EXTRA_LDFLAGS := $(call linkerprefix,$(EXTRA_LDFLAGS))
+O_TO_EXE = $(CC) $(CFLAGS) $(LDFLAGS_$(@)) \
+	-Wl,-Map=$(@).map,--cref -o $@ $(OBJS-y) $(call linkerprefix,$(LDFLAGS)) \
+	$(EXTRA_LDFLAGS) $(call linkerprefix,$(LDLIBS))
+else
+O_TO_EXE = $(LD) $(LDFLAGS) $(LDFLAGS_$(@)) $(EXTRA_LDFLAGS) \
+	-Map=$(@).map --cref -o $@ $(OBJS-y) $(LDLIBS)
+endif
+O_TO_EXE_STR = $(subst ','\'',$(O_TO_EXE)) #'# fix syntax highlight
+O_TO_EXE_DISP = $(if $(V),"$(O_TO_EXE_STR)","  LD $(@)")
+O_TO_EXE_CMD = "cmd_$@ = $(O_TO_EXE_STR)"
+O_TO_EXE_DO = @set -e; \
+	echo $(O_TO_EXE_DISP); \
+	$(O_TO_EXE) && \
+	echo $(O_TO_EXE_CMD) > $(call exe2cmd,$(@))
+
+-include .$(APP).cmd
+
+# path where libraries are retrieved
+LDLIBS_PATH := $(subst -Wl$(comma)-L,,$(filter -Wl$(comma)-L%,$(LDLIBS)))
+LDLIBS_PATH += $(subst -L,,$(filter -L%,$(LDLIBS)))
+
+# list of .a files that are linked to this application
+LDLIBS_NAMES := $(patsubst -l%,lib%.a,$(filter -l%,$(LDLIBS)))
+LDLIBS_NAMES += $(patsubst -Wl$(comma)-l%,lib%.a,$(filter -Wl$(comma)-l%,$(LDLIBS)))
+
+# list of found libraries files (useful for deps). If not found, the
+# library is silently ignored and dep won't be checked
+LDLIBS_FILES := $(wildcard $(foreach dir,$(LDLIBS_PATH),\
+	$(addprefix $(dir)/,$(LDLIBS_NAMES))))
+
+#
+# Compile executable file if needed
+#
+$(APP): $(OBJS-y) $(LDLIBS_FILES) $(DEP_$(APP)) $(LDSCRIPT) FORCE
+	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
+	$(if $(D),\
+		@echo -n "$< -> $@ " ; \
+		echo -n "file_missing=$(call boolean,$(file_missing)) " ; \
+		echo -n "cmdline_changed=$(call boolean,$(call cmdline_changed,$(O_TO_EXE_STR))) " ; \
+		echo -n "depfile_missing=$(call boolean,$(depfile_missing)) " ; \
+		echo "depfile_newer=$(call boolean,$(depfile_newer)) ")
+	$(if $(or \
+		$(file_missing),\
+		$(call cmdline_changed,$(O_TO_EXE_STR)),\
+		$(depfile_missing),\
+		$(depfile_newer)),\
+		$(O_TO_EXE_DO))
+
+#
+# install app in $(RTE_OUTPUT)/app
+#
+$(RTE_OUTPUT)/buildtools/$(APP): $(APP)
+	@echo "  INSTALL-APP $(APP)"
+	@[ -d $(RTE_OUTPUT)/buildtools ] || mkdir -p $(RTE_OUTPUT)/buildtools
+	$(Q)cp -f $(APP) $(RTE_OUTPUT)/buildtools
+
+#
+# install app map file in $(RTE_OUTPUT)/app
+#
+$(RTE_OUTPUT)/buildtools/$(APP).map: $(APP)
+	@echo "  INSTALL-MAP $(APP).map"
+	@[ -d $(RTE_OUTPUT)/buildtools ] || mkdir -p $(RTE_OUTPUT)/buildtools
+	$(Q)cp -f $(APP).map $(RTE_OUTPUT)/buildtools
+
+#
+# Clean all generated files
+#
+.PHONY: clean
+clean: _postclean
+	$(Q)rm -f $(_BUILD_TARGETS) $(_INSTALL_TARGETS) $(_CLEAN_TARGETS)
+
+.PHONY: doclean
+doclean:
+	$(Q)rm -rf $(APP) $(OBJS-all) $(DEPS-all) $(DEPSTMP-all) \
+	  $(CMDS-all) $(INSTALL-FILES-all) .$(APP).cmd
+
+
+include $(RTE_SDK)/mk/internal/rte.compile-post.mk
+include $(RTE_SDK)/mk/internal/rte.install-post.mk
+include $(RTE_SDK)/mk/internal/rte.clean-post.mk
+include $(RTE_SDK)/mk/internal/rte.build-post.mk
+include $(RTE_SDK)/mk/internal/rte.depdirs-post.mk
+
+.PHONY: FORCE
+FORCE:
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index eec5241..fb68af2 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -64,7 +64,8 @@  build: $(ROOTDIRS-y)
 clean: $(CLEANDIRS)
 	@rm -rf $(RTE_OUTPUT)/include $(RTE_OUTPUT)/app \
 		$(RTE_OUTPUT)/hostapp $(RTE_OUTPUT)/lib \
-		$(RTE_OUTPUT)/hostlib $(RTE_OUTPUT)/kmod
+		$(RTE_OUTPUT)/hostlib $(RTE_OUTPUT)/kmod \
+		$(RTE_OUTPUT)/buildtools
 	@[ -d $(RTE_OUTPUT)/include ] || mkdir -p $(RTE_OUTPUT)/include
 	@$(RTE_SDK)/scripts/gen-config-h.sh $(RTE_OUTPUT)/.config \
 		> $(RTE_OUTPUT)/include/rte_config.h