Hi,
David Imhoff has found that flow control got broken in bge
after some recent changes but also that simple "ifconfig bge0"
call done by any user can change current flow control settings.
We've tested it on a bunch of recent cards (5719, 5720), one
old-ish card (5715) but would like others to make sure their
bge's are running fine with this. Changes must be visible
under substantial load from multiple clients.
An elaborate explanation from David:
---------8<---------
If auto polling is disabled bge_link_upd() @l4546 does a call to
mii_pollstat() followed by a explicit call to bge_miibus_statchg().
However bge_miibus_statchg() is also called implicitly from
mii_pollstat() -> brgphy_service(,, MII_POLLSTAT) -> mii_phy_update().
bge_miibus_statchg() @l1056 obtains the flow control state from
mii->mii_media_active and afterwards removes the flow control information
from mii->mii_media_active. So the second time bge_miibus_statchg() is
called the flow control flags in mii->mii_media_active are always zero.
And thus flow control on the card is disabled
Clearing the flow control flags from mii->mii_media_active has one other
disadvantage. That is that the next call to mii_pollstat(), for example
by running 'ifconfig bge0', will trigger bge_miibus_statchg() to be
executed, because mii->mii_media_active and the newly obtained status
don't match.
This last rogue call to bge_miibus_statchg() has a nasty side effect.
Which is that the flow control flags in mii->mii_media_active will
be equal to bge_flow_flags. Thus the flow contol flags will not be
cleared from mii->mii_media_active. At the bottom of the function
flow control is only enabled if the global options _equal_ full
duplex, however the IFM_FLOW flag is also part of the global options.
Thus a simple call to 'ifconfig bge0' can effectively disable
flow control.
---------8<---------
OK's are welcome as well.
diff --git sys/dev/pci/if_bge.c sys/dev/pci/if_bge.c
index 3d396cc..7001b6f 100644
--- sys/dev/pci/if_bge.c
+++ sys/dev/pci/if_bge.c
@@ -1050,14 +1050,12 @@ bge_miibus_statchg(struct device *dev)
/*
* Get flow control negotiation result.
*/
if (IFM_SUBTYPE(mii->mii_media.ifm_cur->ifm_media) == IFM_AUTO &&
- (mii->mii_media_active & IFM_ETH_FMASK) != sc->bge_flowflags) {
+ (mii->mii_media_active & IFM_ETH_FMASK) != sc->bge_flowflags)
sc->bge_flowflags = mii->mii_media_active & IFM_ETH_FMASK;
- mii->mii_media_active &= ~IFM_ETH_FMASK;
- }
if (!BGE_STS_BIT(sc, BGE_STS_LINK) &&
mii->mii_media_status & IFM_ACTIVE &&
IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)
BGE_STS_SETBIT(sc, BGE_STS_LINK);
@@ -1082,11 +1080,11 @@ bge_miibus_statchg(struct device *dev)
mac_mode |= BGE_PORTMODE_MII;
/* Set MAC flow control behavior to match link flow control settings. */
tx_mode &= ~BGE_TXMODE_FLOWCTL_ENABLE;
rx_mode &= ~BGE_RXMODE_FLOWCTL_ENABLE;
- if ((mii->mii_media_active & IFM_GMASK) == IFM_FDX) {
+ if (mii->mii_media_active & IFM_FDX) {
if (sc->bge_flowflags & IFM_ETH_TXPAUSE)
tx_mode |= BGE_TXMODE_FLOWCTL_ENABLE;
if (sc->bge_flowflags & IFM_ETH_RXPAUSE)
rx_mode |= BGE_RXMODE_FLOWCTL_ENABLE;
} else
@@ -4542,11 +4540,10 @@ bge_link_upd(struct bge_softc *sc)
/*
* For controllers that call mii_tick, we have to poll
* link status.
*/
mii_pollstat(mii);
- bge_miibus_statchg(&sc->bge_dev);
}
/* Clear the attention */
CSR_WRITE_4(sc, BGE_MAC_STS, BGE_MACSTAT_SYNC_CHANGED|
BGE_MACSTAT_CFG_CHANGED|BGE_MACSTAT_MI_COMPLETE|
