| Message ID | 20240227204556.17524-6-eddyz87@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | libbpf: type suffixes and autocreate flag for struct_ops maps | expand |
On Tue, Feb 27, 2024 at 10:45:53PM +0200, Eduard Zingerman wrote: > When loading struct_ops programs kernel requires BTF id of the > struct_ops type and member index for attachment point inside that > type. This makes it not possible to have same BPF program used in > struct_ops maps that have different struct_ops type. > Check if libbpf rejects such BPF objects files. > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > --- > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 24 +++++++++++ > .../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 ++ > .../selftests/bpf/prog_tests/bad_struct_ops.c | 42 +++++++++++++++++++ > .../selftests/bpf/progs/bad_struct_ops.c | 17 ++++++++ > 4 files changed, 87 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c > > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > index 0d8437e05f64..69f5eb9ad546 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > @@ -601,6 +601,29 @@ struct bpf_struct_ops bpf_bpf_testmod_ops = { > .owner = THIS_MODULE, > }; > > +static int bpf_dummy_reg2(void *kdata) > +{ > + struct bpf_testmod_ops2 *ops = kdata; > + > + ops->test_1(); > + return 0; > +} > + > +static struct bpf_testmod_ops2 __bpf_testmod_ops2 = { > + .test_1 = bpf_testmod_test_1, > +}; > + > +struct bpf_struct_ops bpf_testmod_ops2 = { > + .verifier_ops = &bpf_testmod_verifier_ops, > + .init = bpf_testmod_ops_init, > + .init_member = bpf_testmod_ops_init_member, > + .reg = bpf_dummy_reg2, > + .unreg = bpf_dummy_unreg, > + .cfi_stubs = &__bpf_testmod_ops2, > + .name = "bpf_testmod_ops2", > + .owner = THIS_MODULE, > +}; > + > extern int bpf_fentry_test1(int a); > > static int bpf_testmod_init(void) > @@ -612,6 +635,7 @@ static int bpf_testmod_init(void) > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set); > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set); > ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops); > + ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2); > if (ret < 0) > return ret; > if (bpf_fentry_test1(0) < 0) > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > index c3b0cf788f9f..3183fff7f246 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > @@ -37,4 +37,8 @@ struct bpf_testmod_ops { > int (*test_maybe_null)(int dummy, struct task_struct *task); > }; > > +struct bpf_testmod_ops2 { > + int (*test_1)(void); > +}; > + > #endif /* _BPF_TESTMOD_H */ > diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > new file mode 100644 > index 000000000000..9c689db4b05b > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <test_progs.h> > +#include "bad_struct_ops.skel.h" > + > +#define EXPECTED_MSG "libbpf: struct_ops reloc" > + > +static libbpf_print_fn_t old_print_cb; > +static bool msg_found; > + > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > +{ > + old_print_cb(level, fmt, args); > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > + msg_found = true; > + > + return 0; > +} Not necessary at all for this patch set / just an observation, but it would be nice to have this be something offered by the core prog_tests framework (meaning, the ability to assert libbpf output for a testcase). > + > +static void test_bad_struct_ops(void) > +{ > + struct bad_struct_ops *skel; > + int err; > + > + old_print_cb = libbpf_set_print(print_cb); > + skel = bad_struct_ops__open_and_load(); > + err = errno; > + libbpf_set_print(old_print_cb); > + if (!ASSERT_NULL(skel, "bad_struct_ops__open_and_load")) > + return; > + > + ASSERT_EQ(err, EINVAL, "errno should be EINVAL"); > + ASSERT_TRUE(msg_found, "expected message"); > + > + bad_struct_ops__destroy(skel); > +} > + > +void serial_test_bad_struct_ops(void) > +{ > + if (test__start_subtest("test_bad_struct_ops")) > + test_bad_struct_ops(); > +} > diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > new file mode 100644 > index 000000000000..9c103afbfdb1 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <vmlinux.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include "../bpf_testmod/bpf_testmod.h" > + > +char _license[] SEC("license") = "GPL"; > + > +SEC("struct_ops/test_1") > +int BPF_PROG(test_1) { return 0; } > + > +SEC(".struct_ops.link") > +struct bpf_testmod_ops testmod_1 = { .test_1 = (void *)test_1 }; Just to make be 100% sure that we're isolating the issue under test, should we also add a .test_2 prog and add it to the struct bpf_testmod_ops map? > + > +SEC(".struct_ops.link") > +struct bpf_testmod_ops2 testmod_2 = { .test_1 = (void *)test_1 }; > -- > 2.43.0 >
On Wed, 2024-02-28 at 12:15 -0600, David Vernet wrote: [...] > > +static libbpf_print_fn_t old_print_cb; > > +static bool msg_found; > > + > > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > > +{ > > + old_print_cb(level, fmt, args); > > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > > + msg_found = true; > > + > > + return 0; > > +} > > Not necessary at all for this patch set / just an observation, but it would be > nice to have this be something offered by the core prog_tests framework > (meaning, the ability to assert libbpf output for a testcase). This might be useful, I will add a utility function for it (probably two). [...] > > diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > > new file mode 100644 > > index 000000000000..9c103afbfdb1 > > --- /dev/null > > +++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > > @@ -0,0 +1,17 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <vmlinux.h> > > +#include <bpf/bpf_helpers.h> > > +#include <bpf/bpf_tracing.h> > > +#include "../bpf_testmod/bpf_testmod.h" > > + > > +char _license[] SEC("license") = "GPL"; > > + > > +SEC("struct_ops/test_1") > > +int BPF_PROG(test_1) { return 0; } > > + > > +SEC(".struct_ops.link") > > +struct bpf_testmod_ops testmod_1 = { .test_1 = (void *)test_1 }; > > Just to make be 100% sure that we're isolating the issue under test, should we > also add a .test_2 prog and add it to the struct bpf_testmod_ops map? You are concerned that error might be confused with libbpf insisting that '.test_2' should be present, right? libbpf allows NULL members but I can add '.test_2' here, no problem. [...]
On Wed, Feb 28, 2024 at 10:06:21PM +0200, Eduard Zingerman wrote: > On Wed, 2024-02-28 at 12:15 -0600, David Vernet wrote: > [...] > > > > +static libbpf_print_fn_t old_print_cb; > > > +static bool msg_found; > > > + > > > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > > > +{ > > > + old_print_cb(level, fmt, args); > > > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > > > + msg_found = true; > > > + > > > + return 0; > > > +} > > > > Not necessary at all for this patch set / just an observation, but it would be > > nice to have this be something offered by the core prog_tests framework > > (meaning, the ability to assert libbpf output for a testcase). > > This might be useful, I will add a utility function for it (probably two). > > [...] > > > > diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > > > new file mode 100644 > > > index 000000000000..9c103afbfdb1 > > > --- /dev/null > > > +++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > > > @@ -0,0 +1,17 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > + > > > +#include <vmlinux.h> > > > +#include <bpf/bpf_helpers.h> > > > +#include <bpf/bpf_tracing.h> > > > +#include "../bpf_testmod/bpf_testmod.h" > > > + > > > +char _license[] SEC("license") = "GPL"; > > > + > > > +SEC("struct_ops/test_1") > > > +int BPF_PROG(test_1) { return 0; } > > > + > > > +SEC(".struct_ops.link") > > > +struct bpf_testmod_ops testmod_1 = { .test_1 = (void *)test_1 }; > > > > Just to make be 100% sure that we're isolating the issue under test, should we > > also add a .test_2 prog and add it to the struct bpf_testmod_ops map? > > You are concerned that error might be confused with libbpf insisting > that '.test_2' should be present, right? > libbpf allows NULL members but I can add '.test_2' here, no problem. Correct, and yes that's true. Feel free to ignore if you think it's cleaner without, totally up to you.
On Tue, Feb 27, 2024 at 12:46 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > When loading struct_ops programs kernel requires BTF id of the > struct_ops type and member index for attachment point inside that > type. This makes it not possible to have same BPF program used in > struct_ops maps that have different struct_ops type. > Check if libbpf rejects such BPF objects files. > > Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> > --- > .../selftests/bpf/bpf_testmod/bpf_testmod.c | 24 +++++++++++ > .../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 ++ > .../selftests/bpf/prog_tests/bad_struct_ops.c | 42 +++++++++++++++++++ > .../selftests/bpf/progs/bad_struct_ops.c | 17 ++++++++ > 4 files changed, 87 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c > > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > index 0d8437e05f64..69f5eb9ad546 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c > @@ -601,6 +601,29 @@ struct bpf_struct_ops bpf_bpf_testmod_ops = { > .owner = THIS_MODULE, > }; > > +static int bpf_dummy_reg2(void *kdata) > +{ > + struct bpf_testmod_ops2 *ops = kdata; > + > + ops->test_1(); > + return 0; > +} > + > +static struct bpf_testmod_ops2 __bpf_testmod_ops2 = { > + .test_1 = bpf_testmod_test_1, > +}; > + > +struct bpf_struct_ops bpf_testmod_ops2 = { > + .verifier_ops = &bpf_testmod_verifier_ops, > + .init = bpf_testmod_ops_init, > + .init_member = bpf_testmod_ops_init_member, > + .reg = bpf_dummy_reg2, > + .unreg = bpf_dummy_unreg, > + .cfi_stubs = &__bpf_testmod_ops2, > + .name = "bpf_testmod_ops2", > + .owner = THIS_MODULE, > +}; > + > extern int bpf_fentry_test1(int a); > > static int bpf_testmod_init(void) > @@ -612,6 +635,7 @@ static int bpf_testmod_init(void) > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set); > ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set); > ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops); > + ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2); > if (ret < 0) > return ret; > if (bpf_fentry_test1(0) < 0) > diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > index c3b0cf788f9f..3183fff7f246 100644 > --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h > @@ -37,4 +37,8 @@ struct bpf_testmod_ops { > int (*test_maybe_null)(int dummy, struct task_struct *task); > }; > > +struct bpf_testmod_ops2 { > + int (*test_1)(void); > +}; > + > #endif /* _BPF_TESTMOD_H */ > diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > new file mode 100644 > index 000000000000..9c689db4b05b > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <test_progs.h> > +#include "bad_struct_ops.skel.h" > + > +#define EXPECTED_MSG "libbpf: struct_ops reloc" > + > +static libbpf_print_fn_t old_print_cb; > +static bool msg_found; > + > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > +{ > + old_print_cb(level, fmt, args); > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > + msg_found = true; > + > + return 0; > +} > + > +static void test_bad_struct_ops(void) > +{ > + struct bad_struct_ops *skel; > + int err; > + > + old_print_cb = libbpf_set_print(print_cb); > + skel = bad_struct_ops__open_and_load(); we want to check that the load step failed specifically, right? So please split open from load, make sure that open succeeds, but load fails > + err = errno; > + libbpf_set_print(old_print_cb); > + if (!ASSERT_NULL(skel, "bad_struct_ops__open_and_load")) > + return; > + > + ASSERT_EQ(err, EINVAL, "errno should be EINVAL"); > + ASSERT_TRUE(msg_found, "expected message"); > + > + bad_struct_ops__destroy(skel); > +} > + > +void serial_test_bad_struct_ops(void) why does it have to be a serial test? > +{ > + if (test__start_subtest("test_bad_struct_ops")) > + test_bad_struct_ops(); > +} > diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > new file mode 100644 > index 000000000000..9c103afbfdb1 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <vmlinux.h> > +#include <bpf/bpf_helpers.h> > +#include <bpf/bpf_tracing.h> > +#include "../bpf_testmod/bpf_testmod.h" > + > +char _license[] SEC("license") = "GPL"; > + > +SEC("struct_ops/test_1") > +int BPF_PROG(test_1) { return 0; } > + > +SEC(".struct_ops.link") > +struct bpf_testmod_ops testmod_1 = { .test_1 = (void *)test_1 }; > + > +SEC(".struct_ops.link") > +struct bpf_testmod_ops2 testmod_2 = { .test_1 = (void *)test_1 }; > -- > 2.43.0 >
On Wed, 2024-02-28 at 15:40 -0800, Andrii Nakryiko wrote: [...] > > +static libbpf_print_fn_t old_print_cb; > > +static bool msg_found; > > + > > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > > +{ > > + old_print_cb(level, fmt, args); > > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > > + msg_found = true; > > + > > + return 0; > > +} > > + > > +static void test_bad_struct_ops(void) > > +{ > > + struct bad_struct_ops *skel; > > + int err; > > + > > + old_print_cb = libbpf_set_print(print_cb); > > + skel = bad_struct_ops__open_and_load(); > > we want to check that the load step failed specifically, right? So > please split open from load, make sure that open succeeds, but load > fails Ok > > > + err = errno; > > + libbpf_set_print(old_print_cb); > > + if (!ASSERT_NULL(skel, "bad_struct_ops__open_and_load")) > > + return; > > + > > + ASSERT_EQ(err, EINVAL, "errno should be EINVAL"); > > + ASSERT_TRUE(msg_found, "expected message"); > > + > > + bad_struct_ops__destroy(skel); > > +} > > + > > +void serial_test_bad_struct_ops(void) > > why does it have to be a serial test? Because it hijacks libbpf print callback. [...]
On Wed, Feb 28, 2024 at 3:44 PM Eduard Zingerman <eddyz87@gmail.com> wrote: > > On Wed, 2024-02-28 at 15:40 -0800, Andrii Nakryiko wrote: > [...] > > > > +static libbpf_print_fn_t old_print_cb; > > > +static bool msg_found; > > > + > > > +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) > > > +{ > > > + old_print_cb(level, fmt, args); > > > + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) > > > + msg_found = true; > > > + > > > + return 0; > > > +} > > > + > > > +static void test_bad_struct_ops(void) > > > +{ > > > + struct bad_struct_ops *skel; > > > + int err; > > > + > > > + old_print_cb = libbpf_set_print(print_cb); > > > + skel = bad_struct_ops__open_and_load(); > > > > we want to check that the load step failed specifically, right? So > > please split open from load, make sure that open succeeds, but load > > fails > > Ok > > > > > > + err = errno; > > > + libbpf_set_print(old_print_cb); > > > + if (!ASSERT_NULL(skel, "bad_struct_ops__open_and_load")) > > > + return; > > > + > > > + ASSERT_EQ(err, EINVAL, "errno should be EINVAL"); > > > + ASSERT_TRUE(msg_found, "expected message"); > > > + > > > + bad_struct_ops__destroy(skel); > > > +} > > > + > > > +void serial_test_bad_struct_ops(void) > > > > why does it have to be a serial test? > > Because it hijacks libbpf print callback. each non-serial test runs in its own *process*, there is no multi-threading here, so it's fine if non-serial test temporarily hijacks print callback, as long as it restores it properly before finishing > > [...]
On Wed, 2024-02-28 at 15:56 -0800, Andrii Nakryiko wrote: [...] > each non-serial test runs in its own *process*, there is no > multi-threading here, so it's fine if non-serial test temporarily > hijacks print callback, as long as it restores it properly before > finishing I missed this detail, thanks.
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c index 0d8437e05f64..69f5eb9ad546 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c @@ -601,6 +601,29 @@ struct bpf_struct_ops bpf_bpf_testmod_ops = { .owner = THIS_MODULE, }; +static int bpf_dummy_reg2(void *kdata) +{ + struct bpf_testmod_ops2 *ops = kdata; + + ops->test_1(); + return 0; +} + +static struct bpf_testmod_ops2 __bpf_testmod_ops2 = { + .test_1 = bpf_testmod_test_1, +}; + +struct bpf_struct_ops bpf_testmod_ops2 = { + .verifier_ops = &bpf_testmod_verifier_ops, + .init = bpf_testmod_ops_init, + .init_member = bpf_testmod_ops_init_member, + .reg = bpf_dummy_reg2, + .unreg = bpf_dummy_unreg, + .cfi_stubs = &__bpf_testmod_ops2, + .name = "bpf_testmod_ops2", + .owner = THIS_MODULE, +}; + extern int bpf_fentry_test1(int a); static int bpf_testmod_init(void) @@ -612,6 +635,7 @@ static int bpf_testmod_init(void) ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set); ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set); ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops, bpf_testmod_ops); + ret = ret ?: register_bpf_struct_ops(&bpf_testmod_ops2, bpf_testmod_ops2); if (ret < 0) return ret; if (bpf_fentry_test1(0) < 0) diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h index c3b0cf788f9f..3183fff7f246 100644 --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h @@ -37,4 +37,8 @@ struct bpf_testmod_ops { int (*test_maybe_null)(int dummy, struct task_struct *task); }; +struct bpf_testmod_ops2 { + int (*test_1)(void); +}; + #endif /* _BPF_TESTMOD_H */ diff --git a/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c new file mode 100644 index 000000000000..9c689db4b05b --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <test_progs.h> +#include "bad_struct_ops.skel.h" + +#define EXPECTED_MSG "libbpf: struct_ops reloc" + +static libbpf_print_fn_t old_print_cb; +static bool msg_found; + +static int print_cb(enum libbpf_print_level level, const char *fmt, va_list args) +{ + old_print_cb(level, fmt, args); + if (level == LIBBPF_WARN && strncmp(fmt, EXPECTED_MSG, strlen(EXPECTED_MSG)) == 0) + msg_found = true; + + return 0; +} + +static void test_bad_struct_ops(void) +{ + struct bad_struct_ops *skel; + int err; + + old_print_cb = libbpf_set_print(print_cb); + skel = bad_struct_ops__open_and_load(); + err = errno; + libbpf_set_print(old_print_cb); + if (!ASSERT_NULL(skel, "bad_struct_ops__open_and_load")) + return; + + ASSERT_EQ(err, EINVAL, "errno should be EINVAL"); + ASSERT_TRUE(msg_found, "expected message"); + + bad_struct_ops__destroy(skel); +} + +void serial_test_bad_struct_ops(void) +{ + if (test__start_subtest("test_bad_struct_ops")) + test_bad_struct_ops(); +} diff --git a/tools/testing/selftests/bpf/progs/bad_struct_ops.c b/tools/testing/selftests/bpf/progs/bad_struct_ops.c new file mode 100644 index 000000000000..9c103afbfdb1 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bad_struct_ops.c @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <vmlinux.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> +#include "../bpf_testmod/bpf_testmod.h" + +char _license[] SEC("license") = "GPL"; + +SEC("struct_ops/test_1") +int BPF_PROG(test_1) { return 0; } + +SEC(".struct_ops.link") +struct bpf_testmod_ops testmod_1 = { .test_1 = (void *)test_1 }; + +SEC(".struct_ops.link") +struct bpf_testmod_ops2 testmod_2 = { .test_1 = (void *)test_1 };
When loading struct_ops programs kernel requires BTF id of the struct_ops type and member index for attachment point inside that type. This makes it not possible to have same BPF program used in struct_ops maps that have different struct_ops type. Check if libbpf rejects such BPF objects files. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- .../selftests/bpf/bpf_testmod/bpf_testmod.c | 24 +++++++++++ .../selftests/bpf/bpf_testmod/bpf_testmod.h | 4 ++ .../selftests/bpf/prog_tests/bad_struct_ops.c | 42 +++++++++++++++++++ .../selftests/bpf/progs/bad_struct_ops.c | 17 ++++++++ 4 files changed, 87 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/bad_struct_ops.c create mode 100644 tools/testing/selftests/bpf/progs/bad_struct_ops.c