Re: [RFC bpf-next v3 02/11] bpf: add struct_ops_tab to btf.

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 9/20/23 8:59 AM, thinker.li@xxxxxxxxx wrote:
> From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
>
> struct_ops_tab will be used to restore registered struct_ops.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
> ---
>   include/linux/btf.h |  9 +++++
>   kernel/bpf/btf.c    | 84 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 93 insertions(+)
>
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 928113a80a95..5fabe23aedd2 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -571,4 +571,13 @@ static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type
>   	return btf_type_is_struct(t);
>   }
>   
> +struct bpf_struct_ops;
> +
> +int btf_add_struct_ops_btf(struct bpf_struct_ops *st_ops,
> +			   struct btf *btf);
> +int btf_add_struct_ops(struct bpf_struct_ops *st_ops,
> +		       struct module *owner);
> +const struct bpf_struct_ops **
> +btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
> +
>   #endif
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index f93e835d90af..3fb9964f8672 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -241,6 +241,12 @@ struct btf_id_dtor_kfunc_tab {
>   	struct btf_id_dtor_kfunc dtors[];
>   };
>   
> +struct btf_struct_ops_tab {
> +	u32 cnt;
> +	u32 capacity;
> +	struct bpf_struct_ops *ops[];
> +};
> +
>   struct btf {
>   	void *data;
>   	struct btf_type **types;
> @@ -258,6 +264,7 @@ struct btf {
>   	struct btf_kfunc_set_tab *kfunc_set_tab;
>   	struct btf_id_dtor_kfunc_tab *dtor_kfunc_tab;
>   	struct btf_struct_metas *struct_meta_tab;
> +	struct btf_struct_ops_tab *struct_ops_tab;
>   
>   	/* split BTF support */
>   	struct btf *base_btf;
> @@ -1688,11 +1695,20 @@ static void btf_free_struct_meta_tab(struct btf *btf)
>   	btf->struct_meta_tab = NULL;
>   }
>   
> +static void btf_free_struct_ops_tab(struct btf *btf)
> +{
> +	struct btf_struct_ops_tab *tab = btf->struct_ops_tab;
> +
> +	kfree(tab);
> +	btf->struct_ops_tab = NULL;
> +}
> +
>   static void btf_free(struct btf *btf)
>   {
>   	btf_free_struct_meta_tab(btf);
>   	btf_free_dtor_kfunc_tab(btf);
>   	btf_free_kfunc_set_tab(btf);
> +	btf_free_struct_ops_tab(btf);
>   	kvfree(btf->types);
>   	kvfree(btf->resolved_sizes);
>   	kvfree(btf->resolved_ids);
> @@ -8601,3 +8617,71 @@ bool btf_type_ids_nocast_alias(struct bpf_verifier_log *log,
>   
>   	return !strncmp(reg_name, arg_name, cmp_len);
>   }
> +
> +int btf_add_struct_ops_btf(struct bpf_struct_ops *st_ops, struct btf *btf)

A few nits.

'struct btf *btf' as the first argument, to be consistent with other similar btf 
functions.

This new function is not used outside of this file, so at least static. I would 
just fold this into btf_add_struct_ops() below which currently is mostly empty 
other than a btf_get/put.

> +{
> +	struct btf_struct_ops_tab *tab;
> +	int i;
> +
> +	/* Assume this function is called for a module when the module is
> +	 * loading.
> +	 */
> +
> +	tab = btf->struct_ops_tab;
> +	if (!tab) {
> +		tab = kzalloc(sizeof(*tab) +
> +			      sizeof(struct bpf_struct_ops *) * 4,
> +			      GFP_KERNEL);

nit. offsetof(struct bpf_struct_ops_tab, ops[4]).

> +		if (!tab)
> +			return -ENOMEM;
> +		tab->capacity = 4;
> +		btf->struct_ops_tab = tab;
> +	}
> +
> +	for (i = 0; i < tab->cnt; i++)
> +		if (tab->ops[i] == st_ops)
> +			return -EEXIST;
> +
> +	if (tab->cnt == tab->capacity) {
> +		struct btf_struct_ops_tab *new_tab;
> +
> +		new_tab = krealloc(tab, sizeof(*tab) +
> +				   sizeof(struct bpf_struct_ops *) *
> +				   tab->capacity * 2, GFP_KERNEL);
> +		if (!new_tab)
> +			return -ENOMEM;
> +		tab = new_tab;
> +		tab->capacity *= 2;
> +		btf->struct_ops_tab = tab;
> +	}
> +
> +	btf->struct_ops_tab->ops[btf->struct_ops_tab->cnt++] = st_ops;
> +
> +	return 0;
> +}
> +
> +int btf_add_struct_ops(struct bpf_struct_ops *st_ops, struct module *owner)
> +{
> +	struct btf *btf = btf_get_module_btf(owner);
> +	int ret;
> +
> +	if (!btf)
> +		return -ENOENT;
> +
> +	ret = btf_add_struct_ops_btf(st_ops, btf);
> +
> +	btf_put(btf);
> +
> +	return ret;
> +}
> +
> +const struct bpf_struct_ops **btf_get_struct_ops(struct btf *btf, u32 *ret_cnt)
> +{
> +	if (!btf)
> +		return NULL;
> +	if (!btf->struct_ops_tab)
> +		return NULL;
> +
> +	*ret_cnt = btf->struct_ops_tab->cnt;
> +	return (const struct bpf_struct_ops **)btf->struct_ops_tab->ops;
> +}