Re: [Qemu-devel] virtio-net with virtio-mmio

[Stefan Hajnoczi]

On Mon, Jan 2, 2012 at 5:19 AM, Ying-Shiuan Pan
<address@hidden> wrote:
> 2011/12/30 Stefan Hajnoczi <address@hidden>
>>
>> On Wed, Dec 28, 2011 at 06:16:42PM +0800, Ying-Shiuan Pan wrote:
>> > I'm very interested in virtio-mmio Peter Maydell did for QEMU,
>> > (http://lists.nongnu.org/archive/html/qemu-devel/2011-11/msg01870.html)
>> >
>> > actually, I've tested the virtio-blk, and it is working.
>> > I applied those patch to QEMU-1.0 and brought the virtio_mmio.c from
>> > Linux-3.2-rc back to Linux-linaro-2.6.38.
>> >
>> > I also found a small bug in virito-mmio.c: virtio_mmio_write(),
>> > Peter forgot to break in each case of switch block.
>> > After fixed the small bug, the virtio-balloon works as well.
>>
>> PMM: Do you have a git branch where you could apply Ying-Shiuan's fix?
> hmm... I've sent Peter my patch :)
>>
>> > Then, when I attempted to enable the virtio-net, the initialization part is
>> > fine,
>> > however, the QEMU crashed and printed this message:
>> > "virtio-net header not in first element"
>> >
>> > It happens when the front-end virtio-net is invoking virtqueue_kick() at
>> > the end of try_fill_recv().
>> > Then, QEMU gets this message and invokes virtio_net_receive(), then the
>> > error happens.
>>
>> virtio-net is looking at a virtqueue element (a packet buffer provided
>> by the guest OS into which QEMU will copy the received packet) but the
>> virtqueue element does not have the expected format.  You can check the
>> virtio specification for the details on the virtio-net buffer layout:
>> http://ozlabs.org/~rusty/virtio-spec/
>>
>> It sounds like the vring is corrupted or QEMU's virtio code is not
>> correctly reading the virtqueue element (which is basically an iovec
>> array).
>>
>> virtio-net is more advanced than virtio-blk in how it uses virtio so
>> it's not surprising that you've discovered an issue.  Debugging this
>> comes down to looking at the vring descriptors and the virtqueue
>> elements that QEMU extracts.
>>
>> It sounds like there's a problem with the rx queue, you could try
>> indirect_desc=off to disable indirect vring descriptors to see if that
>> is part of the problem.
>>
>> Stefan
> I guessed that the problem was in adding header,
> virtio-net should insert its header to the ring buffer before its
> packet, correct me if I'm wrong.
>
> virtio-net driver (backend part) can use add_recvbuf_small() /
> add_recvbuf_big() / add_recvbuf_mergeable()
> depending on what features the virtio-net device (QEMU part) provides.
> I found that both small and big add the header, while the mergeable
> one does not.
> Besides, in that add_recvbuf_big(), the comments mentioned about a QEMU bug??
>
> Anyway, I found a workaround solution, that is disable the
> VIRTIO_NET_F_MRG_RXBUF feature
> and make virtio-net driver use add_recvbuf_big()
>
> However, I am still not clear about how the problem happened.

Can you confirm which "virtio-net header not in first element" error
is occurring?  There are two instances in qemu/hw/virtio-net.c:
virtio_net_receive() and virtio_net_flush_tx().

If you are really seeing the error from virtio_net_receive() then
feature negotiation is broken.  The guest will advertise mergeable
receive buffers and the host (QEMU) should detect this in
virtio_net_set_features().  This causes QEMU to set
VirtIONet->mergeable_rx_bufs to true.

If VirtIONet->mergeable_rx_bufs is true then the error you are seeing
in virtio_net_receive() cannot happen:

    if (!n->mergeable_rx_bufs && elem.in_sg[0].iov_len != guest_hdr_len) {
        error_report("virtio-net header not in first element");
        exit(1);
    }

The only explanation is that the guest and host did not negotiate the
mergeable receive buffers feature properly.  That could mean Peter's
QEMU patches are not calling invoking virtio_net_set_features() at the
appropriate time or something else is broken - probably a virtio-mmio
issue.

AFAIK mergeable receive buffers are used by default and work correctly
under virtio-pci.

If you can debug this a little more we'll figure out what the problem is.

Stefan