From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [net-next, RFC, 4/8] net: core: add recycle capabilities on skbs via page_pool API Date: Sat, 8 Dec 2018 14:45:53 +0100 Message-ID: <20181208144553.03b059c6@redhat.com> References: <154413874729.21735.10644578158550468689.stgit@firesoul> <20181208095758.GA32028@strlen.de> <72f33f12-9222-cbe7-6ff2-e4b4f86fb17c@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Florian Westphal , netdev@vger.kernel.org, "David S. Miller" , Toke =?UTF-8?B?SMO4aWxhbmQtSsO4cmdlbnNl?= =?UTF-8?B?bg==?= , ard.biesheuvel@linaro.org, Jason Wang , ilias.apalodimas@linaro.org, =?UTF-8?B?QmrDtnJu?= =?UTF-8?B?VMO2cGVs?= , w@1wt.eu, Saeed Mahameed , mykyta.iziumtsev@gmail.com, Daniel Borkmann , Alexei Starovoitov , Tariq Toukan , brouer@redhat.com To: Eric Dumazet Return-path: Received: from mx1.redhat.com ([209.132.183.28]:48898 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726124AbeLHNqE (ORCPT ); Sat, 8 Dec 2018 08:46:04 -0500 In-Reply-To: <72f33f12-9222-cbe7-6ff2-e4b4f86fb17c@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 8 Dec 2018 04:29:17 -0800 Eric Dumazet wrote: > On 12/08/2018 01:57 AM, Florian Westphal wrote: > > Jesper Dangaard Brouer wrote: > >> From: Ilias Apalodimas > >> > >> This patch is changing struct sk_buff, and is thus per-definition > >> controversial. > >> > >> Place a new member 'mem_info' of type struct xdp_mem_info, just after > >> members (flags) head_frag and pfmemalloc, And not in between > >> headers_start/end to ensure skb_copy() and pskb_copy() work as-is. > >> Copying mem_info during skb_clone() is required. This makes sure that > >> pages are correctly freed or recycled during the altered > >> skb_free_head() invocation. > > > > I read this to mean that this 'info' isn't accessed/needed until skb > > is freed. Any reason its not added at the end? > > > > This would avoid moving other fields that are probably accessed > > more frequently during processing. > > > > But I do not get why the patch is needed. > > Adding extra cost for each skb destruction is costly. > > I though XDP was all about _not_ having skbs. > > Please let's do not slow down the non XDP stack only to make XDP more > appealing. In general this work is about better cooperation between XDP and netstack. The patch is needed because the page_pool is keeping pages DMA mapped and need a return hook. I doubt that the extra compare-and-branch will affect your use-case on super-scalar CPUs. We (Ilias and I) are actually testing this stuff on low-end ARM64 in-order execution CPUs, which is actually nice as performance effects of our code changes are not hidden by speculative execution units. I'm enforcing (and Ilias agrees) that we do benchmark driven development. I actually invite people to monitor our progress here[1]. So, trust me, I am as concerned as you about any performance regression, and is vigilantly measuring this stuff. (This is more than you can say about a lot of the other stuff that gets accepted on this list). [1] https://github.com/xdp-project/xdp-project/blob/master/areas/arm64/ -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer