Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 06 Dec 2006 09:48:57 -0800
From:      Scott Long <scottl@samsco.org>
To:        Sam Leffler <sam@errno.com>
Cc:        src-committers@FreeBSD.org, Bruce Evans <bde@zeta.org.au>, cvs-src@FreeBSD.org, Marius Strobl <marius@FreeBSD.org>, cvs-all@FreeBSD.org, Gleb Smirnoff <glebius@FreeBSD.org>
Subject:   Re: cvs commit: src/sys/pci if_xl.c if_xlreg.h
Message-ID:  <45770289.5010002@samsco.org>
In-Reply-To: <4576F9F7.9090503@errno.com>
References:  <200612060218.kB62IfVn046324@repoman.freebsd.org> <20061206164242.A32496@delplex.bde.org> <20061206154555.GM32700@FreeBSD.org> <4576F9F7.9090503@errno.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45770289.5010002>