Patchwork [5/5] tap: drain queue in tap_send()

login
register
mail settings
Submitter Mark McLoughlin
Date Oct. 27, 2009, 6:16 p.m.
Message ID <1256667399-3149-6-git-send-email-markmc@redhat.com>
Download mbox | patch
Permalink /patch/37021/
State New
Headers show

Comments

Mark McLoughlin - Oct. 27, 2009, 6:16 p.m.
Okay, let's try re-enabling the drain-entire-queue behaviour, with a
Anthony Liguori - Oct. 30, 2009, 4:21 p.m.
Mark McLoughlin wrote:
> Okay, let's try re-enabling the drain-entire-queue behaviour, with a
> difference - before each subsequent packet, use qemu_can_send_packet()
> to check that we can send it. This is similar to how we check before
> polling the tap fd and avoids having to drop a packet if the receiver
> cannot handle it.
>
> This patch should be a performance improvement since we no longer have
> to go through the mainloop for each packet.
>
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
>   

Could you rebase and rend this patch against master?  I've got 1-4 in 
staging.

The GSO changes make resolving this non trivial (for me at least :-)).

Regards,

Anthony Liguori
Mark McLoughlin - Oct. 30, 2009, 4:34 p.m.
On Fri, 2009-10-30 at 11:21 -0500, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > Okay, let's try re-enabling the drain-entire-queue behaviour, with a
> > difference - before each subsequent packet, use qemu_can_send_packet()
> > to check that we can send it. This is similar to how we check before
> > polling the tap fd and avoids having to drop a packet if the receiver
> > cannot handle it.
> >
> > This patch should be a performance improvement since we no longer have
> > to go through the mainloop for each packet.
> >
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> >   
> 
> Could you rebase and rend this patch against master?  I've got 1-4 in 
> staging.
> 
> The GSO changes make resolving this non trivial (for me at least :-)).

Um, you've lost me :-)

You've got it in staging AFAICS, and anyway ... 5/5 is mostly a revert
of 1/5, so if 1/5 applied so should 5/5

What's in staging looks fine to me, except it doesn't build because of
problems with other patches

Thanks,
Mark.
Anthony Liguori - Oct. 30, 2009, 5:46 p.m.
Mark McLoughlin wrote:
> On Fri, 2009-10-30 at 11:21 -0500, Anthony Liguori wrote:
>   
>> Mark McLoughlin wrote:
>>     
>>> Okay, let's try re-enabling the drain-entire-queue behaviour, with a
>>> difference - before each subsequent packet, use qemu_can_send_packet()
>>> to check that we can send it. This is similar to how we check before
>>> polling the tap fd and avoids having to drop a packet if the receiver
>>> cannot handle it.
>>>
>>> This patch should be a performance improvement since we no longer have
>>> to go through the mainloop for each packet.
>>>
>>> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
>>>   
>>>       
>> Could you rebase and rend this patch against master?  I've got 1-4 in 
>> staging.
>>
>> The GSO changes make resolving this non trivial (for me at least :-)).
>>     
>
> Um, you've lost me :-)
>
> You've got it in staging AFAICS, and anyway ... 5/5 is mostly a revert
> of 1/5, so if 1/5 applied so should 5/5
>
> What's in staging looks fine to me, except it doesn't build because of
> problems with other patches
>   

Ignore it, I accidentally tried backporting the patch targeted for the 
stable tree and then responded to the wrong patch.

Regards,

Anthony Liguori

Patch

difference - before each subsequent packet, use qemu_can_send_packet()
to check that we can send it. This is similar to how we check before
polling the tap fd and avoids having to drop a packet if the receiver
cannot handle it.

This patch should be a performance improvement since we no longer have
to go through the mainloop for each packet.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net/tap.c |   33 ++++++++++++++++++---------------
 1 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index f32226b..5272d7d 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -187,23 +187,26 @@  static void tap_send_completed(VLANClientState *vc, ssize_t len)
 static void tap_send(void *opaque)
 {
     TAPState *s = opaque;
-    uint8_t *buf = s->buf;
     int size;
 
-    size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
-    if (size <= 0) {
-        return;
-    }
+    do {
+        uint8_t *buf = s->buf;
 
-    if (s->has_vnet_hdr && !s->using_vnet_hdr) {
-        buf  += sizeof(struct virtio_net_hdr);
-        size -= sizeof(struct virtio_net_hdr);
-    }
+        size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
+        if (size <= 0) {
+            break;
+        }
 
-    size = qemu_send_packet_async(s->vc, buf, size, tap_send_completed);
-    if (size == 0) {
-        tap_read_poll(s, 0);
-    }
+        if (s->has_vnet_hdr && !s->using_vnet_hdr) {
+            buf  += sizeof(struct virtio_net_hdr);
+            size -= sizeof(struct virtio_net_hdr);
+        }
+
+        size = qemu_send_packet_async(s->vc, buf, size, tap_send_completed);
+        if (size == 0) {
+            tap_read_poll(s, 0);
+        }
+    } while (size > 0 && qemu_can_send_packet(s->vc));
 }
 
 int tap_has_ufo(VLANClientState *vc)