Linux MIPS Architecture development
 help / color / mirror / Atom feed
* Possible thinko in driver/net/sb1250-mac.c?
@ 2005-02-12 13:07 Richard Sandiford
  0 siblings, 0 replies; only message in thread
From: Richard Sandiford @ 2005-02-12 13:07 UTC (permalink / raw)
  To: linux-mips

[ As usual, I have a feeling I'm either showing my ignorance or
  going over well-trodden ground, but here goes.. ]

I was looking at driver/net/sb1250-mac.c, and I noticed that it
effectively maintains a gap of _two_ empty buffers in the receive ring.
As expected, sbdma_fillring() sets things up so that the gap is only a
single buffer (assuming enough free memory):

        for (idx = 0; idx < SBMAC_MAX_RXDESCR-1; idx++) {
                if (sbdma_add_rcvbuffer(d,NULL) != 0)
                        break;
        }

but the first time sbdma_rx_process() is called, it fails to replace the
buffer it reads with a new one.  The reason is that sbdma_rx_process()
is structured like this:

        if (packet in sb was received OK) {
                if (sbdma_add_rcvbuffer(d,NULL) == -ENOBUFS) {
                        ... Drop the packet and add sb back to the ring ...
                        sbdma_add_rcvbuffer(d,sb);
                } else {
                        ... Hand sb off to netif_rx ...
                }
        } else {
                ... Record the error and add sb back to the ring ...
                sbdma_add_rcvbuffer(d,sb);
        }
        d->sbdma_remptr = SBDMA_NEXTBUF(d,sbdma_remptr);

where sbdma_remptr is only updated _after_ calling
sbdma_add_rcvbuffer().  But sbdma_add_rcvbuffer() uses
sbdma_remptr to check whether the ring is full:

	dsc = d->sbdma_addptr;
	nextdsc = SBDMA_NEXTBUF(d,sbdma_addptr);
	if (nextdsc == d->sbdma_remptr) {
		return -ENOSPC;
	}

So when sbdma_add_rcvbuffer() is called for the first time after
sbdma_fillring(), the call to sbdma_add_rcvbuffer() will fail
with ENOSPC (verified with various printk()s) and no buffer will
be added.

I guess this doesn't matter much if the first packet is received OK.
But if it isn't, and the receive code takes the error path, I think
it'll end up leaking a buffer.

Richard

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2005-02-12 13:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-12 13:07 Possible thinko in driver/net/sb1250-mac.c? Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox