Kill off the virtio_net tx mitigation timer

len += qemu_sendv_packet(n->vc, out_sg, out_num);

qemu/hw/virtio-net.c +1 -0

+	kvm_sleep_end();

By removing the tx timer altogether and doing all the copies in the I/O thread, we can keep the I/O churning away in parallel with the guest generating more I/O.

In my tests, this significantly increases guest->host throughput, causes a minor increase in host->guest throughput, reduces CPU utilization somewhat and greatly reduces roundtrip times.

Even aside from the benchmark results, removing the arbitrary 150us timer is a nicer option than coming up with a heuristic to make it vary according to load. Finally, on kernels which don't have a suitably low posix timer latency, we won't be scuppered by effectively having e.g. a 1ms timer.

Note, this highlights that the I/O thread may become a scalability concern and we might want to consider e.g. an I/O thread per device.

Note also that when tuning for a specific workload, which CPU the I/O thread is pinned to is important.

Signed-off-by Mark McLoughlin <[email protected]>

---
 qemu/hw/virtio-net.c |   79 ++++++++++++++++++++++++++++---------------------
 1 files changed, 45 insertions(+), 34 deletions(-)

qemu/hw/virtio-net.c +45 -34

index bc2ede6..0612f5f 100644
@@ -15,6 +15,8 @@
 #include "net.h"
 #include "qemu-timer.h"
 #include "qemu-kvm.h"
+#include "qemu-char.h"
+#include "compatfd.h"
 
 /* from Linux's virtio_net.h */
 

@@ -35,8 +37,6 @@
 #define VIRTIO_NET_F_HOST_ECN	13	/* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO	14	/* Host can handle UFO in. */
 
-#define TX_TIMER_INTERVAL 150000 /* 150 us */
-
 /* The config defining mac address (6 bytes) */
 struct virtio_net_config
 {

@@ -68,8 +68,7 @@ typedef struct VirtIONet
     VirtQueue *rx_vq;
     VirtQueue *tx_vq;
     VLANClientState *vc;
-    QEMUTimer *tx_timer;
-    int tx_timer_active;
+    int tx_eventfds[2];
 } VirtIONet;
 
 /* TODO

@@ -227,13 +226,14 @@ static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
 }
 
 /* TX */
-static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
+static int virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
 {
     VirtQueueElement elem;
     int has_vnet_hdr = tap_has_vnet_hdr(n->vc->vlan->first_client);
+    int num_packets = 0;
 
     if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
-        return;
+        return num_packets;
 
     while (virtqueue_pop(vq, &elem)) {
 	ssize_t len = 0;

@@ -256,38 +256,31 @@ static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq)
 
 	virtqueue_push(vq, &elem, len);
 	virtio_notify(&n->vdev, vq);
+
+	num_packets++;
     }
+
+    return num_packets;
 }
 
 static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIONet *n = to_virtio_net(vdev);
 
-    if (n->tx_timer_active) {
-	vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
-	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_NO_NOTIFY;
-    }
+    vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY;
+    qemu_eventfd_write(n->tx_eventfds[1], 1);
 }
 
-static void virtio_net_tx_timer(void *opaque)
+static void virtio_net_tx_event(void *opaque)
 {
     VirtIONet *n = opaque;
 
-    n->tx_timer_active = 0;
+    qemu_eventfd_read(n->tx_eventfds[0]);
 
-    /* Just in case the driver is not ready on more */
-    if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
-        return;
-
-    n->tx_vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
-    virtio_net_flush_tx(n, n->tx_vq);
+    if (!virtio_net_flush_tx(n, n->tx_vq))
+        n->tx_vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
+    else
+        qemu_eventfd_write(n->tx_eventfds[1], 1);
 }
 
 static void virtio_net_save(QEMUFile *f, void *opaque)

@@ -297,7 +290,6 @@ static void virtio_net_save(QEMUFile *f, void *opaque)
     virtio_save(&n->vdev, f);
 
     qemu_put_buffer(f, n->mac, 6);
