Linux Virtualization - Jan 2008 - [PATCH] virtio_net tx performance fix

>From f582caf612b446e42f1e80d5ef12c5b7322efd03 Mon Sep 17 00:00:00 2001
From: Dor Laor <dor.laor@qumranet.com>
Date: Mon, 28 Jan 2008 02:09:48 +0200
Subject: [PATCH] virtio_net tx performance fix

There was a problem with the location of the notify call in
add_buff function:
When VRING_USED_F_NO_NOTIFY is set, the host does not kick the
guest when packets were transmitted, as a result the guest runs
out of tx buffers sometimes. This is fine but the problem lies
when add_buf fails, it called notify and the host sends all the
pending tx pkts. When enable_cb was called, more_used(vq) returned
false so eventually the skb was dropped.

Moving notify from add_buf to enable_cb fixes this flow problem.
The tx performance boosted from 220Mbps to 850Mbps.

Signed-off-by: Dor Laor <dor.laor@qumranet.com>
---
 drivers/virtio/virtio_ring.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3a28c13..592bbc9 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -87,8 +87,6 @@ static int vring_add_buf(struct virtqueue *_vq,
 	if (vq->num_free < out + in) {
 		pr_debug("Can't add buf len %i - avail = %i\n",
 			 out + in, vq->num_free);
-		/* We notify *even if* VRING_USED_F_NO_NOTIFY is set here. */
-		vq->notify(&vq->vq);
 		END_USE(vq);
 		return -ENOSPC;
 	}
@@ -232,6 +230,7 @@ static bool vring_enable_cb(struct virtqueue *_vq)
 	vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
 	mb();
 	if (unlikely(more_used(vq))) {
+		vq->notify(&vq->vq);
 		vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT;
 		END_USE(vq);
 		return false;
-- 
1.5.3.7


-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-virtio_net-tx-performance-fix.patch
Type: application/mbox
Size: 1676 bytes
Desc: not available
Url :
http://lists.linux-foundation.org/pipermail/virtualization/attachments/20080128/33452e43/0001-virtio_net-tx-performance-fix.bin

Hi Dor,

How are you measuring performance?  The numbers I've gotten with netperf 
before and after your patch are:

tx - 647.27mbit
rx - 89.22

tx - 27.82
rx - 79.93

So this patch is pretty much killing performance for netperf.

Dor Laor wrote:
> There was a problem with the location of the notify call in
> add_buff function:
> When VRING_USED_F_NO_NOTIFY is set, the host does not kick the
> guest when packets were transmitted, as a result the guest runs
> out of tx buffers sometimes.
But even if F_NO_NOTIFY is set, if the tx buffer is full, we notify the 
guest, so this prevents that from happening.
>  This is fine but the problem lies
> when add_buf fails, it called notify and the host sends all the
> pending tx pkts. When enable_cb was called, more_used(vq) returned
> false so eventually the skb was dropped.
>   
I'm having a tough time following this part.  If add_buf fails, we 
notify unconditionally (which is, I think what we want).  I'm not sure 
how that relates to a packet getting dropped though.

Regards,

Anthony Liguori