On 2024/11/6 20:02, Andrii Nakryiko wrote:
On Fri, Nov 1, 2024 at 1:22 PM Juntong Deng <juntong.deng@xxxxxxxxxxx> wrote:
Yes, users can access file->f_op, but there seems to be no way for
users to get references to struct file_operations for the various file
types? For example, how does a user get a reference to socket_file_ops?
See [0]. Libbpf will find it for the BPF program from kallsyms.
[0] https://github.com/torvalds/linux/blob/master/tools/testing/selftests/bpf/progs/test_ksyms.c#L13-L18
Thanks for telling me this method.
Yes, then we don't need bpf_get_file_ops_type().
Yes, I agree that it is feasible.
But there is a question here, should we expose the internal state
structure of the iterator (If we want to embed) ?
I guess that we need two versions of data structures struct bpf_iter_xxx
and struct bpf_iter_xxx_kern is for the purpose of encapsulation?
Yes, that's what we do for iterator state structure, and you should do
that as well. bpf_iter_xxx one will be opaque (see other examples, we
literally add `u64 __opaque[N];` there).
But this bpf_iter_task_file_item will be sort of internal API that is
returned from bpf_iter_task_file_next(). So you'll have something like
struct bpf_iter_task_file {
.... additional state ...
struct bpf_iter_task_file_item item;
};
then you have
struct bpf_iter_task_file_item bpf_iter_task_file_next(struct
bpf_iter_task_file *it)
{
struct bpf_iter_task_file_kern *kit = (void *)it;
...
kit->item.task = <sometask>;
kit->item.file = <file>; /* and so on */
return &kit->item;
}
With two versions of the data structure, users can only manipulate
the iterator using the iterator kfuncs, avoiding users from directly
accessing the internal state.
After we decide to return struct bpf_iter_task_file_item, these members
will not be able to change and users can directly access/change the
internal state of the iterator.
Yes, you have to carefully set up bpf_iter_task_file_item, but you
could expand it in the future without breaking any old users, because
you only return it by pointer and with BPF CO-RE all the field shifts
will be correctly handled.
You are right, in the next patch series version I will use
struct bpf_iter_task_file_item.
Could you please also give some feedback on
"2. Where should we put CRIB related kfuncs?"
in the discussion section of the v3 patch series
cover letter [0]?
[0]:
https://lore.kernel.org/bpf/AM6PR03MB58488FD29EB0D0B89D52AABB99532@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/T/#t
Then I can fix all the things in the v4 patch series.
Many thanks.
