Re: [Qemu-devel] frame reordering in qemu_net_queue_send() ?

[Zhi Yong Wu]

On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo <address@hidden> wrote:
>> Hi,
>> while investigating rx performance for emulated network devices
>> (i am looking at the userspace version, relying on net=tap
>> or similar approaches) i noticed the code
>> in net/queue.c :: qemu_net_queue_send()
>> which look strange to me (same goes for the iov version).
>>
>> The whole function is below, just for reference.
>> My impression is that the call to qemu_net_queue_flush()
>> is misplaced, and it should be before qemu_net_queue_deliver()
>> otherwise the current packet is pushed out before anything
>> was already in the queue.
>>
>> Does it make sense ?
>>
>> cheers
>> luigi
>>
>>    ssize_t qemu_net_queue_send(NetQueue *queue,
>>                                VLANClientState *sender,
>>                                unsigned flags,
>>                                const uint8_t *data,
>>                                size_t size,
>>                                NetPacketSent *sent_cb)
>>    {
>>        ssize_t ret;
>>
>>        if (queue->delivering) {
>>            return qemu_net_queue_append(queue, sender, flags, data, size, 
>> NULL);
>>        }
>>
>>        ret = qemu_net_queue_deliver(queue, sender, flags, data, size);
>>        if (ret == 0) {
>>            qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
>>            return 0;
>>        }
>>
>>        qemu_net_queue_flush(queue);
>
> This of the case where delivering a packet causes additional
> (re-entrant) qemu_net_queue_send() calls to this queue.  We'll be in
If qemu_net_queue_send() are re-entranced, what issue or badness will
it introduce?
I haven't found what issue will take place when queue_flush() is moved
before queue_deliver().

> ->delivering state and queue those packets.  After we've finished
> delivering the initial packet we flush the queue.
>
> This is a weird case but this is how I read the intention of the code.
>
> Jan: maybe slirp can do this re-entrant qemu_net_queue_send()?
>
> Stefan
>



-- 
Regards,

Zhi Yong Wu