Subject: Re: Fix for bug in vr(4)
To: None <tech-kern@NetBSD.org>
From: Julio M. Merino Vidal <jmmv84@gmail.com>
List: tech-kern
Date: 01/22/2005 20:28:11
On Sat, 2005-01-22 at 17:27 +0100, Julio M. Merino Vidal wrote:

> So after the whole day looking for the problem, I found it.  For some
> reason, the card gets zero-length packets which, when passed to bpf,
> cause the failure.  These packets always come with the VR_RXSTAT_RLINK
> flag active, and sometimes with VR_RXSTAT_FIRSTFRAG too.  (I can
> understand the meaning of the later, but not the former.)
> 
> I don't know why this happens, but ignoring these packets avoids the
> crashes.  However, just ignoring them leaves the card in a state which
> is inconsistent.  The driver will keep "receiving" these packets
> periodically and the card does not work (it won't transmit anything).
> 
> Issuing a reset command after receiving any of these zero-length
> packets solves the problem.  After that, the driver does not receive
> them any more and the card works fine.  (I.e., the conditional gets
> triggered only once.)

I've been pointed out that this is not the correct solution (haha, I
thought that).  So, due to the lack of public documentation (*sigh*),
I've looked at what the Linux driver does.

They ensure that every packet comes with the VR_RXSTAT_FIRSTFRAG and
VR_RXSTAT_LASTFRAG bits set.  Otherwise, they treat it as an "oversized
ethernet frame" error.

Indeed, adding this check captures the problem, and if I discard those
packets as erroneous, everything works fine later on (no need for an
ugly reset).  The patch that does this is pasted below.

However...  I've tried the same commands that here produce the failure
under Linux but saw nothing strange (aside from some mysterious
delays).  It may be because their messages are printed at "warning"
level and my current setting is higher than that...  Haven't used linux
for a long while, so I don't know how to change/verify this ;)

Does this look more reasonable?  At least it seems to me.

Cheers,

Index: if_vr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_vr.c,v
retrieving revision 1.71
diff -u -r1.71 if_vr.c
--- if_vr.c	13 Jan 2005 14:51:28 -0000	1.71
+++ if_vr.c	22 Jan 2005 20:22:51 -0000
@@ -643,6 +643,18 @@
 			VR_INIT_RXDESC(sc, i);
 
 			continue;
+		} else if (!(rxstat & VR_RXSTAT_FIRSTFRAG) ||
+		           !(rxstat & VR_RXSTAT_LASTFRAG)) {
+			ifp->if_ierrors++;
+
+			printf("%s: receive error: oversized ethernet frame; "
+			       "size = %d, status = 0x%x\n",
+			       sc->vr_dev.dv_xname,
+			       VR_RXBYTES(le32toh(d->vr_status)), rxstat);
+
+			VR_INIT_RXDESC(sc, i);
+
+			continue;
 		}
 
 		bus_dmamap_sync(sc->vr_dmat, ds->ds_dmamap, 0,
@@ -650,6 +662,7 @@
 
 		/* No errors; receive the packet. */
 		total_len = VR_RXBYTES(le32toh(d->vr_status));
+		KASSERT(total_len > 0);
 
 #ifdef __NO_STRICT_ALIGNMENT
 		/*

-- 
Julio M. Merino Vidal <jmmv84@gmail.com>
http://www.livejournal.com/users/jmmv/
The NetBSD Project - http://www.NetBSD.org/