cvs commit: src/sys/pci if_xl.c if_xlreg.h

[Scott Long]

Sam Leffler wrote:
> Gleb Smirnoff wrote:
> 
>>On Wed, Dec 06, 2006 at 06:01:48PM +1100, Bruce Evans wrote:
>>B> It's a shame to force all NIC drivers to manage the timeout for this.
>>B> Most have a timeout for other purposes so I couldn't see how to save
>>B> much code using a callback, but a callback would be cleaner.  (To avoid
>>B> the race, just move the decrement of the count to drivers.)
>>
>>It is a shame to have a two extra fields in struct ifnet, just for
>>the sake of the drivers that can wedge. It is a shame to go through
>>the whole list of interfaces every second.
>>
>>There are routers with few NICs and dozens of vlan(4) interfaces. There
>>are also PPP concentrators with up to thousand interfaces and only
>>one NIC that really needs to have its watchdog.
>>
> 
> 
> I agree with both sentiments and as the originator of the ifnet watchdog
> mechanism I can only say that it's high time it was replaced by
> something better.  My main worry with this change is that people will
> _blindly_ sweep drivers replacing what was previously a fairly
> lightweight mechanism with something much more expensive.
> 
> 	Sam

As Gleb points out, most drivers already have a private callout for 
stats collecting, so piggybacking off of that doesn't present any
extra load on the system.  I share your concern about blind changes;
the changes made to if_em were meant as a proof of concept and a means
to an end for getting the driver ready for the upcoming release.  I
can't think of any way to keep this functionality in the if layer that
doesn't incur extra locking overhead, other than to create an 
if_slowtimeout() instance per interface.  But it's such a small function
that I don't think that there is much to be gained by trying to keep it
around.  I hope that when people replace the if_watchdog mechanism in 
their driver that they also take the time to make the replacement 
smarter than just doing blind resets, calling into the interrupt 
handler, etc.

Scott