[PATCH v2 bpf-next 0/3] veristat: add better support of freplace programs

[PATCH v2 bpf-next 0/3] veristat: add better support of freplace programs

[Andrii Nakryiko]

Teach veristat how to deal with freplace BPF programs. As they can't be
directly loaded by veristat without custom user-space part that sets correct
target program FD, veristat always fails freplace programs. This patch set
teaches veristat to guess target program type that will be inherited by
freplace program itself, and subtitute it for BPF_PROG_TYPE_EXT (freplace) one
for the purposes of BPF verification.

Patch #1 fixes bug in libbpf preventing overriding freplace with specific
program type.

Patch #2 adds convenient -d flag to request veristat to emit libbpf debug
logs. It help debugging why a specific BPF program fails to load, if the
problem is not due to BPF verification itself.

v1->v2:
  - fix compilation error reported by old GCC (my GCC v11 doesn't produce even
    a warning) and Clang (see CI failure at [0]):

GCC version:

  veristat.c: In function ‘fixup_obj’:
  veristat.c:908:1: error: label at end of compound statement
    908 | skip_freplace_fixup:
        | ^~~~~~~~~~~~~~~~~~~

Clang version:

  veristat.c:909:1: error: label at end of compound statement is a C2x extension [-Werror,-Wc2x-extensions]
  }
  ^
  1 error generated.

  [0] https://github.com/kernel-patches/bpf/actions/runs/4515972059/jobs/7953845335

Andrii Nakryiko (3):
  libbpf: disassociate section handler on explicit
    bpf_program__set_type() call
  veristat: add -d debug mode option to see debug libbpf log
  veristat: guess and substitue underlying program type for freplace
    (EXT) progs

 tools/lib/bpf/libbpf.c                 |   1 +
 tools/testing/selftests/bpf/veristat.c | 126 ++++++++++++++++++++++++-
 2 files changed, 122 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH v2 bpf-next 1/3] libbpf: disassociate section handler on explicit bpf_program__set_type() call

[Andrii Nakryiko]

If user explicitly overrides programs's type with
bpf_program__set_type() API call, we need to disassociate whatever
SEC_DEF handler libbpf determined initially based on program's SEC()
definition, as it's not goind to be valid anymore and could lead to
crashes and/or confusing failures.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index f6a071db5c6e..a34ebb6b8508 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8465,6 +8465,7 @@ int bpf_program__set_type(struct bpf_program *prog, enum bpf_prog_type type)
 		return libbpf_err(-EBUSY);
 
 	prog->type = type;
+	prog->sec_def = NULL;
 	return 0;
 }
 
-- 
2.34.1

[PATCH v2 bpf-next 2/3] veristat: add -d debug mode option to see debug libbpf log

[Andrii Nakryiko]

