Message-ID: <9b5b88da-0d2d-d3f3-6ee1-7e4afc2e329a@intel.com>
Date:   Fri, 3 Mar 2023 14:26:56 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH bpf-next v1 1/2] xdp: recycle Page Pool backed skbs built
 from XDP frames
Content-Language: en-US
To:     Yunsheng Lin <linyunsheng@huawei.com>
CC:     Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        =?UTF-8?Q?Toke_H=c3=b8iland-J=c3=b8rgensen?= <toke@redhat.com>,
        Song Liu <song@kernel.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>, <bpf@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230301160315.1022488-1-aleksander.lobakin@intel.com>
 <20230301160315.1022488-2-aleksander.lobakin@intel.com>
 <36d42e20-b33f-5442-0db7-e9f5ef9d0941@huawei.com>
 <dd811304-44ed-0372-8fe7-00c425a453dd@intel.com>
 <7ffbcac4-f4f2-5579-fd55-35813fbd792c@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <7ffbcac4-f4f2-5579-fd55-35813fbd792c@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?QUJQdGR3LzlMajlnSGIza2xGNGFOYzV6UFNRRkxWeUVHRlp3T3NEblpydWRJ?=
 =?utf-8?B?ZEx4WVVkVTV2ck1HNnN3Kzc3SXhlUkNJakxHRjRkb04yZTB1aUtvMmJTYVJR?=
 =?utf-8?B?VWlheEEzSE96K2dTVTYrNEJVaWVOQW5ZdXBxYW9NTlVLR211TGFFZ3BYSzh0?=
 =?utf-8?B?eVBRTkZDT1RoYXJqWHdIYm5kSGhWRWF3L01BWit4M0RSY09OUEQvb3J5ZzEz?=
 =?utf-8?B?YW8rQVI3Q2IweFk1aUh4QWRWZnZxT1Y5WmlBSjRCbW1sMWRxcy95dlNYTS92?=
 =?utf-8?B?L0FqS0dsTjZCMmtVd3p2dFBKSUUvelY5WkRUZFdWTklXVnU1OURpTnNhQUdM?=
 =?utf-8?B?QzdteTZHZUZkUEtodXNkb1RuRkExTHBJSkdzZ2k4MENsRGFDeWFHcEhLUi9J?=
 =?utf-8?B?dzRwamZGVzcyNEdMS0szN1Jyd0NCcDEzazRsNlViRlZ3L0NoS0N0bzMwcGQ1?=
 =?utf-8?B?Y055RjZDRU0xRUt2RHJIeEpEMDYzTkdQcy9MUzkwME1TVDdZeFh6dzhFY202?=
 =?utf-8?B?YXdXd01jRjlXaWxwVVllVmYzckdrT29XWnRVMWN4SkFzWmh0cmdzRFpTdEkx?=
 =?utf-8?B?OFMyZUdwanVRTWswQTU3UHpkT0NMOWR6UzRHUzBoL3lqK1lKdnFjZ1hHMitV?=
 =?utf-8?B?VmNRdWgxd2FWckFhWjFpNkdBWG5TcjVQVEV1bnFrUUpBbS9HYU05QURMQzRY?=
 =?utf-8?B?Kzl4aWJFNlNZM0d2NSsrb1YwRElNSE1nNEtlK1ovY2VoeVBpYWUrY3JUR2F2?=
 =?utf-8?B?R3hkSDc3QlIrUGJRTnRjaUU4RDdDajZwUXhQWm53VFpicS9XZ3hFbm9lT3Nt?=
 =?utf-8?B?bnJ4SkF6S05SRjdUenRvZDlzL0JEZEFYc2Y1TUdiSjNUVExWY0tBVlJmY0hq?=
 =?utf-8?B?Smk2UkNnRW10dkM1NkFiZE1tcjJLYVpIRnBzd1ZQb3AwWUVDY29jSzVvSTAv?=
 =?utf-8?B?RDNJUG5rdEF0YjE2QnUvQ0Jjamp6QVB2Ri9FbmRNTmtlUWhNVVdTaXdzVUNu?=
 =?utf-8?B?Wm5oUktVUFhrUkZyempubXdnTDlwb2N0d3I0Q25FV3ZiYlJmS1ZSMjRnRFYw?=
 =?utf-8?B?OVNmd1QrVUFSbkJkUm5jOHIvUDB0ZlJ4dW5UcXFjNkEwa0pGUzYzblZFWTVN?=
 =?utf-8?B?SEhUbi8xK3RaWE9KUXd3cEdKMlovTnVJaStBT3JENkFSRlE5bHpkVDMvaEJ4?=
 =?utf-8?B?TUxYTGd4WnBDU3VKdjVHT3B1OUcycE9yQ1ZqRkZYQURRSnRNdHlWYW16QTVR?=
 =?utf-8?B?NUg1S1h1QnFEdlNVdXBBeW1KMFhjb25NTURTcytWa0IxNFZRU3ZNaVVPS0Jx?=
 =?utf-8?B?VlpWQk1FUWJaa3E3OURWU1JtQ1FCM2JBME53MzZFVzFsM0lTaVdyeUpmcWZm?=
 =?utf-8?B?NmhYdlJ6aU9yREh4VDc2SEorbjNleDQ3cWxTYytlOVBoS3gySFhFOUZkcU1B?=
 =?utf-8?B?d0tNTVZqbVpWanRUaU1sTFBDVXNQaXllTk9RRXhXclFZTmJuT3M5UTA1UFJq?=
 =?utf-8?B?ZWdwaVN3Mjhyc1p6QkNFWlpESjBmV1pubzdaMGszZ1JKSXdXeHRSbEN0dVF5?=
 =?utf-8?B?RlFoL0w3QWl4SnVGMXVNcW5yRUk4RDdVc3NBb0xyMERwVUJDa2NHSEFFYzh4?=
 =?utf-8?B?SUxXQWF0VlZ6djg3dWQ4TzBhMG9hbENDU1pZZ0JKRkcraG1VczNlVWtHcXpZ?=
 =?utf-8?B?VXdCbVl6Z05ldDlBK2RTSHFxeUdTQSttMlQxV1BEOXlxWDFyMHVKRmUzdFhy?=
 =?utf-8?B?UXc3NXdNcnFESEEwcWM5QmxmYlJMWjlnWms3aVd2ekNtaXF5Q3lLMy95ZTI5?=
 =?utf-8?B?OTlDK2M4WVJrUlY1K2dMbzNNcW1qQWlTVHRDTjB2blNzdlV2TEdXMG9JUm1V?=
 =?utf-8?B?ZUdjUWlKSFhnOUJ2MnQ1SWlBVm9RT1Q4TUZhUkxwaTREN3RPNWRGdFNSWUlF?=
 =?utf-8?B?SDQzRklYaDl0ZkQ3eGJ1RndUaGZkZUJKN2J5MDdoa2pTZGd4OXlTZ0ZJNWV3?=
 =?utf-8?B?cTNMVmlVQzlZbzJEam1pckkvR25CVm5FZHdlSUExTlJlMXRIdnhLc2oxcW1n?=
 =?utf-8?B?Mm83NTlrNnZ5Ui9oazVFSk1UZHNnbWpsSzhUbEtBbnFjZGV2V2orTDBwMXdo?=
 =?utf-8?B?YVF0VTJZRnVDS3AvQks5MmZJS3dlNVl3cFZBWWZtRGd1YjVicExtYldZQThl?=
 =?utf-8?Q?SbkApd476/gKX44aVkEk1iM=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 353caafd-07fe-473c-c921-08db1beb17ff
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2023 13:27:54.0554
 (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: 3gd3YGfJOOsyeNEtzCxlccPjFP25LJmYyw62fbFeI28d7u/Es5j/xoSYYyyK15pSMh1XlIl+sOZItmWsD8GuPBjWqBeFfHWonBfyLyvYvtQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7924
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Fri, 3 Mar 2023 20:44:24 +0800

> On 2023/3/3 19:22, Alexander Lobakin wrote:
>> From: Yunsheng Lin <linyunsheng@huawei.com>
>> Date: Thu, 2 Mar 2023 10:30:13 +0800

[...]

>> And they are fixed :D
>> No drivers currently which use Page Pool mix PP pages with non-PP. And
> 
> The wireless adapter which use Page Pool *does* mix PP pages with
> non-PP, see below discussion:
> 
> https://lore.kernel.org/netdev/156f3e120bd0757133cb6bc11b76889637b5e0a6.camel@gmail.com/

Ah right, I remember that (also was fixed).
Not that I think it is correct to mix them -- for my PoV, a driver
shoule either give *all* its Rx buffers as PP-backed or not use PP at all.

[...]

>> As Jesper already pointed out, not having a quick way to check whether
>> we have to check ::pp_magic at all can decrease performance. So it's
>> rather a shortcut.
> 
> When we are freeing a page by updating the _refcount, I think
> we are already touching the cache of ::pp_magic.

But no page freeing happens before checking for skb->pp_recycle, neither
in skb_pp_recycle() (skb_free_head() etc.)[0] nor in skb_frag_unref()[1].

> 
> Anyway, I am not sure checking ::pp_magic is correct when a
> page will be passing between different subsystem and back to
> the network stack eventually, checking ::pp_magic may not be
> correct if this happens.
> 
> Another way is to use the bottom two bits in bv_page, see:
> https://www.spinics.net/lists/netdev/msg874099.html
> 
>>
>>>
>>>>  
>>>>  	/* Allow SKB to reuse area used by xdp_frame */
>>>>  	xdp_scrub_frame(xdpf);
>>>>
>>
>> Thanks,
>> Olek
>> .
>>

[0] https://elixir.bootlin.com/linux/latest/source/net/core/skbuff.c#L808
[1]
https://elixir.bootlin.com/linux/latest/source/include/linux/skbuff.h#L3385

Thanks,
Olek