Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dbb5500e-f623-77fb-2606-c3073371e979@oracle.com>
Date:   Thu, 20 Jul 2023 23:34:42 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v2 3/9] bpf/btf: Add a function to search a member of a
 struct/union
Content-Language: en-GB
To:     "Masami Hiramatsu (Google)" <mhiramat@kernel.org>,
        linux-trace-kernel@vger.kernel.org
Cc:     linux-kernel@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>,
        Martin KaFai Lau <martin.lau@linux.dev>, bpf@vger.kernel.org,
        Sven Schnelle <svens@linux.ibm.com>,
        Alexei Starovoitov <ast@kernel.org>
References: <168960739768.34107.15145201749042174448.stgit@devnote2>
 <168960742712.34107.9849785489776347376.stgit@devnote2>
From:   Alan Maguire <alan.maguire@oracle.com>
In-Reply-To: <168960742712.34107.9849785489776347376.stgit@devnote2>
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?cm5PcUZ5SG9OY0htZFBnTENzakpBY0xwV3RySDVnRm5ZK1JkcEx1b2tkSFNu?=
 =?utf-8?B?VnNtYitpYk1zMy9kenVDMVdGQTJibUFTNUJjTHFZUkxTV0dISHdqbDJNM0ky?=
 =?utf-8?B?NjBQemlmbVBObHJXbkNlYW1GLzhhVWIrcllQcE9PR3BnbmdRNjVTQUFKUXZj?=
 =?utf-8?B?YmVqUVgwNDcvU2hDZTZqbkhLbHFCTDhEcm9oMWh3a0MwSEF2NjlzL1pOa0lr?=
 =?utf-8?B?OU4yMEVqdXVORmJwQ2w2NEYwWmdyTGU2N3REVHlVYUJaN25Pdi9BdlpQTm1w?=
 =?utf-8?B?OEpGQThGaDRPUnpVRkFLRlg2bHY4TmozUFRGMWluSjNwR3NsYW9DVnZPTExu?=
 =?utf-8?B?eUNTdUNldVlPclFWazYyaDJVcFJnMXR6d2RBVmt3TU02dGdTWXgxZm9LajBs?=
 =?utf-8?B?Sy9kZStXSUNIQW5TM3FQcnlmb0lyK2lGektLL2ViMnJoMUUzWGpPT0RlWG4v?=
 =?utf-8?B?NlBacWFkaHlsNFJoRFlJaFJLUXM3RFR2RHMvVG9aKzIrMW5RSGJySzVaOThT?=
 =?utf-8?B?NFNCMkdyZ1lhaDdkc2VETUo5VUNVMDkrR1M1ZkJ2bTBML3ZwK2wrYW5xRERK?=
 =?utf-8?B?QngwdE9maUhKUUVGK1NHUm1QQUliamJPRkR4Umx2by8wTGlDMWkwZklOeGl3?=
 =?utf-8?B?ajViV0Y0L0hiN1YyK3lmQUQ0N29KTFNzUUFEZ1Q5TFdGV2tqSHRqckZENGNJ?=
 =?utf-8?B?bjI0NTNBWjVQcXcxT0U0QVZBeHRQUTFycmg2dWk3cWpjN0ErMVZBQVVFeExG?=
 =?utf-8?B?RWUxVjQ2T0FtRE9oMS9tek1QTlAyeTFXS1pEVWZKZC80NnUxZjEwNWIvR3pY?=
 =?utf-8?B?Um5YQTVqZnlCeTQyZEdQa1ZXVlNjMDFXc1lqS09Hcjhaa0lKNllJN0lpMkMr?=
 =?utf-8?B?bHNkQjNFcWpDTURGanpnY3lsSWZKMkZhWTBWTWNjODdWUFZ2WFZwWlJ1TCtK?=
 =?utf-8?B?bUYrTURaMHNzVWw3UDVObExFRXIxODlBRERxL21nanFSVzBmMk93aG5NNEhZ?=
 =?utf-8?B?NWx6dXFyTDB1dDg2L3NqR2Z0SkY0QU5nRjE2dDNWVUZCMjdRQzMxdGRBc1Vy?=
 =?utf-8?B?cG5xRzU5QjU0V3EzWG5sVEV4QVQ1Q0hGT0FDdkNmS2tvMTUrQ1I3R2ZFcVZY?=
 =?utf-8?B?MnVqT0dTWXhkcjlScnhOaWtqbDBUR00wVGdIWTFMYnVadmFkYjVKU1Q4N1Fa?=
 =?utf-8?B?TjViY1d2UmZJdC9mazFSbTBqbk9BTmhYWE9yUVd5N3BzSlROUkpYb2RIb2hJ?=
 =?utf-8?B?bWY3RkpmL3RhMW1jRUFKYlRTdGxmV2FVQnFrdFJISjFyNjFzS0Q2Sy9mZVZX?=
 =?utf-8?B?VzdaWC9QWTFBRkRFVU5kS3h1UW5mTGlRWFJRMXp2SE5sY1NNbnJWMmZMeHI0?=
 =?utf-8?B?WXBxcUNiSVZaWU1jQmlvUitaNEVhWjlvSGtQbHJFSGdjdjZVT2ZCMURoSEg5?=
 =?utf-8?B?cGhrT1hMQmlBeUJzRVJIZkE4WEE2Yzd1VVhxMlJWL0t3dFdNL0l5SFN3T0tG?=
 =?utf-8?B?T3BBY3RIWHVRY3BYdGJpb3EvVCtjSTJxS21xN0NpRkYwNG5zaHo0cUd3S3g0?=
 =?utf-8?B?QkJuTVk4NHVGbzlQeGUxL0o4enZ5Qnp4dUtMaVA5WkhLY3hncmFEVW55b3pk?=
 =?utf-8?B?RXAyWjBuNHZvenZkZFlNRWZHSlI3bEdKY0ZXb3Y3MnRoNWxzWjZKNDVMc3pE?=
 =?utf-8?B?eDh3UzRibWQ3SDA0SzFKT3Nua2ZHWGR6dHhHd1RTNTg5WU5MNmJsM3E2ZitI?=
 =?utf-8?B?WU1oOEZQY1JQNUEwZnNlYTZvOEd0eFVQQjJMK2JjSHd5MmlnMjlMbzI3eVVT?=
 =?utf-8?B?dmlxVXRCNm1RTlJOMElIRDBlWC9rTTdYVnhoS2ZDZVBvWUxHR0pWdlhQbjlC?=
 =?utf-8?B?aVdmYWN6SmVjcmx0VzVjY1VBcXF4Q2NabER6UWluT2dTNVZocmtDZHNmQ0R6?=
 =?utf-8?B?c3N5dlFuZS9DY09TS2xWdFFZcU5FZGU3YVVKT25MKys3a0JZdUI0bVBDdVhM?=
 =?utf-8?B?bklkaGduZkVRNzV2YU12L04xNkE4alBmak01ZjZ1VDI1SGh3L1dqVkU2di9s?=
 =?utf-8?B?dk4wcXk4NmVCMjdEcVg5NkZNUVdxTkFyL1FQL3Z2K2pieVNVVjZpVUkvUHVq?=
 =?utf-8?B?Tjkrd2VjalEvN0R6MXJkODFTbXhGRzZCa2RwOGFjdnNrUmE1OVpRME1wK3RS?=
 =?utf-8?Q?XTI6Zm+EDmnT9TQt7rB59y8=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: o5NF9xJrkA1+pfH00jFgEuEM2xhUgwNJZTqYgTImxHkqEukx8IvDrcpqCOrwHvm3YNBwzMJG7l28T+qlnadvRKHtGN3cJyM+yUek0G91cieVVpxbfegItpYeRAUpt5L5K3w/1zm1SKvYSDBi2iU7fh0LHvW/G7Oi8nkPhoeYn8shs/cVtLPPIsBFkWZ917epQNlCQkZ5YjvENrJu5rh059gCTg8oIOdt8+ysOScsJWrC7j4r/3yghcc7q9B2Jv/VEonP/nNK4cPcMAfFBTX8VHjqE+O0toOZnCoG5T6bP71G107QzehDqLUtf23GmiDTCHYCmP764U/7ghYkgor6Ga0o7hiWSlrIQhNnbdlQd7AG0AwxkA4IEZqXSwikRualrlgq+F2uwFY59froFleV94+GQmHGFWIB8Cm6Q/b4z2MJKOhATmTSzoP7dOwK8Tb+pdLtZQIMfeUwbqLA1aOUxZO1S+dm/BUS7BljOw08spkOFjlzyft/qHo0QEyvTt+fRlCSiqd9EkK6ehJeWMI4lejbfIpo43zsyC43p2eQNOF2BfcFWP9Dz5tHPNUF/kOYR3hw5nzfKhrdRmoFVqMHXfsT10jQnP7yxAdMg7bk32MbF8cF5/Z2xJAPZBTXT2HF8n+sT1XkiIPKVEunNyMIgO6CK+3S9Dx2ORt+mrE9U71gX0BW7NF0F800REY9Dqt2ELNVGxml/diTJSN5UlhImZ1l7MFxYz7VVhcVDnVDhx2RixHqfIs90iWR2f3xEzsHQKg+B1kscX+KmOlJ/v/AXxEjXXIPdk4aD8abKwL9CkRXU0BMQhxd7uZ3rHMZxT+vEq1hc1Z3jD84HWIpuc/DdBzjSO8w3pZTTXh0fW5UOffc2ojqbO8dJqWs4wN+YnDRb+ov8xJ/9vsjsjjlK+KYQQ==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 88ac174d-069f-4ec9-0937-08db8971862c
