Re: [Qemu-devel] [PATCH V2 18/20] virtio-net: multiqueue support

[Jason Wang]

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 <address@hidden>
>> ---
>>  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