Message ID | CAA1vHqKz3xYNF_ZiS508xoH21gLadB3TKqhLxP7nWPcq8Xx0LA@mail.gmail.com |
---|---|
State | New |
Headers | show |
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
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
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. */