From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=54632 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OxniJ-0002Un-Oj for qemu-devel@nongnu.org; Mon, 20 Sep 2010 17:15:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OxneL-0006jL-1Z for qemu-devel@nongnu.org; Mon, 20 Sep 2010 17:11:46 -0400 Received: from adelie.canonical.com ([91.189.90.139]:51811) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OxneK-0006jE-Ov for qemu-devel@nongnu.org; Mon, 20 Sep 2010 17:11:45 -0400 Received: from loganberry.canonical.com ([91.189.90.37]) by adelie.canonical.com with esmtp (Exim 4.69 #1 (Debian)) id 1OxneI-0003H8-Pe for ; Mon, 20 Sep 2010 22:11:43 +0100 Received: from loganberry.canonical.com (localhost [127.0.0.1]) by loganberry.canonical.com (Postfix) with ESMTP id 8C6362E80DB for ; Mon, 20 Sep 2010 21:11:22 +0000 (UTC) MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Date: Mon, 20 Sep 2010 21:02:41 -0000 From: "Edgar E. Iglesias" <638955@bugs.launchpad.net> Sender: bounces@canonical.com References: <20100915132907.2955.78648.malonedeb@gandwana.canonical.com> Message-Id: <20100920210241.GC20298@laped.lan> Subject: [Bug 638955] Re: [Qemu-devel] [PATCH] e1000: Pad short frames to minimum size (60 bytes) Errors-To: bounces@canonical.com Reply-To: Bug 638955 <638955@bugs.launchpad.net> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Mon, Sep 20, 2010 at 10:44:34PM +0200, Michael S. Tsirkin wrote: > On Mon, Sep 20, 2010 at 10:40:35PM +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 len= gth. > > > >> = > > > > 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 check= sum. > > > > 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 cau= se = > > > 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 tha= t 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 existi= ng > > > >> 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 ta= p code so > > > we need to fix it up ourselves. > > = > > = > > This enforces ethernet semantics on the internal links (which is probab= ly > > not good), > = > Plus plus ungood. > When we do add e.g. ipoib support, we'll have to go and hunt these bugs d= own again. > Also will make it impossible to implement any devices that pass in guest = buffers > without FCS and padding. If we dont remove the padding from the device models rx paths, we will continue with code that relies on it and it is IMO wrong. Ethernet MAC's don't padd nor append checksum on receive. I agree with you that it's not great that the internal link protocol has to be strictly ethernet but it seems to me like if that is reality today, with or without Anthonys patch. slirp and tap both require ethernet semantics (except possibly padding and FCS). The addressing and packet headers are ethernet. In the long run, I'd rather see a more flexible internal interconnect that supports mutiple heterogenous link types. In the meantime, I think Anthonys patch is a better workaround than patching the device models. > > but it's IMO much better than changing the devices. > = > How much better? OK, s/much better/better/ :) > = > > It also > > moves the workaround closer to the root of the problem. > > IMO, it's a step in the right direction. > > = > > Acked-by: Edgar E. Iglesias > > = > > = > > > 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 =3D read(tapfd, buf, maxlen); > > > + if (len > 0) { > > > + len =3D MAX(MIN(maxlen, 40), len); > > > + } > = > Let's at least add a comment explaining what does this do? > Also - does tcp backend need this as well? Other backends? A comment sounds good. Cheers, Edgar -- = emulated netcards don't work with recent sunos kernel https://bugs.launchpad.net/bugs/638955 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: hi there, i'm using qemu-kvm backend in version: # qemu-kvm -version QEMU PC emulator version 0.12.5 (qemu-kvm-0.12.5), Copyright (c) 2003-2008 = Fabrice Bellard and there are just *not working any of model=3D$type with combinations of r= ecent sunos (solaris, openindiana, opensolaris, ..) .. you can download for testing purposes iso from here: http://dlc-origin.open= indiana.org/isos/147/ or from here: http://genunix.org/distributions/indian= a/ << osol and oi are also bubuntu-like *live cds, so no need to bother wit= h installing behaviour is as follows: e1000 - receiving doesn't work, transmitting works .. dladm (tool for handl= e ethers) shows that is all ok, correct mode is loaded up, it just seems li= ke this driver works at 100% but .. rtl8169|pcnet - works in 10Mbit mode with several other issues like high cp= u utilization and so .. dladm is unable to recognize options for this kind = of -nic others - just don't work .. i experienced this issue several times in past .. woraround was, that rt= l8169 worked so-so .. with recent sunos kernel it doesn't. it's easy to reproduce, this is why i'm not putting here more then launchin= g script for my virtual machine: # cat openindiana.sh qemu-kvm -hda /home/kvm/openindiana/openindiana.img -m 2048 -localtime -cdr= om /home/kvm/+images/oi-dev-147-x86.iso -boot d \ -vga std -vnc :9 -k en-us -monitor unix:/home/kvm/openindiana/instance,serv= er,nowait \ -net nic,model=3De1000,vlan=3D1 -net tap,ifname=3Doi0,script=3Dno,vlan=3D1 & sleep 2; ip l set oi0 up; ip a a 192.168.99.9/24 dev oi0; regards by daniel