On Sat, Jan 25, 2025 at 2:59 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> When there is bpf_dynptr field in the map key btf type or the map key
> btf type is bpf_dyntr, set BPF_INT_F_DYNPTR_IN_KEY in map_flags.
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> kernel/bpf/syscall.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 07c67ad1a6a07..46b96d062d2db 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1360,6 +1360,34 @@ static struct btf *get_map_btf(int btf_fd)
> return btf;
> }
>
> +static int map_has_dynptr_in_key_type(struct btf *btf, u32 btf_key_id, u32 key_size)
> +{
> + const struct btf_type *type;
> + struct btf_record *record;
> + u32 btf_key_size;
> +
> + if (!btf_key_id)
> + return 0;
> +
> + type = btf_type_id_size(btf, &btf_key_id, &btf_key_size);
> + if (!type || btf_key_size != key_size)
> + return -EINVAL;
> +
> + /* For dynptr key, key BTF type must be struct */
> + if (!__btf_type_is_struct(type))
> + return 0;
> +
> + if (btf_type_is_dynptr(btf, type))
> + return 1;
> +
> + record = btf_parse_fields(btf, type, BPF_DYNPTR, key_size);
> + if (IS_ERR(record))
> + return PTR_ERR(record);
> +
> + btf_record_free(record);
> + return !!record;
> +}
> +
> #define BPF_MAP_CREATE_LAST_FIELD map_token_fd
> /* called via syscall */
> static int map_create(union bpf_attr *attr)
> @@ -1398,6 +1426,14 @@ static int map_create(union bpf_attr *attr)
> btf = get_map_btf(attr->btf_fd);
> if (IS_ERR(btf))
> return PTR_ERR(btf);
> +
> + err = map_has_dynptr_in_key_type(btf, attr->btf_key_type_id, attr->key_size);
> + if (err < 0)
> + goto put_btf;
> + if (err > 0) {
> + attr->map_flags |= BPF_INT_F_DYNPTR_IN_KEY;
I don't like this inband signaling in the uapi field.
The whole refactoring in patch 4 to do patch 6 and
subsequent bpf_map_has_dynptr_key() in various places
feels like reinventing the wheel.
We already have map_check_btf() mechanism that works for
existing special fields inside BTF.
Please use it.
map_has_dynptr_in_key_type() can be done in map_check_btf()
after map is created, no ?
Then when it passes map->map_type check set a bool inside
struct bpf_map, so that bpf_map_has_dynptr_key() can be fast
in the critical path of hashtab.
Or better yet use:
static inline bool bpf_map_has_dynptr_key(const struct bpf_map *map)
{
/* key_record is not NULL when the map key contains bpf_dynptr_user */
return !!map->key_record;
}
since htab_map_hash() has to read key_record anyway,
hence better D$ access.
