Subject: nfe(4) fixes
To: None <current-users@netbsd.org>
From: Quentin Garnier <cube@cubidou.net>
List: tech-kern
Date: 09/23/2007 21:32:48
--HvxUWqUQYk36AMou
Content-Type: multipart/mixed; boundary="LvG3mvTpA2cBjRlj"
Content-Disposition: inline
--LvG3mvTpA2cBjRlj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Hi,
I wonder if many other people experience the same troubles I do with
nfe(4). The attached patch makes it stable for me. Well, stable to
the amount of my testing: previously, cvs up'ing src was enough to
kill it in a matter of seconds, and now I'm unable to make it trip
through a cvs up or a rsync of the repository.
Nevermind the event counters I added, it just helped me figure out a
bit what was happening (or rather, not happening) in the driver.
Here's a list of the issues the patch tries to fix:
- in the case the chip doesn't support jumbo frames, the replacement
mbuf was mapped with bus_dmamap_load_mbuf, which is wrong (at least
according to the source of that function): it expects a real mbuf
chain, not a blank mbuf with an unspecified packet size.
- in the case the chip does support jumbo frames, there are two
issues:
o when the driver runs out of mbufs, it jumps to the label skip,
thus not writing again the physaddr field of the descriptor,
although apparently that field might have been changed by the
hardware (e.g., this is where it stores the VLAN ID when it is
programmed to strip it). In order to fix that, I introduce a
reverse mapping of the mbuf storage space so that we can fetch
back the physical address. Maybe there are better ways to do
that.
o when it runs out of jumbo mbufs, the driver just skips new
packets, potentially deadlocking if the 64 extra mbufs are
still busy. Therefore, if the packet is small enough, let's
get a mbuf cluster for it. That way, unless you manage to
only receive jumbo mbufs, things will eventually cool down when
stressed.
The reason why I cared about non-jumbo support is because I first tried
to make my hardware work while ignoring jumbo support (which I don't use
anyway) by defining NFE_NO_JUMBO at the top of the source file.
Comments before I commit? I might remove the event counters, they're
not very useful now.
--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"You could have made it, spitting out benchmarks
Owe it to yourself not to fail"
Amplifico, Spitting Out Benchmarks, Hometakes Vol. 2, 2005.
--LvG3mvTpA2cBjRlj
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="nfe.diff"
Content-Transfer-Encoding: quoted-printable
Index: if_nfe.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/pci/if_nfe.c,v
retrieving revision 1.17
diff -u -p -r1.17 if_nfe.c
--- if_nfe.c 1 Sep 2007 07:32:30 -0000 1.17
+++ if_nfe.c 23 Sep 2007 19:17:51 -0000
@@ -38,6 +38,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_nfe.c,v 1
#include <sys/kernel.h>
#include <sys/device.h>
#include <sys/socket.h>
+#include <sys/evcnt.h>
=20
#include <machine/bus.h>
=20
@@ -94,7 +95,7 @@ void nfe_start(struct ifnet *);
void nfe_watchdog(struct ifnet *);
int nfe_init(struct ifnet *);
void nfe_stop(struct ifnet *, int);
-struct nfe_jbuf *nfe_jalloc(struct nfe_softc *);
+struct nfe_jbuf *nfe_jalloc(struct nfe_softc *, int);
void nfe_jfree(struct mbuf *, void *, size_t, void *);
int nfe_jpool_alloc(struct nfe_softc *);
void nfe_jpool_free(struct nfe_softc *);
@@ -124,6 +125,18 @@ int nfedebug =3D 0;
#define DPRINTFN(n,x)
#endif
=20
+#ifdef NFE_DEBUG
+static struct evcnt nfe_jallocs =3D
+ EVCNT_INITIALIZER(EVCNT_TYPE_MISC, NULL, "nfe", "jalloc");
+static struct evcnt nfe_jfrees =3D
+ EVCNT_INITIALIZER(EVCNT_TYPE_MISC, NULL, "nfe", "jfree");
+static struct evcnt nfe_failed_jallocs =3D
+ EVCNT_INITIALIZER(EVCNT_TYPE_MISC, NULL, "nfe", "jalloc failed");
+EVCNT_ATTACH_STATIC(nfe_jallocs);
+EVCNT_ATTACH_STATIC(nfe_jfrees);
+EVCNT_ATTACH_STATIC(nfe_failed_jallocs);
+#endif
+
/* deal with naming differences */
=20
#define PCI_PRODUCT_NVIDIA_NFORCE3_LAN2 \
@@ -765,18 +778,34 @@ nfe_rxeof(struct nfe_softc *sc)
}
=20
if (sc->sc_flags & NFE_USE_JUMBO) {
- if ((jbuf =3D nfe_jalloc(sc)) =3D=3D NULL) {
- m_freem(mnew);
- ifp->if_ierrors++;
- goto skip;
- }
- MEXTADD(mnew, jbuf->buf, NFE_JBYTES, 0, nfe_jfree, sc);
+ physaddr =3D
+ sc->rxq.jbuf[sc->rxq.jbufmap[i]].physaddr;
+ if ((jbuf =3D nfe_jalloc(sc, i)) =3D=3D NULL) {
+ if (len > MCLBYTES) {
+ m_freem(mnew);
+ ifp->if_ierrors++;
+ goto skip1;
+ }
+ MCLGET(mnew, M_DONTWAIT);
+ if ((mnew->m_flags & M_EXT) =3D=3D 0) {
+ m_freem(mnew);
+ ifp->if_ierrors++;
+ goto skip1;
+ }
=20
- bus_dmamap_sync(sc->sc_dmat, sc->rxq.jmap,
- mtod(data->m, char *) - (char *)sc->rxq.jpool,
- NFE_JBYTES, BUS_DMASYNC_POSTREAD);
+ memcpy(mtod(mnew, void *),
+ mtod(data->m, const void *), len);
+ m =3D mnew;
+ goto mbufcopied;
+ } else {
+ MEXTADD(mnew, jbuf->buf, NFE_JBYTES, 0, nfe_jfree, sc);
+
+ bus_dmamap_sync(sc->sc_dmat, sc->rxq.jmap,
+ mtod(data->m, char *) - (char *)sc->rxq.jpool,
+ NFE_JBYTES, BUS_DMASYNC_POSTREAD);
=20
- physaddr =3D jbuf->physaddr;
+ physaddr =3D jbuf->physaddr;
+ }
} else {
MCLGET(mnew, M_DONTWAIT);
if ((mnew->m_flags & M_EXT) =3D=3D 0) {
@@ -789,14 +818,15 @@ nfe_rxeof(struct nfe_softc *sc)
data->map->dm_mapsize, BUS_DMASYNC_POSTREAD);
bus_dmamap_unload(sc->sc_dmat, data->map);
=20
- error =3D bus_dmamap_load_mbuf(sc->sc_dmat, data->map,
- mnew, BUS_DMA_READ | BUS_DMA_NOWAIT);
+ error =3D bus_dmamap_load(sc->sc_dmat, data->map,
+ mtod(mnew, void *), MCLBYTES, NULL,
+ BUS_DMA_READ | BUS_DMA_NOWAIT);
if (error !=3D 0) {
m_freem(mnew);
=20
/* try to reload the old mbuf */
- error =3D bus_dmamap_load_mbuf(sc->sc_dmat,
- data->map, data->m,
+ error =3D bus_dmamap_load(sc->sc_dmat, data->map,
+ mtod(data->m, void *), MCLBYTES, NULL,
BUS_DMA_READ | BUS_DMA_NOWAIT);
if (error !=3D 0) {
/* very unlikely that it will fail.. */
@@ -816,6 +846,7 @@ nfe_rxeof(struct nfe_softc *sc)
m =3D data->m;
data->m =3D mnew;
=20
+mbufcopied:
/* finalize mbuf */
m->m_pkthdr.len =3D m->m_len =3D len;
m->m_pkthdr.rcvif =3D ifp;
@@ -853,6 +884,7 @@ nfe_rxeof(struct nfe_softc *sc)
ifp->if_ipackets++;
(*ifp->if_input)(ifp, m);
=20
+skip1:
/* update mapping address in h/w descriptor */
if (sc->sc_flags & NFE_40BIT_ADDR) {
#if defined(__LP64__)
@@ -1090,6 +1122,9 @@ nfe_start(struct ifnet *ifp)
int old =3D sc->txq.queued;
struct mbuf *m0;
=20
+ if ((ifp->if_flags & (IFF_OACTIVE | IFF_RUNNING)) !=3D IFF_RUNNING)
+ return;
+
for (;;) {
IFQ_POLL(&ifp->if_snd, m0);
if (m0 =3D=3D NULL)
@@ -1368,7 +1403,7 @@ nfe_alloc_rx_ring(struct nfe_softc *sc,=20
}
=20
if (sc->sc_flags & NFE_USE_JUMBO) {
- if ((jbuf =3D nfe_jalloc(sc)) =3D=3D NULL) {
+ if ((jbuf =3D nfe_jalloc(sc, i)) =3D=3D NULL) {
printf("%s: could not allocate jumbo buffer\n",
sc->sc_dev.dv_xname);
goto fail;
@@ -1489,13 +1524,22 @@ nfe_free_rx_ring(struct nfe_softc *sc, s
}
=20
struct nfe_jbuf *
-nfe_jalloc(struct nfe_softc *sc)
+nfe_jalloc(struct nfe_softc *sc, int i)
{
struct nfe_jbuf *jbuf;
=20
jbuf =3D SLIST_FIRST(&sc->rxq.jfreelist);
- if (jbuf =3D=3D NULL)
+ if (jbuf =3D=3D NULL) {
+#ifdef NFE_DEBUG
+ nfe_failed_jallocs.ev_count++;
+#endif
return NULL;
+ }
+ sc->rxq.jbufmap[i] =3D
+ ((char *)jbuf->buf - (char *)sc->rxq.jpool) / NFE_JBYTES;
+#ifdef NFE_DEBUG
+ nfe_jallocs.ev_count++;
+#endif
SLIST_REMOVE_HEAD(&sc->rxq.jfreelist, jnext);
return jbuf;
}
@@ -1512,6 +1556,10 @@ nfe_jfree(struct mbuf *m, void *buf, siz
struct nfe_jbuf *jbuf;
int i;
=20
+#ifdef NFE_DEBUG
+ nfe_jfrees.ev_count++;
+#endif
+
/* find the jbuf from the base pointer */
i =3D ((char *)buf - (char *)sc->rxq.jpool) / NFE_JBYTES;
if (i < 0 || i >=3D NFE_JPOOL_COUNT) {
Index: if_nfevar.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/pci/if_nfevar.h,v
retrieving revision 1.2
diff -u -p -r1.2 if_nfevar.h
--- if_nfevar.h 4 Mar 2007 06:02:22 -0000 1.2
+++ if_nfevar.h 23 Sep 2007 19:17:51 -0000
@@ -59,6 +59,7 @@ struct nfe_rx_ring {
void * jpool;
struct nfe_rx_data data[NFE_RX_RING_COUNT];
struct nfe_jbuf jbuf[NFE_JPOOL_COUNT];
+ int jbufmap[NFE_RX_RING_COUNT];
SLIST_HEAD(, nfe_jbuf) jfreelist;
int bufsz;
int cur;
--LvG3mvTpA2cBjRlj--
--HvxUWqUQYk36AMou
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (NetBSD)
iQEVAwUBRva/X9goQloHrPnoAQL+8AgAxFtMB5KgRazegqt8mOuy3MURe9G0VXyi
EkFtWopObz7C/AxYy2BMps8gpc1CAsinZHIxaU5iI7jTTjTHdKKCkQ1V2+Q6s3BB
O5G7ivB7DgINH8Kh20kQ0M2DuDvXKbXZKGak8xwCQsi7Rz6/UHfrquYne+wX4u2f
k08XqfhEZMYZQy7pt2Z9OoAM3qUYEPSvaG5frgQmya60Fhgpdbx0W1urZo7jP/AX
mSYBxigjKoRomLlEfUek+0yFWAL+1J+FwPK3drOllUPsYrO7IWFtkx9sWz/Z/kUW
c2RxqLnfEwuWJ2sRtVO3VczvNsQ7fY0D8tT3ktUSDHgOwk2DhiLreg==
=WJWN
-----END PGP SIGNATURE-----
--HvxUWqUQYk36AMou--