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>