Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 14 Jul 2008 10:36:42 +0900
From:      Pyun YongHyeon <pyunyh@gmail.com>
To:        Dimitry Andric <dimitry@andric.com>
Cc:        freebsd-current@FreeBSD.org
Subject:   Re: Call for testers: re(4) and RTL8168C/RTL8168CP/RTL8111C/RTL8111CP
Message-ID:  <20080714013642.GF36245@cdnetworks.co.kr>
In-Reply-To: <20080714013519.GE36245@cdnetworks.co.kr>
References:  <20080609012657.GD12521@cdnetworks.co.kr> <484D215A.7050700@andric.com> <20080609123206.GF12521@cdnetworks.co.kr> <484D25CC.9050106@andric.com> <20080610050550.GB17874@cdnetworks.co.kr> <484E9377.2050609@andric.com> <20080611005814.GA3529@cdnetworks.co.kr> <48666CD7.9020706@andric.com> <20080630043156.GB79537@cdnetworks.co.kr> <20080714013519.GE36245@cdnetworks.co.kr>

next in thread | previous in thread | raw e-mail | index | archive | help

--oyUTqETQ0mS9luUI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--oyUTqETQ0mS9luUI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="re.link.patch3"

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);
 }
 
 /*

--oyUTqETQ0mS9luUI--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080714013642.GF36245>