From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apHCr-00063U-HK for qemu-devel@nongnu.org; Sun, 10 Apr 2016 11:23:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1apHCn-0001TJ-Uc for qemu-devel@nongnu.org; Sun, 10 Apr 2016 11:23:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59858) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1apHCn-0001T1-N0 for qemu-devel@nongnu.org; Sun, 10 Apr 2016 11:23:17 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (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 0F280C057EC9 for ; Sun, 10 Apr 2016 15:23:16 +0000 (UTC) References: <1459711556-10273-1-git-send-email-wexu@redhat.com> <1459711556-10273-2-git-send-email-wexu@redhat.com> <57031D5D.3010608@redhat.com> <20160405111701-mutt-send-email-mst@redhat.com> From: Wei Xu Message-ID: <570A6FE0.7080005@redhat.com> Date: Sun, 10 Apr 2016 23:23:12 +0800 MIME-Version: 1.0 In-Reply-To: <20160405111701-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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: "Michael S. Tsirkin" , Jason Wang Cc: qemu-devel@nongnu.org, victork@redhat.com, yvugenfi@redhat.com, marcel@redhat.com, dfleytma@redhat.com On 2016=E5=B9=B404=E6=9C=8805=E6=97=A5 16:17, Michael S. Tsirkin wrote: > On Tue, Apr 05, 2016 at 10:05:17AM +0800, Jason Wang wrote: >> >> 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 W= HQL >>> 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_OFF= LOADS_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(VirtIODev= ice *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? That's interesting, but i'm wondering how to test this? could you please=20 point me out? >> - Need we differentiate ipv4 and ipv6 like TSO here? Sure, thanks. >> - And looks like this patch should be squash to following patches. OK. >> >>> } >>> =20 >>> if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) { >>> @@ -582,7 +583,8 @@ static uint64_t virtio_net_guest_offloads_by_feat= ures(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 o= n >> GUEST_RSC. there is an exclusive check when handling set feature command from=20 control queue, so looks it will broke the check if don't include this bit= . >> >>> =20 >>> return guest_offloads_mask & features; >>> } >>> @@ -1089,7 +1091,8 @@ static int receive_filter(VirtIONet *n, const u= int8_t *buf, int size) >>> return 0; >>> } >>> =20 >>> -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 =3D qemu_get_nic_opaque(nc); >>> VirtIONetQueue *q =3D virtio_net_get_subqueue(nc); >>> @@ -1685,6 +1688,26 @@ static int virtio_net_load_device(VirtIODevice= *vdev, QEMUFile *f, >>> return 0; >>> } >>> =20 >>> + >>> +static ssize_t virtio_net_rsc_receive(NetClientState *nc, >>> + const uint8_t *buf, size_t siz= e) >>> +{ >>> + 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 =3D 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. OK. >> >>> + >>> static NetClientInfo net_virtio_info =3D { >>> .type =3D NET_CLIENT_OPTIONS_KIND_NIC, >>> .size =3D sizeof(NICState), >>> @@ -1909,6 +1932,8 @@ static Property virtio_net_properties[] =3D { >>> 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 bel= ieve? > And definitely disable by default. There maybe some windows specific details about this, i'll discuss with=20 Yan and update. > >> Btw, also need a patch for virtio spec. Sure. >> >> Thanks >> >>> DEFINE_PROP_END_OF_LIST(), >>> }; >>> =20 >>> diff --git a/include/standard-headers/linux/virtio_net.h b/include/st= andard-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 pa= ckets */ >>> =20 >>> #ifndef VIRTIO_NET_NO_LEGACY >>> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */