network interface monitoring?

Pyun YongHyeon pyunyh at gmail.com
Thu Jan 24 21:36:30 PST 2008


On Thu, Jan 24, 2008 at 01:07:22PM -0500, Yousif Hassan wrote:
 > On Thu, 2008-01-24 at 10:36 -0600, Brooks Davis wrote:
 > > On Thu, Jan 24, 2008 at 10:58:33AM -0500, Yousif Hassan wrote:
 > > > Thank you to all who responded.
 > > > 
 > > > The suggestion was made to use devd or ifstated.  Both sound like
 > > > excellent tools, but I'm currently being tripped up by a core problem -
 > > > both tools rely on the kernel to notify userland of link state changes
 > > > (which makes complete sense!).  This is all well and good - but the
 > > > current issue I'm seeing is that the link state doesn't get updated
 > > > without running "ifconfig" again - is this by design?  A known "issue?"
 > > > 
 > > > An example:
 > > > 1. Unplug network cable from bfe0
 > > > 2. I run ifconfig
 > > > 3. I see that interface bfe0's status is "no carrier".  Good.
 > > > 4. I plug the cable into bfe0
 > > > 5. Wait... wait... look in /var/log/messages... wait more.. NO STATE
 > > > CHANGE - the longest I've waited was 2 minutes, which is already too
 > > > long
 > > > 6. run "ifconfig" again
 > > > 7. Link state immediately changes, logs to /var/log/messages, devd
 > > > scripts run
 > > > 
 > > > Is this a known behavior?  It seems like the link state changes should
 > > > happen automatically, without something to "trigger" them.  Isn't there
 > > > some kind of poll or interrupt sequence?  I'm on 6.3 RC2 (will upgrade
 > > > to 6.3-RELEASE imminently) but can't imagine this code changed?  Does
 > > > this work differently/better in 7.0?
 > > 
 > > It's known but not well understood and is a driver bug.
 > 
 > I was afraid you'd say that.  Thanks for the info.
 > 
 > I found PR kern/109733: [bge] bge link state issues (regression)
 > which, while referring to a different driver, has some of the same
 > symptoms.
 > 
 > In the meantime, I scripted something that calls ifconfig every 30
 > seconds or so, redirected its output to null, and put it in the
 > background and nice'd it; this seems to do the trick, albeit via a
 > horrid hack.  Due to the bug you mentioned, sometimes link state events
 > queue up, too, and get passed to userland at once, which is no kind of
 > fun, but the script still works.
 > 

Try attached patch. I don't know whether it works or not but it
seems that link state was not correctly tracked down by stock bfe(4).
The attached patch does the following.
 - conversion to callout(9) API.
 - added missing lock in bfe_ifmedia_sts().
 - implemented miibus_statchg method to track link state.
 - use our callout to drive watchdog timer.
 - restart Tx routine if pending queued packets are present in
   watchdog handler.
 - fixed blindly resetting watchdog timer in bfe_txeof().

I guess the above is minimal patch to get correct link state.
If I had the hardware I would have rewritten bfe_encap/bfe_newbuf
to use bus_dmamap_load_mbuf_sg(9). :(

-- 
Regards,
Pyun YongHyeon
-------------- next part --------------
--- sys/dev/bfe/if_bfe.c.orig	2007-12-08 10:25:08.000000000 +0900
+++ sys/dev/bfe/if_bfe.c	2008-01-25 13:44:34.000000000 +0900
@@ -97,7 +97,7 @@
 static void bfe_init				(void *);
 static void bfe_init_locked			(void *);
 static void bfe_stop				(struct bfe_softc *);
-static void bfe_watchdog			(struct ifnet *);
+static void bfe_watchdog			(struct bfe_softc *);
 static int  bfe_shutdown			(device_t);
 static void bfe_tick				(void *);
 static void bfe_txeof				(struct bfe_softc *);
@@ -335,6 +335,7 @@
 	sc = device_get_softc(dev);
 	mtx_init(&sc->bfe_mtx, device_get_nameunit(dev), MTX_NETWORK_LOCK,
 			MTX_DEF);
+	callout_init_mtx(&sc->bfe_stat_co, &sc->bfe_mtx, 0);
 
 	unit = device_get_unit(dev);
 	sc->bfe_dev = dev;
@@ -388,7 +389,6 @@
 	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
 	ifp->if_ioctl = bfe_ioctl;
 	ifp->if_start = bfe_start;
-	ifp->if_watchdog = bfe_watchdog;
 	ifp->if_init = bfe_init;
 	ifp->if_mtu = ETHERMTU;
 	IFQ_SET_MAXLEN(&ifp->if_snd, BFE_TX_QLEN);
@@ -410,7 +410,6 @@
 	}
 
 	ether_ifattach(ifp, sc->bfe_enaddr);
