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

[Zhi Yong Wu]

On Thu, May 31, 2012 at 5:34 PM, Luigi Rizzo <address@hidden> wrote:
>
>
> On Thu, May 31, 2012 at 11:18 AM, 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.
Yeah, this case actually exists, but tcp/ip protocol stack in guest
will make sure this ordering will finally be correct.

By the way, e.g. even if some packets are sent successfully in order,
after internet trasmission, we can not also make sure that these
packets will be received by their destination in its original order.
The tcp/ip protocol stack in destination host will make sure that they
will be sorted by their seq number in these packets.

>> >
>> > 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
>> ->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.
>
>
> i was under the impression that qemu_net_queue_deliver() may also return 0
> if the other side
> (frontend network device) has no room for the packet. This would cause the
> queue to
> contain data on entry in the next call, even without reentrancy. Consider
> this:
>
> t0. qemu_net_queue_send(pkt-A)
>          qemu_net_queue_deliver() returns 0, pkt-A is queued and we return
> t1. qemu_net_queue_send(pkt-B)
>        qemu_net_queue_deliver() returns successfully, pkt-B is sent to the
> frontend
>        then we call qemu_net_queue_flush() and this sends pkt-B to the
> frontend,
>        in the wrong order
>
> cheers
> luigi
>
> --
> -----------------------------------------+-------------------------------
>  Prof. Luigi RIZZO, address@hidden  . Dip. di Ing. dell'Informazione
>  http://www.iet.unipi.it/~luigi/        . Universita` di Pisa
>  TEL      +39-050-2211611               . via Diotisalvi 2
>  Mobile   +39-338-6809875               . 56122 PISA (Italy)
> -----------------------------------------+-------------------------------
>



-- 
Regards,

Zhi Yong Wu