Skip to contentSkip to conversation

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

DISCUSSION Richard Sandiford 1 participant 1 message Feb 12, 2005 MIPS

lore
Share & tools
lore
#0 lore raw

[ 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