-	callout_handle_init(&sc->bfe_stat_ch);
 
 	/*
 	 * Tell the upper layer(s) we support long frames.
@@ -444,13 +443,16 @@
 	sc = device_get_softc(dev);
 
 	KASSERT(mtx_initialized(&sc->bfe_mtx), ("bfe mutex not initialized"));
-	BFE_LOCK(sc);
 
 	ifp = sc->bfe_ifp;
 
 	if (device_is_attached(dev)) {
+		BFE_LOCK(sc);
 		bfe_stop(sc);
-		ether_ifdetach(ifp);
+		BFE_UNLOCK(sc);
+		callout_drain(&sc->bfe_stat_co);
+		if (ifp != NULL)
+			ether_ifdetach(ifp);
 	}
 
 	bfe_chip_reset(sc);
@@ -460,7 +462,6 @@
 		device_delete_child(dev, sc->bfe_miibus);
 
 	bfe_release_resources(sc);
-	BFE_UNLOCK(sc);
 	mtx_destroy(&sc->bfe_mtx);
 
 	return (0);
@@ -548,7 +549,42 @@
 static void
 bfe_miibus_statchg(device_t dev)
 {
-	return;
+	struct bfe_softc *sc;
+	struct mii_data *mii;
+	u_int32_t val, flow;
+
+	sc = device_get_softc(dev);
+	mii = device_get_softc(sc->bfe_miibus);
+
+	if ((mii->mii_media_status & IFM_ACTIVE) != 0) {
+		if (IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)
+			sc->bfe_link = 1;
+	} else
+		sc->bfe_link = 0;
+
+	/* XXX Should stop Rx/Tx engine before chainging MAC. */
+	val = CSR_READ_4(sc, BFE_TX_CTRL);
+	val &= ~BFE_TX_DUPLEX;
+	if ((IFM_OPTIONS(mii->mii_media_active) & IFM_FDX) != 0) {
+		val |= BFE_TX_DUPLEX;
+		flow = 0;
+#ifdef notyet
+		flow = CSR_READ_4(sc, BFE_RXCONF);
+		flow &= ~BFE_RXCONF_FLOW;
+		if ((IFM_OPTIONS(sc->sc_mii->mii_media_active) &
+		    IFM_ETH_RXPAUSE) != 0)
+			flow |= BFE_RXCONF_FLOW;
+		CSR_WRITE_4(sc, BFE_RXCONF, flow);
+		/*
+		 * It seems that the hardware Tx pause issues so enable
+		 * only Rx pause.
+		 */
+		flow = CSR_READ_4(sc, BFE_MAC_FLOW);
+		flow &= ~BFE_FLOW_PAUSE_ENAB;
+		CSR_WRITE_4(sc, BFE_MAC_FLOW, flow);
+#endif
+	}
+	CSR_WRITE_4(sc, BFE_TX_CTRL, val);
 }
 
 static void
@@ -1152,10 +1188,9 @@
 		sc->bfe_tx_cons = i;
 		ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 	}
-	if(sc->bfe_tx_cnt == 0)
-		ifp->if_timer = 0;
-	else
-		ifp->if_timer = 5;
+
+	if (sc->bfe_tx_cnt == 0)
+		sc->bfe_watchdog_timer = 0;
 }
 
 /* Pass a received packet up the stack */
