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. -- Gaëtan Rivet 6WIND
