Re: Random network lossage on nfe(4)

[Quentin Garnier <cube%cubidou.net@localhost>]

On Thu, Apr 17, 2008 at 04:11:54PM -0400, Christos Zoulas wrote:
> On Apr 17, 10:10pm, cube%cubidou.net@localhost (Quentin Garnier) wrote:
> -- Subject: Re: Random network lossage on nfe(4)
> 
> | > So, if putting it back corrupts regular MTU frames I consider this a
> | > serious regression. Considering than 99.9% of the population runs without
> | > jumbo frames, I don't see it as a serious issue. Before my machine was
> | > completely unusable.
> | 
> | What I mean is that it hides the problem.  My 4.99.31 ran just fine with
> | the jumbo frame code in, and I do experience the issue with 4.99.58.
> 
> Hmm, perhaps the change is unrelated then. Can you enable jumbo again and
> see if it still fails?

Can anyone else try the attached patch?  It seems to do the trick for
me, from quick tests.  I'd appreciate a review from someone more
confident that I am with the kmutex_t stuff (notably, is the
mutex_create call correct?).

If my patch is indeed correct, it means the problem appeared when the
kernel was made pre-emptible.

-- 
Quentin Garnier - cube%cubidou.net@localhost - cube%NetBSD.org@localhost
"See the look on my face from staying too long in one place
[...] every time the morning breaks I know I'm closer to falling"
KT Tunstall, Saving My Face, Drastic Fantastic, 2007.

Index: if_nfe.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_nfe.c,v
retrieving revision 1.33
diff -u -u -r1.33 if_nfe.c
--- if_nfe.c    17 Apr 2008 20:16:46 -0000      1.33
+++ if_nfe.c    17 Apr 2008 23:04:20 -0000
@@ -33,6 +33,7 @@
 #include <sys/types.h>
 #include <sys/sockio.h>
 #include <sys/mbuf.h>
+#include <sys/mutex.h>
 #include <sys/queue.h>
 #include <sys/kernel.h>
 #include <sys/device.h>
@@ -112,6 +113,8 @@
 CFATTACH_DECL_NEW(nfe, sizeof(struct nfe_softc), nfe_match, nfe_attach,
     NULL, NULL);
 
+/* #define NFE_NO_JUMBO */
+
 #ifdef NFE_DEBUG
 int nfedebug = 0;
 #define DPRINTF(x)     do { if (nfedebug) printf x; } while (0)
@@ -323,10 +326,12 @@
                    NFE_READ(sc, NFE_PWR2_CTL) & ~NFE_PWR2_WAKEUP_MASK);
        }
 
-#ifdef notyet
+#ifndef NFE_NO_JUMBO
        /* enable jumbo frames for adapters that support it */
-       if (sc->sc_flags & NFE_JUMBO_SUP)
+       if (sc->sc_flags & NFE_JUMBO_SUP) {
                sc->sc_flags |= NFE_USE_JUMBO;
+               mutex_init(&sc->rxq.mtx, MUTEX_SPIN, IPL_NET);
+       }
 #endif
 
        /* Check for reversed ethernet address */
@@ -1545,12 +1550,15 @@
 {
        struct nfe_jbuf *jbuf;
 
+       mutex_enter(&sc->rxq.mtx);
        jbuf = SLIST_FIRST(&sc->rxq.jfreelist);
+       if (jbuf != NULL)
+               SLIST_REMOVE_HEAD(&sc->rxq.jfreelist, jnext);
+       mutex_exit(&sc->rxq.mtx);
        if (jbuf == NULL)
                return NULL;
        sc->rxq.jbufmap[i] =
            ((char *)jbuf->buf - (char *)sc->rxq.jpool) / NFE_JBYTES;
-       SLIST_REMOVE_HEAD(&sc->rxq.jfreelist, jnext);
        return jbuf;
 }
 
@@ -1576,7 +1584,9 @@
        jbuf = &sc->rxq.jbuf[i];
 
        /* ..and put it back in the free list */
+       mutex_enter(&sc->rxq.mtx);
        SLIST_INSERT_HEAD(&sc->rxq.jfreelist, jbuf, jnext);
+       mutex_exit(&sc->rxq.mtx);
 
        if (m != NULL)
                pool_cache_put(mb_cache, m);
Index: if_nfevar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_nfevar.h,v
retrieving revision 1.8
diff -u -u -r1.8 if_nfevar.h
--- if_nfevar.h 17 Apr 2008 19:12:26 -0000      1.8
+++ if_nfevar.h 17 Apr 2008 23:04:20 -0000
@@ -64,6 +64,7 @@
        int                     bufsz;
        int                     cur;
        int                     next;
+       kmutex_t                mtx;
 };
 
 struct nfe_softc {

Attachment: pgpeNCHyVk3__.pgp
Description: PGP signature