@@ -1412,7 +1447,8 @@
 	if (!sc->bfe_link && ifp->if_snd.ifq_len < 10)
 		return;
 
-	if (ifp->if_drv_flags & IFF_DRV_OACTIVE)
+	if ((ifp->if_drv_flags & (IFF_DRV_RUNNING | IFF_DRV_OACTIVE)) !=
+	    IFF_DRV_RUNNING)
 		return;
 
 	while(sc->bfe_tx_ring[idx].bfe_mbuf == NULL) {
@@ -1448,7 +1484,7 @@
 		/*
 		 * Set a timeout in case the chip goes out to lunch.
 		 */
-		ifp->if_timer = 5;
+		sc->bfe_watchdog_timer = 5;
 	}
 }
 
@@ -1465,9 +1501,12 @@
 {
 	struct bfe_softc *sc = (struct bfe_softc*)xsc;
 	struct ifnet *ifp = sc->bfe_ifp;
+	struct mii_data *mii;
 
 	BFE_LOCK_ASSERT(sc);
 
+	mii = device_get_softc(sc->bfe_miibus);
+
 	if (ifp->if_drv_flags & IFF_DRV_RUNNING)
 		return;
 
@@ -1488,11 +1527,14 @@
 	/* Enable interrupts */
 	CSR_WRITE_4(sc, BFE_IMASK, BFE_IMASK_DEF);
 
-	bfe_ifmedia_upd(ifp);
+	/* Clear link state and change media. */
+	sc->bfe_link = 0;
+	mii_mediachg(mii);
+
 	ifp->if_drv_flags |= IFF_DRV_RUNNING;
 	ifp->if_drv_flags &= ~IFF_DRV_OACTIVE;
 
-	sc->bfe_stat_ch = timeout(bfe_tick, sc, hz);
+	callout_reset(&sc->bfe_stat_co, hz, bfe_tick, sc);
 }
 
 /*
@@ -1503,20 +1545,22 @@
 {
 	struct bfe_softc *sc;
 	struct mii_data *mii;
+	int error;
 
 	sc = ifp->if_softc;
+	BFE_LOCK(sc);
 
 	mii = device_get_softc(sc->bfe_miibus);
-	sc->bfe_link = 0;
 	if (mii->mii_instance) {
 		struct mii_softc *miisc;
 		for (miisc = LIST_FIRST(&mii->mii_phys); miisc != NULL;
 				miisc = LIST_NEXT(miisc, mii_list))
 			mii_phy_reset(miisc);
 	}
-	mii_mediachg(mii);
+	error = mii_mediachg(mii);
+	BFE_UNLOCK(sc);
 
-	return (0);
+	return (error);
 }
 
 /*
@@ -1528,10 +1572,12 @@
 	struct bfe_softc *sc = ifp->if_softc;
 	struct mii_data *mii;
 
+	BFE_LOCK(sc);
 	mii = device_get_softc(sc->bfe_miibus);
 	mii_pollstat(mii);
 	ifmr->ifm_active = mii->mii_media_active;
 	ifmr->ifm_status = mii->mii_media_status;
+	BFE_UNLOCK(sc);
 }
 
 static int
@@ -1576,22 +1622,25 @@
 }
 
 static void
-bfe_watchdog(struct ifnet *ifp)
+bfe_watchdog(struct bfe_softc *sc)
 {
-	struct bfe_softc *sc;
+	struct ifnet *ifp;
 
-	sc = ifp->if_softc;
+	BFE_LOCK_ASSERT(sc);
 
-	BFE_LOCK(sc);
+	if (sc->bfe_watchdog_timer == 0 || --sc->bfe_watchdog_timer)
+		return;
+
+	ifp = sc->bfe_ifp;
 
 	printf("bfe%d: watchdog timeout -- resetting\n", sc->bfe_unit);
 
+	ifp->if_oerrors++;
 	ifp->if_drv_flags &= ~IFF_DRV_RUNNING;
 	bfe_init_locked(sc);
 
-	ifp->if_oerrors++;
-
-	BFE_UNLOCK(sc);
+	if (!IFQ_DRV_IS_EMPTY(&ifp->if_snd))
+		bfe_start_locked(ifp);
 }
 
 static void
@@ -1600,27 +1649,13 @@
 	struct bfe_softc *sc = xsc;
 	struct mii_data *mii;
 
-	if (sc == NULL)
-		return;
-
-	BFE_LOCK(sc);
+	BFE_LOCK_ASSERT(sc);
 
 	mii = device_get_softc(sc->bfe_miibus);
-
-	bfe_stats_update(sc);
-	sc->bfe_stat_ch = timeout(bfe_tick, sc, hz);
-
-	if(sc->bfe_link) {
-		BFE_UNLOCK(sc);
-		return;
-	}
-
 	mii_tick(mii);
-	if (!sc->bfe_link && mii->mii_media_status & IFM_ACTIVE &&
-			IFM_SUBTYPE(mii->mii_media_active) != IFM_NONE)
-		sc->bfe_link++;
-
-	BFE_UNLOCK(sc);
+	bfe_stats_update(sc);
+	bfe_watchdog(sc);
+	callout_reset(&sc->bfe_stat_co, hz, bfe_tick, sc);
 }
 
 /*
@@ -1634,13 +1669,13 @@
 
 	BFE_LOCK_ASSERT(sc);
 
-	untimeout(bfe_tick, sc, sc->bfe_stat_ch);
-
 	ifp = sc->bfe_ifp;
+	ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
+	sc->bfe_link = 0;
+	callout_stop(&sc->bfe_stat_co);
+	sc->bfe_watchdog_timer = 0;
 
 	bfe_chip_halt(sc);
 	bfe_tx_ring_free(sc);
 	bfe_rx_ring_free(sc);
-
-	ifp->if_drv_flags &= ~(IFF_DRV_RUNNING | IFF_DRV_OACTIVE);
 }
--- sys/dev/bfe/if_bfereg.h.orig	2006-05-29 03:44:39.000000000 +0900
+++ sys/dev/bfe/if_bfereg.h	2008-01-25 13:36:23.000000000 +0900
@@ -73,6 +73,10 @@
 #define BFE_CTRL_LED        0x000000e0 /* Onchip EPHY LED Control */
 #define BFE_CTRL_LED_SHIFT  5
 
