qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
	638955@bugs.launchpad.net,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] e1000: Pad short frames to minimum size (60 bytes)
Date: Sun, 19 Sep 2010 13:18:01 +0200	[thread overview]
Message-ID: <20100919111801.GC7350@redhat.com> (raw)
In-Reply-To: <AANLkTi=AM+o53WqPyjTycA+Xtqv+E1zRW2bUBAnc=Oqe@mail.gmail.com>

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
> <edgar.iglesias@gmail.com> 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.
> 
> 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

Not all nic devices have to be emulate ethernet, so not all devices want
the padding, e.g. virtio does not.  It's also easy to imagine an
ethernet device that strips the padding: would be silly to add it
just to have it stripped.

If we really want to do this generically, we could implement a function dealing
with the padding, and call it from relevant devices.

But I think the patch looks fine as is.

-- 
MST

  reply	other threads:[~2010-09-19 11:24 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-18 20:43 [Qemu-devel] [PATCH] e1000: Pad short frames to minimum size (60 bytes) Stefan Hajnoczi
2010-09-18 20:57 ` Hervé Poussineau
2010-09-18 21:12   ` Stefan Hajnoczi
2010-09-20  8:42     ` Kevin Wolf
2010-09-20  8:51       ` Edgar E. Iglesias
2010-09-20  9:13       ` Stefan Hajnoczi
2010-09-18 21:27 ` Edgar E. Iglesias
2010-09-19  6:36   ` Stefan Hajnoczi
2010-09-19 11:18     ` Michael S. Tsirkin [this message]
2010-09-19 12:04       ` Edgar E. Iglesias
2010-09-19 12:03         ` Michael S. Tsirkin
2010-09-20  8:50     ` Kevin Wolf
2010-09-20  9:03       ` Edgar E. Iglesias
2010-09-20  9:34         ` Stefan Hajnoczi
2010-09-20 10:42     ` Michael S. Tsirkin
2010-09-20 20:31       ` Anthony Liguori
2010-09-20 20:35         ` Michael S. Tsirkin
2010-09-20 20:40         ` Edgar E. Iglesias
2010-09-20 20:44           ` Michael S. Tsirkin
2010-09-20 20:51         ` [Bug 638955] " Edgar E. Iglesias
2010-09-21  9:17           ` Michael S. Tsirkin
2010-09-21  9:21             ` Edgar E. Iglesias
2010-09-21  9:16         ` [Bug 638955] " Stefan Hajnoczi
2011-02-21 21:13           ` [Qemu-devel] " Stefan Weil
2010-09-20 17:52 ` [Qemu-devel] Re: [PATCH] e1000: " Michael S. Tsirkin
  -- strict thread matches above, loose matches on Subject: below --
2010-09-15 13:29 [Qemu-devel] [Bug 638955] [NEW] emulated netcards don't work with recent sunos kernel daniel pecka
2010-09-17  8:47 ` [Qemu-devel] [Bug 638955] " daniel pecka
2010-09-17 15:52 ` daniel pecka
2010-09-20  8:02 ` daniel pecka
2010-09-20  9:47 ` Stefan Hajnoczi
2010-09-20 20:57 ` [Bug 638955] Re: [Qemu-devel] [PATCH] e1000: Pad short frames to minimum size (60 bytes) Anthony Liguori
2010-09-20 21:02 ` Edgar E. Iglesias
2010-09-21  8:49 ` [Qemu-devel] [Bug 638955] Re: emulated netcards don't work with recent sunos kernel daniel pecka
2010-10-02 19:23 ` daniel pecka
2010-10-12 14:40   ` [Qemu-devel] " Stefan Hajnoczi
2011-01-03 13:40 ` [Qemu-devel] " daniel pecka
2011-01-04  9:36   ` [Qemu-devel] " Stefan Hajnoczi
2011-01-04 10:04 ` [Qemu-devel] " daniel pecka
2011-01-04 17:51 ` Stefan Weil
2011-01-29 17:41 ` Daniel Kvasnicka
2011-02-28 19:06 ` geppz
2011-03-01  9:50   ` [Qemu-devel] " Stefan Hajnoczi
2011-03-01 15:14 ` [Qemu-devel] " geppz
2011-03-05 20:14 ` Stefan Hajnoczi
2011-03-06 12:32 ` Stefan Hajnoczi
2011-03-07 18:43 ` geppz
2014-10-05 20:57 ` dblade
2014-10-06  8:53   ` Stefan Hajnoczi
2014-10-07  1:46 ` dblade
2015-09-08 22:14 ` Jan Vlug
2015-09-10 19:44 ` Jan Vlug
2016-08-12  4:58 ` T. Huth

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=20100919111801.GC7350@redhat.com \
    --to=mst@redhat.com \
    --cc=638955@bugs.launchpad.net \
    --cc=edgar.iglesias@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@linux.vnet.ibm.com \
    /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;
as well as URLs for NNTP newsgroup(s).