From patchwork Fri Aug 27 22:37:18 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 62878 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 1AB41B7101 for ; Sat, 28 Aug 2010 08:41:04 +1000 (EST) Received: from localhost ([127.0.0.1]:48345 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Op7bY-0000cn-8k for incoming@patchwork.ozlabs.org; Fri, 27 Aug 2010 18:41:00 -0400 Received: from [140.186.70.92] (port=48426 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Op7Y9-0007jB-D9 for qemu-devel@nongnu.org; Fri, 27 Aug 2010 18:37:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1Op7Y3-0004Fs-Gd for qemu-devel@nongnu.org; Fri, 27 Aug 2010 18:37:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34730) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1Op7Y3-0004Fl-54 for qemu-devel@nongnu.org; Fri, 27 Aug 2010 18:37:23 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o7RMbLCo029786 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 27 Aug 2010 18:37:21 -0400 Received: from s20.home (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o7RMbIK5031072; Fri, 27 Aug 2010 18:37:19 -0400 From: Alex Williamson To: qemu-devel@nongnu.org Date: Fri, 27 Aug 2010 16:37:18 -0600 Message-ID: <20100827223718.2696.64669.stgit@s20.home> In-Reply-To: <20100827223659.2696.3589.stgit@s20.home> References: <20100827223659.2696.3589.stgit@s20.home> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. Cc: jes.sorensen@redhat.com, alex.williamson@redhat.com, kvm@vger.kernel.org Subject: [Qemu-devel] [PATCH 2/5] 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. 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 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 | 4 +++- hw/s390-virtio-bus.h | 2 ++ hw/syborg_virtio.c | 6 +++++- hw/virtio-net.c | 23 ++++++++++++++++------- hw/virtio-pci.c | 6 +++++- hw/virtio.h | 2 +- 6 files changed, 32 insertions(+), 11 deletions(-) diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index 4da0b40..1483362 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -110,7 +110,8 @@ static int s390_virtio_net_init(VirtIOS390Device *dev) { VirtIODevice *vdev; - vdev = virtio_net_init((DeviceState *)dev, &dev->nic, dev->txtimer); + vdev = virtio_net_init((DeviceState *)dev, &dev->nic, + dev->txtimer, dev->txburst); if (!vdev) { return -1; } @@ -328,6 +329,7 @@ static VirtIOS390DeviceInfo s390_virtio_net = { .qdev.props = (Property[]) { DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic), DEFINE_PROP_UINT32("txtimer", VirtIOS390Device, txtimer, 1), + DEFINE_PROP_INT32("txburst", VirtIOS390Device, txburst, 256), DEFINE_PROP_END_OF_LIST(), }, }; diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h index 922daf2..808aea0 100644 --- a/hw/s390-virtio-bus.h +++ b/hw/s390-virtio-bus.h @@ -45,6 +45,8 @@ typedef struct VirtIOS390Device { uint32_t max_virtserial_ports; /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ uint32_t txtimer; + /* Max tx packets for virtio-net to burst at a time */ + int32_t txburst; } VirtIOS390Device; typedef struct VirtIOS390Bus { diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c index c8d731a..7b76972 100644 --- a/hw/syborg_virtio.c +++ b/hw/syborg_virtio.c @@ -70,6 +70,8 @@ typedef struct { uint32_t host_features; /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ uint32_t txtimer; + /* Max tx packets for virtio-net to burst at a time */ + int32_t txburst; } SyborgVirtIOProxy; static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset) @@ -286,7 +288,8 @@ static int syborg_virtio_net_init(SysBusDevice *dev) VirtIODevice *vdev; SyborgVirtIOProxy *proxy = FROM_SYSBUS(SyborgVirtIOProxy, dev); - vdev = virtio_net_init(&dev->qdev, &proxy->nic, proxy->txtimer); + vdev = virtio_net_init(&dev->qdev, &proxy->nic, + proxy->txtimer, proxy->txburst); return syborg_virtio_init(proxy, vdev); } @@ -298,6 +301,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = { DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic), DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features), DEFINE_PROP_UINT32("txtimer", SyborgVirtIOProxy, txtimer, 1), + DEFINE_PROP_INT32("txburst", SyborgVirtIOProxy, txburst, 256), DEFINE_PROP_END_OF_LIST(), } }; diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 9ef29f0..ac4aa8f 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) @@ -905,7 +913,7 @@ static void virtio_net_vmstate_change(void *opaque, int running, int reason) } VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, - uint32_t txtimer) + uint32_t txtimer, int32_t txburst) { VirtIONet *n; @@ -942,6 +950,7 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, n->tx_timeout = txtimer; } } + n->tx_burst = txburst; n->mergeable_rx_bufs = 0; n->promisc = 1; /* for compatibility */ diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index e3b9897..e025c09 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -109,6 +109,8 @@ typedef struct { uint32_t max_virtserial_ports; /* Timeout value for virtio-net txtimer, 1 = default, >1 = ns timeout */ uint32_t txtimer; + /* Max tx packets for virtio-net to burst at a time */ + int32_t txburst; } VirtIOPCIProxy; /* virtio device */ @@ -615,7 +617,8 @@ static int virtio_net_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; - vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, proxy->txtimer); + vdev = virtio_net_init(&pci_dev->qdev, &proxy->nic, + proxy->txtimer, proxy->txburst); vdev->nvectors = proxy->nvectors; virtio_init_pci(proxy, vdev, @@ -693,6 +696,7 @@ static PCIDeviceInfo virtio_info[] = { DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features), DEFINE_NIC_PROPERTIES(VirtIOPCIProxy, nic), DEFINE_PROP_UINT32("txtimer", VirtIOPCIProxy, txtimer, 1), + DEFINE_PROP_INT32("txburst", VirtIOPCIProxy, txburst, 256), DEFINE_PROP_END_OF_LIST(), }, .qdev.reset = virtio_pci_reset, diff --git a/hw/virtio.h b/hw/virtio.h index 77d05e0..4051889 100644 --- a/hw/virtio.h +++ b/hw/virtio.h @@ -186,7 +186,7 @@ void virtio_bind_device(VirtIODevice *vdev, const VirtIOBindings *binding, /* Base devices. */ VirtIODevice *virtio_blk_init(DeviceState *dev, BlockConf *conf); VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, - uint32_t txtimer); + uint32_t txtimer, int32_t txburst); VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t max_nr_ports); VirtIODevice *virtio_balloon_init(DeviceState *dev); #ifdef CONFIG_LINUX