[RFC] Remove tx_timer from virtio_net

[Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

Avi and I were talking this afternoon and he suggested that we should 
remove the tx_timer from the virtio_net front-end and replace it with a 
tx_timer in the backend.

Since the backend can suppress notifications, this is appealing since it 
gives much more flexibility to the backend in determining how to do tx 
batching.  I've done a quick implementation and performance is pretty good.

We may need an ABI change, however.  When the backend disables 
notifications, there is absolutely no way for the frontend to notify 
anymore.  In the case where the queue fills up, it cannot flush since 
the backend has disabled notifications.  To work around this, I had to 
least notifications enabled and check for the case where the queue fills 
up manually.

I think a possible solution to this would be to differentiate between a 
soft and hard notify.  We would introduce a VRING_USED_F_NOTIFY_ON_FULL 
and a VRING_AVAIL_F_NOTIFY_ON_FULL.

The NO_NOTIFY variants would indicate that the other end never sends a 
notify, whereas NOTIFY_ON_FULL would indicate that the other end never 
sends a notify unless the queue fills up.

Regards,

Anthony Liguori

[-- Attachment #2: virtio_net_tx_timer.diff --]
[-- Type: text/x-patch, Size: 2207 bytes --]

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 3940743..1006dee 100644
--- a/qemu/hw/virtio-net.c
+++ b/qemu/hw/virtio-net.c
@@ -14,6 +14,7 @@
 #include "virtio.h"
 #include "net.h"
 #include "pc.h"
+#include "qemu-timer.h"
 
 /* from Linux's virtio_net.h */
 
@@ -28,6 +29,10 @@
 #define VIRTIO_NET_F_TSO6	4
 #define VIRTIO_NET_F_MAC	5
 
+#define USE_TX_TIMER
+
+#define TX_TIMER_INTERVAL (1000 / 500)
+
 /* The config defining mac address (6 bytes) */
 struct virtio_net_config
 {
@@ -63,6 +68,8 @@ typedef struct VirtIONet
     int tap_fd;
     struct VirtIONet *next;
     int do_notify;
+    QEMUTimer *tx_timer;
+    int tx_timer_active;
 } VirtIONet;
 
 static VirtIONet *VirtIONetHead = NULL;
@@ -222,10 +229,10 @@ again:
 }
 
 /* TX */
-static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
+static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
 {
-    VirtIONet *n = to_virtio_net(vdev);
     VirtQueueElement elem;
+    int count = 0;
 
     if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
         return;
@@ -241,11 +248,36 @@ static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
 	    len += elem.out_sg[i].iov_len;
 	}
 
+	count++;
+
 	virtqueue_push(vq, &elem, sizeof(struct virtio_net_hdr) + len);
 	virtio_notify(&n->vdev, vq);
     }
 }
 
+static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIONet *n = to_virtio_net(vdev);
+
+    if (n->tx_timer_active &&
+	(vq->vring.avail->idx - vq->last_avail_idx) == 64) {
+	qemu_del_timer(n->tx_timer);
+	n->tx_timer_active = 0;
+	virtio_net_flush_tx(n, vq);
+    } else {
+	qemu_mod_timer(n->tx_timer, qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
+	n->tx_timer_active = 1;
+    }
+}
+
+static void virtio_net_tx_timer(void *opaque)
+{
+    VirtIONet *n = opaque;
+
+    n->tx_timer_active = 0;
+    virtio_net_flush_tx(n, n->tx_vq);
+}
+
 void *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     VirtIONet *n;
@@ -270,5 +302,8 @@ void *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
         VirtIONetHead = n;
     }
 
+    n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
+    n->tx_timer_active = 0;
+
     return &n->vdev;
 }

[-- Attachment #3: virtio:net_remove-tx-timer.patch --]
[-- Type: text/x-patch, Size: 2529 bytes --]

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 4082099..797a3ed 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -23,7 +23,6 @@
 #include <linux/virtio.h>
 #include <linux/virtio_net.h>
 #include <linux/scatterlist.h>
-#include <linux/hrtimer.h>
 
 static int napi_weight = 128;
 module_param(napi_weight, int, 0444);
@@ -40,15 +39,9 @@ struct virtnet_info
 	struct net_device *dev;
 	struct napi_struct napi;
 
-	/* TX coalescing. */
-	struct hrtimer tx_timer;
-
 	/* Number of input buffers, and max we've ever had. */
 	unsigned int num, max;
 
-	/* Number of queued output buffers, and max we've ever had. */
-	unsigned int out_num, out_max;
-
 	/* Receive & send queues. */
 	struct sk_buff_head recv;
 	struct sk_buff_head send;
@@ -238,20 +231,6 @@ again:
 	return received;
 }
 
-static enum hrtimer_restart kick_xmit(struct hrtimer *t)
-{
-	struct virtnet_info *vi = container_of(t,struct virtnet_info,tx_timer);
-
-	BUG_ON(!in_softirq());
-	BUG_ON(in_irq());
-	netif_tx_lock(vi->dev);
-	vi->svq->vq_ops->kick(vi->svq);
-	vi->out_num = 0;
-	netif_tx_unlock(vi->dev);
-
-	return HRTIMER_NORESTART;
-}
-
 static int start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct virtnet_info *vi = netdev_priv(dev);
@@ -305,13 +284,8 @@ again:
 			goto again;
 
 		pr_debug("%s: virtio not prepared to send\n", dev->name);
-		if (vi->out_max != vi->out_num)
-			printk("%s: out_max changed from %u to %u\n",
-			       dev->name, vi->out_max, vi->out_num);
-		vi->out_max = vi->out_num;
-		vi->out_num = 0;
+
 		/* Kick off send immediately. */
-		hrtimer_cancel(&vi->tx_timer);
 		vi->svq->vq_ops->kick(vi->svq);
 		netif_stop_queue(dev);
 
@@ -327,13 +301,8 @@ again:
 		return NETDEV_TX_BUSY;
 	}
 
-	if (++vi->out_num == vi->out_max) {
-		hrtimer_cancel(&vi->tx_timer);
-		vi->svq->vq_ops->kick(vi->svq);
-		vi->out_num = 0;
-	} else if (!hrtimer_is_queued(&vi->tx_timer))
-		hrtimer_start(&vi->tx_timer, ktime_set(0,500000),
-			      HRTIMER_MODE_REL);
+	vi->svq->vq_ops->kick(vi->svq);
+
 	return 0;
 }
 
@@ -425,10 +394,6 @@ static int virtnet_probe(struct virtio_device *vdev)
 	netif_napi_add(dev, &vi->napi, virtnet_poll, napi_weight);
 	vi->dev = dev;
 	vi->vdev = vdev;
-	memset(&vi->tx_timer, 0, sizeof(vi->tx_timer));
-	hrtimer_init(&vi->tx_timer, CLOCK_REALTIME, HRTIMER_MODE_REL);
-	vi->tx_timer.function = kick_xmit;
-	vi->out_max = -1U;
 
 	/* We expect two virtqueues, receive then send. */
 	vi->rvq = vdev->config->find_vq(vdev, 0, skb_recv_done);

[-- Attachment #4: Type: text/plain, Size: 278 bytes --]

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

[-- Attachment #5: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel