Date: Fri, 25 Jan 2008 14:10:59 +0900 From: Pyun YongHyeon <pyunyh@gmail.com> To: Yousif Hassan <yousif@alumni.jmu.edu> Cc: freebsd-net@freebsd.org, Brooks Davis <brooks@freebsd.org>, "Bruce M. Simpson" <bms@freebsd.org> Subject: Re: network interface monitoring? Message-ID: <20080125051059.GA22410@cdnetworks.co.kr> In-Reply-To: <1201198042.19736.5.camel@localhost> References: <1201125022.2106.67.camel@localhost> <20080123222047.GA14264@lor.one-eyed-alien.net> <1201190313.2591.7.camel@localhost> <20080124163634.GA25331@lor.one-eyed-alien.net> <1201198042.19736.5.camel@localhost>
next in thread | previous in thread | raw e-mail | index | archive | help
--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
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
--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="bfe.patch"
--- 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 */
--6c2NcOVqGQ03X4Wi--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080125051059.GA22410>