Re: [dpdk-dev] [PATCH v3 5/9] pmdinfogen: move to drivers subdirectory

[Thomas Monjalon <thomas@monjalon.net>]

21/06/2017 13:39, Gaëtan Rivet:
> On Wed, Jun 21, 2017 at 12:00:24PM +0200, Thomas Monjalon wrote:
> > 21/06/2017 11:40, Gaëtan Rivet:
> > > On Wed, Jun 21, 2017 at 09:57:18AM +0200, Thomas Monjalon wrote:
> > > > Another (probably better) solution is to keep basic definitions
> > > > and helpers in EAL:
> > > > 	- rte_pci.h keeps only some PCI definitions and helpers
> > > > like rte_pci_addr and eal_parse_pci_BDF() in EAL
> > > > 	- bus management is done in the PCI driver
> > > > 
> > > > For pmdinfogen, we just need struct rte_pci_id.
> > > > Other tools or applications will probably need this kind of basic
> > > > struct and functions available in EAL.
> > > 
> > > I mostly agree, this proposal should be kept to a minimum at first for
> > > this release and carefully expanded afterward.
> > > 
> > > If that's ok, I will propose a new version of this patchset with a new
> > > librte_pci, that might fix both pmdinfogen and librte_kni.
> > 
> > Why creating a new librte_pci instead of just keeping it in EAL?
> 
> While I agree that it makes sense to have PCI helpers shared among
> several subsystems, I do not see a reason for the EAL to rely on it.
> 
> The EAL is the bedrock of the whole system. Having those helpers within
> would mean that one expects them to be used to build this bedrock. It
> would be misleading.
> 
> In the context of a framework, aimed at being used by others, an okay
> architecture is one that works. A good architecture is one that
> intrinsically convey meaning and explains its goal to developers relying
> on it. I think that having this PCI lib within EAL just because nothing
> prevents us from doing so is mistaken, in this regard.
> 
> Conversely, the argument about being conservative in the changes,
> especially to an essential part such as the EAL, is obsolete for this
> release and this subsystem, as deep changes are necessary anyway,
> and the design should be right from the get go to allow further stability.
> 
> Finally, the PCI lib and bus is also an example for other developers. I
> do not think that all other hardware buses should be allowed in adding
> their own specific helpers to the EAL. In this regard, there is no
> reason to have an exception made just for the PCI lib.

You get a point :)