[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.7] hw/net/spapr_llan: Delay flushing of th
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH for-2.7] hw/net/spapr_llan: Delay flushing of the RX queue while adding new RX buffers |
Date: |
Fri, 1 Apr 2016 12:25:56 +1100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Thu, Mar 31, 2016 at 01:47:05PM +0200, Thomas Huth wrote:
> Currently, the spapr-vlan device is trying to flush the RX queue
> after each RX buffer that has been added by the guest via the
> H_ADD_LOGICAL_LAN_BUFFER hypercall. In case the receive buffer pool
> was empty before, we only pass single packets to the guest this
> way. This can cause very bad performance if a sender is trying
> to stream fragmented UDP packets to the guest. For example when
> using the UDP_STREAM test from netperf with UDP packets that are
> much bigger than the MTU size, almost all UDP packets are dropped
> in the guest since the chances are quite high that at least one of
> the fragments got lost on the way.
>
> When flushing the receive queue, it's much better if we'd have
> a bunch of receive buffers available already, so that fragmented
> packets can be passed to the guest in one go. To do this, the
> spapr_vlan_receive() function should return 0 instead of -1 if there
> are no more receive buffers available, so that receive_disabled = 1
> gets temporarily set for the receive queue, and we have to delay
> the queue flushing at the end of h_add_logical_lan_buffer() a little
> bit by using a timer, so that the guest gets a chance to add multiple
> RX buffers before we flush the queue again.
>
> This improves the UDP_STREAM test with the spapr-vlan device a lot:
> Running
> netserver -p 44444 -L <guestip> -f -D -4
> in the guest, and
> netperf -p 44444 -L <hostip> -H <guestip> -t UDP_STREAM -l 60 -- -m 16384
> in the host, I get the following values _without_ this patch:
>
> Socket Message Elapsed Messages
> Size Size Time Okay Errors Throughput
> bytes bytes secs # # 10^6bits/sec
>
> 229376 16384 60.00 1738970 0 3798.83
> 229376 60.00 23 0.05
>
> That "0.05" means that almost all UDP packets got lost/discarded
> at the receiving side.
> With this patch applied, the value look much better:
>
> Socket Message Elapsed Messages
> Size Size Time Okay Errors Throughput
> bytes bytes secs # # 10^6bits/sec
>
> 229376 16384 60.00 1789104 0 3908.35
> 229376 60.00 22818 49.85
>
> Signed-off-by: Thomas Huth <address@hidden>
> ---
> As a reference: When running the same test with a "e1000" NIC
> instead of "spapr-vlan", I get a throughput of ~85 up to 100 MBits/s
> ... so the spapr-vlan is still not as good as other NICs here, but
> at least it's much better than before.
Nice work!
Applied to ppc-for-2.7.
>
> hw/net/spapr_llan.c | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
> index a647f25..d604d55 100644
> --- a/hw/net/spapr_llan.c
> +++ b/hw/net/spapr_llan.c
> @@ -109,6 +109,7 @@ typedef struct VIOsPAPRVLANDevice {
> target_ulong buf_list;
> uint32_t add_buf_ptr, use_buf_ptr, rx_bufs;
> target_ulong rxq_ptr;
> + QEMUTimer *rxp_timer;
> uint32_t compat_flags; /* Compatability flags for migration
> */
> RxBufPool *rx_pool[RX_MAX_POOLS]; /* Receive buffer descriptor pools */
> } VIOsPAPRVLANDevice;
> @@ -205,7 +206,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc,
> const uint8_t *buf,
> }
>
> if (!dev->rx_bufs) {
> - return -1;
> + return 0;
> }
>
> if (dev->compat_flags & SPAPRVLAN_FLAG_RX_BUF_POOLS) {
> @@ -214,7 +215,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc,
> const uint8_t *buf,
> bd = spapr_vlan_get_rx_bd_from_page(dev, size);
> }
> if (!bd) {
> - return -1;
> + return 0;
> }
>
> dev->rx_bufs--;
> @@ -265,6 +266,13 @@ static NetClientInfo net_spapr_vlan_info = {
> .receive = spapr_vlan_receive,
> };
>
> +static void spapr_vlan_flush_rx_queue(void *opaque)
> +{
> + VIOsPAPRVLANDevice *dev = opaque;
> +
> + qemu_flush_queued_packets(qemu_get_queue(dev->nic));
> +}
> +
> static void spapr_vlan_reset_rx_pool(RxBufPool *rxp)
> {
> /*
> @@ -301,6 +309,9 @@ static void spapr_vlan_realize(VIOsPAPRDevice *sdev,
> Error **errp)
> dev->nic = qemu_new_nic(&net_spapr_vlan_info, &dev->nicconf,
> object_get_typename(OBJECT(sdev)),
> sdev->qdev.id, dev);
> qemu_format_nic_info_str(qemu_get_queue(dev->nic),
> dev->nicconf.macaddr.a);
> +
> + dev->rxp_timer = timer_new_us(QEMU_CLOCK_VIRTUAL,
> spapr_vlan_flush_rx_queue,
> + dev);
> }
>
> static void spapr_vlan_instance_init(Object *obj)
> @@ -331,6 +342,11 @@ static void spapr_vlan_instance_finalize(Object *obj)
> dev->rx_pool[i] = NULL;
> }
> }
> +
> + if (dev->rxp_timer) {
> + timer_del(dev->rxp_timer);
> + timer_free(dev->rxp_timer);
> + }
> }
>
> void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
> @@ -628,7 +644,13 @@ static target_ulong h_add_logical_lan_buffer(PowerPCCPU
> *cpu,
>
> dev->rx_bufs++;
>
> - qemu_flush_queued_packets(qemu_get_queue(dev->nic));
> + /*
> + * Give guest some more time to add additional RX buffers before we
> + * flush the receive queue, so that e.g. fragmented IP packets can
> + * be passed to the guest in one go later (instead of passing single
> + * fragments if there is only one receive buffer available).
> + */
> + timer_mod(dev->rxp_timer, qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) + 500);
>
> return H_SUCCESS;
> }
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature