From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Neil Horman <nhorman@tuxdriver.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 21:39:43 +0200 [thread overview]
Message-ID: <1507554.Pcj3Og5Zqt@xps13> (raw)
In-Reply-To: <20160525191345.GF14322@hmsreliant.think-freely.org>
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.
next prev parent reply other threads:[~2016-05-25 19:39 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
2016-05-25 17:39 ` Thomas Monjalon
2016-05-25 19:13 ` Neil Horman
2016-05-25 19:39 ` Thomas Monjalon [this message]
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=1507554.Pcj3Og5Zqt@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=nhorman@tuxdriver.com \
--cc=pmatilai@redhat.com \
--cc=stephen@networkplumber.org \
/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).