Date: Mon, 29 Jul 2002 09:37:16 -0700 From: Luigi Rizzo <rizzo@icir.org> To: Ander Song <song@rdsk.net> Cc: Guy Helmer <ghelmer@palisadesys.com>, freebsd-net@FreeBSD.ORG Subject: Re: Crashes in fxp driver with polling enabled Message-ID: <20020729093716.A43572@iguana.icir.org> In-Reply-To: <3D4531DB.2B59416C@rdsk.net>; from song@rdsk.net on Mon, Jul 29, 2002 at 08:15:23PM %2B0800 References: <3D33B8C6.2FF275F1@rdsk.net> <FPEBKMIFGFHCGLLKBLMMCEPJCAAA.ghelmer@palisadesys.com> <20020729013321.C38758@iguana.icir.org> <3D4531DB.2B59416C@rdsk.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 29, 2002 at 08:15:23PM +0800, Ander Song wrote:
> The patch should work. Here we just set count = -1 when RNR happen (just
i thought about that, but it seems to defeat one of the purposes
of polling (at least they way i implemented it), which is to control
the amount of CPU dedicated to network vs. non-network processing.
Basically, you get RNR under heavy (but controlled) non-network
load, and if your response is to set count = -1 you end up giving
more resources to the network processing than you really want.
> before rcvloop), so all the packet will be fetched (so the patch is only
> 2 lines). It seems that the uninitialized rfa_status fields can cause
> the bad DMA operation. For testing purpose, we zero all the rfa_status
> and everything is fine even without the count = -1 patch.
The thing is, the driver does initialize rfa_status to zero before loading
the buffer, and the chip sets it when a packet is received.
The problem leading to the panic is (i believe) that the old driver
gave a restart command to the receive unit on buffers which had
already the C bit set, so the would be used by the network
controller and by the protocol stack. The former would
trash the status & length fields, the latter would trash the
physical address pointers in the rfa.
cheers
luigi
>
> Luigi Rizzo wrote:
> >
> > On Tue, Jul 16, 2002 at 08:52:02AM -0500, Guy Helmer wrote:
> > > On Tuesday, July 16, 2002 1:10 AM, Song Bo Run [mailto:song@rdsk.net] wrote:
> > > > Hello, Guy
> > > >
> > > > Here we are encountering almost exactly the same problem as you are,
> > > > except that we are using OpenBSD. We have ported Luigi's polling code
> > > > to OpenBSD 2.9 and are using fxp driver. Our testing attack is just ping
> > > > -f with 65500 bytes data.
> >
> > could you try the following patch and see if it
> > fixes the problem ? I have tested it on a few boxes which failed
> > without it and they can sustain the attack with this patch.
> >
> > cheers
> > luigi
> >
> > Index: if_fxp.c
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/dev/fxp/if_fxp.c,v
> > retrieving revision 1.110.2.22
> > diff -u -b -w -r1.110.2.22 if_fxp.c
> > --- if_fxp.c 9 Jul 2002 00:37:42 -0000 1.110.2.22
> > +++ if_fxp.c 29 Jul 2002 07:56:50 -0000
> > @@ -245,6 +245,9 @@
> > DRIVER_MODULE(if_fxp, cardbus, fxp_driver, fxp_devclass, 0, 0);
> > DRIVER_MODULE(miibus, fxp, miibus_driver, miibus_devclass, 0, 0);
> >
> > +static int fxp_rnr;
> > +SYSCTL_INT(_hw, OID_AUTO, fxp_rnr, CTLFLAG_RW, &fxp_rnr,0,"fxp rnr events");
> > +
> > /*
> > * Inline function to copy a 16-bit aligned 32-bit quantity.
> > */
> > @@ -1268,10 +1271,20 @@
> > * Process receiver interrupts. If a no-resource (RNR)
> > * condition exists, get whatever packets we can and
> > * re-start the receiver.
> > + * When using polling, we do not process the list to completion,
> > + * so when we get an RNR interrupt we must defer the restart
> > + * until we hit the last buffer with the C bit set.
> > + * If we run out of cycles and rfa_headm has the C bit set,
> > + * record the pending RNR in an unused status bit, so that the
> > + * info will be used in the subsequent polling cycle.
> > */
> > if (statack & (FXP_SCB_STATACK_FR | FXP_SCB_STATACK_RNR)) {
> > struct mbuf *m;
> > struct fxp_rfa *rfa;
> > + int rnr = (statack & FXP_SCB_STATACK_RNR) ? 1 : 0;
> > +
> > + if (rnr)
> > + fxp_rnr++;
> > rcvloop:
> > m = sc->rfa_headm;
> > rfa = (struct fxp_rfa *)(m->m_ext.ext_buf +
> > @@ -1281,6 +1294,9 @@
> > if (count < 0 || count-- > 0)
> > #endif /* DEVICE_POLLING */
> > if (rfa->rfa_status & FXP_RFA_STATUS_C) {
> > +#define FXP_RFA_RNRMARK 0x4000 /* used to mark a pending RNR intr */
> > + if (rfa->rfa_status & FXP_RFA_RNRMARK)
> > + rnr = 1;
> > /*
> > * Remove first packet from the chain.
> > */
> > @@ -1328,7 +1344,10 @@
> > }
> > goto rcvloop;
> > }
> > - if (statack & FXP_SCB_STATACK_RNR) {
> > + if (rnr) {
> > + if (rfa->rfa_status & FXP_RFA_STATUS_C)
> > + rfa->rfa_status |= FXP_RFA_RNRMARK;
> > + else {
> > fxp_scb_wait(sc);
> > CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
> > vtophys(sc->rfa_headm->m_ext.ext_buf) +
> > @@ -1336,6 +1355,7 @@
> > fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
> > }
> > }
> > + }
> > }
> >
> > /*
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020729093716.A43572>