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

2012-05-31 Thread Stefan Hajnoczi
On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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
-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



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

2012-05-31 Thread Luigi Rizzo
On Thu, May 31, 2012 at 11:18 AM, Stefan Hajnoczi stefa...@gmail.comwrote:

 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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
 -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, ri...@iet.unipi.it  . 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)
-+---


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

2012-05-31 Thread Paolo Bonzini
Il 31/05/2012 11:34, Luigi Rizzo ha scritto:
 
 
 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.

Correct.

 This would cause the queue to
 contain data on entry in the next call, even without reentrancy.

I think this is related to the other thread that you started.

If you do a qemu_net_queue_flush() whenever buffers become ready, it
will probably fix this bug too.

Paolo




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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 11:34, Luigi Rizzo wrote:
 On Thu, May 31, 2012 at 11:18 AM, Stefan Hajnoczi stefa...@gmail.comwrote:
 
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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
 -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

This ordering requires a state change in the frontend (from can't
receive to can receive) that is not possible. Such a change could
only be triggered if the guest interacts with the NIC. But we are
holding a lock that prevents this while walking the queue.

Jan

then we call qemu_net_queue_flush() and this sends pkt-B to the
 frontend,
in the wrong order
 
 cheers
 luigi
 

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 11:18, Stefan Hajnoczi wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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
 -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()?

Yep, when working with a VLAN queue (*): transmission of the NIC will
directly be processed by slirp in the sender's context and can make it
send a reply, also in this context.

Jan

(*) Makes me wonder if this was considered for the hub patches.

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Zhi Yong Wu
On Thu, May 31, 2012 at 5:34 PM, Luigi Rizzo ri...@iet.unipi.it wrote:


 On Thu, May 31, 2012 at 11:18 AM, Stefan Hajnoczi stefa...@gmail.com
 wrote:

 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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, ri...@iet.unipi.it  . 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



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

2012-05-31 Thread Paolo Bonzini
Il 31/05/2012 15:07, Zhi Yong Wu ha scritto:
 Yeah, this case actually exists, but tcp/ip protocol stack in guest
 will make sure this ordering will finally be correct.

Nevertheless it's not good, and the latest Windows Logo tests will fail
if you reorder frames.

Paolo



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

2012-05-31 Thread Luigi Rizzo
On Thu, May 31, 2012 at 03:23:12PM +0200, Jan Kiszka wrote:
 On 2012-05-31 15:19, Paolo Bonzini wrote:
  Il 31/05/2012 15:07, Zhi Yong Wu ha scritto:
  Yeah, this case actually exists, but tcp/ip protocol stack in guest
  will make sure this ordering will finally be correct.
  
  Nevertheless it's not good, and the latest Windows Logo tests will fail
  if you reorder frames.
 
 Can we really enter qemu_net_queue_send with delivering == 0 and a
 non-empty queue?

i have no idea.

it doesn't help that comments are using very very
sparingly throughout the code...

cheers
luigi



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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 15:19, Paolo Bonzini wrote:
 Il 31/05/2012 15:07, Zhi Yong Wu ha scritto:
 Yeah, this case actually exists, but tcp/ip protocol stack in guest
 will make sure this ordering will finally be correct.
 
 Nevertheless it's not good, and the latest Windows Logo tests will fail
 if you reorder frames.

Can we really enter qemu_net_queue_send with delivering == 0 and a
non-empty queue?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 15:49, Luigi Rizzo wrote:
 On Thu, May 31, 2012 at 03:23:12PM +0200, Jan Kiszka wrote:
 On 2012-05-31 15:19, Paolo Bonzini wrote:
 Il 31/05/2012 15:07, Zhi Yong Wu ha scritto:
 Yeah, this case actually exists, but tcp/ip protocol stack in guest
 will make sure this ordering will finally be correct.

 Nevertheless it's not good, and the latest Windows Logo tests will fail
 if you reorder frames.

 Can we really enter qemu_net_queue_send with delivering == 0 and a
 non-empty queue?
 
 i have no idea.
 
 it doesn't help that comments are using very very
 sparingly throughout the code...

You may find more hints in the commit logs.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Zhi Yong Wu
On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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



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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 15:45, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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
before and after may be fine - if we really need it before.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Zhi Yong Wu
On Thu, May 31, 2012 at 9:19 PM, Paolo Bonzini pbonz...@redhat.com wrote:
 Il 31/05/2012 15:07, Zhi Yong Wu ha scritto:
 Yeah, this case actually exists, but tcp/ip protocol stack in guest
 will make sure this ordering will finally be correct.

 Nevertheless it's not good, and the latest Windows Logo tests will fail
Yeah, it surely need to be improved.
 if you reorder frames.
I don't know the details for Windows logo tests. but i think that one
good protocol stack implementation should tolerate this case.

 Paolo



-- 
Regards,

Zhi Yong Wu



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

2012-05-31 Thread Zhi Yong Wu
On Thu, May 31, 2012 at 9:48 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2012-05-31 15:45, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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.

 Jan

 --
 Siemens AG, Corporate Technology, CT T DE IT 1
 Corporate Competence Center Embedded Linux



-- 
Regards,

Zhi Yong Wu



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

2012-05-31 Thread Jan Kiszka
On 2012-05-31 16:10, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 9:48 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2012-05-31 15:45, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



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

2012-05-31 Thread Zhi Yong Wu
On Thu, May 31, 2012 at 10:16 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2012-05-31 16:10, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 9:48 PM, Jan Kiszka jan.kis...@siemens.com wrote:
 On 2012-05-31 15:45, Zhi Yong Wu wrote:
 On Thu, May 31, 2012 at 5:18 PM, Stefan Hajnoczi stefa...@gmail.com 
 wrote:
 On Wed, May 30, 2012 at 8:59 AM, Luigi Rizzo ri...@iet.unipi.it 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



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

2012-05-30 Thread Luigi Rizzo
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);

return ret;
}




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

2012-05-30 Thread Luigi Rizzo
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);

return ret;
}