Linux MIPS Architecture development
 help / color / mirror / Atom feed
From: Richard Sandiford <rsandifo@redhat.com>
To: linux-mips@linux-mips.org
Subject: Possible thinko in driver/net/sb1250-mac.c?
Date: Sat, 12 Feb 2005 13:07:04 +0000	[thread overview]
Message-ID: <87wttekjuv.fsf@firetop.home> (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

                 reply	other threads:[~2005-02-12 13:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wttekjuv.fsf@firetop.home \
    --to=rsandifo@redhat.com \
    --cc=linux-mips@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox