[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEf4Bzb_V4p7XvE6e2CUfbKhfjh96i-_mCYa-osWiSGwWObmrQ@mail.gmail.com>
Date: Fri, 23 Jan 2026 10:54:37 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Leon Hwang <leon.hwang@...ux.dev>
Cc: bpf@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, John Fastabend <john.fastabend@...il.com>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
Shuah Khan <shuah@...nel.org>, Christian Brauner <brauner@...nel.org>,
Seth Forshee <sforshee@...nel.org>, Yuichiro Tsuji <yuichtsu@...zon.com>,
Andrey Albershteyn <aalbersh@...hat.com>, Willem de Bruijn <willemb@...gle.com>,
Jason Xing <kerneljasonxing@...il.com>, Tao Chen <chen.dylane@...ux.dev>,
Mykyta Yatsenko <yatsenko@...a.com>, Kumar Kartikeya Dwivedi <memxor@...il.com>,
Anton Protopopov <a.s.protopopov@...il.com>, Amery Hung <ameryhung@...il.com>,
Rong Tao <rongtao@...tc.cn>, linux-kernel@...r.kernel.org, linux-api@...r.kernel.org,
linux-kselftest@...r.kernel.org, kernel-patches-bot@...com
Subject: Re: [RESEND PATCH bpf-next v6 2/9] libbpf: Add support for extended
bpf syscall
On Thu, Jan 22, 2026 at 5:41 PM Leon Hwang <leon.hwang@...ux.dev> wrote:
>
>
>
> On 23/1/26 08:53, Andrii Nakryiko wrote:
> > On Tue, Jan 20, 2026 at 7:26 AM Leon Hwang <leon.hwang@...ux.dev> wrote:
> >>
> >> To support the extended BPF syscall introduced in the previous commit,
> >> introduce the following internal APIs:
> >>
> >> * 'sys_bpf_ext()'
> >> * 'sys_bpf_ext_fd()'
> >> They wrap the raw 'syscall()' interface to support passing extended
> >> attributes.
> >> * 'probe_sys_bpf_ext()'
> >> Check whether current kernel supports the BPF syscall common attributes.
> >>
> >> Signed-off-by: Leon Hwang <leon.hwang@...ux.dev>
> >> ---
> >> tools/lib/bpf/bpf.c | 32 ++++++++++++++++++++++++++++++++
> >> tools/lib/bpf/features.c | 8 ++++++++
> >> tools/lib/bpf/libbpf_internal.h | 3 +++
> >> 3 files changed, 43 insertions(+)
> >>
> >> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> >> index 21b57a629916..ed9c6eaeb656 100644
> >> --- a/tools/lib/bpf/bpf.c
> >> +++ b/tools/lib/bpf/bpf.c
> >> @@ -69,6 +69,38 @@ static inline __u64 ptr_to_u64(const void *ptr)
> >> return (__u64) (unsigned long) ptr;
> >> }
> >>
> >> +static inline int sys_bpf_ext(enum bpf_cmd cmd, union bpf_attr *attr,
> >> + unsigned int size,
> >> + struct bpf_common_attr *attr_common,
> >> + unsigned int size_common)
> >> +{
> >> + cmd = attr_common ? (cmd | BPF_COMMON_ATTRS) : (cmd & ~BPF_COMMON_ATTRS);
> >> + return syscall(__NR_bpf, cmd, attr, size, attr_common, size_common);
> >> +}
> >> +
> >> +static inline int sys_bpf_ext_fd(enum bpf_cmd cmd, union bpf_attr *attr,
> >> + unsigned int size,
> >> + struct bpf_common_attr *attr_common,
> >> + unsigned int size_common)
> >> +{
> >> + int fd;
> >> +
> >> + fd = sys_bpf_ext(cmd, attr, size, attr_common, size_common);
> >> + return ensure_good_fd(fd);
> >> +}
> >> +
> >> +int probe_sys_bpf_ext(void)
> >> +{
> >> + const size_t attr_sz = offsetofend(union bpf_attr, prog_token_fd);
> >> + union bpf_attr attr;
> >> +
> >> + memset(&attr, 0, attr_sz);
> >> + /* This syscall() will return error always. */
> >
> > I'll cite myself from the last review:
> >
> >> But fd should really not be >= 0, and if it is -- it's some problem,
> >> so I'd return an error in that case to keep us aware, which is why I'm
> >> saying I'd just return inside if (fd >= 0) { }
> >
> > I didn't say let's just ignore syscall return with (void) cast and
> > happily check errno no matter what, did I? Drop the comment, and
> > handle fd >= 0 case explicitly, please.
> >
>
> My mistake — sorry for the misunderstanding.
>
> You’re right; the return value should not be ignored. In the next
> revision, I’ll handle the fd >= 0 case explicitly and drop the comment.
> The logic will be updated along the lines of:
>
> fd = syscall(__NR_bpf, BPF_PROG_LOAD | BPF_COMMON_ATTRS,
> &attr, attr_sz, NULL, sizeof(struct bpf_common_attr));
> if (fd >= 0) {
> close(fd);
> return 0;
> }
> return errno == EFAULT;
>
well no, it should be
fd = syscall(...);
if (fd >= 0) {
close(fd);
return -EINVAL;
}
return errno == EFAULT ? 1 : 0;
> Thanks,
> Leon
>
>
Powered by blists - more mailing lists