From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48778) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VyS6m-0007Zq-QH for qemu-devel@nongnu.org; Wed, 01 Jan 2014 15:09:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VyS6i-0007Hv-C7 for qemu-devel@nongnu.org; Wed, 01 Jan 2014 15:09:40 -0500 Received: from mail-wi0-f171.google.com ([209.85.212.171]:46866) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VyS6i-0007Hg-5p for qemu-devel@nongnu.org; Wed, 01 Jan 2014 15:09:36 -0500 Received: by mail-wi0-f171.google.com with SMTP id bz8so18124086wib.16 for ; Wed, 01 Jan 2014 12:09:34 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1387563997-1845-1-git-send-email-roy.franz@linaro.org> <1387563997-1845-3-git-send-email-roy.franz@linaro.org> Date: Wed, 1 Jan 2014 12:09:34 -0800 Message-ID: From: Roy Franz Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 2/2] net: Fix lan9118 buffer length handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , Patch Tracking , "qemu-devel@nongnu.org Developers" , Stefan Hajnoczi , Anthony Liguori On Fri, Dec 27, 2013 at 5:25 PM, Peter Crosthwaite wrote: > On Sat, Dec 21, 2013 at 4:26 AM, Roy Franz wrote: >> The 9118 ethernet controller supports transmission of multi-buffer packets >> with arbitrary byte alignment of the start and end bytes. All writes to >> the packet fifo are 32 bits, so the controller discards bytes at the beginning >> and end of each buffer based on the 'Data start offset' and 'Buffer size' >> of the TX command 'A' format. >> >> This patch uses the provided buffer length to limit the bytes transmitted. >> Previously all the bytes of the last 32-bit word written to the TX fifo >> were added to the internal transmit buffer structure resulting in more bytes >> being transmitted than were submitted to the hardware in the command. This >> resulted in extra bytes being inserted into the middle of multi-buffer >> packets when the non-final buffers had non-32bit aligned ending addresses. >> >> Signed-off-by: Roy Franz >> --- >> hw/net/lan9118.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c >> index c5d6f14..712bb41 100644 >> --- a/hw/net/lan9118.c >> +++ b/hw/net/lan9118.c >> @@ -773,11 +773,10 @@ static void tx_fifo_push(lan9118_state *s, uint32_t val) >> in FIFO words. Empirical results show it to be little-endian. >> */ >> /* TODO: FIFO overflow checking. */ >> - while (n--) { >> + while (n-- && s->txp->buffer_size--) { > > can you just get n right in the first place (line 766): > > n = MIN(4, s->txp_buffer_size); > > rather than doing min calculation on two parallel iteration variables? > > Regards, > Peter > >> s->txp->data[s->txp->len] = val & 0xff; >> s->txp->len++; >> val >>= 8; >> - s->txp->buffer_size--; >> } >> s->txp->fifo_used++; >> } >> -- >> 1.7.10.4 >> >> Hi Peter, I'll take a look at this (and your other comments on the extract32) when I am back at work next week. Thanks, Roy