From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:43873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QbUHl-0005NP-DW for qemu-devel@nongnu.org; Tue, 28 Jun 2011 05:08:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QbUHi-0004Uo-Ot for qemu-devel@nongnu.org; Tue, 28 Jun 2011 05:08:44 -0400 Received: from mel.act-europe.fr ([194.98.77.210]:58115) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QbUHi-0004UL-6z for qemu-devel@nongnu.org; Tue, 28 Jun 2011 05:08:42 -0400 Message-ID: <4E099A15.4050004@adacore.com> Date: Tue, 28 Jun 2011 11:08:37 +0200 From: Fabien Chouteau MIME-Version: 1.0 References: <1309178511-20027-1-git-send-email-chouteau@adacore.com> <4E08925F.3030605@adacore.com> <4E08ABF4.1010206@adacore.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Make SLIRP Ethernet packets size to 64 bytes minimuma List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org On 28/06/2011 10:34, Stefan Hajnoczi wrote: > On Mon, Jun 27, 2011 at 5:12 PM, Fabien Chouteau wrote: >> On 27/06/2011 17:39, Stefan Hajnoczi wrote: >>> On Mon, Jun 27, 2011 at 3:23 PM, Fabien Chouteau wrote: >>>> On 27/06/2011 15:50, Stefan Hajnoczi wrote: >>>>> On Mon, Jun 27, 2011 at 1:41 PM, Fabien Chouteau wrote: >>>>>> >>>>>> Signed-off-by: Fabien Chouteau >>>>>> --- >>>>>> slirp/slirp.c | 4 ++-- >>>>>> 1 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> Any particular bug that this fixes? >>>>> >>>>> There have been 64 byte minimum padding patches to several emulated >>>>> NICs. There has also been discussion about where the best place to do >>>>> this is. Why is this patch necessary? >>>>> >>>> >>>> This patch is necessary because some NICs are configured to drop short frames, >>>> therefore the OS will not receive some of the packets generated by Qemu. >>>> >>>> There's a first patch to fix this issue: >>>> http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=dbf3c4b4baceb91eb64d09f787cbe92d65188813 >>>> >>>> My patch fixes two other sources of short frames. >>> >>> Thanks for the explanation. I stepped back from the discussion on >>> where the right place to fix this is last time around. Now I'm >>> wondering why do anything in slirp when the other sources (tap, ...) >>> aren't padding to 64 bytes? >> >> I think that packets generated by Qemu must follow RFC. For other sources, qemu >> should keep original size when possible and put padding otherwise. > > slirp interfaces to net.h which does not emulate Ethernet. For > example, it doesn't carry the Frame Check Sequence (FCS) as per the > Ethernet standard. NICs that want to report FCS to the guest > calculate it themselves, just like they do with 64-byte padding. So I > find it weird to say we should honor Ethernet when the transport we > are using is not Ethernet. In this case it's always Ethernet frames, look at the two patched lines, there's an Ethernet header: uint8_t arp_req[max(ETH_HLEN + sizeof(struct arphdr), 64)]; slirp_output(slirp->opaque, buf, max(ip_data_len + ETH_HLEN, 64)); > > This patch doesn't hurt but we'd be just as well off without it. > > Did you do this to fix a bug? If so, then something else in QEMU > needs to be fixed, not slirp. When Qemu generates bad Ethernet frames, I think it's a bug. And again, this is the extension of a previous patch. If this patch is not valid then we should revert the first, it's also a question of consistency. -- Fabien Chouteau