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
--- 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 */
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to