-    qemu_put_be32(f, n->tx_timer_active);
 }
 
 static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)

@@ -310,12 +302,16 @@ static int virtio_net_load(QEMUFile *f, void *opaque, int version_id)
     virtio_load(&n->vdev, f);
 
     qemu_get_buffer(f, n->mac, 6);
-    n->tx_timer_active = qemu_get_be32(f);
 
-    if (n->tx_timer_active) {
-	qemu_mod_timer(n->tx_timer,
-		       qemu_get_clock(vm_clock) + TX_TIMER_INTERVAL);
-    }
+    return 0;
+}
+
+static int virtio_net_uninit(PCIDevice *dev)
+{
+    VirtIONet *n = (VirtIONet *)dev;
+
+    close(n->tx_eventfds[0]);
+    close(n->tx_eventfds[1]);
 
     return 0;
 }

@@ -324,13 +320,23 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
 {
     VirtIONet *n;
     static int virtio_net_id;
+    int eventfds[2];
+
+    if (qemu_eventfd(eventfds) == -1) {
+        fprintf(stderr, "Failed to create eventfds : %s\n",
+                strerror(errno));
+        return NULL;
+    }
 
     n = (VirtIONet *)virtio_init_pci(bus, "virtio-net", 6900, 0x1000,
 				     0, VIRTIO_ID_NET,
 				     0x02, 0x00, 0x00,
 				     6, sizeof(VirtIONet));
-    if (!n)
+    if (!n) {
+	close(eventfds[0]);
+	close(eventfds[1]);
 	return NULL;
+    }
 
     n->vdev.get_config = virtio_net_update_config;
     n->vdev.get_features = virtio_net_get_features;

@@ -341,8 +347,13 @@ PCIDevice *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
     n->vc = qemu_new_vlan_client(nd->vlan, virtio_net_receive,
                                  virtio_net_can_receive, n);
 
-    n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n);
-    n->tx_timer_active = 0;
+    fcntl(eventfds[0], F_SETFL, O_NONBLOCK);
+    n->tx_eventfds[0] = eventfds[0];
+    fcntl(eventfds[1], F_SETFL, O_NONBLOCK);
+    n->tx_eventfds[1] = eventfds[1];
+
+    n->vdev.pci_dev.unregister = virtio_net_uninit;
+    qemu_set_fd_handler2(n->tx_eventfds[0], NULL, virtio_net_tx_event, NULL, n);
 
     register_savevm("virtio-net", virtio_net_id++, 1,
 		    virtio_net_save, virtio_net_load, n);

Instead of using an event fd, perhaps you could just schedule a bottom half? I think that would be a whole lot cleaner.

(quoted diff) +1 -3

+static void virtio_net_tx_bh(void *opaque)
 {
     VirtIONet *n = opaque;
 
-    n->tx_timer_active = 0;
-
-    /* Just in case the driver is not ready on more */

By removing the tx timer altogether and doing all the copies in the I/O thread, we can keep the I/O churning away in parallel with the guest generating more I/O.

In my tests, this significantly increases guest->host throughput, causes a minor increase in host->guest throughput, reduces CPU utilization somewhat and greatly reduces roundtrip times.

Even aside from the benchmark results, removing the arbitrary 150us timer is a nicer option than coming up with a heuristic to make it vary according to load. Finally, on kernels which don't have a suitably low posix timer latency, we won't be scuppered by effectively having e.g. a 1ms timer.

Note, this highlights that the I/O thread may become a scalability concern and we might want to consider e.g. an I/O thread per device.

Note also that when tuning for a specific workload, which CPU the I/O thread is pinned to is important.

Note also that when tuning for a specific workload, which CPU the I/O thread is pinned to is important.

Quoted 3 lines · Mark McLoughlin

Note also that when tuning for a specific workload, which CPU the I/O thread is pinned to is important.

Hi Mark:

Can you give an example of when that has a noticeable affect?

For example, if the guest handles network interrupts on vcpu0 and it is pinned to pcpu0 where should the IO thread be pinned for best performance?

Quoted 11 lines · David S. Ahern

Quoted 3 lines · Mark McLoughlin

Note also that when tuning for a specific workload, which CPU the I/O thread is pinned to is important.

Hi Mark:

Can you give an example of when that has a noticeable affect?

For example, if the guest handles network interrupts on vcpu0 and it is pinned to pcpu0 where should the IO thread be pinned for best performance?

Basically, the I/O thread is where packets are copied too and from host kernel space at the moment.

If there are other copies of the packets anywhere, you want those to copy from a cache.

With my netperf guest->host benchmark, you actually have four copies going on:

1) netperf process in guest copying to guest kernel space

