All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark McLoughlin <markmc@redhat.com>
To: qemu-devel@nongnu.org
Cc: Sven Rudolph <Sven_Rudolph@drewag.de>,
	Scott Tsai <scottt.tw@gmail.com>,
	Mark McLoughlin <markmc@redhat.com>
Subject: [Qemu-devel] [PATCH 1/5] tap: disable draining queue in one go
Date: Tue, 27 Oct 2009 18:16:35 +0000	[thread overview]
Message-ID: <1256667399-3149-2-git-send-email-markmc@redhat.com> (raw)
In-Reply-To: <1256667399-3149-1-git-send-email-markmc@redhat.com>

If qemu_send_packet_async() returns zero, it means the packet has been
queued and the sent callback will be invoked once it has been flushed.

This is only possible where the NIC's receive() handler returns zero
and promises to notify the networking core that room is available in its
queue again.

In the case where the receive handler does not have this capability
(and its queue fills up) it returns -1 and the networking core does not
queue up the packet. This condition is indicated by a -1 return from
qemu_send_packet_async().

Currently, tap handles this condition simply by dropping the packet. It
should do its best to avoid getting into this situation by checking such
NIC's have room for a packet before copying the packet from the tap
interface.

tap_send() used to achieve this by only reading a single packet before
returning to the mainloop. That way, tap_can_send() is called before
reading each packet.

tap_send() was changed to completely drain the tap interface queue
without taking into account the situation where the NIC returns an
error and the packet is not queued. Let's start fixing this by
reverting to the previous behaviour of reading one packet at a time.

Reported-by: Scott Tsai <scottt.tw@gmail.com>
Tested-by: Sven Rudolph <Sven_Rudolph@drewag.de>
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 net/tap.c |   33 +++++++++++++++------------------
 1 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 60354e4..f32226b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -187,26 +187,23 @@ 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;
 
-    do {
-        uint8_t *buf = s->buf;
-
-        size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
-        if (size <= 0) {
-            break;
-        }
+    size = tap_read_packet(s->fd, s->buf, sizeof(s->buf));
+    if (size <= 0) {
+        return;
+    }
 
-        if (s->has_vnet_hdr && !s->using_vnet_hdr) {
-            buf  += sizeof(struct virtio_net_hdr);
-            size -= sizeof(struct virtio_net_hdr);
-        }
+    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);
+    size = qemu_send_packet_async(s->vc, buf, size, tap_send_completed);
+    if (size == 0) {
+        tap_read_poll(s, 0);
+    }
 }
 
 int tap_has_ufo(VLANClientState *vc)
-- 
1.6.2.5

  reply	other threads:[~2009-10-27 18:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-27 18:16 [Qemu-devel] [PATCH 0/5] Fix packet queueing to allow full tap queue drain Mark McLoughlin
2009-10-27 18:16 ` Mark McLoughlin [this message]
2009-10-27 18:16 ` [Qemu-devel] [PATCH 2/5] net: disable receiving if client returns zero Mark McLoughlin
2009-10-27 18:16 ` [Qemu-devel] [PATCH 3/5] net/queue: queue packets even if sender doesn't supply a callback Mark McLoughlin
2009-10-27 18:16 ` [Qemu-devel] [PATCH 4/5] virtio-net: split the has_buffers() logic from can_receive() Mark McLoughlin
2009-10-27 18:16 ` [Qemu-devel] [PATCH 5/5] tap: drain queue in tap_send() Mark McLoughlin
2009-10-30 16:21   ` Anthony Liguori
2009-10-30 16:34     ` Mark McLoughlin
2009-10-30 17:46       ` Anthony Liguori
2009-10-28  3:36 ` [Qemu-devel] Re: [PATCH 0/5] Fix packet queueing to allow full tap queue drain Scott Tsai
2009-10-28  8:49   ` Mark McLoughlin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1256667399-3149-2-git-send-email-markmc@redhat.com \
    --to=markmc@redhat.com \
    --cc=Sven_Rudolph@drewag.de \
    --cc=qemu-devel@nongnu.org \
    --cc=scottt.tw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.