Re: [Qemu-devel] [PATCH] fix unbounded qemu NetQueue

[Luigi Rizzo]

On Thu, Jan 17, 2013 at 2:21 AM, Stefan Hajnoczi <address@hidden> wrote:

On Thu, Jan 17, 2013 at 07:07:11AM +0100, Luigi Rizzo wrote:
> The comment at the beginning of net/queue.c says that packets that
> cannot be sent by qemu_net_queue_send() should not be enqueued
> unless a callback is set.
>
> This patch implements this behaviour, that prevents a queue to grow
> unbounded (e.g. when a network backend is not connected).
>
> Also for good measure the patch implements bounded size queues
> (though it should not be necessary now because each source can only have
> one packet queued). When a packet is dropped because excessive
> queue size the callback is not supposed to be called.

Although I appreciate the semantics that the comment tries to establish,
the code doesn't behave like this today and we cannot drop packets in
cases where we relied on queuing them.

More changes will be required to make the hub, USB, pcap scenario I
described previously work.

i see. then the other option would be to drop packets only

if the queue is oversize AND the callback is not set:

+    if (queue->nq_count >= queue->nq_maxlen && !sent_cb)
+       return;

so we should be able to use the queue to store packets when the

USB guest is slow, and avoid dropping precious packet with the callback set

(there should be only one of them for each source) ?

The queue might grow slightly overlimit but still be kept under control.

I cannot think of another way to handle the callback. I think we cannot run

it on a drop as it would cause a premature restart of the sender.

(at a cursory inspection of the code, just tap.c and virtio-net.c use

callbacks)

cheers

luigi