Received-SPF: pass (google.com: domain of linux-kernel+bounces-132808-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <21be2825-167f-4357-8f6c-02fe065d7490@intel.com>
Date: Fri, 5 Apr 2024 12:15:45 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v9 2/9] iavf: kill "legacy-rx" for good
Content-Language: en-US
To: Alexander Lobakin <aleksander.lobakin@intel.com>, "David S. Miller"
	<davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski
	<kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
CC: Alexander Duyck <alexanderduyck@fb.com>, Yunsheng Lin
	<linyunsheng@huawei.com>, Jesper Dangaard Brouer <hawk@kernel.org>, "Ilias
 Apalodimas" <ilias.apalodimas@linaro.org>, Christoph Lameter <cl@linux.com>,
	Vlastimil Babka <vbabka@suse.cz>, Andrew Morton <akpm@linux-foundation.org>,
	<nex.sw.ncis.osdt.itp.upstreaming@intel.com>, <netdev@vger.kernel.org>,
	<intel-wired-lan@lists.osuosl.org>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>
References: <20240404154402.3581254-1-aleksander.lobakin@intel.com>
 <20240404154402.3581254-3-aleksander.lobakin@intel.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20240404154402.3581254-3-aleksander.lobakin@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UmxyMG1jQkNEbFhiK3JDanBEQlVYUTdVSHVrUHVnMTVpOWJOcG5tTHp2WXBY?=
 =?utf-8?B?L3FvWWdKVy82QUR5cHloMDZVK29jVjdHNEVDM3IyUHhCUXdOeWNaMVRwK2tw?=
 =?utf-8?B?VFREQTlhUHAxVm1zS1ArbnAvN0lkelhRRkJIdWJKSmsvUGRUWmlwZjNQVXY1?=
 =?utf-8?B?K2EybWhVM2tIcSsvcTN6U2NOZThDRFJvaVNicEx3YVhFUGF6bWJjNHI1L1do?=
 =?utf-8?B?Q3FEN1FObC9tckpoTENxTUpCenNTamxEeXRtWWI1L1NXTEhDU293WE1UcnRw?=
 =?utf-8?B?cDdhcEgzSkVtandnTXBQbDNYU2NGelNIb2Rwd0xCdmttTi81NHpEd0VwMnpN?=
 =?utf-8?B?MzMxbmVPY1ViK1V5QUhPMVdSREdRZWV6aHFja1hRRE42bGZ5SHJETjVITDFu?=
 =?utf-8?B?bVNOSjVlTnhBVmdhNTRsUm43Lzg0OElvN2o0elc3M2tBaVZQSGJkNEZWcmRG?=
 =?utf-8?B?SlpUS1F3MzlzQ3NPMjVSRytHOEZ5dnlCd05TYStzL3dCMFpDc1FvMVhKeHBB?=
 =?utf-8?B?SEJDQ3FkTExqV0Y2eVVpNVdVeUx2U3hLajYzd01wbTZhM05ZOVBpOHozNlpE?=
 =?utf-8?B?VlpWZlRjYVp3Yi9lYU5nUGtSeVl1dnloQVVaQ2p6QlZoeGJMSVk4NkFWdXV1?=
 =?utf-8?B?TCt1WGRQN1hFbEZNS1QwanlvREFWZXp0aXZ3aHlFQ05MV3M4SEhoQnpmOEd3?=
 =?utf-8?B?bm16cmJHeGFHYjZaS2dzcGVWd1FZaU9LWjV0TjkzYU1OSUtkU3Q2OVFGVUlp?=
 =?utf-8?B?T0pIL01VTEdINm52UHphdHdkUkFhQjJIdXh2UUxvbmE3dms2bXk3dURCQjV3?=
 =?utf-8?B?K3N5VENOMW9jM3JmN0JHclRkSGxmSjZ0bndsWnROVERMN0ZiZ0pYS1J2bHRC?=
 =?utf-8?B?S283Z2xaVDBrcTJLMjV4ZmFTTkt3cnhlQ2ZDM2ppRllVVzhuYUpURFovbWFp?=
 =?utf-8?B?WFVlUk9ZV2M5RE5oblVEVWtxZXVORXNrZDZoNDd6NUFQSDVjSmdIVkhDL3N0?=
 =?utf-8?B?R3k5SzFMSzdPaHlJeklxYVdOVzB1bU5weERQVVZOQTNHang5N29ubHhub1Ew?=
 =?utf-8?B?VGJQQ0oyLy9oN1I2TktCVGNFTDVmWEIrTVhYaWhpOTlUVmhPanZzOW9hQ21X?=
 =?utf-8?B?VUlPc0NETGlFeU9uaG5NTm0zVGdKNmJ1dGZsM1BSWVpNS1V3eTIyNFlSS0lw?=
 =?utf-8?B?OHJDWWZ3d21kTEh3RnBCbncycmg5VnJwdHZneVE1VmUxN3F1UndDdWRtTmpI?=
 =?utf-8?B?Sk0ySWI2alBjOUUwWFFZNnZORVFZSlJ6Z3pNb2Z0aUM5S1BVMm0vdTdGcnpI?=
 =?utf-8?B?OWpBL3dPYTlvVkQ1bVZ0NHFzNlErZE5qM3R4SXRDczY0Rmw0Q1pkamMyMWtB?=
 =?utf-8?B?Mkk1dDhtVXZlVStOMDNvZUhJTDBMaXVLNEFkR0ZVMkNTd1g2eDVFM1RQaDlm?=
 =?utf-8?B?dzZYdSthSzg0Q1RxQkhucTBEc21PNU56b0RSTkg5d05OQ2k3bHV2b29jbWxL?=
 =?utf-8?B?NXloVVRQM1R2RGdTK1pWTHhza1FJYW1WYnU2emlXcnkwM1hSRktDVUdGM2s1?=
 =?utf-8?B?a1pKN2c5NHE2WjFIdkhpSmplSGFWc05ML244TUF5dU5yTGwvUmw1NUpaYXpE?=
 =?utf-8?B?RHpYZjVYQktsVFhZRHJnYys0anh1YUJaM013NnB5UmsxVE12TTZNcGtTYS9K?=
 =?utf-8?B?NGZZVlVoaEJvSjlua3BTRE1Ebk5ZcjZzdXNudHNjZFgvZUlEN2tvZERrZnVR?=
 =?utf-8?B?bW1ERjQySmlRUEF6NUwxQWJ0bXZ1cnJRT2xub1UrWW1KaGw2MG9pd0NjWmJ3?=
 =?utf-8?B?bzVRTEZzNHhvSENjUXRDbWs2d2VEcDU2UE1pUUkydG16bTBsQWZxdEphT0wz?=
 =?utf-8?B?Q2cxQVNUOElqWWV1UFpkM21JS0YrMkNBanM3dG9Fd3hnaGxZcWNzQklvbUhp?=
 =?utf-8?B?SDF2RXRUWXJRYzZOUlRSeUoweHhIbU5nbko2dzllSlhadnJkZTFXeUs1cWdS?=
 =?utf-8?B?ZFpwQXc5ODVkT3hSV0E2eEJuM2NxaVVxbmwwbEtRZVZ2OWMxV3I5WFRWTnRY?=
 =?utf-8?B?Ty9IVzlHclhWN2plV3gvbHhCd0ZKUkVXdTBnYTJYb2Nyc2tPUGRLRXBxLzF3?=
 =?utf-8?B?M3ZCOVZKV3lZWnU3czg0UWlKa2dGRDNQMFpKb1U0alNzdlBqbmhhSkhzUlVG?=
 =?utf-8?Q?THh8EjWnfYM8WKLG8ovXWyY=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b24d7ffc-344b-48e8-0aea-08dc55596333
