2017-12-01 10:23:03

by Quentin Monnet

[permalink] [raw]
Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name

Thanks Roman!
One comment in-line.

2017-11-30 13:42 UTC+0000 ~ Roman Gushchin <[email protected]>
> 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 <[email protected]>
> Cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> ---
> 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;


2017-12-01 22:46:12

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name

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 <[email protected]>
> > 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 <[email protected]>
> > Cc: Alexei Starovoitov <[email protected]>
> > Cc: Daniel Borkmann <[email protected]>
> > Cc: Jakub Kicinski <[email protected]>
> > ---
> > 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;
> }

2017-12-04 12:35:16

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name

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 <[email protected]>
> > > 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 <[email protected]>
> > > Cc: Alexei Starovoitov <[email protected]>
> > > Cc: Daniel Borkmann <[email protected]>
> > > Cc: Jakub Kicinski <[email protected]>
> > > ---
> > > 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 13:12:43

by Quentin Monnet

[permalink] [raw]
Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name

2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <[email protected]>
> 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 <[email protected]>
>>>> 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 <[email protected]>
>>>> Cc: Alexei Starovoitov <[email protected]>
>>>> Cc: Daniel Borkmann <[email protected]>
>>>> Cc: Jakub Kicinski <[email protected]>
>>>> ---
>>>> 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

2017-12-04 13:22:49

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH net-next 1/5] libbpf: add ability to guess program type based on section name

On Mon, Dec 04, 2017 at 01:12:33PM +0000, Quentin Monnet wrote:
> 2017-12-04 12:34 UTC+0000 ~ Roman Gushchin <[email protected]>
> > 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 <[email protected]>
> >>>> 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 <[email protected]>
> >>>> Cc: Alexei Starovoitov <[email protected]>
> >>>> Cc: Daniel Borkmann <[email protected]>
> >>>> Cc: Jakub Kicinski <[email protected]>
> >>>> ---
> >>>> 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!