Re: [RFC bpf-next v3 4/5] bpf: Add a new dynptr type for CGRUP_SOCKOPT.

Kui-Feng Lee
2023-08-17T19:00:10Z

On 8/16/23 18:25, Alexei Starovoitov wrote:
> On Tue, Aug 15, 2023 at 10:47:11AM -0700, thinker.li@gmail.com wrote:
>>
>> +BTF_SET8_START(cgroup_common_btf_ids)
>> +BTF_ID_FLAGS(func, bpf_sockopt_dynptr_copy_to, KF_SLEEPABLE)
>> +BTF_ID_FLAGS(func, bpf_sockopt_dynptr_alloc, KF_SLEEPABLE)
>> +BTF_ID_FLAGS(func, bpf_sockopt_dynptr_install, KF_SLEEPABLE)
>> +BTF_ID_FLAGS(func, bpf_sockopt_dynptr_release, KF_SLEEPABLE)
>> +BTF_ID_FLAGS(func, bpf_sockopt_dynptr_from, KF_SLEEPABLE)
>> +BTF_SET8_END(cgroup_common_btf_ids)
>
> These shouldn't be sockopt specific.
> If we want dynptr to represent a pointer to a user contiguous user memory
> we should use generic kfunc that do so.
>
> I suspect a single new kfunc: bpf_dynptr_from_user_mem() would do.
> New dynptr type can be hidden in the kernel and all existing
> kfuncs dynptr_slice, dynptr_data, dynptr_write could be made to work
> with user memory.
>
> But I think we have to step back. Why do we need this whole thing in the first place?
> _why_ sockopt bpf progs needs to read and write user memory?
>
> Yes there is one page limit, but what is the use case to actually read and write
> beyond that? iptables sockopt was mentioned, but I don't think bpf prog can do
> anything useful with iptables binary blobs. They are hard enough for kernel to parse.

The ideal behind the design is let the developers of filters to decide
when to replace the existing buffer. And, access the content of
buffers just like accessing raw pointers. However, seems almost everyone
love to use *_read() & *_write(). I will move to that direction.