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

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

[Ying-Shiuan Pan]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

Hi, all:

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.

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.

The parameters I passed to qemu-system-arm is
qemu-system-arm XXXXXXX -netdev type=tap,id=mynet -global
virtio-net-mmio.netdev=mynet
I don't know this is correct or not.

Does anyone have any idea of this problem?
Or give me some directions to solve this bug?

Any comment is welcome. I really appreciate your help!!


----
Best Regards,
Ying-Shiuan Pan

[-- Attachment #2: Type: text/html, Size: 1444 bytes --]

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

[Stefan Hajnoczi]

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?

> 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

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

[Ying-Shiuan Pan]

Hi, Stefan

Thanks for your response.


2011/12/30 Stefan Hajnoczi <stefanha@gmail.com>
>
> 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.
If I luckily figure out, I'll post it. :)

So~ currently, virito-balloon, virtio-blk, and virtio-net can work
properly  with virtio-mmio. :D

The following is the *workaround solution* which is for virtio-net
driver part (frontend).

diff --git a/virtio_net.c b/virtio_net.c
index 82dba5a..811e2d9 100644
--- a/virtio_net.c
+++ b/virtio_net.c
@@ -955,8 +955,10 @@ static int virtnet_probe(struct virtio_device *vdev)
            virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
                vi->big_packets = true;

+#ifndef ARM
        if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
                vi->mergeable_rx_bufs = true;
+#endif

        /* We expect two virtqueues, receive then send,
         * and optionally control. */


----
Best Regards,
Ying-Shiuan Pan

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

[Stefan Hajnoczi]

On Mon, Jan 2, 2012 at 5:19 AM, Ying-Shiuan Pan
<yingshiuan.pan@gmail.com> wrote:
> 2011/12/30 Stefan Hajnoczi <stefanha@gmail.com>
>>
>> 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

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

[Ying-Shiuan Pan]

2012/1/3 Stefan Hajnoczi <stefanha@gmail.com>:
> On Mon, Jan 2, 2012 at 5:19 AM, Ying-Shiuan Pan
> <yingshiuan.pan@gmail.com> wrote:
>> 2011/12/30 Stefan Hajnoczi <stefanha@gmail.com>
>>>
>>> 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