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

[Zhi Yong Wu]

On Thu, May 31, 2012 at 10:16 PM, Jan Kiszka <address@hidden> wrote:
> On 2012-05-31 16:10, Zhi Yong Wu wrote:
>> On Thu, May 31, 2012 at 9:48 PM, Jan Kiszka <address@hidden> wrote:
>>> On 2012-05-31 15:45, Zhi Yong Wu wrote:
>>>> 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().
>>>>
>>>
>>> Delayed packets that are hold back during delivering==1. Having a flush
>> Sorry, i don't get what it mean? You know, english is not my first
>> language.:) You mean that delayed packets may be reenqueued?
>>> before and after may be fine - if we really need it before.
>>>
>
> I might have been to brief as well: The purpose of the flush after
> delivery is the case when we re-entered qemu_net_queue_send while
> delivering. If queue->delivering is 1 on entry, we don't deliver but
> queue the packet. Once the returned from qemu_net_queue_deliver to the
> root qemu_net_queue_send, the queue of hold back packets has to be
> flushed to avoid delays.
Yeah, current solution has been this.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux



-- 
Regards,

Zhi Yong Wu