[Qemu-devel] [PATCH 15/16] net: refactor packet queueing code

[Mark McLoughlin]

The packet queue code is fairly standalone, has some complex details and
easily reusable. It makes sense to split it out on its own. This patch
doesn't contain any functional changes.

Signed-off-by: Mark McLoughlin <address@hidden>
---
 Makefile      |    3 +-
 net-queue.c   |  250 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net-queue.h   |   64 +++++++++++++++
 net.c         |  138 +++++--------------------------
 net.h         |   18 +----
 qemu-common.h |    1 +
 6 files changed, 342 insertions(+), 132 deletions(-)
 create mode 100644 net-queue.c
 create mode 100644 net-queue.h

diff --git a/Makefile b/Makefile
index c552739..e94dd2c 100644
--- a/Makefile
+++ b/Makefile
@@ -96,7 +96,8 @@ obj-y += usb-serial.o usb-net.o usb-bus.o
 obj-y += sd.o ssi-sd.o
 obj-y += bt.o bt-host.o bt-vhci.o bt-l2cap.o bt-sdp.o bt-hci.o bt-hid.o 
usb-bt.o
 obj-y += bt-hci-csr.o
-obj-y += buffered_file.o migration.o migration-tcp.o net.o qemu-sockets.o
+obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o
+obj-y += net.o net-queue.o
 obj-y += qemu-char.o aio.o net-checksum.o savevm.o
 obj-y += msmouse.o ps2.o
 obj-y += qdev.o qdev-properties.o ssi.o
