From patchwork Tue Oct 29 01:51:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 286708 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 2F5ED2C00E3 for ; Tue, 29 Oct 2013 12:57:17 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757085Ab3J2B5N (ORCPT ); Mon, 28 Oct 2013 21:57:13 -0400 Received: from ozlabs.org ([203.10.76.45]:52116 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756651Ab3J2B5M (ORCPT ); Mon, 28 Oct 2013 21:57:12 -0400 Received: by ozlabs.org (Postfix, from userid 1011) id 8A3202C00CD; Tue, 29 Oct 2013 12:57:11 +1100 (EST) From: Rusty Russell To: Michael Dalton , "David S. Miller" Cc: netdev@vger.kernel.org, Eric Dumazet , "Michael S. Tsirkin" , Daniel Borkmann , virtualization@lists.linux-foundation.org, Michael Dalton Subject: Re: [PATCH net-next] virtio_net: migrate mergeable rx buffers to page frag allocators In-Reply-To: <1383000258-1458-1-git-send-email-mwdalton@google.com> References: <1383000258-1458-1-git-send-email-mwdalton@google.com> User-Agent: Notmuch/0.15.2 (http://notmuchmail.org) Emacs/23.4.1 (i686-pc-linux-gnu) Date: Tue, 29 Oct 2013 12:21:56 +1030 Message-ID: <87ppqoetdv.fsf@rustcorp.com.au> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Michael Dalton writes: > The virtio_net driver's mergeable receive buffer allocator > uses 4KB packet buffers. For MTU-sized traffic, SKB truesize > is > 4KB but only ~1500 bytes of the buffer is used to store > packet data, reducing the effective TCP window size > substantially. This patch addresses the performance concerns > with mergeable receive buffers by allocating MTU-sized packet > buffers using page frag allocators. If more than MAX_SKB_FRAGS > buffers are needed, the SKB frag_list is used. > > Signed-off-by: Michael Dalton Hi Michael, Nice work! Just one comment. Your patch highlights the anachronistic name MAX_PACKET_LEN: it was from the original implementation which only supported 1500 byte packets, and only used in one place. Please apply a first patch like this, then come up with a new constant name (GOOD_PACKET_LEN?) for that value. Because it's not the maximum packet we can receive for mergable buffers. Thanks, Rusty. Subject: virtio_net: remove anachronistic MAX_PACKET_LEN constant. From: Rusty Russell The initial implementation of virtio_net only allowed ethernet-style MTU packets; with more recent features this isn't true. Move the constant into the function where it's used. Signed-off-by: Rusty Russell --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 057ea13..dcbfccd 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -35,8 +35,6 @@ static bool csum = true, gso = true; module_param(csum, bool, 0444); module_param(gso, bool, 0444); -/* FIXME: MTU in config. */ -#define MAX_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN) #define GOOD_COPY_LEN 128 #define VIRTNET_DRIVER_VERSION "1.0.0" @@ -434,12 +432,13 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp) struct sk_buff *skb; struct skb_vnet_hdr *hdr; int err; + const int len = ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN; - skb = __netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN, gfp); + skb = __netdev_alloc_skb_ip_align(vi->dev, len, gfp); if (unlikely(!skb)) return -ENOMEM; - skb_put(skb, MAX_PACKET_LEN); + skb_put(skb, len); hdr = skb_vnet_hdr(skb); sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr);