Call for testers: re(4) and
RTL8168C/RTL8168CP/RTL8111C/RTL8111CP
Pyun YongHyeon
pyunyh at gmail.com
Mon Jul 14 01:38:54 UTC 2008
On Mon, Jul 14, 2008 at 10:35:19AM +0900, To Dimitry Andric wrote:
> On Mon, Jun 30, 2008 at 01:31:56PM +0900, To Dimitry Andric wrote:
> > On Sat, Jun 28, 2008 at 06:54:47PM +0200, Dimitry Andric wrote:
> > > On 2008-06-11 02:58, Pyun YongHyeon wrote:
> > > > > This seems to work better, although it still takes quite some time
> > > > > (~10s) for the interfaces to go up at boot time. I haven't yet been
> > > > > able to get them "stuck", however, so that's good. :)
> > > > Hmm, that's interesting. Can you spot where re(4) spends its time?
> > > > Did RELENG_7 also have this issue?
> > >
> > > Apparently it's experiencing timeouts, I usually get these:
> > >
> > > re0: link state changed to DOWN
> > > re0: watchdog timeout
> > ^^^^^^^^^^^^^^^^
> > Because link state changed to DOWN re(4) should not queue
> > transmitting packets anymore until it get a valid link. Trying to
> > send further packets would cause watchdong timeouts as above.
> > This indicates re(4) failed to detect link loss event.
> > What makes me wonder is why the link state was changed to DOWN.
> > Do you have a clue(e.g. switching hub down etc)?
> >
> > > re0: 3 link states coalesced
> > ^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Hmm, I guess you've encountered another bug. The link states
> > coalescing message indicates a bug in PHY driver and link state
> > handling of re(4). ATM the link state handling of re(4) is in very
> > bad state and it doesn't correctly drive MII_TICK. re(4) just relys
> > on link status change interrupt of controller but re(4) failed to
> > determine what's current link event is for (The event could be link
> > up or down or auto-negotiation complete etc). In addition, all
> > RealTek controllers lack proper programming interface to tell MAC
> > negotiated speed/duplex/flow-controls which in turn taking proper
> > action to the event very hard.
> >
> > I guess re(4) should not rely on link status change interrupt but
> > it should fall back to traditional polling mechanism which will
> > enable correct tracking of link establishment. Also the link up/
> > down handling should be changed to process mii(4) posted events.
> > All these change requires a lot of code change and needs more
> > testing. I think I may have to commit accumulated patches for newer
> > RTL8168 family before going to that direction. The patch is not
> > perfect to address all issues for RTL8168 family but it allows
> > recognition of the new hardware and make it usable in most cases.
> >
> > > re0: link state changed to UP
> > > re1: link state changed to DOWN
> > >
> > > I've been running all tests under RELENG_7, btw. Note also, these
> > > delays don't always happen, in some cases the interfaces react very
> > > quickly. In rare cases, they don't work at all, until you manually
> > > ifconfig down and up them a few times.
> > >
> > > What's funny though, is that the interfaces seem to start in DOWN mode:
> > >
> > > [...booting...]
> > > Mounting local file systems:.
> > > Setting hostname: tensor.andric.com.
> > > re0: link state changed to DOWN
> > > re1: link state changed to DOWN
> > > lo0: flags=8049<UP,LOOPBACK,RUNNING,MULTICAST> metric 0 mtu 16384
> > > inet6 ::1 prefixlen 128
> > > inet6 fe80::1%lo0 prefixlen 64 scopeid 0x5
> > > inet 127.0.0.1 netmask 0xff000000
> > > re0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
> > > options=399b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,TSO4,WOL_UCAST,WOL_MCAST,WOL_MAGIC>
> > > ether 00:30:18:a6:f1:a8
> > > inet6 fe80::230:18ff:fea6:f1a8%re0 prefixlen 64 tentative scopeid 0x1
> > > inet 87.251.56.140 netmask 0xffffffc0 broadcast 87.251.56.191
> > > media: Ethernet autoselect (none)
> > > status: no carrier
> > > re1: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
> > > options=399b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,TSO4,WOL_UCAST,WOL_MCAST,WOL_MAGIC>
> > > ether 00:30:18:a6:f1:a9
> > > inet6 fe80::230:18ff:fea6:f1a9%re1 prefixlen 64 tentative scopeid 0x2
> > > inet 192.168.0.1 netmask 0xffffff00 broadcast 192.168.0.255
> > > media: Ethernet autoselect (none)
> > > status: no carrier
> > > [...more initialization...]
> > > net.inet6.ip6.forwarding: 0 -> 1
> > > net.inet6.ip6.accept_rtadv: 0 -> 0
> > > re0: link state changed to UP
> > > re1: link state changed to UP
> > >
> > > and only then do they "really" go up... :)
> > >
> >
> > I can't sure due to bugs in link state handling in driver but
> > generally it's normal. Establishing a link with link partner takes
> > time and sometimes it would even take 10 seconds or more.
> >
> > > Do you have any good suggestions on where I could put some debug
> > > printfs in re to find out what it's timing out on?
> > >
> >
> > Before doing that it would be more appropriate to fix link state
> > handing in driver. I'll let you know when I have a patch for link
> > handling clean-up.
> >
>
> Here is patch for re(4) link handling.
> Copy if_re.c and if_rlreg.h from HEAD to RELENG_7 and apply
> attached one. If you still see watchdog timeouts, please turn off
> TSO and let me know how it goes.
> One user reported TSO issues on 8169 family controllers but I
> can't reproduce this on my 8169 hardware so it could be related
> with silicon bug of sepecific revision of the hardware.
Forgot to attach patch.
Here we go.
>
> > >
> > > > Plugging/unplugging UTP cable to ethernet controller during boot
> > > > change the long delay? How about disabling WOL before system
> > > > shutdown?(e.g. ifconfig re0 -wol)
> > >
> > > Plugging/unplugging the cable doesn't seem to make much difference, and
> > > neither does disabling WOL before shutdown (or altogether)...
> > >
> >
> > Ok.
> >
> > Thanks for reporting.
--
Regards,
Pyun YongHyeon
-------------- next part --------------
Index: sys/pci/if_rlreg.h
===================================================================
--- sys/pci/if_rlreg.h (revision 180504)
+++ sys/pci/if_rlreg.h (working copy)
@@ -839,6 +839,7 @@
#define RL_FLAG_PAR 0x0020
#define RL_FLAG_DESCV2 0x0040
#define RL_FLAG_MACSTAT 0x0080
+#define RL_FLAG_FASTETHER 0x0100
#define RL_FLAG_LINK 0x8000
};
Index: sys/dev/re/if_re.c
===================================================================
--- sys/dev/re/if_re.c (revision 180504)
+++ sys/dev/re/if_re.c (working copy)
@@ -596,7 +596,38 @@
re_miibus_statchg(dev)
device_t dev;
{
+ struct rl_softc *sc;
+ struct ifnet *ifp;
+ struct mii_data *mii;
+ sc = device_get_softc(dev);
+ mii = device_get_softc(sc->rl_miibus);
+ ifp = sc->rl_ifp;
+ if (mii == NULL || ifp == NULL ||
+ (ifp->if_drv_flags & IFF_DRV_RUNNING) == 0)
+ return;
+
+ sc->rl_flags &= ~RL_FLAG_LINK;
+ if ((mii->mii_media_status & IFM_AVALID) != 0) {
+ switch (IFM_SUBTYPE(mii->mii_media_active)) {
+ case IFM_10_T:
+ case IFM_100_TX:
+ sc->rl_flags |= RL_FLAG_LINK;
+ break;
+ case IFM_1000_T:
+ if ((sc->rl_flags & RL_FLAG_FASTETHER) != 0)
+ break;
+ sc->rl_flags |= RL_FLAG_LINK;
+ break;
+ default:
+ break;
+ }
+ }
+ /*
+ * RealTek controllers does not provide any interface to
+ * Tx/Rx MACs for resolved speed, duplex and flow-control
+ * parameters.
+ */
}
/*
@@ -1238,17 +1269,18 @@
switch (hw_rev->rl_rev) {
case RL_HWREV_8139CPLUS:
- sc->rl_flags |= RL_FLAG_NOJUMBO;
+ sc->rl_flags |= RL_FLAG_NOJUMBO | RL_FLAG_FASTETHER;
break;
case RL_HWREV_8100E:
case RL_HWREV_8101E:
sc->rl_flags |= RL_FLAG_NOJUMBO | RL_FLAG_INVMAR |
- RL_FLAG_PHYWAKE;
+ RL_FLAG_PHYWAKE | RL_FLAG_FASTETHER;
break;
case RL_HWREV_8102E:
case RL_HWREV_8102EL:
sc->rl_flags |= RL_FLAG_NOJUMBO | RL_FLAG_INVMAR |
- RL_FLAG_PHYWAKE | RL_FLAG_DESCV2 | RL_FLAG_MACSTAT;
+ RL_FLAG_PHYWAKE | RL_FLAG_DESCV2 | RL_FLAG_MACSTAT |
+ RL_FLAG_FASTETHER;
break;
case RL_HWREV_8168_SPIN1:
case RL_HWREV_8168_SPIN2:
@@ -2049,30 +2081,14 @@
{
struct rl_softc *sc;
struct mii_data *mii;
- struct ifnet *ifp;
sc = xsc;
- ifp = sc->rl_ifp;
RL_LOCK_ASSERT(sc);
- re_watchdog(sc);
-
mii = device_get_softc(sc->rl_miibus);
mii_tick(mii);
- if ((sc->rl_flags & RL_FLAG_LINK) != 0) {
- if (!(mii->mii_media_status & IFM_ACTIVE))
- sc->rl_flags &= ~RL_FLAG_LINK;
- } else {
- if (mii->mii_media_status & IFM_ACTIVE &&
- IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE) {
- sc->rl_flags |= RL_FLAG_LINK;
- if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
- taskqueue_enqueue_fast(taskqueue_fast,
- &sc->rl_txtask);
- }
- }
-
+ re_watchdog(sc);
callout_reset(&sc->rl_stat_callout, hz, re_tick, sc);
}
@@ -2189,11 +2205,6 @@
re_init_locked(sc);
}
- if (status & RL_ISR_LINKCHG) {
- callout_stop(&sc->rl_stat_callout);
- re_tick(sc);
- }
-
if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
taskqueue_enqueue_fast(taskqueue_fast, &sc->rl_txtask);
@@ -2698,14 +2709,15 @@
{
struct rl_softc *sc;
struct mii_data *mii;
+ int error;
sc = ifp->if_softc;
mii = device_get_softc(sc->rl_miibus);
RL_LOCK(sc);
- mii_mediachg(mii);
+ error = mii_mediachg(mii);
RL_UNLOCK(sc);
- return (0);
+ return (error);
}
/*
@@ -2856,6 +2868,7 @@
re_watchdog(sc)
struct rl_softc *sc;
{
+ struct ifnet *ifp;
RL_LOCK_ASSERT(sc);
@@ -2863,11 +2876,14 @@
return;
device_printf(sc->rl_dev, "watchdog timeout\n");
- sc->rl_ifp->if_oerrors++;
+ ifp = sc->rl_ifp;
+ ifp->if_oerrors++;
re_txeof(sc);
re_rxeof(sc);
re_init_locked(sc);
+ if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
+ taskqueue_enqueue_fast(taskqueue_fast, &sc->rl_txtask);
}
/*
More information about the freebsd-current
mailing list