From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Watson Subject: Re: [net-next v5 3/3] net/tls: Remove redundant array allocation. Date: Wed, 1 Aug 2018 13:52:07 -0700 Message-ID: <20180801205207.GB6180@davejwatson-mba.dhcp.thefacebook.com> References: <20180719162613.27184-1-vakul.garg@nxp.com> <20180719162613.27184-4-vakul.garg@nxp.com> <20180721.192532.520509909556885779.davem@davemloft.net> <20180723163509.GA91424@davejwatson-mba.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: "netdev@vger.kernel.org" , "borisp@mellanox.com" , "aviadye@mellanox.com" , Doron Roberts-Kedes , David Miller To: Vakul Garg Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:36940 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729639AbeHAWjA (ORCPT ); Wed, 1 Aug 2018 18:39:00 -0400 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On 08/01/18 01:49 PM, Vakul Garg wrote: > > I don't think this patch is safe as-is. sgin_arr is a stack array of size > > MAX_SKB_FRAGS (+ overhead), while my read of skb_cow_data is that it > > walks the whole chain of skbs from skb->next, and can return any number of > > segments. Therefore we need to heap allocate. I think I copied the IPSEC > > code here. > > > > For perf though, we could use the stack array if skb_cow_data returns <= > > MAX_SKB_FRAGS. > > skb_cow_data() is being called only when caller passes sgout=NULL (i.e. > non-zero copy case). In case of zero-copy, we do not call skb_cow_data() > and just assume that MAX_SKB_FRAGS+2 sized scatterlist array sgin_arr[] > is sufficient. This assumption could be wrong. So skb_cow_data() should be > called unconditionally to determine number of scatterlist entries required > for skb. I agree it is best to unify them. I was originally worried about perf with the extra allocation (which you proposed fixing by merging with the crypto allocation, which would be great), and the perf of skb_cow_data(). Zerocopy doesn't require skb_cow_data(), but we do have to walk the skbs to calculate nsg correctly. However skb_cow_data perf might be fine after your fix "strparser: Call skb_unclone conditionally".