[Qemu-devel] Re: [PATCHv3] virtio-net: correct packet length math

[Michael S. Tsirkin]

On Fri, Jun 25, 2010 at 12:47:03PM +0530, Amit Shah wrote:
> On (Thu) Jun 24 2010 [18:54:07], Michael S. Tsirkin wrote:
> > We were requesting too much when checking buffer
> > length: size already includes host header length.
> > 
> > Further, we should not exit if we get a packet that
> > is too long, since this might not be under control
> > of the guest. Just drop the packet.
> 
> control of the host?

Well, host too I guess. What I was trying to say, it might not
be the fault of the guest that it got a packet
that is too long.

> > @@ -579,19 +581,32 @@ static ssize_t virtio_net_receive(VLANClientState 
> > *nc, const uint8_t *buf, size_
> >                  mhdr = (struct virtio_net_hdr_mrg_rxbuf *)sg[0].iov_base;
> >  
> >              offset += receive_header(n, sg, elem.in_num,
> > -                                     buf + offset, size - offset, hdr_len);
> > -            total += hdr_len;
> > +                                     buf + offset, size - offset, 
> > guest_hdr_len);
> > +            total += guest_hdr_len;
> >          }
> >  
> >          /* copy in packet.  ugh */
> >          len = iov_from_buf(sg, elem.in_num,
> >                             buf + offset, size - offset);
> >          total += len;
> > +        offset += len;
> > +        /* If buffers can't be merged, at this point we
> > +         * must have consumed the complete packet.
> > +         * Otherwise, drop it. */
> > +        if (!n->mergeable_rx_bufs && offset < size) {
> > +#if 0
> > +            fprintf(stderr, "virtio-net truncated non-mergeable packet: "
> > +
> > +                    "i %zd mergeable %d offset %zd, size %zd, "
> > +                    "guest hdr len %zd, host hdr len %zd\n",
> > +                    i, n->mergeable_rx_bufs,
> > +                    offset, size, guest_hdr_len, host_hdr_len);
> > +#endif
> > +            return size;
> > +        }
> 
> Before returning, won't you have to finish off the virtqueue operations
> -- fill, flush, kick, etc.?
> 
>               Amit

No, this would consume a buffer. We just want to drop the packet
without side-effects. But we do need to decrement the inuse counter.
I'll respin the patch.

-- 
MST