linux-mips
[Top] [All Lists]

Possible thinko in driver/net/sb1250-mac.c?

To: linux-mips@linux-mips.org
Subject: Possible thinko in driver/net/sb1250-mac.c?
From: Richard Sandiford <rsandifo@redhat.com>
Date: Sat, 12 Feb 2005 13:07:04 +0000
Original-recipient: rfc822;linux-mips@linux-mips.org
Sender: linux-mips-bounce@linux-mips.org
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux)
[ 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

<Prev in Thread] Current Thread [Next in Thread>
  • Possible thinko in driver/net/sb1250-mac.c?, Richard Sandiford <=