I've got test report for the BCM5723/BCM5784. It would be
great if someone with a 5703 or 5704 could try this.
On Thu, Jun 13, 2013 at 18:09 +0200, Mike Belopuhov wrote:
> 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|
