On 2/15/23 10:44, Stanislav Fomichev wrote:
On 02/15, Kui-Feng Lee wrote:
Thank you for the feedback.
On 2/14/23 18:39, Stanislav Fomichev wrote:
> On 02/14, Kui-Feng Lee wrote:
[..]
>
> > +��� if (map->map_type != BPF_MAP_TYPE_STRUCT_OPS)
> > +������� return -EINVAL;
> > +
> > +��� link = kzalloc(sizeof(*link), GFP_USER);
> > +��� if (!link) {
> > +������� err = -ENOMEM;
> > +������� goto err_out;
> > +��� }
> > +��� bpf_link_init(link, BPF_LINK_TYPE_STRUCT_OPS,
> > &bpf_struct_ops_map_lops, NULL);
> > +��� link->map = map;
> > +
> > +��� err = bpf_link_prime(link, &link_primer);
> > +��� if (err)
> > +������� goto err_out;
> > +
> > +��� return bpf_link_settle(&link_primer);
> > +
> > +err_out:
> > +��� bpf_map_put(map);
> > +��� kfree(link);
> > +��� return err;
> > +}
> > +
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index cda8d00f3762..54e172d8f5d1 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -2738,7 +2738,9 @@ static void bpf_link_free(struct bpf_link
*link)
> > ����� if (link->prog) {
> > ��������� /* detach BPF program, clean up used resources */
> > ��������� link->ops->release(link);
> > -������� bpf_prog_put(link->prog);
> > +������� if (link->type != BPF_LINK_TYPE_STRUCT_OPS)
> > +����������� bpf_prog_put(link->prog);
> > +������� /* The struct_ops links clean up map by them-selves. */
>
> Why not more generic:
>
> if (link->prog)
> ����bpf_prog_put(link->prog);
>
> ?
The `prog` and `map` functions are now occupying the same space. I'm
afraid
this check won't work.
Hmm, good point. In this case: why not have separate prog/map pointers
instead of a union? Are we 100% sure struct_ops is unique enough
and there won't ever be another map-based links?
Good question!
bpf_link is used to attach a single BPF program with a hook now. This
patch takes things one step further, allowing the attachment of
struct_ops. We can think of it as attaching a set of BPF programs to a
pre-existing set of hooks. I would say that bpf_link now represents the
attachments of a set of BPF programs to a pre-defined set of hooks. The
size of a set is one for the case of attaching single BPF program.
Is creating a new map-based link indicative of introducing an entirely
distinct type of map that reflects a set of BPF programs? If so, why
doesn't struct_ops work? If it happens, we may need to create a function
to validate the attach_type associated with any given map type.
