Re: [PATCH bpf-next v2 10/20] bpf: Introduce bpf_dynptr_user

[Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>]

On Sat, Jan 25, 2025 at 2:59 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> For bpf map with dynptr key support, the userspace application will use
> bpf_dynptr_user to represent the bpf_dynptr in the map key and pass it
> to bpf syscall. The bpf syscall will copy from bpf_dynptr_user to
> construct a corresponding bpf_dynptr_kern object when the map key is an
> input argument, and copy to bpf_dynptr_user from a bpf_dynptr_kern
> object when the map key is an output argument.
>
> For now the size of bpf_dynptr_user must be the same as bpf_dynptr, but
> the last u32 field is not used, so make it a reserved field.
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
>  include/uapi/linux/bpf.h       | 6 ++++++
>  tools/include/uapi/linux/bpf.h | 6 ++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 2acf9b3363717..7d96685513c55 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -7335,6 +7335,12 @@ struct bpf_dynptr {
>         __u64 __opaque[2];
>  } __attribute__((aligned(8)));
>
> +struct bpf_dynptr_user {
> +       __bpf_md_ptr(void *, data);
> +       __u32 size;
> +       __u32 reserved;
> +} __attribute__((aligned(8)));

Pls add a comment explaining that bpf_dynptr_user is for user space only
and bpf progs should continue using bpf_dynptr.
May be give an example that to use bpf_dynptr in map key
the bpf prog should write:

+struct mixed_dynptr_key {
+ int id;
+ struct bpf_dynptr name;
+};

while to access that map the user space should write:

+struct id_dname_key {
+ int id;
+ struct bpf_dynptr_user name;
+};

tbh the api is kinda ugly, since in the past we always had user space
and bpf prog reuse the same struct names.
Here the top struct names have to be different,
but have to have the same layout.

Maybe let's try the following:
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index fff6cdb8d11a..55d225961dbf 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -7335,7 +7335,14 @@ struct bpf_wq {
 } __attribute__((aligned(8)));

 struct bpf_dynptr {
+       union {
        __u64 __opaque[2];
+       struct {
+               __bpf_md_ptr(void *, data);
+               __u32 size;
+               __u32 reserved;
+       };
+       };
 } __attribute__((aligned(8)));

Then bpf prog and user space can use the same key type.