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
> >> <edgar.igles...@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.
> >>
> > 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 <aligu...@us.ibm.com>
> 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 <aligu...@us.ibm.com>
>
> 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).
> + }
> + return len;
> }
> #endif
>
> --
> 1.7.0.4
>
