From patchwork Thu Sep 2 15:00:57 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 63494 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id CC40BB7183 for ; Fri, 3 Sep 2010 01:19:07 +1000 (EST) Received: from localhost ([127.0.0.1]:54826 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OrBWL-0002rk-RS for incoming@patchwork.ozlabs.org; Thu, 02 Sep 2010 11:16:09 -0400 Received: from [140.186.70.92] (port=39091 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OrBRb-0000Hj-Qv for qemu-devel@nongnu.org; Thu, 02 Sep 2010 11:11:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OrBHh-0001PR-AM for qemu-devel@nongnu.org; Thu, 02 Sep 2010 11:01:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27619) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OrBHg-0001P7-Uj for qemu-devel@nongnu.org; Thu, 02 Sep 2010 11:01:01 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o82F0wdg018394 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 2 Sep 2010 11:00:59 -0400 Received: from s20.home (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o82F0vr3023871; Thu, 2 Sep 2010 11:00:58 -0400 From: Alex Williamson To: qemu-devel@nongnu.org Date: Thu, 02 Sep 2010 09:00:57 -0600 Message-ID: <20100902150057.11862.4754.stgit@s20.home> In-Reply-To: <20100902150041.11862.65901.stgit@s20.home> References: <20100902150041.11862.65901.stgit@s20.home> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. Cc: chrisw@redhat.com, kvm@vger.kernel.org, quintela@redhat.com, jes.sorensen@redhat.com, mst@redhat.com, alex.williamson@redhat.com Subject: [Qemu-devel] [PATCH v2 2/4] virtio-net: Limit number of packets sent per TX flush X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org If virtio_net_flush_tx() is called with notification disabled, we can race with the guest, processing packets at the same rate as they get produced. The trouble is that this means we have no guaranteed exit condition from the function and can spend minutes in there. Currently flush_tx is only called with notification on, which seems to limit us to one pass through the queue per call. An upcoming patch changes this. Also add an option to set this value on the command line as different workloads may wish to use different values. We can't necessarily support any random value, so this is a developer option: x-txburst= Usage: -device virtio-net-pci,x-txburst=64 # 64 packets per tx flush One pass through the queue (256) seems to be a good default value for this, balancing latency with throughput. We use a signed int for x-txburst because 2^31 packets in a burst would take many, many minutes to process and it allows us to easily return a negative value value from virtio_net_flush_tx() to indicate a back-off or error condition. Signed-off-by: Alex Williamson --- hw/s390-virtio-bus.c | 2 ++ hw/syborg_virtio.c | 2 ++ hw/virtio-net.c | 21 +++++++++++++++------ hw/virtio-net.h | 8 ++++++++ hw/virtio-pci.c | 2 ++ 5 files changed, 29 insertions(+), 6 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index d5cb24e..092e65f 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -330,6 +330,8 @@ static VirtIOS390DeviceInfo s390_virtio_net = { DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic), DEFINE_PROP_UINT32("x-txtimer", VirtIOS390Device, net.txtimer, TX_TIMER_INTERVAL), + DEFINE_PROP_INT32("x-txburst", VirtIOS390Device, + net.txburst, TX_BURST), DEFINE_PROP_END_OF_LIST(), }, }; diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index 5665189..3c3f3b0 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -298,6 +298,8 @@ static SysBusDeviceInfo syborg_virtio_net_info = { DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features), DEFINE_PROP_UINT32("x-txtimer", SyborgVirtIOProxy, net.txtimer, TX_TIMER_INTERVAL), + DEFINE_PROP_INT32("x-txburst", SyborgVirtIOProxy, + net.txburst, TX_BURST), DEFINE_PROP_END_OF_LIST(), } }; diff --git a/hw/virtio-net.c b/hw/virtio-net.c index d5b03ab..55f3d94 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -37,6 +37,7 @@ typedef struct VirtIONet NICState *nic; QEMUTimer *tx_timer; uint32_t tx_timeout; + int32_t tx_burst; int tx_timer_active; uint32_t has_vnet_hdr; uint8_t has_ufo; @@ -620,7 +621,7 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_ return size; } -static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq); +static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq); static void virtio_net_tx_complete(VLANClientState *nc, ssize_t len) { @@ -636,16 +637,18 @@ static void virtio_net_tx_complete(VLANClientState *nc, ssize_t len) } /* TX */ -static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) +static int32_t virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) { VirtQueueElement elem; + int32_t num_packets = 0; - if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) - return; + if (!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) { + return num_packets; + } if (n->async_tx.elem.out_num) { virtio_queue_set_notification(n->tx_vq, 0); - return; + return num_packets; } while (virtqueue_pop(vq, &elem)) { @@ -682,14 +685,19 @@ static void virtio_net_flush_tx(VirtIONet *n, VirtQueue *vq) virtio_queue_set_notification(n->tx_vq, 0); n->async_tx.elem = elem; n->async_tx.len = len; - return; + return -EBUSY; } len += ret; virtqueue_push(vq, &elem, len); virtio_notify(&n->vdev, vq); + + if (++num_packets >= n->tx_burst) { + break; + } } + return num_packets; } static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq) @@ -934,6 +942,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, n->tx_timer = qemu_new_timer(vm_clock, virtio_net_tx_timer, n); n->tx_timer_active = 0; n->tx_timeout = net->txtimer; + n->tx_burst = net->txburst; n->mergeable_rx_bufs = 0; n->promisc = 1; /* for compatibility */ diff --git a/hw/virtio-net.h b/hw/virtio-net.h index 46a2e1c..a2d1545 100644 --- a/hw/virtio-net.h +++ b/hw/virtio-net.h @@ -49,9 +49,17 @@ #define TX_TIMER_INTERVAL 150000 /* 150 us */ +/* Limit the number of packets that can be sent via a single flush + * of the TX queue. This gives us a guaranteed exit condition and + * ensures fairness in the io path. 256 conveniently matches the + * length of the TX queue and shows a good balance of performance + * and latency. */ +#define TX_BURST 256 + typedef struct virtio_net_conf { uint32_t txtimer; + int32_t txburst; } virtio_net_conf; /* Maximum packet size we can receive from tap device: header + 64k */ diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 1af48e2..3a5b3e6 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -693,6 +693,8 @@ static PCIDeviceInfo virtio_info[] = { DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic), DEFINE_PROP_UINT32("x-txtimer", VirtIOPCIProxy, net.txtimer, TX_TIMER_INTERVAL), + DEFINE_PROP_INT32("x-txburst", VirtIOPCIProxy, + net.txburst, TX_BURST), DEFINE_PROP_END_OF_LIST(), }, .qdev.reset = virtio_pci_reset,