Add -d option to allow requesting libbpf debug logs from veristat.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/veristat.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 83231456d3c5..263df32fbda8 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -135,6 +135,7 @@ static struct env {
 	char **filenames;
 	int filename_cnt;
 	bool verbose;
+	bool debug;
 	bool quiet;
 	int log_level;
 	enum resfmt out_fmt;
@@ -169,7 +170,7 @@ static int libbpf_print_fn(enum libbpf_print_level level, const char *format, va
 {
 	if (!env.verbose)
 		return 0;
-	if (level == LIBBPF_DEBUG /* && !env.verbose */)
+	if (level == LIBBPF_DEBUG  && !env.debug)
 		return 0;
 	return vfprintf(stderr, format, args);
 }
@@ -186,6 +187,7 @@ static const struct argp_option opts[] = {
 	{ NULL, 'h', NULL, OPTION_HIDDEN, "Show the full help" },
 	{ "verbose", 'v', NULL, 0, "Verbose mode" },
 	{ "log-level", 'l', "LEVEL", 0, "Verifier log level (default 0 for normal mode, 1 for verbose mode)" },
+	{ "debug", 'd', NULL, 0, "Debug mode (turns on libbpf debug logging)" },
 	{ "quiet", 'q', NULL, 0, "Quiet mode" },
 	{ "emit", 'e', "SPEC", 0, "Specify stats to be emitted" },
 	{ "sort", 's', "SPEC", 0, "Specify sort order" },
@@ -212,6 +214,10 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 	case 'v':
 		env.verbose = true;
 		break;
+	case 'd':
+		env.debug = true;
+		env.verbose = true;
+		break;
 	case 'q':
 		env.quiet = true;
 		break;
-- 
2.34.1

[PATCH v2 bpf-next 3/3] veristat: guess and substitue underlying program type for freplace (EXT) progs

[Andrii Nakryiko]

SEC("freplace") (i.e., BPF_PROG_TYPE_EXT) programs are not loadable as
is through veristat, as kernel expects actual program's FD during
BPF_PROG_LOAD time, which veristat has no way of knowing.

Unfortunately, freplace programs are a pretty important class of
programs, especially when dealing with XDP chaining solutions, which
rely on EXT programs.

So let's do our best and teach veristat to try to guess the original
program type, based on program's context argument type. And if guessing
process succeeds, we manually override freplace/EXT with guessed program
type using bpf_program__set_type() setter to increase chances of proper
BPF verification.

We rely on BTF and maintain a simple lookup table. This process is
obviously not 100% bulletproof, as valid program might not use context
and thus wouldn't have to specify correct type. Also, __sk_buff is very
ambiguous and is the context type across many different program types.
We pick BPF_PROG_TYPE_CGROUP_SKB for now, which seems to work fine in
practice so far. Similarly, some program types require specifying attach
type, and so we pick one out of possible few variants.

Best effort at its best. But this makes veristat even more widely
applicable.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/veristat.c | 118 ++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 263df32fbda8..a422fa986dc4 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -15,6 +15,7 @@
 #include <sys/sysinfo.h>
 #include <sys/stat.h>
 #include <bpf/libbpf.h>
+#include <bpf/btf.h>
 #include <libelf.h>
 #include <gelf.h>
 #include <float.h>
@@ -778,7 +779,59 @@ static int parse_verif_log(char * const buf, size_t buf_sz, struct verif_stats *
 	return 0;
 }
 
-static void fixup_obj(struct bpf_object *obj)
+static int guess_prog_type_by_ctx_name(const char *ctx_name,
+				       enum bpf_prog_type *prog_type,
+				       enum bpf_attach_type *attach_type)
+{
+	/* We need to guess program type based on its declared context type.
+	 * This guess can't be perfect as many different program types might
+	 * share the same context type.  So we can only hope to reasonably
+	 * well guess this and get lucky.
+	 *
+	 * Just in case, we support both UAPI-side type names and
+	 * kernel-internal names.
+	 */
+	static struct {
+		const char *uapi_name;
+		const char *kern_name;
+		enum bpf_prog_type prog_type;
+		enum bpf_attach_type attach_type;
+	} ctx_map[] = {
+		/* __sk_buff is most ambiguous, for now we assume cgroup_skb */
+		{ "__sk_buff", "sk_buff", BPF_PROG_TYPE_CGROUP_SKB, BPF_CGROUP_INET_INGRESS },
+		{ "bpf_sock", "sock", BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND },
+		{ "bpf_sock_addr", "bpf_sock_addr_kern",  BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND },
+		{ "bpf_sock_ops", "bpf_sock_ops_kern", BPF_PROG_TYPE_SOCK_OPS, BPF_CGROUP_SOCK_OPS },
+		{ "sk_msg_md", "sk_msg", BPF_PROG_TYPE_SK_MSG, BPF_SK_MSG_VERDICT },
+		{ "bpf_cgroup_dev_ctx", "bpf_cgroup_dev_ctx", BPF_PROG_TYPE_CGROUP_DEVICE, BPF_CGROUP_DEVICE },
+		{ "bpf_sysctl", "bpf_sysctl_kern", BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL },
+		{ "bpf_sockopt", "bpf_sockopt_kern", BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT },
+		{ "sk_reuseport_md", "sk_reuseport_kern", BPF_PROG_TYPE_SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE },
+		{ "bpf_sk_lookup", "bpf_sk_lookup_kern", BPF_PROG_TYPE_SK_LOOKUP, BPF_SK_LOOKUP },
+		{ "xdp_md", "xdp_buff", BPF_PROG_TYPE_XDP, BPF_XDP },
+		/* tracing types with no expected attach type */
+		{ "bpf_user_pt_regs_t", "pt_regs", BPF_PROG_TYPE_KPROBE },
+		{ "bpf_perf_event_data", "bpf_perf_event_data_kern", BPF_PROG_TYPE_PERF_EVENT },
+		{ "bpf_raw_tracepoint_args", NULL, BPF_PROG_TYPE_RAW_TRACEPOINT },
+	};
+	int i;
+
+	if (!ctx_name)
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(ctx_map); i++) {
+		if (strcmp(ctx_map[i].uapi_name, ctx_name) == 0 ||
+		    (!ctx_map[i].kern_name || strcmp(ctx_map[i].kern_name, ctx_name) == 0)) {
+			*prog_type = ctx_map[i].prog_type;
+			*attach_type = ctx_map[i].attach_type;
+			return 0;
+		}
+	}
+
+	return -ESRCH;
+}
+
+static void fixup_obj(struct bpf_object *obj, struct bpf_program *prog, const char *filename)
 {
 	struct bpf_map *map;
 
@@ -798,18 +851,75 @@ static void fixup_obj(struct bpf_object *obj)
 				bpf_map__set_max_entries(map, 1);
 		}
 	}
+
+	/* SEC(freplace) programs can't be loaded with veristat as is,
+	 * but we can try guessing their target program's expected type by
+	 * looking at the type of program's first argument and substituting
+	 * corresponding program type
+	 */
+	if (bpf_program__type(prog) == BPF_PROG_TYPE_EXT) {
+		const struct btf *btf = bpf_object__btf(obj);
+		const char *prog_name = bpf_program__name(prog);
+		enum bpf_prog_type prog_type;
+		enum bpf_attach_type attach_type;
+		const struct btf_type *t;
+		const char *ctx_name;
+		int id;
+
+		if (!btf)
+			goto skip_freplace_fixup;
+
+		id = btf__find_by_name_kind(btf, prog_name, BTF_KIND_FUNC);
+		t = btf__type_by_id(btf, id);
+		t = btf__type_by_id(btf, t->type);
+		if (!btf_is_func_proto(t) || btf_vlen(t) != 1)
+			goto skip_freplace_fixup;
+
+		/* context argument is a pointer to a struct/typedef */
+		t = btf__type_by_id(btf, btf_params(t)[0].type);
+		while (t && btf_is_mod(t))
+			t = btf__type_by_id(btf, t->type);
+		if (!t || !btf_is_ptr(t))
+			goto skip_freplace_fixup;
+		t = btf__type_by_id(btf, t->type);
+		while (t && btf_is_mod(t))
+			t = btf__type_by_id(btf, t->type);
+		if (!t)
+			goto skip_freplace_fixup;
+
+		ctx_name = btf__name_by_offset(btf, t->name_off);
+
+		if (guess_prog_type_by_ctx_name(ctx_name, &prog_type, &attach_type) == 0) {
+			bpf_program__set_type(prog, prog_type);
+			bpf_program__set_expected_attach_type(prog, attach_type);
+
+			if (!env.quiet) {
+				printf("Using guessed program type '%s' for %s/%s...\n",
+					libbpf_bpf_prog_type_str(prog_type),
+					filename, prog_name);
+			}
+		} else {
+			if (!env.quiet) {
+				printf("Failed to guess program type for freplace program with context type name '%s' for %s/%s. Consider using canonical type names to help veristat...\n",
+					ctx_name, filename, prog_name);
+			}
+		}
+	}
+skip_freplace_fixup:
+	return;
 }
 
 static int process_prog(const char *filename, struct bpf_object *obj, struct bpf_program *prog)
 {
 	const char *prog_name = bpf_program__name(prog);
+	const char *base_filename = basename(filename);
 	size_t buf_sz = sizeof(verif_log_buf);
 	char *buf = verif_log_buf;
 	struct verif_stats *stats;
 	int err = 0;
 	void *tmp;
 
-	if (!should_process_file_prog(basename(filename), bpf_program__name(prog))) {
+	if (!should_process_file_prog(base_filename, bpf_program__name(prog))) {
 		env.progs_skipped++;
 		return 0;
 	}
@@ -835,12 +945,12 @@ static int process_prog(const char *filename, struct bpf_object *obj, struct bpf
 	verif_log_buf[0] = '\0';
 
 	/* increase chances of successful BPF object loading */
-	fixup_obj(obj);
+	fixup_obj(obj, prog, base_filename);
 
 	err = bpf_object__load(obj);
 	env.progs_processed++;
 
-	stats->file_name = strdup(basename(filename));
+	stats->file_name = strdup(base_filename);
 	stats->prog_name = strdup(bpf_program__name(prog));
 	stats->stats[VERDICT] = err == 0; /* 1 - success, 0 - failure */
 	parse_verif_log(buf, buf_sz, stats);
-- 
2.34.1

Re: [PATCH v2 bpf-next 1/3] libbpf: disassociate section handler on explicit bpf_program__set_type() call

[Stanislav Fomichev]

On 03/24, Andrii Nakryiko wrote:
> If user explicitly overrides programs's type with
> bpf_program__set_type() API call, we need to disassociate whatever
> SEC_DEF handler libbpf determined initially based on program's SEC()
> definition, as it's not goind to be valid anymore and could lead to
> crashes and/or confusing failures.

> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>   tools/lib/bpf/libbpf.c | 1 +
>   1 file changed, 1 insertion(+)

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index f6a071db5c6e..a34ebb6b8508 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8465,6 +8465,7 @@ int bpf_program__set_type(struct bpf_program *prog,  
> enum bpf_prog_type type)
>   		return libbpf_err(-EBUSY);

>   	prog->type = type;
> +	prog->sec_def = NULL;
>   	return 0;
>   }

Surprisingly, but it breaks the following selftest:

serial_test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
serial_test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
serial_test_bpf_obj_id:PASS:get-fd-by-notexist-link-id 0 nsec
serial_test_bpf_obj_id:FAIL:prog_attach prog #0, err -95
   #17      bpf_obj_id:FAIL

(saw in the bot, reproduced locally)

> --
> 2.34.1

Re: [PATCH v2 bpf-next 3/3] veristat: guess and substitue underlying program type for freplace (EXT) progs

[Alexei Starovoitov]

On Fri, Mar 24, 2023 at 04:27:45PM -0700, Andrii Nakryiko wrote:
> +static int guess_prog_type_by_ctx_name(const char *ctx_name,
> +				       enum bpf_prog_type *prog_type,
> +				       enum bpf_attach_type *attach_type)
> +{
> +	/* We need to guess program type based on its declared context type.
> +	 * This guess can't be perfect as many different program types might
> +	 * share the same context type.  So we can only hope to reasonably
> +	 * well guess this and get lucky.
> +	 *
> +	 * Just in case, we support both UAPI-side type names and
> +	 * kernel-internal names.
> +	 */
> +	static struct {
> +		const char *uapi_name;
> +		const char *kern_name;
> +		enum bpf_prog_type prog_type;
> +		enum bpf_attach_type attach_type;
> +	} ctx_map[] = {
> +		/* __sk_buff is most ambiguous, for now we assume cgroup_skb */
> +		{ "__sk_buff", "sk_buff", BPF_PROG_TYPE_CGROUP_SKB, BPF_CGROUP_INET_INGRESS },
> +		{ "bpf_sock", "sock", BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND },
> +		{ "bpf_sock_addr", "bpf_sock_addr_kern",  BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND },
> +		{ "bpf_sock_ops", "bpf_sock_ops_kern", BPF_PROG_TYPE_SOCK_OPS, BPF_CGROUP_SOCK_OPS },
> +		{ "sk_msg_md", "sk_msg", BPF_PROG_TYPE_SK_MSG, BPF_SK_MSG_VERDICT },
> +		{ "bpf_cgroup_dev_ctx", "bpf_cgroup_dev_ctx", BPF_PROG_TYPE_CGROUP_DEVICE, BPF_CGROUP_DEVICE },
> +		{ "bpf_sysctl", "bpf_sysctl_kern", BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL },
> +		{ "bpf_sockopt", "bpf_sockopt_kern", BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT },
> +		{ "sk_reuseport_md", "sk_reuseport_kern", BPF_PROG_TYPE_SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE },
> +		{ "bpf_sk_lookup", "bpf_sk_lookup_kern", BPF_PROG_TYPE_SK_LOOKUP, BPF_SK_LOOKUP },
> +		{ "xdp_md", "xdp_buff", BPF_PROG_TYPE_XDP, BPF_XDP },
> +		/* tracing types with no expected attach type */
> +		{ "bpf_user_pt_regs_t", "pt_regs", BPF_PROG_TYPE_KPROBE },
> +		{ "bpf_perf_event_data", "bpf_perf_event_data_kern", BPF_PROG_TYPE_PERF_EVENT },
> +		{ "bpf_raw_tracepoint_args", NULL, BPF_PROG_TYPE_RAW_TRACEPOINT },
> +	};
> +	int i;
> +
> +	if (!ctx_name)
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(ctx_map); i++) {
> +		if (strcmp(ctx_map[i].uapi_name, ctx_name) == 0 ||
> +		    (!ctx_map[i].kern_name || strcmp(ctx_map[i].kern_name, ctx_name) == 0)) {

I'm struggling to understand above A || (B || C) logic.
() are redundant?

> +			*prog_type = ctx_map[i].prog_type;
> +			*attach_type = ctx_map[i].attach_type;
> +			return 0;
> +		}
> +	}
> +
> +	return -ESRCH;

This will never be hit, since "bpf_raw_tracepoint_args", NULL is last and
it will always succeed.

The idea is to always succeed in guessing and default to raw_tp ?
... and there should be only one entry with kern_name == NULL...
But it's not mentioned anywhere in the comments.
A weird way to implement a default.
I'm definitely missing something.

Re: [PATCH v2 bpf-next 1/3] libbpf: disassociate section handler on explicit bpf_program__set_type() call

[Andrii Nakryiko]

On Fri, Mar 24, 2023 at 8:01 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> On 03/24, Andrii Nakryiko wrote:
> > If user explicitly overrides programs's type with
> > bpf_program__set_type() API call, we need to disassociate whatever
> > SEC_DEF handler libbpf determined initially based on program's SEC()
> > definition, as it's not goind to be valid anymore and could lead to
> > crashes and/or confusing failures.
>
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >   tools/lib/bpf/libbpf.c | 1 +
> >   1 file changed, 1 insertion(+)
>
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index f6a071db5c6e..a34ebb6b8508 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -8465,6 +8465,7 @@ int bpf_program__set_type(struct bpf_program *prog,
> > enum bpf_prog_type type)
> >               return libbpf_err(-EBUSY);
>
> >       prog->type = type;
> > +     prog->sec_def = NULL;
> >       return 0;
> >   }
>
> Surprisingly, but it breaks the following selftest:
>
> serial_test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
> serial_test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
> serial_test_bpf_obj_id:PASS:get-fd-by-notexist-link-id 0 nsec
> serial_test_bpf_obj_id:FAIL:prog_attach prog #0, err -95
>    #17      bpf_obj_id:FAIL
>
> (saw in the bot, reproduced locally)
>

my bad for not testing thoroughly enough, will take a look at what's
going on, thanks!

> > --
> > 2.34.1
>

Re: [PATCH v2 bpf-next 3/3] veristat: guess and substitue underlying program type for freplace (EXT) progs

[Andrii Nakryiko]

On Sat, Mar 25, 2023 at 9:59 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Mar 24, 2023 at 04:27:45PM -0700, Andrii Nakryiko wrote:
> > +static int guess_prog_type_by_ctx_name(const char *ctx_name,
> > +                                    enum bpf_prog_type *prog_type,
> > +                                    enum bpf_attach_type *attach_type)
> > +{
> > +     /* We need to guess program type based on its declared context type.
> > +      * This guess can't be perfect as many different program types might
> > +      * share the same context type.  So we can only hope to reasonably
> > +      * well guess this and get lucky.
> > +      *
> > +      * Just in case, we support both UAPI-side type names and
> > +      * kernel-internal names.
> > +      */
> > +     static struct {
> > +             const char *uapi_name;
> > +             const char *kern_name;
> > +             enum bpf_prog_type prog_type;
> > +             enum bpf_attach_type attach_type;
> > +     } ctx_map[] = {
> > +             /* __sk_buff is most ambiguous, for now we assume cgroup_skb */
> > +             { "__sk_buff", "sk_buff", BPF_PROG_TYPE_CGROUP_SKB, BPF_CGROUP_INET_INGRESS },
> > +             { "bpf_sock", "sock", BPF_PROG_TYPE_CGROUP_SOCK, BPF_CGROUP_INET4_POST_BIND },
> > +             { "bpf_sock_addr", "bpf_sock_addr_kern",  BPF_PROG_TYPE_CGROUP_SOCK_ADDR, BPF_CGROUP_INET4_BIND },
> > +             { "bpf_sock_ops", "bpf_sock_ops_kern", BPF_PROG_TYPE_SOCK_OPS, BPF_CGROUP_SOCK_OPS },
> > +             { "sk_msg_md", "sk_msg", BPF_PROG_TYPE_SK_MSG, BPF_SK_MSG_VERDICT },
> > +             { "bpf_cgroup_dev_ctx", "bpf_cgroup_dev_ctx", BPF_PROG_TYPE_CGROUP_DEVICE, BPF_CGROUP_DEVICE },
> > +             { "bpf_sysctl", "bpf_sysctl_kern", BPF_PROG_TYPE_CGROUP_SYSCTL, BPF_CGROUP_SYSCTL },
> > +             { "bpf_sockopt", "bpf_sockopt_kern", BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT },
> > +             { "sk_reuseport_md", "sk_reuseport_kern", BPF_PROG_TYPE_SK_REUSEPORT, BPF_SK_REUSEPORT_SELECT_OR_MIGRATE },
> > +             { "bpf_sk_lookup", "bpf_sk_lookup_kern", BPF_PROG_TYPE_SK_LOOKUP, BPF_SK_LOOKUP },
> > +             { "xdp_md", "xdp_buff", BPF_PROG_TYPE_XDP, BPF_XDP },
> > +             /* tracing types with no expected attach type */
> > +             { "bpf_user_pt_regs_t", "pt_regs", BPF_PROG_TYPE_KPROBE },
> > +             { "bpf_perf_event_data", "bpf_perf_event_data_kern", BPF_PROG_TYPE_PERF_EVENT },
> > +             { "bpf_raw_tracepoint_args", NULL, BPF_PROG_TYPE_RAW_TRACEPOINT },
> > +     };
> > +     int i;
> > +
> > +     if (!ctx_name)
> > +             return -EINVAL;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(ctx_map); i++) {
> > +             if (strcmp(ctx_map[i].uapi_name, ctx_name) == 0 ||
> > +                 (!ctx_map[i].kern_name || strcmp(ctx_map[i].kern_name, ctx_name) == 0)) {
>
> I'm struggling to understand above A || (B || C) logic.
> () are redundant?

oh, shoot, should have been A || (B && C). Last minute refactoring
gone wrong. kern_name is optional (for bpf_raw_tracepoint_args), so
this is just to avoid strcmp() on NULL.

>
> > +                     *prog_type = ctx_map[i].prog_type;
> > +                     *attach_type = ctx_map[i].attach_type;
> > +                     return 0;
> > +             }
> > +     }
> > +
> > +     return -ESRCH;
>
> This will never be hit, since "bpf_raw_tracepoint_args", NULL is last and
> it will always succeed.
>
> The idea is to always succeed in guessing and default to raw_tp ?
> ... and there should be only one entry with kern_name == NULL...

No-no, it's much simpler than that. It should have been A || (B && C)
above. If we can't guess, we shouldn't just assume raw_tp, that wasn't
the intent.

Unfortunately I didn't see your comments before sending v3, I'll send
v4 with a fix for NULL check.


> But it's not mentioned anywhere in the comments.
> A weird way to implement a default.
> I'm definitely missing something.