Re: bge diff needs testing

2013-06-20 Thread Rob Sessink
 I've got test report for the BCM5723/BCM5784.  It would be
 great if someone with a 5703 or 5704 could try this.

Hoi Mike, 

Over the weekend I would be able to test this on a HP DL360 G3 with 5703. 

Regards Rob



Re: bge diff needs testing

2013-06-17 Thread Mike Belopuhov

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|