X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8102.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2024 10:15:58.5332
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: DvEUFzDm1s/lkgxQyV1AKsXa7VMKPQl7Ltsx8uG8ld7o5SCxgfAiR5EEHEmEOxaNxN8qpPLAH/J+VjxgPw6YeXjqQryo74GWLlPuB6BU6Mo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR11MB4646
X-OriginatorOrg: intel.com

On 4/4/24 17:43, Alexander Lobakin wrote:
> Ever since build_skb() became stable, the old way with allocating an skb
> for storing the headers separately, which will be then copied manually,
> was slower, less flexible, and thus obsolete.
> 
> * It had higher pressure on MM since it actually allocates new pages,
>    which then get split and refcount-biased (NAPI page cache);
> * It implies memcpy() of packet headers (40+ bytes per each frame);
> * the actual header length was calculated via eth_get_headlen(), which
>    invokes Flow Dissector and thus wastes a bunch of CPU cycles;
> * XDP makes it even more weird since it requires headroom for long and
>    also tailroom for some time (since mbuf landed). Take a look at the
>    ice driver, which is built around work-arounds to make XDP work with
>    it.
> 
> Even on some quite low-end hardware (not a common case for 100G NICs) it
> was performing worse.
> The only advantage "legacy-rx" had is that it didn't require any
> reserved headroom and tailroom. But iavf didn't use this, as it always
> splits pages into two halves of 2k, while that save would only be useful
> when striding. And again, XDP effectively removes that sole pro.
> 
> There's a train of features to land in IAVF soon: Page Pool, XDP, XSk,
> multi-buffer etc. Each new would require adding more and more Danse
> Macabre for absolutely no reason, besides making hotpath less and less
> effective.
> Remove the "feature" with all the related code. This includes at least
> one very hot branch (typically hit on each new frame), which was either
> always-true or always-false at least for a complete NAPI bulk of 64
> frames, the whole private flags cruft, and so on. Some stats:
> 
> Function: add/remove: 0/4 grow/shrink: 0/7 up/down: 0/-721 (-721)
> RO Data: add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-40 (-40)
> 
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> ---
>   drivers/net/ethernet/intel/iavf/iavf.h        |   2 +-
>   drivers/net/ethernet/intel/iavf/iavf_txrx.h   |  27 +---
>   .../net/ethernet/intel/iavf/iavf_ethtool.c    | 140 ------------------
>   drivers/net/ethernet/intel/iavf/iavf_main.c   |  10 +-
>   drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  82 +---------
>   .../net/ethernet/intel/iavf/iavf_virtchnl.c   |   3 +-
>   6 files changed, 8 insertions(+), 256 deletions(-)
> 
Awesome!
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>