| Message ID | 20200724203830.81531-2-alexei.starovoitov@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: Populate bpffs with map and prog iterators | expand |
On Fri, Jul 24, 2020 at 1:39 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > From: Alexei Starovoitov <ast@kernel.org> > > Refactor the code a bit to extract bpf_link_get_by_id() helper. The commit log doesn't match the code: bpf_link_get_by_id() vs. bpf_link_by_id(). > It's similar to existing bpf_prog_by_id(). > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > Acked-by: Andrii Nakryiko <andriin@fb.com> Other than that, Acked-by: Song Liu <songliubraving@fb.com> > --- > include/linux/bpf.h | 1 + > kernel/bpf/syscall.c | 46 +++++++++++++++++++++++++++----------------- > 2 files changed, 29 insertions(+), 18 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 8357be349133..d5c4e2cc24a0 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1355,6 +1355,7 @@ int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog, > struct btf *btf, const struct btf_type *t); > > struct bpf_prog *bpf_prog_by_id(u32 id); > +struct bpf_link *bpf_link_by_id(u32 id); > > const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id); > #else /* !CONFIG_BPF_SYSCALL */ > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index ee290b1f2d9e..42eb0d622980 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3984,40 +3984,50 @@ static int link_update(union bpf_attr *attr) > return ret; > } > > -static int bpf_link_inc_not_zero(struct bpf_link *link) > +static struct bpf_link *bpf_link_inc_not_zero(struct bpf_link *link) > { > - return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? 0 : -ENOENT; > + return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? link : ERR_PTR(-ENOENT); > } > > -#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id > - > -static int bpf_link_get_fd_by_id(const union bpf_attr *attr) > +struct bpf_link *bpf_link_by_id(u32 id) > { > struct bpf_link *link; > - u32 id = attr->link_id; > - int fd, err; > > - if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID)) > - return -EINVAL; > - > - if (!capable(CAP_SYS_ADMIN)) > - return -EPERM; > + if (!id) > + return ERR_PTR(-ENOENT); > > spin_lock_bh(&link_idr_lock); > - link = idr_find(&link_idr, id); > /* before link is "settled", ID is 0, pretend it doesn't exist yet */ > + link = idr_find(&link_idr, id); > if (link) { > if (link->id) > - err = bpf_link_inc_not_zero(link); > + link = bpf_link_inc_not_zero(link); > else > - err = -EAGAIN; > + link = ERR_PTR(-EAGAIN); > } else { > - err = -ENOENT; > + link = ERR_PTR(-ENOENT); > } > spin_unlock_bh(&link_idr_lock); > + return link; > +} > > - if (err) > - return err; > +#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id > + > +static int bpf_link_get_fd_by_id(const union bpf_attr *attr) > +{ > + struct bpf_link *link; > + u32 id = attr->link_id; > + int fd; > + > + if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID)) > + return -EINVAL; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + link = bpf_link_by_id(id); > + if (IS_ERR(link)) > + return PTR_ERR(link); > > fd = bpf_link_new_fd(link); > if (fd < 0) > -- > 2.23.0 >
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 8357be349133..d5c4e2cc24a0 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1355,6 +1355,7 @@ int btf_check_type_match(struct bpf_verifier_env *env, struct bpf_prog *prog, struct btf *btf, const struct btf_type *t); struct bpf_prog *bpf_prog_by_id(u32 id); +struct bpf_link *bpf_link_by_id(u32 id); const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id); #else /* !CONFIG_BPF_SYSCALL */ diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index ee290b1f2d9e..42eb0d622980 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3984,40 +3984,50 @@ static int link_update(union bpf_attr *attr) return ret; } -static int bpf_link_inc_not_zero(struct bpf_link *link) +static struct bpf_link *bpf_link_inc_not_zero(struct bpf_link *link) { - return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? 0 : -ENOENT; + return atomic64_fetch_add_unless(&link->refcnt, 1, 0) ? link : ERR_PTR(-ENOENT); } -#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id - -static int bpf_link_get_fd_by_id(const union bpf_attr *attr) +struct bpf_link *bpf_link_by_id(u32 id) { struct bpf_link *link; - u32 id = attr->link_id; - int fd, err; - if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID)) - return -EINVAL; - - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; + if (!id) + return ERR_PTR(-ENOENT); spin_lock_bh(&link_idr_lock); - link = idr_find(&link_idr, id); /* before link is "settled", ID is 0, pretend it doesn't exist yet */ + link = idr_find(&link_idr, id); if (link) { if (link->id) - err = bpf_link_inc_not_zero(link); + link = bpf_link_inc_not_zero(link); else - err = -EAGAIN; + link = ERR_PTR(-EAGAIN); } else { - err = -ENOENT; + link = ERR_PTR(-ENOENT); } spin_unlock_bh(&link_idr_lock); + return link; +} - if (err) - return err; +#define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id + +static int bpf_link_get_fd_by_id(const union bpf_attr *attr) +{ + struct bpf_link *link; + u32 id = attr->link_id; + int fd; + + if (CHECK_ATTR(BPF_LINK_GET_FD_BY_ID)) + return -EINVAL; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + link = bpf_link_by_id(id); + if (IS_ERR(link)) + return PTR_ERR(link); fd = bpf_link_new_fd(link); if (fd < 0)