2) qemu process in the host copying between internal buffers

3) qemu process in the host copying to host kernel space

4) netserver process in the host copying into its buffers

My machine has four CPUs, with two 6Mb L2 caches - each cache is shared between two of the CPUs, so I set things up as follows:

pcpu#3 - netserver, I/O thread, vcpu#0
pcup#4 - vcpu#1, virtio_net irq, netperf

which (hopefully) ensures that we're only doing one copy using RAM and the rest are using the L1/L2 caches.

Cheers,
Mark.

My machine has four CPUs, with two 6Mb L2 caches - each cache is shared between two of the CPUs, so I set things up as follows:

pcpu#3 - netserver, I/O thread, vcpu#0
pcup#4 - vcpu#1, virtio_net irq, netperf

which (hopefully) ensures that we're only doing one copy using RAM and the rest are using the L1/L2 caches.

Hey,

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

Below are the results I got from benchmarking guest->host and host->guest on my machine.

There's enough numbers there to make anyone blind, but basically there are results for current kvm-userspace.git, with the no-tx-timer patch applied and with the drop-the-mutex patch applied.

Also, I've included results that show what difference some tuning makes with all the patches applied. The tuning basically just involves pinning the I/O thread and the netperf/netserver processes in both the host and guest to two physical CPUs which share a L2 cache.

