| Message ID | 20171130134302.2840-2-guro@fb.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpftool: cgroup bpf operations | expand |
Thanks Roman! One comment in-line. 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com> > 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. > > Signed-off-by: Roman Gushchin <guro@fb.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Jakub Kicinski <jakub.kicinski@netronome.com> > --- > tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 47 insertions(+) > > 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_TRACEPOINT); > BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); > BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > > +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) == 0) > + return BPF_PROG_TYPE_SOCKET_FILTER; > + if (strncmp(prog->section_name, "kprobe/", 7) == 0) > + return BPF_PROG_TYPE_KPROBE; > + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) > + return BPF_PROG_TYPE_KPROBE; > + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) > + return BPF_PROG_TYPE_TRACEPOINT; > + if (strncmp(prog->section_name, "xdp", 3) == 0) > + return BPF_PROG_TYPE_XDP; > + if (strncmp(prog->section_name, "perf_event", 10) == 0) > + return BPF_PROG_TYPE_PERF_EVENT; > + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) > + return BPF_PROG_TYPE_CGROUP_SKB; > + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) > + return BPF_PROG_TYPE_CGROUP_SOCK; > + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) > + return BPF_PROG_TYPE_CGROUP_DEVICE; > + if (strncmp(prog->section_name, "sockops", 7) == 0) > + return BPF_PROG_TYPE_SOCK_OPS; > + if (strncmp(prog->section_name, "sk_skb", 6) == 0) > + return BPF_PROG_TYPE_SK_SKB; 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: #define SOCKET_FILTER_SEC_PREFIX "socket" #define KPROBE_SEC_PREFIX "kprobe/" […] #define TRY_TYPE(string, __TYPE) \ do { \ if (!strncmp(string, __TYPE ## _SEC_PREFIX, \ sizeof(__TYPE ## _SEC_PREFIX))) \ return BPF_PROG_TYPE_ ## __TYPE; \ } while(0); static enum bpf_prog_type bpf_program__guess_type(struct bpf_program *prog) { if (!prog->section_name) goto err; TRY_TYPE(prog->section_name, SOCKET_FILTER); TRY_TYPE(prog->section_name, KPROBE); […] err: pr_warning("…", prog->section_name); return BPF_PROG_TYPE_UNSPEC; } > + > +err: > + pr_warning("failed to guess program type based on section name %s\n", > + prog->section_name); > + > + return BPF_PROG_TYPE_UNSPEC; > +} > + > int bpf_map__fd(struct bpf_map *map) > { > return map ? map->fd : -EINVAL;
On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote: > Thanks Roman! > One comment in-line. > > 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com> > > 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. > > > > Signed-off-by: Roman Gushchin <guro@fb.com> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > Cc: Jakub Kicinski <jakub.kicinski@netronome.com> > > --- > > tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 47 insertions(+) > > > > 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_TRACEPOINT); > > BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); > > BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > > > > +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) == 0) > > + return BPF_PROG_TYPE_SOCKET_FILTER; > > + if (strncmp(prog->section_name, "kprobe/", 7) == 0) > > + return BPF_PROG_TYPE_KPROBE; > > + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) > > + return BPF_PROG_TYPE_KPROBE; > > + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) > > + return BPF_PROG_TYPE_TRACEPOINT; > > + if (strncmp(prog->section_name, "xdp", 3) == 0) > > + return BPF_PROG_TYPE_XDP; > > + if (strncmp(prog->section_name, "perf_event", 10) == 0) > > + return BPF_PROG_TYPE_PERF_EVENT; > > + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) > > + return BPF_PROG_TYPE_CGROUP_SKB; > > + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) > > + return BPF_PROG_TYPE_CGROUP_SOCK; > > + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) > > + return BPF_PROG_TYPE_CGROUP_DEVICE; > > + if (strncmp(prog->section_name, "sockops", 7) == 0) > > + return BPF_PROG_TYPE_SOCK_OPS; > > + if (strncmp(prog->section_name, "sk_skb", 6) == 0) > > + return BPF_PROG_TYPE_SK_SKB; > > 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: > > #define SOCKET_FILTER_SEC_PREFIX "socket" > #define KPROBE_SEC_PREFIX "kprobe/" > […] > > #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 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 *prog) > { > if (!prog->section_name) > goto err; > > TRY_TYPE(prog->section_name, SOCKET_FILTER); > TRY_TYPE(prog->section_name, KPROBE); > […] > > err: > pr_warning("…", > prog->section_name); > > return BPF_PROG_TYPE_UNSPEC; > }
On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote: > On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote: > > Thanks Roman! > > One comment in-line. > > > > 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com> > > > 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. > > > > > > Signed-off-by: Roman Gushchin <guro@fb.com> > > > Cc: Alexei Starovoitov <ast@kernel.org> > > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > > Cc: Jakub Kicinski <jakub.kicinski@netronome.com> > > > --- > > > tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 47 insertions(+) > > > > > > 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_TRACEPOINT); > > > BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); > > > BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > > > > > > +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) == 0) > > > + return BPF_PROG_TYPE_SOCKET_FILTER; > > > + if (strncmp(prog->section_name, "kprobe/", 7) == 0) > > > + return BPF_PROG_TYPE_KPROBE; > > > + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) > > > + return BPF_PROG_TYPE_KPROBE; > > > + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) > > > + return BPF_PROG_TYPE_TRACEPOINT; > > > + if (strncmp(prog->section_name, "xdp", 3) == 0) > > > + return BPF_PROG_TYPE_XDP; > > > + if (strncmp(prog->section_name, "perf_event", 10) == 0) > > > + return BPF_PROG_TYPE_PERF_EVENT; > > > + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) > > > + return BPF_PROG_TYPE_CGROUP_SKB; > > > + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) > > > + return BPF_PROG_TYPE_CGROUP_SOCK; > > > + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) > > > + return BPF_PROG_TYPE_CGROUP_DEVICE; > > > + if (strncmp(prog->section_name, "sockops", 7) == 0) > > > + return BPF_PROG_TYPE_SOCK_OPS; > > > + if (strncmp(prog->section_name, "sk_skb", 6) == 0) > > > + return BPF_PROG_TYPE_SK_SKB; > > > > 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: > > > > #define SOCKET_FILTER_SEC_PREFIX "socket" > > #define KPROBE_SEC_PREFIX "kprobe/" > > […] > > > > #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 > a macro that wraps the strncmp() with sizeof would be enough? Without > the return inside? Hm, I'm not sure that using macroses here makes the code much easier to read. Maybe we can use just strcmp() instead? As we compare with hardcoded strings, there is no real difference. Something like this: if (!strcmp("socket", prog->section_name)) return BPF_PROG_TYPE_SOCKET_FILTER; if (!strcmp("kprobe/", prog->section_name)) return BPF_PROG_TYPE_KPROBE; if (!strcmp("kretprobe/", prog->section_name)) return BPF_PROG_TYPE_KPROBE; if (!strcmp("tracepoint/", prog->section_name)) return BPF_PROG_TYPE_TRACEPOINT; if (!strcmp("xdp", prog->section_name)) return BPF_PROG_TYPE_XDP; if (!strcmp("perf_event", prog->section_name)) return BPF_PROG_TYPE_PERF_EVENT; if (!strcmp("cgroup/skb", prog->section_name)) return BPF_PROG_TYPE_CGROUP_SKB; if (!strcmp("cgroup/sock", prog->section_name)) return BPF_PROG_TYPE_CGROUP_SOCK; if (!strcmp("cgroup/dev", prog->section_name)) return BPF_PROG_TYPE_CGROUP_DEVICE; if (!strcmp("sockops", prog->section_name)) return BPF_PROG_TYPE_SOCK_OPS; if (!strcmp("sk_skb", prog->section_name)) return BPF_PROG_TYPE_SK_SKB; Thanks!
2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <guro@fb.com> > On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote: >> On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote: >>> Thanks Roman! >>> One comment in-line. >>> >>> 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com> >>>> 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. >>>> >>>> Signed-off-by: Roman Gushchin <guro@fb.com> >>>> Cc: Alexei Starovoitov <ast@kernel.org> >>>> Cc: Daniel Borkmann <daniel@iogearbox.net> >>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com> >>>> --- >>>> tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 47 insertions(+) >>>> >>>> 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_TRACEPOINT); >>>> BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); >>>> BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); >>>> >>>> +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) == 0) >>>> + return BPF_PROG_TYPE_SOCKET_FILTER; >>>> + if (strncmp(prog->section_name, "kprobe/", 7) == 0) >>>> + return BPF_PROG_TYPE_KPROBE; >>>> + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) >>>> + return BPF_PROG_TYPE_KPROBE; >>>> + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) >>>> + return BPF_PROG_TYPE_TRACEPOINT; >>>> + if (strncmp(prog->section_name, "xdp", 3) == 0) >>>> + return BPF_PROG_TYPE_XDP; >>>> + if (strncmp(prog->section_name, "perf_event", 10) == 0) >>>> + return BPF_PROG_TYPE_PERF_EVENT; >>>> + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) >>>> + return BPF_PROG_TYPE_CGROUP_SKB; >>>> + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) >>>> + return BPF_PROG_TYPE_CGROUP_SOCK; >>>> + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) >>>> + return BPF_PROG_TYPE_CGROUP_DEVICE; >>>> + if (strncmp(prog->section_name, "sockops", 7) == 0) >>>> + return BPF_PROG_TYPE_SOCK_OPS; >>>> + if (strncmp(prog->section_name, "sk_skb", 6) == 0) >>>> + return BPF_PROG_TYPE_SK_SKB; >>> >>> 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: >>> >>> #define SOCKET_FILTER_SEC_PREFIX "socket" >>> #define KPROBE_SEC_PREFIX "kprobe/" >>> […] >>> >>> #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 >> a macro that wraps the strncmp() with sizeof would be enough? Without >> the return inside? > > Hm, I'm not sure that using macroses here makes the code much easier to read. > Maybe we can use just strcmp() instead? > As we compare with hardcoded strings, there is no real difference. > Something like this: > > if (!strcmp("socket", prog->section_name)) > return BPF_PROG_TYPE_SOCKET_FILTER; > if (!strcmp("kprobe/", prog->section_name)) > return BPF_PROG_TYPE_KPROBE; > if (!strcmp("kretprobe/", prog->section_name)) > return BPF_PROG_TYPE_KPROBE; > if (!strcmp("tracepoint/", prog->section_name)) > return BPF_PROG_TYPE_TRACEPOINT; > if (!strcmp("xdp", prog->section_name)) > return BPF_PROG_TYPE_XDP; > if (!strcmp("perf_event", prog->section_name)) > return BPF_PROG_TYPE_PERF_EVENT; > if (!strcmp("cgroup/skb", prog->section_name)) > return BPF_PROG_TYPE_CGROUP_SKB; > if (!strcmp("cgroup/sock", prog->section_name)) > return BPF_PROG_TYPE_CGROUP_SOCK; > if (!strcmp("cgroup/dev", prog->section_name)) > return BPF_PROG_TYPE_CGROUP_DEVICE; > if (!strcmp("sockops", prog->section_name)) > return BPF_PROG_TYPE_SOCK_OPS; > if (!strcmp("sk_skb", prog->section_name)) > return BPF_PROG_TYPE_SK_SKB; > > Thanks! > I do not believe this works. For kprobes for example, the idea was to compare "kprobe/" with only the beginning of prog->section_name, right? The string prog->section_name is supposed to be longer than this (e.g. "kprobe/sys_write"), and strcmp() will not detect a match in this case. Quentin
On Mon, Dec 04, 2017 at 01:12:33PM +0000, Quentin Monnet wrote: > 2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <guro@fb.com> > > On Fri, Dec 01, 2017 at 02:46:06PM -0800, Jakub Kicinski wrote: > >> On Fri, 1 Dec 2017 10:22:57 +0000, Quentin Monnet wrote: > >>> Thanks Roman! > >>> One comment in-line. > >>> > >>> 2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <guro@fb.com> > >>>> 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. > >>>> > >>>> Signed-off-by: Roman Gushchin <guro@fb.com> > >>>> Cc: Alexei Starovoitov <ast@kernel.org> > >>>> Cc: Daniel Borkmann <daniel@iogearbox.net> > >>>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com> > >>>> --- > >>>> tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > >>>> 1 file changed, 47 insertions(+) > >>>> > >>>> 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_TRACEPOINT); > >>>> BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); > >>>> BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > >>>> > >>>> +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) == 0) > >>>> + return BPF_PROG_TYPE_SOCKET_FILTER; > >>>> + if (strncmp(prog->section_name, "kprobe/", 7) == 0) > >>>> + return BPF_PROG_TYPE_KPROBE; > >>>> + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) > >>>> + return BPF_PROG_TYPE_KPROBE; > >>>> + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) > >>>> + return BPF_PROG_TYPE_TRACEPOINT; > >>>> + if (strncmp(prog->section_name, "xdp", 3) == 0) > >>>> + return BPF_PROG_TYPE_XDP; > >>>> + if (strncmp(prog->section_name, "perf_event", 10) == 0) > >>>> + return BPF_PROG_TYPE_PERF_EVENT; > >>>> + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) > >>>> + return BPF_PROG_TYPE_CGROUP_SKB; > >>>> + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) > >>>> + return BPF_PROG_TYPE_CGROUP_SOCK; > >>>> + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) > >>>> + return BPF_PROG_TYPE_CGROUP_DEVICE; > >>>> + if (strncmp(prog->section_name, "sockops", 7) == 0) > >>>> + return BPF_PROG_TYPE_SOCK_OPS; > >>>> + if (strncmp(prog->section_name, "sk_skb", 6) == 0) > >>>> + return BPF_PROG_TYPE_SK_SKB; > >>> > >>> 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: > >>> > >>> #define SOCKET_FILTER_SEC_PREFIX "socket" > >>> #define KPROBE_SEC_PREFIX "kprobe/" > >>> […] > >>> > >>> #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 > >> a macro that wraps the strncmp() with sizeof would be enough? Without > >> the return inside? > > > > Hm, I'm not sure that using macroses here makes the code much easier to read. > > Maybe we can use just strcmp() instead? > > As we compare with hardcoded strings, there is no real difference. > > Something like this: > > > > if (!strcmp("socket", prog->section_name)) > > return BPF_PROG_TYPE_SOCKET_FILTER; > > if (!strcmp("kprobe/", prog->section_name)) > > return BPF_PROG_TYPE_KPROBE; > > if (!strcmp("kretprobe/", prog->section_name)) > > return BPF_PROG_TYPE_KPROBE; > > if (!strcmp("tracepoint/", prog->section_name)) > > return BPF_PROG_TYPE_TRACEPOINT; > > if (!strcmp("xdp", prog->section_name)) > > return BPF_PROG_TYPE_XDP; > > if (!strcmp("perf_event", prog->section_name)) > > return BPF_PROG_TYPE_PERF_EVENT; > > if (!strcmp("cgroup/skb", prog->section_name)) > > return BPF_PROG_TYPE_CGROUP_SKB; > > if (!strcmp("cgroup/sock", prog->section_name)) > > return BPF_PROG_TYPE_CGROUP_SOCK; > > if (!strcmp("cgroup/dev", prog->section_name)) > > return BPF_PROG_TYPE_CGROUP_DEVICE; > > if (!strcmp("sockops", prog->section_name)) > > return BPF_PROG_TYPE_SOCK_OPS; > > if (!strcmp("sk_skb", prog->section_name)) > > return BPF_PROG_TYPE_SK_SKB; > > > > Thanks! > > > > I do not believe this works. For kprobes for example, the idea was to > compare "kprobe/" with only the beginning of prog->section_name, right? > The string prog->section_name is supposed to be longer than this (e.g. > "kprobe/sys_write"), and strcmp() will not detect a match in this case. Ah, true, I've missed this part. Ok, I'll try to master something with macroses. Thanks!
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_TRACEPOINT); BPF_PROG_TYPE_FNS(xdp, BPF_PROG_TYPE_XDP); BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); +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) == 0) + return BPF_PROG_TYPE_SOCKET_FILTER; + if (strncmp(prog->section_name, "kprobe/", 7) == 0) + return BPF_PROG_TYPE_KPROBE; + if (strncmp(prog->section_name, "kretprobe/", 10) == 0) + return BPF_PROG_TYPE_KPROBE; + if (strncmp(prog->section_name, "tracepoint/", 11) == 0) + return BPF_PROG_TYPE_TRACEPOINT; + if (strncmp(prog->section_name, "xdp", 3) == 0) + return BPF_PROG_TYPE_XDP; + if (strncmp(prog->section_name, "perf_event", 10) == 0) + return BPF_PROG_TYPE_PERF_EVENT; + if (strncmp(prog->section_name, "cgroup/skb", 10) == 0) + return BPF_PROG_TYPE_CGROUP_SKB; + if (strncmp(prog->section_name, "cgroup/sock", 11) == 0) + return BPF_PROG_TYPE_CGROUP_SOCK; + if (strncmp(prog->section_name, "cgroup/dev", 10) == 0) + return BPF_PROG_TYPE_CGROUP_DEVICE; + if (strncmp(prog->section_name, "sockops", 7) == 0) + return BPF_PROG_TYPE_SOCK_OPS; + if (strncmp(prog->section_name, "sk_skb", 6) == 0) + return BPF_PROG_TYPE_SK_SKB; + +err: + pr_warning("failed to guess program type based on section name %s\n", + prog->section_name); + + return BPF_PROG_TYPE_UNSPEC; +} + int bpf_map__fd(struct bpf_map *map) { return map ? map->fd : -EINVAL; @@ -1832,6 +1867,18 @@ int bpf_prog_load(const char *file, enum bpf_prog_type type, return -ENOENT; } + /* + * If type is not specified, try to guess it based on + * section name. + */ + if (type == BPF_PROG_TYPE_UNSPEC) { + type = bpf_program__guess_type(prog); + if (type == BPF_PROG_TYPE_UNSPEC) { + bpf_object__close(obj); + return -EINVAL; + } + } + bpf_program__set_type(prog, type); err = bpf_object__load(obj); if (err) {
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. Signed-off-by: Roman Gushchin <guro@fb.com> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Jakub Kicinski <jakub.kicinski@netronome.com> --- tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)