From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=33849 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Poata-0004g9-6g for qemu-devel@nongnu.org; Sun, 13 Feb 2011 07:17:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PoatY-0002Ad-BK for qemu-devel@nongnu.org; Sun, 13 Feb 2011 07:17:42 -0500 Received: from moutng.kundenserver.de ([212.227.17.8]:57012) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PoatX-00028k-Lh for qemu-devel@nongnu.org; Sun, 13 Feb 2011 07:17:39 -0500 Message-ID: <4D57CBDE.2000207@mail.berlios.de> Date: Sun, 13 Feb 2011 13:17:34 +0100 From: Stefan Weil MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] eepro100: pad to ensure minimum packet size References: <4D552D5802000048000A9D56@novprvoes0310.provo.novell.com> In-Reply-To: <4D552D5802000048000A9D56@novprvoes0310.provo.novell.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bruce Rogers Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" Am 11.02.2011 20:36, schrieb Bruce Rogers: > Recent gpxe e100pro drivers will drop small packets because the emulated > nic will report an error for small frames. In the qemu model we should > instead have the e100pro pad out the received frames to be the minimum > size and not report this case as an error. > > Signed-off-by: Bruce Rogers > --- > hw/eepro100.c | 23 +++++++++++++---------- > 1 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/hw/eepro100.c b/hw/eepro100.c > index edf48f6..03b6934 100644 > --- a/hw/eepro100.c > +++ b/hw/eepro100.c > @@ -1645,6 +1645,8 @@ static int nic_can_receive(VLANClientState *nc) > #endif > } > > +#define MIN_BUF_SIZE 60 /* Min. octets in an ethernet frame sans FCS */ > + > static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, > size_t size) > { > /* TODO: > @@ -1653,6 +1655,7 @@ static ssize_t nic_receive(VLANClientState *nc, > const uint8_t * buf, size_t size > */ > EEPRO100State *s = DO_UPCAST(NICState, nc, nc)->opaque; > uint16_t rfd_status = 0xa000; > + uint8_t min_buf[MIN_BUF_SIZE]; > static const uint8_t broadcast_macaddr[6] = > { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; > > @@ -1660,15 +1663,15 @@ static ssize_t nic_receive(VLANClientState > *nc, const uint8_t * buf, size_t size > /* CSMA is disabled. */ > logout("%p received while CSMA is disabled\n", s); > return -1; > - } else if (size < 64 && (s->configuration[7] & BIT(0))) { > - /* Short frame and configuration byte 7/0 (discard short receive) set: > - * Short frame is discarded */ > - logout("%p received short frame (%zu byte)\n", s, size); > - s->statistics.rx_short_frame_errors++; > -#if 0 > - return -1; > -#endif > - } else if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] > & BIT(3))) { > + } > + /* Pad to minimum Ethernet frame length */ > + if (size < sizeof(min_buf)) { > + memcpy(min_buf, buf, size); > + memset(&min_buf[size], 0, sizeof(min_buf) - size); > + buf = min_buf; > + size = sizeof(min_buf); > + } > + if ((size > MAX_ETH_FRAME_SIZE + 4) && !(s->configuration[18] & > BIT(3))) { > /* Long frame and configuration byte 18/3 (long receive ok) not set: > * Long frames are discarded. */ > logout("%p received long frame (%zu byte), ignored\n", s, size); > @@ -1744,7 +1747,7 @@ static ssize_t nic_receive(VLANClientState *nc, > const uint8_t * buf, size_t size > "(%zu bytes); data truncated\n", rfd_size, size); > size = rfd_size; > } > - if (size < 64) { > + if (size < MIN_BUF_SIZE) { > rfd_status |= 0x0080; > } > TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n", Could you please give more details of the test scenario which fails? I'd like to reproduce it here and find a better solution. The configuration bit "discard short frame" exists in real hardware, so removing the code which emulates this behavior is not the correct solution - even if it helps in a special case. No acknowledge for this patch from me. Regards, Stefan Weil