Received-SPF: pass (google.com: domain of linux-kernel+bounces-138644-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <91486cf6-c496-4459-8379-257383d031a1@intel.com>
Date: Wed, 10 Apr 2024 15:36:13 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v9 7/9] libeth: add Rx buffer management
To: Kees Cook <keescook@chromium.org>
CC: Jakub Kicinski <kuba@kernel.org>, "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>, Paolo Abeni <pabeni@redhat.com>,
	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-8-aleksander.lobakin@intel.com>
 <20240405212513.0d189968@kernel.org>
 <1dda8fd5-233b-4b26-95cc-f4eb339a7f88@intel.com>
 <755c17b2-0ec2-49dd-9352-63e5d2f1ba4c@intel.com>
 <202404090909.51BAC81A6@keescook>
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Content-Language: en-US
In-Reply-To: <202404090909.51BAC81A6@keescook>
Content-Type: text/plain; charset="UTF-8"
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?MllYdDE1OWtFM0pEcWFjZ2ZPdS9pZ0MvWVR4T1ZQNUg0a0lpQmYzeWFQcmxk?=
 =?utf-8?B?UFJmRjlRNW9udFZDaXUzTzdtQmtJaGFMMjQ4ZDdZM1hLUExQVENJN3BoMkR4?=
 =?utf-8?B?L0FVTzdieGdQQ0hVdS84cTF1NlNqUkd2VkJjak52bElwbmlXZ093UzBUWWM2?=
 =?utf-8?B?UzExTGNLVTFsc3RadHdNNWpUSUdMRFZKTkZyYWRCall5REVaTm1qbmF1Z0xu?=
 =?utf-8?B?VWVLMms0N2ZkZDFZdkNXdHU1ZjQ5S2EyK01BSENwNGFycEpqaFk3Mk5lYmhw?=
 =?utf-8?B?KytJV2JNdWhSUnRQUkNYQzMvRkJrMU0wL1NTejZCc0M0dkpGZ0g2QW1GOHZ0?=
 =?utf-8?B?L2t6djRSMDladTZDYjRzQXkza1RKMk9Ed1Y4T24zZjlWb1NUeTIwZ3pvRU1F?=
 =?utf-8?B?WFBYeENjSlZXcHVXQWZPVitNWFE3anJQYlN5Q3RKUVFBd1BTcDI5UUI0RXdw?=
 =?utf-8?B?NkZXMWNtSE9zd01pSm9IZ3BzTlBIUnl0Z1NwaktZSUtkNXd5dVRQZ3lJQXlN?=
 =?utf-8?B?WDVUVHZPbnp4d3FQdHhiT252ZkJLbElsaERhQVMrOUU2MCtPajVQTkZmcjlv?=
 =?utf-8?B?Z3NEK2hZNWpVRTFXemRUeTE4OWpRUTA2elhmdklPT21aWkJBV2JjaVZ6K3B2?=
 =?utf-8?B?UkdHRkRoTzFZTE1EbCtteGNEYzE4bW1ZdkVrb0d4SVB3VWlUaENnNGtuQ0Nh?=
 =?utf-8?B?K2RLWEZJR04wNG4zcENIcldLQmZOTVI0cnZTTXlPMGkrdmJNaFdTd01xamRN?=
 =?utf-8?B?aFRvUzk4ZGlOb0EyZFFZZDVGYVFvdlRqVk16Y2xwemJIZFZ0aU1LSzR6ejNa?=
 =?utf-8?B?OFhlSEhJT2s4dkk5d2dLRmRzT01Yek1MRExkcDNaM0hKdVRwRms2WENsOW1i?=
 =?utf-8?B?eFBteWQyUzBoekhVUmpYZGZGTkJSSEVRWG1CSVVVMnhxdmFHdDcyVTJidmox?=
 =?utf-8?B?TnBLaHZpR0x1Z0dkUjhSaklNYzNzcFczc2tLNy9IVDhnS1ZYaVA3UWV4UXo3?=
 =?utf-8?B?ZWZQMmw0dm8rc2FwUE1NUUk0YThQMTkwZko2am02emJWRTRhZmcydnVkdEtY?=
 =?utf-8?B?bE5IaWcvTCtBcldtYjk5MlFiUnRGemJUUThaQjRteWMxczg1N3dhRmZuTGhV?=
 =?utf-8?B?Zm9YSnBEV09yUkNHRkNMbHlWRXNVd1ZhUkNhT0VMdmcwZkxZTElQVjQ3TmZM?=
 =?utf-8?B?aFh6eTgxK092TVdwM0FzNU9tc25ENnN1N3RzT1lleHgveHZwbHd3b0g1c2p3?=
 =?utf-8?B?bnc2NGN3WG9JODk2alhSVzJPZFZHaDdiRktRMmNXRFhCV3JWMVFMc1p1emN4?=
 =?utf-8?B?czRNdkQ0OWZHYU40Qzhodi9aT245Y3lDbGN2VG41Q2dPbWR0U2NBZmREOFhv?=
 =?utf-8?B?NVkySTFQcWFOT1ZjWEJ6ckY5WFJBWXNCTEtOT3FGNEU5NGVTazF0Z2ZCa25x?=
 =?utf-8?B?NGY2S1NUZVczUFc4eXFyVE0xaVVlSkoyblE0TlZ2VUpXRzVMZjlLNEdzaGVl?=
 =?utf-8?B?SVpjaDNDL3lRTzA5cGd3YndvNU12TEg2QlYzNUR0WVd2bFg3UXpqS3FHa09m?=
 =?utf-8?B?QjRjZDVJR2o3Q0hxNDQ4MFk1TkU1V2VoZ3lHbWZrenJYWW5ubjcyRFlXZFVT?=
 =?utf-8?B?VmFiMS9oZU81N1VWTWgwYi9HcCtnMkljQmtDWldOWC9iUEltcUR5QTBoTUdE?=
 =?utf-8?B?L0FhMloxNU1MMjZjcWsyeVliaTlCNlcza2RSeEoxK0VMRHppVmFkeStUeGg4?=
 =?utf-8?B?eGdIOHhLc3RhMFBCTDdYSkVaTlNranUxZzZObE50cWs0c0lleWJEN1ZMYWdn?=
 =?utf-8?B?WTc1ZkxIV1FkY3Zjb1ZvTnZ2bGIvbnJtQ1QrSHNZeEZQaVFacXMydmxCeTA2?=
 =?utf-8?B?K0xCZ1I4eURLMVV1SFdqbmdiemEwOGNIckxPSXhxN1lhSnNOZkxrelZJaUZm?=
 =?utf-8?B?bXdhMDNvUVowcWdlRVlIc1JwVWg2bFZkazJJU1BYY2lGOVVNczkxRmVENTVz?=
 =?utf-8?B?QTZ4YWZ6cHNZaWZTUFZoaHVZb1VmNDNoZFFwT0J6aUFOQmpKNDllV2RZeWRD?=
 =?utf-8?B?T2dwQ1ZrTEE3aU5nUGVQLy9Pd2lZR1ZPN1B4THpzNm9KVFFtMmZ0SFlUd0VV?=
 =?utf-8?B?bWhFTmRlbS84OS9nZVZHTVViK29hZmRBSmtOWmFZT012dmIraUhJWVlmYUhB?=
 =?utf-8?B?bUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: e9d312b9-eb38-434c-1610-08dc59637e8a
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB8718.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2024 13:38:24.0617
 (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: +cKITuaKihKS3J8yUSSx1QUZhywZ10+Yz524QAYr7KrWIjl+51Qp/JP0hUMF0mCH/lqDAy15H6qT5OzsvAp0FtY97i11hEtdR0PJ5sy4MSU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB7932
X-OriginatorOrg: intel.com

From: Kees Cook <keescook@chromium.org>
Date: Tue, 9 Apr 2024 09:17:53 -0700

> On Mon, Apr 08, 2024 at 11:45:32AM +0200, Alexander Lobakin wrote:
>> From: Alexander Lobakin <aleksander.lobakin@intel.com>
>> Date: Mon, 8 Apr 2024 11:11:12 +0200
>>
>>> From: Jakub Kicinski <kuba@kernel.org>
>>> Date: Fri, 5 Apr 2024 21:25:13 -0700
>>>
>>>> On Thu,  4 Apr 2024 17:44:00 +0200 Alexander Lobakin wrote:
>>>>> +/**
>>>>> + * struct libeth_fq - structure representing a buffer queue
>>>>> + * @fp: hotpath part of the structure
>>>>
>>>> Second time this happens this week, so maybe some tooling change in 6.9
>>>> but apparently kdoc does not want to know about the tagged struct:
>>>>
>>>> include/net/libeth/rx.h:69: warning: Excess struct member 'fp' description in 'libeth_fq'
>>>
>>> Oh no, maybe we should teach kdoc to parse struct_group*()?
>>
>> scripts/kernel-doc apparently can handle them...
>>
>> + Kees
>>
> 
> Ah, hm, scripts/kernel-doc throws away the early arguments of
> struct_group_tagged, but I suspect it needs to create a synthetic member
> for the tag. i.e. instead of:
> 
> 	struct_group_tagged(tag, name, members...)
> 
> becoming
> 
> 	members...
> 
> it needs to become
> 
> 	struct tag name;
> 	members...
> 
> It seems this is the first place anyone has tried to document the tagged
> struct name! :)

It makes sense and TBH I expected kdoc to warn that an element
description is missing :D

> 
> Does this work? I haven't tested it...
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 967f1abb0edb..64a19228d5dd 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -1151,7 +1151,8 @@ sub dump_struct($$) {
>          # - first eat non-declaration parameters and rewrite for final match
>          # - then remove macro, outer parens, and trailing semicolon
>          $members =~ s/\bstruct_group\s*\(([^,]*,)/STRUCT_GROUP(/gos;
> -        $members =~ s/\bstruct_group_(attr|tagged)\s*\(([^,]*,){2}/STRUCT_GROUP(/gos;
> +        $members =~ s/\bstruct_group_attr\s*\(([^,]*,){2}/STRUCT_GROUP(/gos;
> +        $members =~ s/\bstruct_group_tagged\s*\(([^,]*,)([^,]*,)/struct $1 $2; STRUCT_GROUP(/gos;

This one does not. We need to exclude ','s from the groups...
      +        $members =~
s/\bstruct_group_tagged\s*\(([^,]*),([^,]*),/struct $1 $2;
STRUCT_GROUP(/gos;

That one is fine. $members:

include/net/libeth/rx.h:91: warning: struct libeth_fq_fp  fp;
STRUCT_GROUP( struct page_pool        *pp; struct libeth_fqe
*fqes; u32                     truesize; u32                     count;
); enum libeth_fqe_type    type:2; bool                    hsplit:1;
bool                    xdp:1; u32                     buf_len; int
               nid;

So you almost fixed it :D

Which tree this should go through? Should I include this patch to this
series with libeth or it's better to push this through kees/linux and
then pull to net-next?


>          $members =~ s/\b__struct_group\s*\(([^,]*,){3}/STRUCT_GROUP(/gos;
>          $members =~ s/\bSTRUCT_GROUP(\(((?:(?>[^)(]+)|(?1))*)\))[^;]*;/$2/gos;

Thanks,
Olek