diff --git a/net-queue.c b/net-queue.c
new file mode 100644
index 0000000..75457f0
--- /dev/null
+++ b/net-queue.c
@@ -0,0 +1,250 @@
+/*
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2009 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "net-queue.h"
+#include "qemu-queue.h"
+
+/* The delivery handler may only return zero if it will call
+ * qemu_net_queue_flush() when it determines that it is once again able
+ * to deliver packets. It must also call qemu_net_queue_purge() in its
+ * cleanup path.
+ *
+ * If a sent callback is provided to send(), the caller must handle a
+ * zero return from the delivery handler by not sending any more packets
+ * until we have invoked the callback. Only in that case will we queue
+ * the packet.
+ *
+ * If a sent callback isn't provided, we just drop the packet to avoid
+ * unbounded queueing.
+ */
+
+struct NetPacket {
+    QTAILQ_ENTRY(NetPacket) entry;
+    VLANClientState *sender;
+    int size;
+    NetPacketSent *sent_cb;
+    uint8_t data[0];
+};
+
+struct NetQueue {
+    NetPacketDeliver *deliver;
+    NetPacketDeliverIOV *deliver_iov;
+    void *opaque;
+
+    QTAILQ_HEAD(packets, NetPacket) packets;
+
+    unsigned delivering : 1;
+};
+
+NetQueue *qemu_new_net_queue(NetPacketDeliver *deliver,
+                             NetPacketDeliverIOV *deliver_iov,
+                             void *opaque)
+{
+    NetQueue *queue;
+
+    queue = qemu_mallocz(sizeof(NetQueue));
+
+    queue->deliver = deliver;
+    queue->deliver_iov = deliver_iov;
+    queue->opaque = opaque;
+
+    QTAILQ_INIT(&queue->packets);
+
+    queue->delivering = 0;
+
+    return queue;
+}
+
+void qemu_del_net_queue(NetQueue *queue)
+{
+    NetPacket *packet, *next;
+
+    QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+        QTAILQ_REMOVE(&queue->packets, packet, entry);
+        qemu_free(packet);
+    }
+
+    qemu_free(queue);
+}
+
+static ssize_t qemu_net_queue_append(NetQueue *queue,
+                                     VLANClientState *sender,
+                                     const uint8_t *buf,
+                                     size_t size,
+                                     NetPacketSent *sent_cb)
+{
+    NetPacket *packet;
+
+    packet = qemu_malloc(sizeof(NetPacket) + size);
+    packet->sender = sender;
+    packet->size = size;
+    packet->sent_cb = sent_cb;
+    memcpy(packet->data, buf, size);
+
+    QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
+
+    return size;
+}
+
+static ssize_t qemu_net_queue_append_iov(NetQueue *queue,
+                                         VLANClientState *sender,
+                                         const struct iovec *iov,
+                                         int iovcnt,
+                                         NetPacketSent *sent_cb)
+{
+    NetPacket *packet;
+    size_t max_len = 0;
+    int i;
+
+    for (i = 0; i < iovcnt; i++) {
+        max_len += iov[i].iov_len;
+    }
+
+    packet = qemu_malloc(sizeof(NetPacket) + max_len);
+    packet->sender = sender;
+    packet->sent_cb = sent_cb;
+    packet->size = 0;
+
+    for (i = 0; i < iovcnt; i++) {
+        size_t len = iov[i].iov_len;
+
+        memcpy(packet->data + packet->size, iov[i].iov_base, len);
+        packet->size += len;
+    }
+
+    QTAILQ_INSERT_TAIL(&queue->packets, packet, entry);
+
+    return packet->size;
+}
+
+static ssize_t qemu_net_queue_deliver(NetQueue *queue,
+                                      VLANClientState *sender,
+                                      const uint8_t *data,
+                                      size_t size)
+{
+    ssize_t ret = -1;
+
+    queue->delivering = 1;
+    ret = queue->deliver(sender, data, size, queue->opaque);
+    queue->delivering = 0;
+
+    return ret;
+}
+
+static ssize_t qemu_net_queue_deliver_iov(NetQueue *queue,
+                                          VLANClientState *sender,
+                                          const struct iovec *iov,
+                                          int iovcnt)
+{
+    ssize_t ret = -1;
+
+    queue->delivering = 1;
+    ret = queue->deliver_iov(sender, iov, iovcnt, queue->opaque);
+    queue->delivering = 0;
+
+    return ret;
+}
+
+ssize_t qemu_net_queue_send(NetQueue *queue,
+                            VLANClientState *sender,
+                            const uint8_t *data,
+                            size_t size,
+                            NetPacketSent *sent_cb)
+{
+    ssize_t ret;
+
+    if (queue->delivering) {
+        return qemu_net_queue_append(queue, sender, data, size, NULL);
+    }
+
+    ret = qemu_net_queue_deliver(queue, sender, data, size);
+    if (ret == 0 && sent_cb != NULL) {
+        qemu_net_queue_append(queue, sender, data, size, sent_cb);
+        return 0;
+    }
+
+    qemu_net_queue_flush(queue);
+
+    return ret;
+}
+
+ssize_t qemu_net_queue_send_iov(NetQueue *queue,
+                                VLANClientState *sender,
+                                const struct iovec *iov,
+                                int iovcnt,
+                                NetPacketSent *sent_cb)
+{
+    ssize_t ret;
+
+    if (queue->delivering) {
+        return qemu_net_queue_append_iov(queue, sender, iov, iovcnt, NULL);
+    }
+
+    ret = qemu_net_queue_deliver_iov(queue, sender, iov, iovcnt);
+    if (ret == 0 && sent_cb != NULL) {
+        qemu_net_queue_append_iov(queue, sender, iov, iovcnt, sent_cb);
+        return 0;
+    }
+
+    qemu_net_queue_flush(queue);
+
+    return ret;
+}
+
+void qemu_net_queue_purge(NetQueue *queue, VLANClientState *from)
+{
+    NetPacket *packet, *next;
+
+    QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) {
+        if (packet->sender == from) {
+            QTAILQ_REMOVE(&queue->packets, packet, entry);
+            qemu_free(packet);
+        }
+    }
+}
+
+void qemu_net_queue_flush(NetQueue *queue)
+{
+    while (!QTAILQ_EMPTY(&queue->packets)) {
+        NetPacket *packet;
+        int ret;
+
+        packet = QTAILQ_FIRST(&queue->packets);
+        QTAILQ_REMOVE(&queue->packets, packet, entry);
+
+        ret = qemu_net_queue_deliver(queue,
+                                     packet->sender,
+                                     packet->data,
+                                     packet->size);
+        if (ret == 0 && packet->sent_cb != NULL) {
+            QTAILQ_INSERT_HEAD(&queue->packets, packet, entry);
+            break;
+        }
+
+        if (packet->sent_cb) {
+            packet->sent_cb(packet->sender, ret);
+        }
+
+        qemu_free(packet);
+    }
+}
diff --git a/net-queue.h b/net-queue.h
new file mode 100644
index 0000000..ea17df6
--- /dev/null
+++ b/net-queue.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2009 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_NET_QUEUE_H
+#define QEMU_NET_QUEUE_H
+
+#include "qemu-common.h"
+
+typedef struct NetPacket NetPacket;
+typedef struct NetQueue NetQueue;
+
+typedef void (NetPacketSent) (VLANClientState *sender, ssize_t ret);
+
+typedef ssize_t (NetPacketDeliver) (VLANClientState *sender,
+                                    const uint8_t *buf,
+                                    size_t size,
+                                    void *opaque);
+
+typedef ssize_t (NetPacketDeliverIOV) (VLANClientState *sender,
+                                       const struct iovec *iov,
+                                       int iovcnt,
+                                       void *opaque);
+
+NetQueue *qemu_new_net_queue(NetPacketDeliver *deliver,
+                             NetPacketDeliverIOV *deliver_iov,
+                             void *opaque);
+void qemu_del_net_queue(NetQueue *queue);
+
+ssize_t qemu_net_queue_send(NetQueue *queue,
+                            VLANClientState *sender,
+                            const uint8_t *data,
+                            size_t size,
+                            NetPacketSent *sent_cb);
+
+ssize_t qemu_net_queue_send_iov(NetQueue *queue,
+                                VLANClientState *sender,
+                                const struct iovec *iov,
+                                int iovcnt,
+                                NetPacketSent *sent_cb);
+
+void qemu_net_queue_purge(NetQueue *queue, VLANClientState *from);
+void qemu_net_queue_flush(NetQueue *queue);
+
+#endif /* QEMU_NET_QUEUE_H */
diff --git a/net.c b/net.c
index 5d78f04..2c7b2ef 100644
--- a/net.c
+++ b/net.c
@@ -422,15 +422,16 @@ int qemu_can_send_packet(VLANClientState *sender)
     return 0;
 }
 
