From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41201) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anGN0-000347-1p for qemu-devel@nongnu.org; Mon, 04 Apr 2016 22:05:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anGMw-0008T8-Pb for qemu-devel@nongnu.org; Mon, 04 Apr 2016 22:05:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36892) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anGMw-0008T0-Iv for qemu-devel@nongnu.org; Mon, 04 Apr 2016 22:05:26 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 53F05388864 for ; Tue, 5 Apr 2016 02:05:25 +0000 (UTC) References: <1459711556-10273-1-git-send-email-wexu@redhat.com> <1459711556-10273-2-git-send-email-wexu@redhat.com> From: Jason Wang Message-ID: <57031D5D.3010608@redhat.com> Date: Tue, 5 Apr 2016 10:05:17 +0800 MIME-Version: 1.0 In-Reply-To: <1459711556-10273-2-git-send-email-wexu@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [ RFC Patch v4 1/3] virtio-net rsc: add a new host offload(rsc) feature bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: wexu@redhat.com, qemu-devel@nongnu.org Cc: marcel@redhat.com, victork@redhat.com, dfleytma@redhat.com, yvugenfi@redhat.com, mst@redhat.com On 04/04/2016 03:25 AM, wexu@redhat.com wrote: > From: Wei Xu > > A new feature bit 'VIRTIO_NET_F_GUEST_RSC' is introduced to support WHQL > Receive-Segment-Offload test, this feature will coalesce tcp packets in > IPv4/6 for userspace virtio-net driver. > > This feature can be enabled either by 'ACK' the feature when loading > the driver in the guest, or by sending the 'VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET' > command to the host via control queue. > > Signed-off-by: Wei Xu > --- > hw/net/virtio-net.c | 29 +++++++++++++++++++++++++++-- > include/standard-headers/linux/virtio_net.h | 1 + > 2 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 5798f87..bd91a4b 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -537,6 +537,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features, > virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4); > virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6); > virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN); > + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_RSC); Several questions here: - I think RSC should work even without vnet_hdr? - Need we differentiate ipv4 and ipv6 like TSO here? - And looks like this patch should be squash to following patches. > } > > if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) { > @@ -582,7 +583,8 @@ static uint64_t virtio_net_guest_offloads_by_features(uint32_t features) > (1ULL << VIRTIO_NET_F_GUEST_TSO4) | > (1ULL << VIRTIO_NET_F_GUEST_TSO6) | > (1ULL << VIRTIO_NET_F_GUEST_ECN) | > - (1ULL << VIRTIO_NET_F_GUEST_UFO); > + (1ULL << VIRTIO_NET_F_GUEST_UFO) | > + (1ULL << VIRTIO_NET_F_GUEST_RSC); Looks like this is unnecessary since we won't set peer offload based on GUEST_RSC. > > return guest_offloads_mask & features; > } > @@ -1089,7 +1091,8 @@ static int receive_filter(VirtIONet *n, const uint8_t *buf, int size) > return 0; > } > > -static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t size) > +static ssize_t virtio_net_do_receive(NetClientState *nc, > + const uint8_t *buf, size_t size) > { > VirtIONet *n = qemu_get_nic_opaque(nc); > VirtIONetQueue *q = virtio_net_get_subqueue(nc); > @@ -1685,6 +1688,26 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, > return 0; > } > > + > +static ssize_t virtio_net_rsc_receive(NetClientState *nc, > + const uint8_t *buf, size_t size) > +{ > + return virtio_net_do_receive(nc, buf, size); > +} > + > +static ssize_t virtio_net_receive(NetClientState *nc, > + const uint8_t *buf, size_t size) > +{ > + VirtIONet *n; > + > + n = qemu_get_nic_opaque(nc); > + if (n->curr_guest_offloads & VIRTIO_NET_F_GUEST_RSC) { > + return virtio_net_rsc_receive(nc, buf, size); > + } else { > + return virtio_net_do_receive(nc, buf, size); > + } > +} The changes here looks odd since it did nothing. Like I've mentioned, better merge the patch into following ones. > + > static NetClientInfo net_virtio_info = { > .type = NET_CLIENT_OPTIONS_KIND_NIC, > .size = sizeof(NICState), > @@ -1909,6 +1932,8 @@ static Property virtio_net_properties[] = { > TX_TIMER_INTERVAL), > DEFINE_PROP_INT32("x-txburst", VirtIONet, net_conf.txburst, TX_BURST), > DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx), > + DEFINE_PROP_BIT("guest_rsc", VirtIONet, host_features, > + VIRTIO_NET_F_GUEST_RSC, true), Need to compat the bit for old machine type to unbreak migration I believe? Btw, also need a patch for virtio spec. Thanks > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/standard-headers/linux/virtio_net.h b/include/standard-headers/linux/virtio_net.h > index a78f33e..5b95762 100644 > --- a/include/standard-headers/linux/virtio_net.h > +++ b/include/standard-headers/linux/virtio_net.h > @@ -55,6 +55,7 @@ > #define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow > * Steering */ > #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > +#define VIRTIO_NET_F_GUEST_RSC 24 /* Guest can coalesce tcp packets */ > > #ifndef VIRTIO_NET_NO_LEGACY > #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */