Date: Tue, 09 Jan 2007 16:14:16 -0800 (PST) From: John Polstra <jdp@polstra.com> To: Oleg Bulyzhin <oleg@freebsd.org> Cc: Doug Barton <dougb@freebsd.org>, net@freebsd.org Subject: Re: [Fwd: Re: bge Ierr rate increase from 5.3R -> 6.1R] Message-ID: <XFMail.20070109161416.jdp@polstra.com> In-Reply-To: <20070109235649.GC5246@lath.rinet.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On 09-Jan-2007 Oleg Bulyzhin wrote:
> On Mon, Jan 08, 2007 at 04:33:29PM -0800, John Polstra wrote:
>> On 30-Dec-2006 Bruce Evans wrote:
>> > More debugging showed that almost any of the reads of the phy in mii
>> > cause an input error,
>>
>> The errors appear to be caused by the code in bge_miibus_{read,write}reg
>> that clears and then restores the BGE_MIMODE_AUTOPOLL bit of the
>> BGE_MI_MODE register. If you remove those chunks of code, the errors
>> go away even when mii_tick is called periodically, and the chip
>> performs quite well under heavy traffic load in both directions.
>
> I've seen this kind of ierrors (LINK_LOST error flag in rx buffer descriptor)
> only on bcm5701 chip. Other chips i tried (5700 and 5721) seems are not
> affected.
The packet loss I observed was on a 5704. I didn't check the
LINK_LOST flags.
>> I agree with you that we shouldn't need to call mii_tick periodically,
>> except perhaps for a few very old variants of the Broadcom chip.
>>
>> John
> mii_tick periodic calls are requried - MII layer use it for link
> autonegotiation. Though i guess it's possible to avoid phy access if we
> have link up.
> I see two different ways to solve this:
> 1) convert driver for using phy interrupts as default (like linux driver),
> thus BGE_MI_MODE read/write can be avoided.
> 2) do not touch PHY inside bge_tick() if we have link up. Actually, this
> check was in driver but was removed as workaround of existed link state
> issues (due to some bugs in if_bge.c and brgphy.c link loss events were
> detected with few seconds delay or even not detected at all).
> Those bugs are fixed now, so we can revive this check (bge_link_upd() can
> be simplified a bit too).
I've been using #2 today, and it's working well so far. I
implemented it like this. (Ignore the version numbers; I'm working
in a private repository.)
--- if_bge.c 8 Jan 2007 22:46:51 -0000 1.26
+++ if_bge.c 9 Jan 2007 22:52:43 -0000 1.27
@@ -3122,8 +3122,8 @@ bge_tick(void *xsc)
if ((sc->bge_flags & BGE_FLAG_TBI) == 0) {
mii = device_get_softc(sc->bge_miibus);
- /* Don't mess with the PHY in IPMI/ASF mode */
- if (!((sc->bge_asf_mode & ASF_STACKUP) && (sc->bge_link)))
+ /* Don't mess with the PHY unless link is down. */
+ if (!sc->bge_link)
mii_tick(mii);
} else {
/*
John
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20070109161416.jdp>