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

On Wed, Jun 21, 2017 at 12:00:24PM +0200, Thomas Monjalon wrote:

21/06/2017 11:40, Gaëtan Rivet:

quoted

On Wed, Jun 21, 2017 at 09:57:18AM +0200, Thomas Monjalon wrote:

quoted

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.

-- 
Gaëtan Rivet
6WIND