X-MS-Exchange-CrossTenant-AuthSource: BLAPR10MB5267.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jul 2023 22:34:48.0385
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gaIN/oGoSNsGLgvIm237It2dQlXmG0DJ5UoFpz7WPb4XPR68r7X+WaYbPukdMkp512vyOMXaUpWheKm481cpmw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR10MB7516
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-20_10,2023-07-20_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 suspectscore=0 phishscore=0 bulkscore=0 mlxscore=0 malwarescore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2306200000 definitions=main-2307200192
X-Proofpoint-ORIG-GUID: qBZBmsdLrbRaUO-x9YUllAUeCJrnnXbD
X-Proofpoint-GUID: qBZBmsdLrbRaUO-x9YUllAUeCJrnnXbD
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 17/07/2023 16:23, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Add btf_find_struct_member() API to search a member of a given data structure
> or union from the member's name.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

A few small things below, but

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  include/linux/btf.h |    3 +++
>  kernel/bpf/btf.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 98fbbcdd72ec..097fe9b51562 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -225,6 +225,9 @@ const struct btf_type *btf_find_func_proto(struct btf *btf,
>  					   const char *func_name);
>  const struct btf_param *btf_get_func_param(const struct btf_type *func_proto,
>  					   s32 *nr);
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> +						const struct btf_type *type,
> +						const char *member_name);
>  
>  #define for_each_member(i, struct_type, member)			\
>  	for (i = 0, member = btf_type_member(struct_type);	\
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index e015b52956cb..452ffb0393d6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1992,6 +1992,44 @@ const struct btf_param *btf_get_func_param(const struct btf_type *func_proto, s3
>  		return NULL;
>  }
>  
> +/*
> + * Find a member of data structure/union by name and return it.
> + * Return NULL if not found, or -EINVAL if parameter is invalid.
> + */
> +const struct btf_member *btf_find_struct_member(struct btf *btf,
> +						const struct btf_type *type,
> +						const char *member_name)
> +{
> +	const struct btf_member *members, *ret;
> +	const char *name;
> +	int i, vlen;
> +
> +	if (!btf || !member_name || !btf_type_is_struct(type))
> +		return ERR_PTR(-EINVAL);
> +
> +	vlen = btf_type_vlen(type);
> +	members = (const struct btf_member *)(type + 1);
> +
> +	for (i = 0; i < vlen; i++) {

could use for_each_member() here I think, or perhaps use
btf_type_member(type) when getting member pointer above.

> +		if (!members[i].name_off) {
> +			/* unnamed union: dig deeper */
> +			type = btf_type_by_id(btf, members[i].type);
> +			if (!IS_ERR_OR_NULL(type)) {
> +				ret = btf_find_struct_member(btf, type,
> +							     member_name);

You'll need to skip modifiers before calling btf_find_struct_member()
here I think; it's possible to have a const anonymous union for example,
so to get to the union you'd need to skip the modifiers first. Otherwise
you could fail the btf_type_is_struct() test on re-entry.


> +				if (!IS_ERR_OR_NULL(ret))
> +					return ret;
> +			}
> +		} else {
> +			name = btf_name_by_offset(btf, members[i].name_off);
> +			if (name && !strcmp(member_name, name))
> +				return &members[i];
> +		}
> +	}
> +
> +	return NULL;
> +}
> +
>  static u32 btf_resolved_type_id(const struct btf *btf, u32 type_id)
>  {
>  	while (type_id < btf->start_id)
> 
>