On 04/13/2013 09:17 PM, Aurelien Jarno wrote:
> On Fri, Jan 25, 2013 at 06:35:41PM +0800, Jason Wang wrote:
>> This patch implements both userspace and vhost support for multiple queue
>> virtio-net (VIRTIO_NET_F_MQ). This is done by introducing an array of
>> VirtIONetQueue to VirtIONet.
>>
>> Signed-off-by: Jason Wang <jasow...@redhat.com>
>> ---
>> hw/virtio-net.c | 317
>> +++++++++++++++++++++++++++++++++++++++++++------------
>> hw/virtio-net.h | 28 +++++-
>> 2 files changed, 275 insertions(+), 70 deletions(-)
> This patch breaks virtio-net in Minix, even with multiqueue disable. I
> don't know virtio enough to know if it is a Minix or a QEMU problem.
> However I have been able to identify the part of the commit causing the
> failure:
Hi Aurelien:
Thanks for the work.
>
>> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
>> index ef522d5..cec91a7 100644
>> --- a/hw/virtio-net.c
>> +++ b/hw/virtio-net.c
> ...
>
>> +static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue, int
>> ctrl)
>> +{
>> + VirtIODevice *vdev = &n->vdev;
>> + int i, max = multiqueue ? n->max_queues : 1;
>> +
>> + n->multiqueue = multiqueue;
>> +
>> + for (i = 2; i <= n->max_queues * 2 + 1; i++) {
>> + virtio_del_queue(vdev, i);
>> + }
>> +
> The for loop above is something which is new, even with multiqueue
> disabled. Even with max_queues=1 it calls virtio_del_queue with i = 2
> and i = 3. Disabling this loop makes the code to work as before.
Looks like a bug here, need to change n->max_queues * 2 + 1 to
n->max_queues * 2. The reason we need to del queue 2 each time because
vq 2 has different meaning is multiqueue and single queue. In single
queue, vq 2 maybe ctrl vq, but in multiqueue mode it was rx1.
Let's see whether this small change works.
>
> On the Minix side it triggers the following assertion:
>
> | virtio.c:370: assert "q->vaddr != NULL" failed, function "free_phys_queue"
>
>
> | virtio_net(73141): panic: assert failed
>
> This correspond to this function in lib/libvirtio/virtio.c:
>
> | static void
> | free_phys_queue(struct virtio_queue *q)
> | {
> | assert(q != NULL);
> | assert(q->vaddr != NULL);
> |
> | free_contig(q->vaddr, q->ring_size);
> | q->vaddr = NULL;
> | q->paddr = 0;
> | q->num = 0;
> | free_contig(q->data, sizeof(q->data[0]));
> | q->data = NULL;
> | }
>
> Do you have an idea if the problem is on the Minix side or on the QEMU
> side?
>
Haven't figured out the relationship between virtqueue dynamic del/add
and q->vaddr here. If the above changes does not work, I guess this
problem happens only for ctrl vq (vq2)? And when does this happen?
Rebooting?
Thanks a lot
