Re: [RFC PATCH 00/21] Reduce code duplication across Intel NIC drivers

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Nov 26, 2024 at 03:57:42PM +0100, Thomas Monjalon wrote:
> 25/11/2024 17:31, Bruce Richardson:
> > On Mon, Nov 25, 2024 at 05:25:47PM +0100, David Marchand wrote:
> > > Hello Bruce,
> > > 
> > > On Fri, Nov 22, 2024 at 1:54 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > This RFC attempts to reduce the amount of code duplication across a
> > > > number of Intel NIC drivers, specifically: ixgbe, i40e, iavf, and ice.
> > > 
> > > Thanks for starting this effort!
> > > 
> > > >
> > > > The first patch extract a function from the Rx side, otherwise the
> > > > majority of the changes are on the Tx side, leading to a converged Tx
> > > > queue structure across the 4 drivers, and a large number of common
> > > > functions.
> > > >
> > > > Open question:
> > > > * How should common code across drivers within a single device class be
> > > >   managed?
> > > >   - For now, I've created an "intel_eth" folder within the "common"
> > > >     driver directory, thinking about it after, it  implies to me that
> > > >     it is common across driver classes.
> > > >   - Would it be better to create an "intel_common" directory within the
> > > >     "net" folder?
> > > 
> > > common/ drivers currently host code that is device class agnostic,
> > > like providing helpers to talk with hw.
> > > No common/ driver has a dependency on some device class library.
> > > 
> > > This series adds code that is not built into a library so there is no
> > > need to express dependencies in meson.
> > > But if the need arises, could it become a problem? (adding a
> > > dependency to lib/ethdev to some drivers/common/xx/).
> > > 
> > > 
> > > For now, I prefer the second proposition and have this code hosted in
> > > drivers/net/.
> > > 
> > Thanks for the feedback. While when I started this prototyping I felt that
> > common was the right place for it, at this point I'm now tending towards
> > this second location - keeping it in net.
> > Any other thoughts on the relative merits of the various locations?
> 
> We just need to know in which order building the common directory.
> It can be before bus drivers or later, but you cannot have bus common code,
> and some ethdev code at the same time.
> If you just want to share code inside drivers/net/, I suppose it is OK to keep it there.
> In any choice you do, you will have to maintain some restrictions on the content due to the location.
> 
Yes, good point. However, that would only apply if the common code was
being built into a separate component to be linked into the other
components using it. The initial prototype work has resulted in header
files only, and my thinking was that even if .c files are added, they would
not be compiled into a .a file, but rather compiled as source into each
individual driver using them. This would allow for a certain amount of
ifdef usage, for example, where things may have slight differences.
However, I think we cross that bridge when we come to it.

/Bruce