Re: Splitting e1000 (Was: Re: e1000: backport ich9 support from 7.5.5 ?)

[Jeff Garzik <jeff@garzik.org>]

Arjan van de Ven wrote:
> Jeff Garzik wrote:
>> Is this is the attitude, what's the point of even posting the driver 
>> for review?  Intel posted e1000new on June 29.  Feedback was then posted. 
> 
> and feedback is being incorperated.
> 
>> Ten days later, without a single revision, Intel declares its own 
>> driver clean and working and good enough for merging as-is.
> 
> No. Arjan looked at driver and sees a rather clean driver compared to 
> some of the other recently merged drivers, sees a list of relatively 
> simple todos (some of which obviously aren't merge stoppers, others are) 
> and hopes that this driver will be treated objectively without the 
> appearance of having different levels of bars depending on what your 
> email address ends with.


Ignoring small potatoes, the merge stoppers in my mind are:

1) Transition plan.  I strongly oppose switching all e1000 users en 
masse to a new driver, especially so soon.  Flag day transitions to 
unproven drivers suck.  Defaults don't work:  users use the old driver 
until the default changes, which means the new driver gets little field 
testing.

Regardless of my opinion of old-e1000 maintainability, top priority is 
to keep users running on a stable driver until new driver is stable.  I 
would propose merging a new driver with only the PCI IDs not already in 
the kernel, get that stable, then consider moving the rest of the 
PCI-Express PCI IDs (or others?).


2) Internal API.  An "it can do anything" API is a hint that the driver 
should be structured differently.  Perhaps a divorce between pre-PCIe 
and PCIe will help things (or 8257x vs other?).  I tend to think that 
both e1000 and e1000new could be cleaned up substantially by such a 
split.  Also, specifically for PHYs, we already have a phy layer that 
can be used a focal point for PHY modularity.

Overall, within minor chip revs you'll probably create standard 
branches.  But within major chip revs, you really should be looking at 
separate code paths rather than trying to shoehorn a wide variety of 
chips down the same (highly modular!) hot path.  That slows down 
everybody to the same speed (least common denominator), and makes it 
more difficult to follow the code path for a single chip.

If the chips are sufficiently different, it is better to write two 
distinct RX/TX/etc. code paths than to create a single, highly modular 
yet highly dense code path.  Easier to read.  Easier to tune.  Easier to 
leave chip errata behind, as time progresses.


3) Looming over everything else, is I wonder if Intel has recognized 
that a maintainership problem exists: lack of ongoing cleanups and lack 
of "focus" on issues not directly related to enabling new features?

My only "bias" against Intel is that I am -nervous- that past history 
will continue, with regards to the "stuff it into the driver and move 
on" driver maintenance regime.  Even most clean driver in the world will 
fall upon hard times if under-maintained.

Receiving feedback, disappearing for months, and then reappearing with 
"blessed" changes is not adequate.  netdev@vger.kernel.org and 
netdev-2.6.git need to be the primary vehicle for e1000[new] 
development, maintained through small incremental changes streaming into 
the kernel.  (though I do understand the constraints of hardware 
go-public release dates)

	Jeff



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/