Re: [PATCH bpf-next 2/3] bpf/helpers: introduce bpf_dynptr_copy kfunc

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-02-18 at 19:00 +0000, Mykyta Yatsenko wrote:
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Introducing bpf_dynptr_copy kfunc allowing copying data from one dynptr to
> another. This functionality is useful in scenarios such as capturing XDP
> data to a ring buffer.
> The implementation consists of 4 branches:
>   * A fast branch for contiguous buffer capacity in both source and
> destination dynptrs
>   * 3 branches utilizing __bpf_dynptr_read and __bpf_dynptr_write to copy
> data to/from non-contiguous buffer
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

>  kernel/bpf/helpers.c  | 37 +++++++++++++++++++++++++++++++++++++
>  kernel/bpf/verifier.c |  3 +++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 2833558c3009..ac5fbdfc504d 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2770,6 +2770,42 @@ __bpf_kfunc int bpf_dynptr_clone(const struct bpf_dynptr *p,
>  	return 0;
>  }

Nit: it would be nice to have a docstring here, as for other dynptr functions.

> +__bpf_kfunc int bpf_dynptr_copy(struct bpf_dynptr *dst_ptr, u32 dst_off,
> +				struct bpf_dynptr *src_ptr, u32 src_off, u32 size)

[...]

> @@ -3174,6 +3210,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_null)
>  BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
>  BTF_ID_FLAGS(func, bpf_dynptr_size)
>  BTF_ID_FLAGS(func, bpf_dynptr_clone)
> +BTF_ID_FLAGS(func, bpf_dynptr_copy)
>  #ifdef CONFIG_NET
>  BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
>  #endif
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e7bc74171c99..3c567bfcc582 100644

Nit: There is no need to add bpf_dynptr_copy to special kfuncs list.
     This list is maintained to get BTF ids of several kfuncs w/o lookup,
     like so: 'special_kfunc_list[KF_bpf_dynptr_slice]'.
     Which is useful when writing custom semantic rules for such functions.
     There are no special rules for bpf_dynptr_copy, hence entry in the
     special_kfunc_list is not needed.

> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11781,6 +11781,7 @@ enum special_kfunc_type {
>  	KF_bpf_dynptr_slice,
>  	KF_bpf_dynptr_slice_rdwr,
>  	KF_bpf_dynptr_clone,
> +	KF_bpf_dynptr_copy,
>  	KF_bpf_percpu_obj_new_impl,
>  	KF_bpf_percpu_obj_drop_impl,
>  	KF_bpf_throw,
> @@ -11819,6 +11820,7 @@ BTF_ID(func, bpf_dynptr_from_xdp)
>  BTF_ID(func, bpf_dynptr_slice)
>  BTF_ID(func, bpf_dynptr_slice_rdwr)
>  BTF_ID(func, bpf_dynptr_clone)
> +BTF_ID(func, bpf_dynptr_copy)
>  BTF_ID(func, bpf_percpu_obj_new_impl)
>  BTF_ID(func, bpf_percpu_obj_drop_impl)
>  BTF_ID(func, bpf_throw)
> @@ -11857,6 +11859,7 @@ BTF_ID_UNUSED
>  BTF_ID(func, bpf_dynptr_slice)
>  BTF_ID(func, bpf_dynptr_slice_rdwr)
>  BTF_ID(func, bpf_dynptr_clone)
> +BTF_ID(func, bpf_dynptr_copy)
>  BTF_ID(func, bpf_percpu_obj_new_impl)
>  BTF_ID(func, bpf_percpu_obj_drop_impl)
>  BTF_ID(func, bpf_throw)