+#define BFE_MAC_FLOW        0x000000AC /* MAC Flow Control */
+#define BFE_FLOW_RX_HIWAT   0x000000ff /* Onchip FIFO HI Water Mark */
+#define BFE_FLOW_PAUSE_ENAB 0x00008000 /* Enable Pause Frame Generation */
+
 #define BFE_RCV_LAZY        0x00000100 /* Lazy Interrupt Control */
 #define BFE_LAZY_TO_MASK    0x00ffffff /* Timeout */
 #define BFE_LAZY_FC_MASK    0xff000000 /* Frame Count */
@@ -504,7 +508,7 @@
     void                    *bfe_intrhand;
     struct resource         *bfe_irq;
     struct resource         *bfe_res;
-    struct callout_handle   bfe_stat_ch;
+    struct callout          bfe_stat_co;
     struct bfe_hw_stats     bfe_hwstats;
     struct bfe_desc         *bfe_tx_list, *bfe_rx_list;
     struct bfe_data         bfe_tx_ring[BFE_TX_LIST_CNT]; /* XXX */
@@ -517,6 +521,7 @@
     u_int32_t               bfe_rx_cnt, bfe_rx_prod, bfe_rx_cons;
     u_int32_t               bfe_tx_dma, bfe_rx_dma;
     u_int32_t               bfe_link;
+    int                     bfe_watchdog_timer;
     u_int8_t                bfe_phyaddr; /* Address of the card's PHY */
     u_int8_t                bfe_mdc_port;
     u_int8_t                bfe_unit;   /* interface number */


More information about the freebsd-net mailing list