(Yes, the 1k buffer size results are weird - we think there's a bug in recent kernels that causes us not to coalesce these small buffers into a large GSO packet before sending)

Anyway, the results in all their glory:

                                |       guest->host tput     |      host->guest tput
  netperf, 10x20s runs (Gb/s)   |    min/ mean/   max/stddev |   min/  mean/   max/stddev
  ------------------------------+----------------------------+---------------------------
  kvm-userspace.git, 1k         | 0.600/ 0.645/ 0.670/ 0.025 | 5.170/ 5.285/ 5.470/ 0.087
  kvm-userspace.git, 16k        | 3.070/ 3.350/ 3.710/ 0.248 | 5.950/ 6.374/ 6.760/ 0.261
  kvm-userspace.git, 65k        | 4.950/ 6.041/ 7.170/ 0.639 | 5.480/ 5.642/ 5.810/ 0.092

  no tx timer, 1k               | 0.720/ 0.790/ 0.850/ 0.040 | 4.950/ 5.172/ 5.370/ 0.128
  no tx timer, 16k              | 4.120/ 4.512/ 4.740/ 0.190 | 4.900/ 5.480/ 6.230/ 0.416
  no tx timer, 65k              | 5.510/ 7.702/ 9.600/ 1.153 | 4.490/ 5.208/ 5.690/ 0.408

Hey,

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

Quoted 7 lines · Mark McLoughlin

Hey,

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

What will it do to small packet, multi-flow loads (simulated by ping -f -l 30 $external)?

Where does the benefit come from?

Is the overhead of managing the timer too high, or does it fire too late and so we sleep? If the latter, can we tune it dynamically?

For example, if the guest sees it is making a lot of progress without the host catching up (waiting on the tx timer), it can kick_I_really_mean_this_now(), to get the host to notice.

Quoted 10 lines · Avi Kivity

Quoted 8 lines · Mark McLoughlin

Hey,

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

Quoted 3 lines

What will it do to small packet, multi-flow loads (simulated by ping -f -l 30 $external)?

It should improve the latency - the packets will be flushed more quickly than the 150us timeout without blocking the guest.

Quoted 2 lines

Where does the benefit come from?

There are two things going on here, I think.

First is that the timer affects latency, removing the timeout helps that.

Second is that currently when we fill up the ring we block the guest vcpu and flush. Thus, while we're copying a entire ring full of packets that guest isn't making progress. Doing the copying in the I/O thread helps there.

Note - the only net I/O we currently do in the vcpu thread at the moment is when the guest is saturating the link. Any other timer, all the I/O is done in the I/O thread by virtue of the timer.

Quoted 5 lines

Is the overhead of managing the timer too high, or does it fire too late and so we sleep? If the latter, can we tune it dynamically?

For example, if the guest sees it is making a lot of progress without the host catching up (waiting on the tx timer), it can kick_I_really_mean_this_now(), to get the host to notice.

It does that already - if the ring fills up the guests forces a kick which causes the host to flush the ring in the vcpu thread.

Quoted 13 lines · Mark McLoughlin

Quoted 10 lines · Avi Kivity

Quoted 6 lines · Mark McLoughlin

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

What will it do to small packet, multi-flow loads (simulated by ping -f -l 30 $external)?

Quoted 4 lines

It should improve the latency - the packets will be flushed more quickly than the 150us timeout without blocking the guest.

But it will increase overhead, since suddenly we aren't queueing anymore. One vmexit per small packet.

Quoted 8 lines

Quoted 2 lines

Where does the benefit come from?

There are two things going on here, I think.

First is that the timer affects latency, removing the timeout helps that.

If the timer affects latency, then something is very wrong. We're lacking an adjustable window.

The way I see it, the notification window should be adjusted according to the current workload. If the link is idle, the window should be one packet -- notify as soon as something is queued. As the workload increases, the window increases to (safety_factor * allowable_latency / packet_rate). The timer is set to allowable_latency to catch changes in workload.

For example:

- allowable_latency 1ms (implies 1K vmexits/sec desired)
- current packet_rate 20K packets/sec
- safety_factor 0.8

So we request notifications every 0.8 * 20K * 1m = 16 packets, and set the timer to 1ms. Usually we get a notification every 16 packets, just before timer expiration. If the workload increases, we get notifications sooner, so we increase the window. If the workload drops, the timer fires and we decrease the window.

The timer should never fire on an all-out benchmark, or in a ping test.

Quoted 5 lines

Second is that currently when we fill up the ring we block the guest vcpu and flush. Thus, while we're copying a entire ring full of packets that guest isn't making progress. Doing the copying in the I/O thread helps there.

We're hurting our cache, and this won't work well with many nics. At the very least this should be done in a dedicated thread.

Quoted 2 lines

But it will increase overhead, since suddenly we aren't queueing anymore. One vmexit per small packet.

Yes in theory, but the packet copies are acting to mitigate exits since we don't re-enable notifications again until we're sure the ring is empty.

With copyless, though, we'd have an unacceptable vmexit rate.

Quoted 12 lines

If the timer affects latency, then something is very wrong. We're lacking an adjustable window.

The way I see it, the notification window should be adjusted according to the current workload. If the link is idle, the window should be one packet -- notify as soon as something is queued. As the workload increases, the window increases to (safety_factor * allowable_latency / packet_rate). The timer is set to allowable_latency to catch changes in workload.

For example:

- allowable_latency 1ms (implies 1K vmexits/sec desired)
- current packet_rate 20K packets/sec
- safety_factor 0.8

So we request notifications every 0.8 * 20K * 1m = 16 packets, and set the timer to 1ms. Usually we get a notification every 16 packets, just before timer expiration. If the workload increases, we get notifications sooner, so we increase the window. If the workload drops, the timer fires and we decrease the window.

Quoted 2 lines

The timer should never fire on an all-out benchmark, or in a ping test.

Yeah, I do like the sound of this.

However, since it requires a new guest feature and I don't expect it'll improve the situation over the proposed patch until we have copyless transmit, I think we should do this as part of the copyless effort.

One thing I'd worry about with this scheme is all-out receive - e.g. any delay in returning a TCP ACK to the sending side, might cause us to hit the TCP window size.

Quoted 2 lines

We're hurting our cache, and this won't work well with many nics. At the very least this should be done in a dedicated thread.

A thread per nic is doable, but it'd be especially tricky on the receive side without more "short-cut the one producer, one consumer case" work.

Hey,
So, I went off and spent some time gathering more data on this stuff
and putting it together in a more consumable fashion.

Here are some graphs showing the effect some of these changes have on
throughput, cpu utilization and vmexit rate:

http://markmc.fedorapeople.org/virtio-netperf/2008-11-06/

The results are a little surprising, and I'm not sure I've fully
digested them yet but some conclusions:

1) Disabling notifications from the guest for longer helps; you see an increase in cpu utilization and vmexit rate, but that can be accounted for by the extra data we're transferring

