[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYojfFiMn6VeUkxUsdSTdFK0A4MzKQxhCCp_OowkseznQ@mail.gmail.com>
Date: Tue, 4 Aug 2020 18:38:28 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Song Liu <songliubraving@...com>
Cc: open list <linux-kernel@...r.kernel.org>,
bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Kernel Team <Kernel-team@...com>,
john fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...omium.org>,
Jesper Dangaard Brouer <brouer@...hat.com>,
Daniel Xu <dlxu@...com>
Subject: Re: [PATCH bpf-next 2/5] libbpf: support BPF_PROG_TYPE_USER programs
On Mon, Aug 3, 2020 at 6:18 PM Song Liu <songliubraving@...com> wrote:
>
>
>
> > On Aug 2, 2020, at 6:40 PM, Andrii Nakryiko <andrii.nakryiko@...il.com> wrote:
> >
> > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@...com> wrote:
> >>
>
> [...]
>
> >
> >> };
> >>
> >> LIBBPF_API int bpf_prog_test_run_xattr(struct bpf_prog_test_run_attr *test_attr);
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index b9f11f854985b..9ce175a486214 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -6922,6 +6922,7 @@ static const struct bpf_sec_def section_defs[] = {
> >> BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT),
> >> BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT),
> >> BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL),
> >> + BPF_PROG_SEC("user", BPF_PROG_TYPE_USER),
> >
> > let's do "user/" for consistency with most other prog types (and nice
> > separation between prog type and custom user name)
>
> About "user" vs. "user/", I still think "user" is better.
>
> Unlike kprobe and tracepoint, user prog doesn't use the part after "/".
> This is similar to "perf_event" for BPF_PROG_TYPE_PERF_EVENT, "xdl" for
> BPF_PROG_TYPE_XDP, etc. If we specify "user" here, "user/" and "user/xxx"
> would also work. However, if we specify "user/" here, programs that used
> "user" by accident will fail to load, with a message like:
>
> libbpf: failed to load program 'user'
>
> which is confusing.
xdp, perf_event and a bunch of others don't enforce it, that's true,
they are a bit of a legacy, unfortunately. But all the recent ones do,
and we explicitly did that for xdp_dev/xdp_cpu, for instance.
Specifying just "user" in the spec would allow something nonsensical
like "userargh", for instance, due to this being treated as a prefix.
There is no harm to require users to do "user/my_prog", though.
Alternatively, we could introduce a new convention in the spec,
something like "user?", which would accept either "user" or
"user/something", but not "user/" nor "userblah". We can try that as
well.
>
> Thanks,
> Song
>
> [...]
>
Powered by blists - more mailing lists