From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38584 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OxzAu-0006UJ-N5 for qemu-devel@nongnu.org; Tue, 21 Sep 2010 05:30:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Oxz4H-0005Qg-5l for qemu-devel@nongnu.org; Tue, 21 Sep 2010 05:23:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46152) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Oxz4G-0005QV-SE for qemu-devel@nongnu.org; Tue, 21 Sep 2010 05:23:17 -0400 Date: Tue, 21 Sep 2010 11:17:07 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] e1000: Pad short frames to minimum size (60 bytes) Message-ID: <20100921091707.GB4365@redhat.com> References: <1284842625-13920-1-git-send-email-stefanha@linux.vnet.ibm.com> <20100918212710.GB3981@laped.lan> <20100920104259.GA23898@redhat.com> <4C97C4A4.1030801@codemonkey.ws> <20100920205136.GB20298@laped.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100920205136.GB20298@laped.lan> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Edgar E. Iglesias" Cc: Stefan Hajnoczi , 638955@bugs.launchpad.net, Stefan Hajnoczi , qemu-devel@nongnu.org On Mon, Sep 20, 2010 at 10:51:36PM +0200, Edgar E. Iglesias wrote: > On Mon, Sep 20, 2010 at 03:31:32PM -0500, Anthony Liguori wrote: > > On 09/20/2010 05:42 AM, Michael S. Tsirkin wrote: > > > On Sun, Sep 19, 2010 at 07:36:51AM +0100, Stefan Hajnoczi wrote: > > > > > >> On Sat, Sep 18, 2010 at 10:27 PM, Edgar E. Iglesias > > >> wrote: > > >> > > >>> This doesn't look right. AFAIK, MAC's dont pad on receive. > > >>> > > >> I agree. NICs that do padding will do it on transmit, not receive. > > >> Anything coming in on the wire should already have the minimum length. > > >> > > > QEMU never gets access to the wire. > > > Our APIs do not really pass complete ethernet packets: > > > we forward packets without checksum and padding. > > > > > > I think it makes complete sense to keep this and > > > handle padding in devices because we > > > have devices that pass the frame to guest without padding and checksum. > > > It should be easy to replace padding code in devices that > > > need it with some kind of macro. > > > > > > > Would this not also address the problem? It sounds like the root cause > > is the tap code, not the devices.. > > > > Regards, > > > > Anthony Liguori > > > > > > > >> In QEMU that isn't true today and that's why rtl8139, pcnet, and > > >> ne2000 already do this same padding. This patch is the smallest > > >> change to cover e1000. > > >> > > >> > > >>> IMO this kind of padding should somehow be done by the bridge that forwards > > >>> packets into the qemu vlan (e.g slirp or the generic tap bridge). > > >>> > > >> That should work and we can then drop the padding code from existing > > >> NICs. I'll take a look. > > >> > > >> Stefan > > >> > > > > > > > > From f77c3143f3fbefdfa2f0cc873c2665b5aa78e8c9 Mon Sep 17 00:00:00 2001 > > From: Anthony Liguori > > Date: Mon, 20 Sep 2010 15:29:31 -0500 > > Subject: [PATCH] tap: make sure packets are at least 40 bytes long > > > > This is required by ethernet drivers but not enforced in the Linux tap code so > > we need to fix it up ourselves. > > > > Signed-off-by: Anthony Liguori > > > > diff --git a/net/tap.c b/net/tap.c > > index 4afb314..822241a 100644 > > --- a/net/tap.c > > +++ b/net/tap.c > > @@ -179,7 +179,13 @@ static int tap_can_send(void *opaque) > > #ifndef __sun__ > > ssize_t tap_read_packet(int tapfd, uint8_t *buf, int maxlen) > > { > > - return read(tapfd, buf, maxlen); > > + ssize_t len; > > + > > + len = read(tapfd, buf, maxlen); > > + if (len > 0) { > > + len = MAX(MIN(maxlen, 40), len); > > > A small detail :) > 40 -> 64 (including a dummy FCS). I don't think so: e1000 at least has code to tack the FCS on, so we'll end up with a 68 bytes. > > + } > > + return len; > > } > > #endif > > > > -- > > 1.7.0.4 > >