[PATCH 1/3] Remove the TX timer from virtio_net

[PATCH 1/3] Remove the TX timer from virtio_net

[Anthony Liguori]

Doing tx mitigation in the host gets equivalent performance and offers greater
flexibility.  It also simplifies the guest code.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

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);

-------------------------------------------------------------------------
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

[PATCH 2/3] Add support for VRING_USED_F_NOTIFY_ON_FULL

[Anthony Liguori]

This patch adds support for VRING_USED_F_NOTIFY_ON_FULL.  When this bit is
set, virtio ring notifications will be suppressed unless the virtio ring is
full.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 1302ac4..ee7936f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -146,7 +146,9 @@ static void vring_kick(struct virtqueue *_vq)
 	/* Need to update avail index before checking if we should notify */
 	mb();
 
-	if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
+	if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY) &&
+	    (!(vq->vring.used->flags & VRING_USED_F_NOTIFY_ON_FULL) ||
+	     (vq->num_free == 0)))
 		/* Prod other side to tell it about changes. */
 		vq->notify(&vq->vq);
 
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index ea3be89..ae6092e 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -17,8 +17,13 @@
 
 /* This means don't notify other side when buffer added. */
 #define VRING_USED_F_NO_NOTIFY	1
+/* Don't notify the other side unless the buffer is full */
+#define VRING_USED_F_NOTIFY_ON_FULL 2
+
 /* This means don't interrupt guest when buffer consumed. */
 #define VRING_AVAIL_F_NO_INTERRUPT	1
+/* Don't notify the other side unless the buffer is full */
+#define VRING_AVAIL_F_NOTIFY_ON_FULL 2
 
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
 struct vring_desc

-------------------------------------------------------------------------
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

[PATCH 3/3][QEMU] Support TX mitigation in QEMU's virtio_net device

[Anthony Liguori]

This patch adds support for TX mitigation in QEMU using the new NOTIFY_ON_FULL
virtio ring flag.

Signed-off-by: Anthony Liguori <aliguori-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

diff --git a/qemu/hw/virtio-net.c b/qemu/hw/virtio-net.c
index 3940743..5fd4114 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,39 @@ 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) {
+	vq->vring.used->flags &= ~VRING_USED_F_NOTIFY_ON_FULL;
+	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;
+	vq->vring.used->flags |= VRING_USED_F_NOTIFY_ON_FULL;
+    }
+}
+
+static void virtio_net_tx_timer(void *opaque)
+{
+    VirtIONet *n = opaque;
+
+    n->tx_vq->vring.used->flags &= ~VRING_USED_F_NOTIFY_ON_FULL;
+    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 +305,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;
 }
diff --git a/qemu/hw/virtio.h b/qemu/hw/virtio.h
index dee97ba..21b96a3 100644
--- a/qemu/hw/virtio.h
+++ b/qemu/hw/virtio.h
@@ -39,6 +39,7 @@
 
 /* This means don't notify other side when buffer added. */
 #define VRING_USED_F_NO_NOTIFY	1
+#define VRING_USED_F_NOTIFY_ON_FULL  2
 /* This means don't interrupt guest when buffer consumed. */
 #define VRING_AVAIL_F_NO_INTERRUPT	1
 

-------------------------------------------------------------------------
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

Re: [PATCH 3/3][QEMU] Support TX mitigation in QEMU's virtio_net device

[Avi Kivity]

Anthony Liguori wrote:
> This patch adds support for TX mitigation in QEMU using the new NOTIFY_ON_FULL
> virtio ring flag.
>
>   

With this  (modified to use VRING_USED_F_NO_NOTIFY as Rusty suggests) 
and the latest virtio patchqueue (now available as the virtio branch on 
kvm.git) I get better latency, but pings from the guest are still lost 
after 12-14 replies.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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