-static int
-qemu_deliver_packet(VLANClientState *sender, const uint8_t *buf, int size)
+static ssize_t qemu_vlan_deliver_packet(VLANClientState *sender,
+                                        const uint8_t *buf,
+                                        size_t size,
+                                        void *opaque)
 {
+    VLANState *vlan = opaque;
     VLANClientState *vc;
     int ret = -1;
 
-    sender->vlan->delivering = 1;
-
-    QTAILQ_FOREACH(vc, &sender->vlan->clients, next) {
+    QTAILQ_FOREACH(vc, &vlan->clients, next) {
         ssize_t len;
 
         if (vc == sender) {
@@ -447,24 +448,15 @@ qemu_deliver_packet(VLANClientState *sender, const 
uint8_t *buf, int size)
         ret = (ret >= 0) ? ret : len;
     }
 
-    sender->vlan->delivering = 0;
-
     return ret;
 }
 
 void qemu_purge_queued_packets(VLANClientState *vc)
 {
-    VLANPacket *packet, *next;
-
     if (!vc->vlan)
         return;
 
-    QTAILQ_FOREACH_SAFE(packet, &vc->vlan->send_queue, entry, next) {
-        if (packet->sender == vc) {
-            QTAILQ_REMOVE(&vc->vlan->send_queue, packet, entry);
-            qemu_free(packet);
-        }
-    }
+    qemu_net_queue_purge(vc->vlan->send_queue, vc);
 }
 
 void qemu_flush_queued_packets(VLANClientState *vc)
@@ -472,47 +464,13 @@ void qemu_flush_queued_packets(VLANClientState *vc)
     if (!vc->vlan)
         return;
 
-    while (!QTAILQ_EMPTY(&vc->vlan->send_queue)) {
-        VLANPacket *packet;
-        int ret;
-
-        packet = QTAILQ_FIRST(&vc->vlan->send_queue);
-        QTAILQ_REMOVE(&vc->vlan->send_queue, packet, entry);
-
-        ret = qemu_deliver_packet(packet->sender, packet->data, packet->size);
-        if (ret == 0 && packet->sent_cb != NULL) {
-            QTAILQ_INSERT_HEAD(&vc->vlan->send_queue, packet, entry);
-            break;
-        }
-
-        if (packet->sent_cb)
-            packet->sent_cb(packet->sender, ret);
-
-        qemu_free(packet);
-    }
-}
-
-static void qemu_enqueue_packet(VLANClientState *sender,
-                                const uint8_t *buf, int size,
-                                NetPacketSent *sent_cb)
-{
-    VLANPacket *packet;
-
-    packet = qemu_malloc(sizeof(VLANPacket) + size);
-    packet->sender = sender;
-    packet->size = size;
-    packet->sent_cb = sent_cb;
-    memcpy(packet->data, buf, size);
-
-    QTAILQ_INSERT_TAIL(&sender->vlan->send_queue, packet, entry);
+    qemu_net_queue_flush(vc->vlan->send_queue);
 }
 
 ssize_t qemu_send_packet_async(VLANClientState *sender,
                                const uint8_t *buf, int size,
                                NetPacketSent *sent_cb)
 {
-    int ret;
-
     if (sender->link_down || !sender->vlan) {
         return size;
     }
@@ -522,20 +480,8 @@ ssize_t qemu_send_packet_async(VLANClientState *sender,
     hex_dump(stdout, buf, size);
 #endif
 
-    if (sender->vlan->delivering) {
-        qemu_enqueue_packet(sender, buf, size, NULL);
-        return size;
-    }
-
-    ret = qemu_deliver_packet(sender, buf, size);
-    if (ret == 0 && sent_cb != NULL) {
-        qemu_enqueue_packet(sender, buf, size, sent_cb);
-        return 0;
-    }
-
-    qemu_flush_queued_packets(sender);
-
-    return ret;
+    return qemu_net_queue_send(sender->vlan->send_queue,
+                               sender, buf, size, sent_cb);
 }
 
 void qemu_send_packet(VLANClientState *vc, const uint8_t *buf, int size)
@@ -571,15 +517,16 @@ static ssize_t calc_iov_length(const struct iovec *iov, 
int iovcnt)
     return offset;
 }
 
-static int qemu_deliver_packet_iov(VLANClientState *sender,
-                                   const struct iovec *iov, int iovcnt)
+static ssize_t qemu_vlan_deliver_packet_iov(VLANClientState *sender,
+                                            const struct iovec *iov,
+                                            int iovcnt,
+                                            void *opaque)
 {
+    VLANState *vlan = opaque;
     VLANClientState *vc;
-    int ret = -1;
-
-    sender->vlan->delivering = 1;
+    ssize_t ret = -1;
 
-    QTAILQ_FOREACH(vc, &sender->vlan->clients, next) {
+    QTAILQ_FOREACH(vc, &vlan->clients, next) {
         ssize_t len;
 
         if (vc == sender) {
@@ -600,61 +547,19 @@ static int qemu_deliver_packet_iov(VLANClientState 
*sender,
         ret = (ret >= 0) ? ret : len;
     }
 
-    sender->vlan->delivering = 0;
-
     return ret;
 }
 
-static ssize_t qemu_enqueue_packet_iov(VLANClientState *sender,
-                                       const struct iovec *iov, int iovcnt,
-                                       NetPacketSent *sent_cb)
-{
-    VLANPacket *packet;
-    size_t max_len = 0;
-    int i;
-
-    max_len = calc_iov_length(iov, iovcnt);
-
-    packet = qemu_malloc(sizeof(VLANPacket) + max_len);
-    packet->sender = sender;
-    packet->sent_cb = sent_cb;
-    packet->size = 0;
-
-    for (i = 0; i < iovcnt; i++) {
-        size_t len = iov[i].iov_len;
-
-        memcpy(packet->data + packet->size, iov[i].iov_base, len);
-        packet->size += len;
-    }
-
-    QTAILQ_INSERT_TAIL(&sender->vlan->send_queue, packet, entry);
-
-    return packet->size;
-}
-
 ssize_t qemu_sendv_packet_async(VLANClientState *sender,
                                 const struct iovec *iov, int iovcnt,
                                 NetPacketSent *sent_cb)
 {
-    int ret;
-
     if (sender->link_down || !sender->vlan) {
         return calc_iov_length(iov, iovcnt);
     }
 
-    if (sender->vlan->delivering) {
-        return qemu_enqueue_packet_iov(sender, iov, iovcnt, NULL);
-    }
-
-    ret = qemu_deliver_packet_iov(sender, iov, iovcnt);
-    if (ret == 0 && sent_cb != NULL) {
-        qemu_enqueue_packet_iov(sender, iov, iovcnt, sent_cb);
-        return 0;
-    }
-
-    qemu_flush_queued_packets(sender);
-
-    return ret;
+    return qemu_net_queue_send_iov(sender->vlan->send_queue,
+                                   sender, iov, iovcnt, sent_cb);
 }
 
 ssize_t
@@ -2348,7 +2253,10 @@ VLANState *qemu_find_vlan(int id, int allocate)
     vlan = qemu_mallocz(sizeof(VLANState));
     vlan->id = id;
     QTAILQ_INIT(&vlan->clients);
-    QTAILQ_INIT(&vlan->send_queue);
+
+    vlan->send_queue = qemu_new_net_queue(qemu_vlan_deliver_packet,
+                                          qemu_vlan_deliver_packet_iov,
+                                          vlan);
 
     QTAILQ_INSERT_TAIL(&vlans, vlan, next);
 
diff --git a/net.h b/net.h
index fa59c3d..e79f524 100644
--- a/net.h
+++ b/net.h
@@ -5,11 +5,10 @@
 #include "qemu-common.h"
 #include "qdict.h"
 #include "qemu-option.h"
+#include "net-queue.h"
 
 /* VLANs support */
 
-typedef struct VLANClientState VLANClientState;
-
 typedef int (NetCanReceive)(VLANClientState *);
 typedef ssize_t (NetReceive)(VLANClientState *, const uint8_t *, size_t);
 typedef ssize_t (NetReceiveIOV)(VLANClientState *, const struct iovec *, int);
@@ -34,25 +33,12 @@ struct VLANClientState {
     char info_str[256];
 };
 
-typedef struct VLANPacket VLANPacket;
-
-typedef void (NetPacketSent) (VLANClientState *, ssize_t);
-
-struct VLANPacket {
-    QTAILQ_ENTRY(VLANPacket) entry;
-    VLANClientState *sender;
-    int size;
-    NetPacketSent *sent_cb;
-    uint8_t data[0];
-};
-
 struct VLANState {
     int id;
     QTAILQ_HEAD(, VLANClientState) clients;
     QTAILQ_ENTRY(VLANState) next;
     unsigned int nb_guest_devs, nb_host_devs;
-    QTAILQ_HEAD(send_queue, VLANPacket) send_queue;
-    int delivering;
+    NetQueue *send_queue;
 };
 
 VLANState *qemu_find_vlan(int id, int allocate);
diff --git a/qemu-common.h b/qemu-common.h
index 12e7dd0..9ad2f0d 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -178,6 +178,7 @@ typedef struct TextConsole TextConsole;
 typedef TextConsole QEMUConsole;
 typedef struct CharDriverState CharDriverState;
 typedef struct VLANState VLANState;
+typedef struct VLANClientState VLANClientState;
 typedef struct QEMUFile QEMUFile;
 typedef struct i2c_bus i2c_bus;
 typedef struct i2c_slave i2c_slave;
-- 
1.6.2.5