From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name Date: Fri, 1 Dec 2017 14:46:06 -0800 Message-ID: <20171201144606.5ddd0272@cakuba.netronome.com> References: <20171130134302.2840-1-guro@fb.com> <20171130134302.2840-2-guro@fb.com> <614dc433-9445-52df-a202-f7cb1cb79ca1@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Roman Gushchin , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@fb.com, ast@kernel.org, daniel@iogearbox.net, kafai@fb.com To: Quentin Monnet Return-path: In-Reply-To: <614dc433-9445-52df-a202-f7cb1cb79ca1@netronome.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote: > Thanks Roman! > One comment in-line. >=20 > 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin > > The bpf_prog_load() function will guess program type if it's not > > specified explicitly. This functionality will be used to implement > > loading of different programs without asking a user to specify > > the program type. In first order it will be used by bpftool. > >=20 > > Signed-off-by: Roman Gushchin > > Cc: Alexei Starovoitov > > Cc: Daniel Borkmann > > Cc: Jakub Kicinski > > --- > > tools/lib/bpf/libbpf.c | 47 ++++++++++++++++++++++++++++++++++++++++++= +++++ > > 1 file changed, 47 insertions(+) > >=20 > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > > index 5aa45f89da93..9f2410beaa18 100644 > > --- a/tools/lib/bpf/libbpf.c > > +++ b/tools/lib/bpf/libbpf.c > > @@ -1721,6 +1721,41 @@ BPF_PROG_TYPE_FNS(tracepoint, BPF_PROG_TYPE_TRAC= EPOINT); > > BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); > > BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > > =20 > > +static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *= prog) > > +{ > > + if (!prog->section_name) > > + goto err; > > + > > + if (strncmp(prog->section_name, "socket", 6) =3D=3D 0) > > + return BPF_PROG_TYPE_SOCKET_FILTER; > > + if (strncmp(prog->section_name, "kprobe/", 7) =3D=3D 0) > > + return BPF_PROG_TYPE_KPROBE; > > + if (strncmp(prog->section_name, "kretprobe/", 10) =3D=3D 0) > > + return BPF_PROG_TYPE_KPROBE; > > + if (strncmp(prog->section_name, "tracepoint/", 11) =3D=3D 0) > > + return BPF_PROG_TYPE_TRACEPOINT; > > + if (strncmp(prog->section_name, "xdp", 3) =3D=3D 0) > > + return BPF_PROG_TYPE_XDP; > > + if (strncmp(prog->section_name, "perf_event", 10) =3D=3D 0) > > + return BPF_PROG_TYPE_PERF_EVENT; > > + if (strncmp(prog->section_name, "cgroup/skb", 10) =3D=3D 0) > > + return BPF_PROG_TYPE_CGROUP_SKB; > > + if (strncmp(prog->section_name, "cgroup/sock", 11) =3D=3D 0) > > + return BPF_PROG_TYPE_CGROUP_SOCK; > > + if (strncmp(prog->section_name, "cgroup/dev", 10) =3D=3D 0) > > + return BPF_PROG_TYPE_CGROUP_DEVICE; > > + if (strncmp(prog->section_name, "sockops", 7) =3D=3D 0) > > + return BPF_PROG_TYPE_SOCK_OPS; > > + if (strncmp(prog->section_name, "sk_skb", 6) =3D=3D 0) > > + return BPF_PROG_TYPE_SK_SKB; =20 >=20 > I do not really like these hard-coded lengths, maybe we could work out > something nicer with a bit of pre-processing work? Perhaps something like: >=20 > #define SOCKET_FILTER_SEC_PREFIX "socket" > #define KPROBE_SEC_PREFIX "kprobe/" > [=E2=80=A6] >=20 > #define TRY_TYPE(string, __TYPE) \ > do { \ > if (!strncmp(string, __TYPE ## _SEC_PREFIX, \ > sizeof(__TYPE ## _SEC_PREFIX))) \ > return BPF_PROG_TYPE_ ## __TYPE; \ > } while(0); I like the suggestion, but I think return and goto statements hiding inside macros are slightly frowned upon in the netdev. Perhaps just=20 a macro that wraps the strncmp() with sizeof would be enough? Without the return inside? > static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *pro= g) > { > if (!prog->section_name) > goto err; >=20 > TRY_TYPE(prog->section_name, SOCKET_FILTER); > TRY_TYPE(prog->section_name, KPROBE); > [=E2=80=A6] >=20 > err: > pr_warning("=E2=80=A6", > prog->section_name); >=20 > return BPF_PROG_TYPE_UNSPEC; > }