[dpdk-dev] [PATCH 09/11] eal: move PCI table macro

Neil Horman nhorman at tuxdriver.com
Fri Jul 8 15:56:43 CEST 2016


On Fri, Jul 08, 2016 at 10:49:25AM +0200, David Marchand wrote:
> Hello Thomas, Neil,
> 
> (will be back in a couple of days, thanks Thomas for pointing this thread)
> 
> 
> On Thu, Jul 7, 2016 at 6:25 PM, Thomas Monjalon
> <thomas.monjalon at 6wind.com> wrote:
> > 2016-07-07 12:11, Neil Horman:
> >> On Thu, Jul 07, 2016 at 05:36:28PM +0200, Thomas Monjalon wrote:
> >> > Remove include of rte_pci.h in the generic header rte_dev.h
> >> > and move the macro DRIVER_REGISTER_PCI_TABLE in rte_pci.h.
> > [...]
> >>
> >> This seems strange to me, in that its odd for the driver information export
> >> macros to be spread out in multiple locations.  Specifically it enjoins the use
> >> of the DRV_EXP_TAG macro, which helps centralize tag naming.  Perhaps the happy
> >> medium is to place all the export macros (includnig PMD_REGISTER_DRIVER) into
> >> its own pmd_register.h header?
> >
> > I don't know.
> > David, your opinion?
> 
> - The suggestion I did offline to Thomas was to move pci stuff in pci headers.
> We are trying to move from the "all pci" code in eal to accomodate for
> other "buses" / architectures.
I get that, but I'm not sure that applies here.  The macro in question is
specific to pci busses, and if there is additional bus information to export, it
will have its own macro (e.g. DRIVER_REGISTER_USB_TABLE or some such).  While
I could see that being an argument for putting each macro in with its own bus
type, I think thats the wrong organization here, in that people writing drivers
will want to know what export macros are available and will expect to look in a
single place for it.

> Having a pci macro in a generic header like rte_dev.h is wrong to me.
> Moving this to a new header like pmd_register.h sounds like a new
> generic header with pci specific stuff in it.
Well, yes, but I see that as no different than rte_ethdev.c or rte_pdump.c.
both of those files will need to know about all the different types of busses
you support and have to include those corresponding header files (i.e. they will
have to include rte_pci.h, rte_usb.h, rte_i2c.h, etc).  This is really no
different in my mind. 

> So, I am not sure I follow you Neil.
> 
> Can you elaborate ?
> 
I suppose the best way to describe it is that while I understand and support the
desire to separate and abstract bus information away from device function, I
think theres a pragmatic descision here to prioritize functional domain over
header inclusion.  That is to say, I think when people are writing a driver, it
will be helpful to have all the export macros in a single location so they know
what information they can export, and that includes registration of various bus
type identifiers.  So a file like pmd_registration.h that includes rte_pci.h,
rte_usb.h, rte_i2c.h, etc is more useful to a developer, than spreading these
macros out to those various header files, for the sake of avoiding a potentially
unneeded include.

> 
> - Why do you want to centralise the tag naming ?
> To avoid collisions ?
Yes, and to centralize that information.  Since the pmdinfogen tool needs to
know what those tag names are as well, its useful to keep them in the same area
to maintain co-ordination.  Its also useful because it means we can use one
macro to define tag naming convention, instead of having to re-implement or
dead-reckon it in multiple files.

> Well, adding those tags should not happen that often and I think we
> can maintain this with careful reviews.
I don't agree with that.  This discussion is based on the fact that you expect
that we will be adding additional bus types in the future correct?  Well, given
that we have a pci bus specific export macro, I would expect that to proliferate
to every other bus type as well, and so we can expect to at least have a new
tag added for every bus that is added, in addition to any other bus agnostic
information people wish to export (just off hand, looking at the linux modinfo
section, we might expect module author, module version, alias names, licensing
infomration, and others to be potential export candidates).  So, depending on
how much this is adopted, I think we can potentially expect a great deal of
additional tagging to be needed.

Neil



More information about the dev mailing list