2) Flushing (when the ring is full) in the I/O thread doesn't seem to help anything; strangely, it has a detrimental effect on host->guest traffic where I would expect us to hit this case at all.

I suspect we may not actually be hitting the full ring condition in these tests at all.

4) Removing the tx timer doesn't have a huge affect on guest->host, except for 32 byte buffers where we see a huge increase in vmexits and a drop in throughput. Bizarrely, we don't see this effect with 64 byte buffers.

However, it does have a pretty significant impact on host->guest, which makes sense since in that case we'll just have a steady stream of TCP ACK packets so if small guest->host packets are affected badly, so will the ACK packets.

5) The drop-mutex patch is a nice win overall, except for a huge

     increase in vmexits for sub-4k guest->host packets. Strange.

Quoted 10 lines · Mark McLoughlin

Quoted 2 lines · Avi Kivity

Where does the benefit come from?

There are two things going on here, I think.

First is that the timer affects latency, removing the timeout helps that.

If the timer affects latency, then something is very wrong. We're lacking an adjustable window.

The way I see it, the notification window should be adjusted according to the current workload. If the link is idle, the window should be one packet -- notify as soon as something is queued. As the workload increases, the window increases to (safety_factor * allowable_latency / packet_rate). The timer is set to allowable_latency to catch changes in workload.

For example:

- allowable_latency 1ms (implies 1K vmexits/sec desired)
- current packet_rate 20K packets/sec
- safety_factor 0.8

So we request notifications every 0.8 * 20K * 1m = 16 packets, and set the timer to 1ms. Usually we get a notification every 16 packets, just before timer expiration. If the workload increases, we get notifications sooner, so we increase the window. If the workload drops, the timer fires and we decrease the window.

The timer should never fire on an all-out benchmark, or in a ping test.

The way I see this (continuing with your example figures) playing out is:

- If we have a packet rate of <2.5K packets/sec, we essentially have zero added latency - each packet causes a vmexit and the packet is dispatched immediately

- As soon as we go above 2.5k we add, on average, an additional ~400us delay to each packet

- This is almost identical to our current scheme with an 800us timer, except that flushes are typically triggered by a vmexit instead of the timer expiring

I don't think this is the effect you're looking for? Am I missing something?

Hey,

The main patch in this series is 5/6 - it just kills off the virtio_net tx mitigation timer and does all the tx I/O in the I/O thread.

Below are the results I got from benchmarking guest->host and host->guest on my machine.

There's enough numbers there to make anyone blind, but basically there are results for current kvm-userspace.git, with the no-tx-timer patch applied and with the drop-the-mutex patch applied.

Also, I've included results that show what difference some tuning makes with all the patches applied. The tuning basically just involves pinning the I/O thread and the netperf/netserver processes in both the host and guest to two physical CPUs which share a L2 cache.

(Yes, the 1k buffer size results are weird - we think there's a bug in recent kernels that causes us not to coalesce these small buffers into a large GSO packet before sending)