From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH 1/5] VSOCK: support fill mergeable rx buffer in guest Date: Wed, 7 Nov 2018 14:17:53 +0800 Message-ID: References: <5BDFF4FE.1080500@huawei.com> <1801c013-6cfb-1d07-3401-49f536e01983@redhat.com> <5BE13331.7050901@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: netdev@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org To: jiangyiwen , stefanha@redhat.com Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59490 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726516AbeKGPqz (ORCPT ); Wed, 7 Nov 2018 10:46:55 -0500 In-Reply-To: <5BE13331.7050901@huawei.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018/11/6 下午2:22, jiangyiwen wrote: > On 2018/11/6 11:38, Jason Wang wrote: >> On 2018/11/5 下午3:45, jiangyiwen wrote: >>> In driver probing, if virtio has VIRTIO_VSOCK_F_MRG_RXBUF feature, >>> it will fill mergeable rx buffer, support for host send mergeable >>> rx buffer. It will fill a page everytime to compact with small >>> packet and big packet. >>> >>> Signed-off-by: Yiwen Jiang >>> --- >>> include/linux/virtio_vsock.h | 3 ++ >>> net/vmw_vsock/virtio_transport.c | 72 +++++++++++++++++++++++++++++----------- >>> 2 files changed, 56 insertions(+), 19 deletions(-) >>> >>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h >>> index e223e26..bf84418 100644 >>> --- a/include/linux/virtio_vsock.h >>> +++ b/include/linux/virtio_vsock.h >>> @@ -14,6 +14,9 @@ >>> #define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL >>> #define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64) >>> >>> +/* Virtio-vsock feature */ >>> +#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */ >>> + >>> enum { >>> VSOCK_VQ_RX = 0, /* for host to guest data */ >>> VSOCK_VQ_TX = 1, /* for guest to host data */ >>> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c >>> index 5d3cce9..2040a9e 100644 >>> --- a/net/vmw_vsock/virtio_transport.c >>> +++ b/net/vmw_vsock/virtio_transport.c >>> @@ -64,6 +64,7 @@ struct virtio_vsock { >>> struct virtio_vsock_event event_list[8]; >>> >>> u32 guest_cid; >>> + bool mergeable; >>> }; >>> >>> static struct virtio_vsock *virtio_vsock_get(void) >>> @@ -256,6 +257,25 @@ static int virtio_transport_send_pkt_loopback(struct virtio_vsock *vsock, >>> return 0; >>> } >>> >>> +static int fill_mergeable_rx_buff(struct virtqueue *vq) >>> +{ >>> + void *page = NULL; >>> + struct scatterlist sg; >>> + int err; >>> + >>> + page = (void *)get_zeroed_page(GFP_KERNEL); >> Any reason to use zeroed page? > In previous version, the entire structure of virtio_vsock_pkt is preallocated > in guest use kzalloc, it is a contiguous zeroed physical memory, but host only > need to fill virtio_vsock_hdr size. > > However, in mergeable rx buffer version, we only fill a page in vring descriptor > in guest, and I will reserve size of virtio_vsock_pkt in host instead of write > the total size of virtio_vsock_pkt, for the correctness of structure value, > we should set zeroed page in advance. I may miss something, but it looks to me only the header needs to be zeroed. > >>> + if (!page) >>> + return -ENOMEM; >>> + >>> + sg_init_one(&sg, page, PAGE_SIZE); >> FYI, for virtio-net we have several optimizations for mergeable rx buffer: >> >> - skb_page_frag_refill() which can use high order page and reduce the stress of page allocator >> > You're right, initially I want to use a memory poll to manage the rx buffer, > and then use this in the later optimized patch. Your advice is very great. > >> - we don't use fixed buffer size, instead we use EWMA to estimate the possible rx buffer size to avoid internal fragmentation >> > Ok, I analysis the feature and consider add it into my patches. > >> If we can try to reuse virtio-net driver, we will get those nice features. >> > Yes, after all virtio-net has a very good ecological environment, and it also > do many performance optimization, it is actually a good idea. > Yes, so my suggestion is to consider to reuse them (unless we found something that is a real blocker) instead of duplicating codes, features a bugs. Thanks