Re: [PATCH bpf-next v2 1/2] bpf: add bpf_dynptr_memset() kfunc

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 6/30/25 2:05 PM, Ihor Solodrai wrote:
> On 6/25/25 4:27 AM, Mykyta Yatsenko wrote:
>> On 6/24/25 21:52, Ihor Solodrai wrote:
>>> Currently there is no straightforward way to fill dynptr memory with a
>>> value (most commonly zero). One can do it with bpf_dynptr_write(), but
>>> a temporary buffer is necessary for that.
>>>
>>> Implement bpf_dynptr_memset() - an analogue of memset() from libc.
>>>
>>> Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
>>> ---
>>>   kernel/bpf/helpers.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 48 insertions(+)
>>>
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index b71e428ad936..b8a7dbc971b4 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -2906,6 +2906,53 @@ __bpf_kfunc int bpf_dynptr_copy(struct 
>>> bpf_dynptr *dst_ptr, u32 dst_off,
>>>       return 0;
>>>   }
>>> +/**
>>> + * bpf_dynptr_memset() - Fill dynptr memory with a constant byte.
>>> + * @ptr: Destination dynptr - where data will be filled
>>> + * @ptr_off: Offset into the dynptr to start filling from
>>> + * @size: Number of bytes to fill
>>> + * @val: Constant byte to fill the memory with
>>> + *
>>> + * Fills the size bytes of the memory area pointed to by ptr
>>> + * at offset ptr_off with the constant byte val.
>>> + * Returns 0 on success; negative error, otherwise.
>>> + */
>>> + __bpf_kfunc int bpf_dynptr_memset(struct bpf_dynptr *ptr, u32 
>>> ptr_off, u32 size, u8 val)
>>> + {
>>> +    struct bpf_dynptr_kern *p = (struct bpf_dynptr_kern *)ptr;
>>> +    char buf[256];
>>> +    u32 chunk_sz;
>>> +    void* slice;
>>> +    u32 offset;
>>> +    int err;
>>> +
>>> +    if (__bpf_dynptr_is_rdonly(p))
>>> +        return -EINVAL;
>>> +
>>> +    err = bpf_dynptr_check_off_len(p, ptr_off, size);
>>> +    if (err)
>>> +        return err;
>>> +
>>> +    slice = bpf_dynptr_slice_rdwr(ptr, ptr_off, NULL, size);
>>> +    if (likely(slice)) {
>>> +        memset(slice, val, size);
>>> +        return 0;
>>> +    }
>>
>> bpf_dynptr_slice_rdwr is doing rdonly and off_len checks anyways, so 
>> perhaps we can
>> avoid calling __bpf_dynptr_is_rdonly and bpf_dynptr_check_off_len 
>> before bpf_dynptr_slice_rdwr,
>> that'll make fast path a little faster.
> 
> Yes, there are quite a bit of repetitive checks.
> Same in bpf_dynptr_write() in the slow path.
> 
> In case of bpf_dynptr_slice_rdwr() the issue is that it returns a
> pointer, and if we only check for null pointer we lose the information
> about the error cause (is it readonly or size?). So I think it's
> better to leave explicit checks.

I just realized that I might have misunderstood your comment.
Did you mean to put the checks after bpf_dynptr_slice_rdwr()
call instead of before?

> 
>>
>>> +
>>> +    /* Non-linear data under the dynptr, write from a local buffer */
>>> +    chunk_sz = min_t(u32, sizeof(buf), size);
>>> +    memset(buf, val, chunk_sz);
>>> +
>>> +    for (offset = ptr_off; offset < ptr_off + size; offset += 
>>> chunk_sz) {
>>> +        chunk_sz = min_t(u32, sizeof(buf), size - offset);
>>> +        err = __bpf_dynptr_write(p, offset, buf, chunk_sz, 0);
>>> +        if (err)
>>> +            return err;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
>>>   {
>>>       return obj;
>>> @@ -3364,6 +3411,7 @@ 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)
>>> +BTF_ID_FLAGS(func, bpf_dynptr_memset)
>>>   #ifdef CONFIG_NET
>>>   BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
>>>   #endif
>>
>