Re: mpsafe tx for re(4)

[Dimitris Papastamos]

On Mon, Dec 28, 2015 at 09:13:34PM +1000, David Gwynne wrote:
> this builds on jmatthew@'s last commit and adds mpsafe tx.
> 
> ive beat on it pretty hard, but more eyes/tests are appreciated.
> 
> ok?

Been running with this on my router since yesterday without issues.
I will continue to run with this patch and report back if anything
goes wrong.

mpsafe tx for re(4)

[David Gwynne]

this builds on jmatthew@'s last commit and adds mpsafe tx.

ive beat on it pretty hard, but more eyes/tests are appreciated.

ok?

Index: re.c
===
RCS file: /cvs/src/sys/dev/ic/re.c,v
retrieving revision 1.188
diff -u -p -r1.188 re.c
--- re.c28 Dec 2015 05:49:15 -  1.188
+++ re.c28 Dec 2015 07:51:23 -
@@ -1042,6 +1042,7 @@ re_attach(struct rl_softc *sc, const cha
ifp->if_softc = sc;
strlcpy(ifp->if_xname, sc->sc_dev.dv_xname, IFNAMSIZ);
ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
+   ifp->if_xflags = IFXF_MPSAFE;
ifp->if_ioctl = re_ioctl;
ifp->if_start = re_start;
ifp->if_watchdog = re_watchdog;
@@ -1497,12 +1498,12 @@ re_txeof(struct rl_softc *sc)
 * to restart the channel here to flush them out. This only
 * seems to be required with the PCIe devices.
 */
-   if (tx_free < sc->rl_ldata.rl_tx_desc_cnt)
+   if (ifq_is_oactive(>if_snd))
+   ifq_restart(>if_snd);
+   else if (tx_free < sc->rl_ldata.rl_tx_desc_cnt)
CSR_WRITE_1(sc, sc->rl_txstart, RL_TXSTART_START);
-   else {
-   ifq_clr_oactive(>if_snd);
+   else
ifp->if_timer = 0;
-   }
 
return (1);
 }
@@ -1589,7 +1590,7 @@ re_intr(void *arg)
 * masks.
 */
re_rxeof(sc);
-   tx = re_txeof(sc);
+   re_txeof(sc);
} else
CSR_WRITE_4(sc, RL_TIMERCNT, 1); /* reload */
} else if (tx | rx) {
@@ -1602,12 +1603,6 @@ re_intr(void *arg)
}
}
 
-   if (!IFQ_IS_EMPTY(>if_snd)) {
-   KERNEL_LOCK();
-   re_start(ifp);
-   KERNEL_UNLOCK();
-   }
-
CSR_WRITE_2(sc, RL_IMR, sc->rl_intrs);
 
return (claimed);
@@ -1691,7 +1686,7 @@ re_encap(struct rl_softc *sc, struct mbu
 
/* FALLTHROUGH */
default:
-   return (ENOBUFS);
+   return (ENOMEM);
}
 
nsegs = map->dm_nsegs;
@@ -1715,8 +1710,8 @@ re_encap(struct rl_softc *sc, struct mbu
nsegs++;
}
 
-   if (sc->rl_ldata.rl_tx_free - (*used + nsegs) <= 1) {
-   error = EFBIG;
+   if (*used + nsegs + 1 >= sc->rl_ldata.rl_tx_free) {
+   error = ENOBUFS;
goto fail_unload;
}
 
@@ -1840,36 +1835,41 @@ re_start(struct ifnet *ifp)
struct mbuf *m;
int idx, used = 0, txq_free, error;
 
-   if (!(ifp->if_flags & IFF_RUNNING) || ifq_is_oactive(>if_snd))
-   return;
-   if ((sc->rl_flags & RL_FLAG_LINK) == 0)
+   if (!ISSET(sc->rl_flags, RL_FLAG_LINK)) {
+   IFQ_PURGE(>if_snd);
return;
+   }
 
txq_free = sc->rl_ldata.rl_txq_considx;
idx = sc->rl_ldata.rl_txq_prodidx;
-   if (idx >= txq_free)
+   if (txq_free <= idx)
txq_free += RL_TX_QLEN;
txq_free -= idx;
 
-   while (txq_free > 1) {
+   for (;;) {
+   if (txq_free <= 1) {
+   ifq_set_oactive(>if_snd);
+   break;
+   }
+
m = ifq_deq_begin(>if_snd);
if (m == NULL)
break;
 
error = re_encap(sc, m, >rl_ldata.rl_txq[idx], );
-   if (error != 0 && error != ENOBUFS) {
+   if (error == 0)
+   ifq_deq_commit(>if_snd, m);
+   else if (error == ENOBUFS) {
ifq_deq_rollback(>if_snd, m);
ifq_set_oactive(>if_snd);
break;
-   } else if (error != 0) {
+   } else {
ifq_deq_commit(>if_snd, m);
m_freem(m);
ifp->if_oerrors++;
continue;
}
 
-   ifq_deq_commit(>if_snd, m);
-
 #if NBPFILTER > 0
if (ifp->if_bpf)
bpf_mtap_ether(ifp->if_bpf, m, BPF_DIRECTION_OUT);
@@ -1882,10 +1882,11 @@ re_start(struct ifnet *ifp)
return;

ifp->if_timer = 5;
-   sc->rl_ldata.rl_txq_prodidx = idx;
atomic_sub_int(>rl_ldata.rl_tx_free, used);
KASSERT(sc->rl_ldata.rl_tx_free >= 0);
 
+   sc->rl_ldata.rl_txq_prodidx = idx;
+
CSR_WRITE_1(sc, sc->rl_txstart, RL_TXSTART_START);
 }
 
@@ -2112,7 +2113,6 @@ re_watchdog(struct ifnet *ifp)
sc = ifp->if_softc;
s = splnet();
printf("%s: watchdog timeout\n", sc->sc_dev.dv_xname);
-   ifp->if_oerrors++;
 
re_txeof(sc);
re_rxeof(sc);
@@ -2140,7 +2140,6 @@ re_stop(struct ifnet *ifp)