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

[Ying-Shiuan Pan]

2012/1/3 Stefan Hajnoczi <address@hidden>:
> 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().

Yes, in virtio_net_receive().
In the probing phase, the virtnet_probe() is invoked. Almost at the
end of virnet_probe(),
try_fill_recv() is invoked, and after try_fill_recv() fill the RX
virtqueue, it will kick the virtqueue,
Then, virtio_net_receive() in QEMU is triggered.

If I modify the virtio_net_receive() to make it ignore this error,
the the problem also occurs in 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.

Oh!!!!!!!  Great!!!!!!
That's the key point!!!!!

Because virtio_net register its own set_features() method, and some
features are setting in that function.
While virtio_blk and virtio_balloon does not use set_features(),
that's why they did not encounter such problem.

>
> 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
I will submit patches later, thanks a lot!!

----
Best Regards,
潘穎軒Ying-Shiuan Pan