Re: bge diff needs testing
On 27 June 2013 14:39, Rob Sessink wrote: >>> 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 > > Hello, > > I saw the commit is already in, so just for the record, this also works with > a BCM5702/5703 A2. > > Regards Rob > Thanks for testing!
Re: bge diff needs testing
>> 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
Hello,
I saw the commit is already in, so just for the record, this also works with a
BCM5702/5703 A2.
Regards Rob
OpenBSD 5.3-current (GENERIC) #0: Sun Jun 23 11:46:47 CEST 2013
r...@tbg-dbsd.tbg.lan:/usr/src/sys/arch/i386/compile/GENERIC
cpu0: Intel(R) Xeon(TM) CPU 2.40GHz ("GenuineIntel" 686-class) 2.40 GHz
cpu0:
FPU,V86,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,CNXT-ID,xTPR,PERF
real mem = 2146996224 (2047MB)
avail mem = 2100490240 (2003MB)
mainbus0 at root
bios0 at mainbus0: AT/286+ BIOS, date 12/31/99, BIOS32 rev. 0 @ 0xf, SMBIOS
rev. 2.3 @ 0xec000 (42 entries)
bios0: vendor HP version "P31" date 03/01/2003
bios0: HP ProLiant DL360 G3
acpi0 at bios0: rev 0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC SPCR
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 6 (boot processor)
cpu0: apic clock running at 133MHz
cpu at mainbus0: not configured
ioapic0 at mainbus0: apid 2 pa 0xfec0, version 11, 16 pins
ioapic1 at mainbus0: apid 3 pa 0xfec01000, version 11, 16 pins
ioapic2 at mainbus0: apid 4 pa 0xfec02000, version 11, 16 pins
ioapic3 at mainbus0: apid 5 pa 0xfec03000, version 11, 16 pins
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 1 (PCI1)
acpiprt2 at acpi0: bus 4 (PCI2)
acpicpu0 at acpi0
acpitz0 at acpi0: critical temperature is 31 degC
bios0: ROM list: 0xc/0x8000 0xc8000/0x4000 0xcc000/0x1800 0xcd800/0x1800
0xee000/0x2000!
pci0 at mainbus0 bus 0: configuration mode 1 (bios)
pchb0 at pci0 dev 0 function 0 "ServerWorks CNB20-HE Host" rev 0x31
pchb1 at pci0 dev 0 function 1 "ServerWorks CNB20-HE Host" rev 0x00
pchb2 at pci0 dev 0 function 2 "ServerWorks CNB20-HE Host" rev 0x00
pci1 at pchb2 bus 1
em0 at pci1 dev 1 function 0 "Intel 82546EB" rev 0x01: apic 3 int 12, address
00:02:a5:48:29:c4
em1 at pci1 dev 1 function 1 "Intel 82546EB" rev 0x01: apic 3 int 11, address
00:02:a5:48:29:c5
bge0 at pci1 dev 2 function 0 "Broadcom BCM5703X" rev 0x02, BCM5702/5703 A2
(0x1002): apic 3 int 14, address 00:0b:cd:4e:8e:a7
brgphy0 at bge0 phy 1: BCM5703 10/100/1000baseT PHY, rev. 2
Re: bge diff needs testing
> 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
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|
bge diff needs testing
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|
