May 2022 - Linux-kselftest-mirror

[PATCH AUTOSEL 5.18 157/159] selftests/bpf: fix btf_dump/btf_dump due to recent clang change

by Sasha Levin

From: Yonghong Song <yhs(a)fb.com> [ Upstream commit 4050764cbaa25760aab40857f723393c07898474 ] Latest llvm-project upstream had a change of behavior related to qualifiers on function return type ([1]). This caused selftests btf_dump/btf_dump failure. The following example shows what changed. $ cat t.c typedef const char * const (* const (* const fn_ptr_arr2_t[5])())(char * (*)(int)); struct t { int a; fn_ptr_arr2_t l; }; int foo(struct t *arg) { return arg->a; } Compiled with latest upstream llvm15, $ clang -O2 -g -target bpf -S -emit-llvm t.c The related generated debuginfo IR looks like: !16 = !DIDerivedType(tag: DW_TAG_typedef, name: "fn_ptr_arr2_t", file: !1, line: 1, baseType: !17) !17 = !DICompositeType(tag: DW_TAG_array_type, baseType: !18, size: 320, elements: !32) !18 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !19) !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !20, size: 64) !20 = !DISubroutineType(types: !21) !21 = !{!22, null} !22 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !23, size: 64) !23 = !DISubroutineType(types: !24) !24 = !{!25, !28} !25 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !26, size: 64) !26 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !27) !27 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) You can see two intermediate const qualifier to pointer are dropped in debuginfo IR. With llvm14, we have following debuginfo IR: !16 = !DIDerivedType(tag: DW_TAG_typedef, name: "fn_ptr_arr2_t", file: !1, line: 1, baseType: !17) !17 = !DICompositeType(tag: DW_TAG_array_type, baseType: !18, size: 320, elements: !34) !18 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !19) !19 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !20, size: 64) !20 = !DISubroutineType(types: !21) !21 = !{!22, null} !22 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !23) !23 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !24, size: 64) !24 = !DISubroutineType(types: !25) !25 = !{!26, !30} !26 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !27) !27 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !28, size: 64) !28 = !DIDerivedType(tag: DW_TAG_const_type, baseType: !29) !29 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) All const qualifiers are preserved. To adapt the selftest to both old and new llvm, this patch removed the intermediate const qualifier in const-to-ptr types, to make the test succeed again. [1] https://reviews.llvm.org/D125919 Reported-by: Mykola Lysenko <mykolal(a)fb.com> Signed-off-by: Yonghong Song <yhs(a)fb.com> Link: https://lore.kernel.org/r/20220523152044.3905809-1-yhs@fb.com Signed-off-by: Alexei Starovoitov <ast(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c index 1c7105fcae3c..4ee4748133fe 100644 --- a/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c +++ b/tools/testing/selftests/bpf/progs/btf_dump_test_case_syntax.c @@ -94,7 +94,7 @@ typedef void (* (*signal_t)(int, void (*)(int)))(int); typedef char * (*fn_ptr_arr1_t[10])(int **); -typedef char * (* const (* const fn_ptr_arr2_t[5])())(char * (*)(int)); +typedef char * (* (* const fn_ptr_arr2_t[5])())(char * (*)(int)); struct struct_w_typedefs { int_t a; -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 148/159] selftests/bpf: Add missing trampoline program type to trampoline_count test

by Sasha Levin

From: Yuntao Wang <ytcoode(a)gmail.com> [ Upstream commit b23316aabffa835ecc516cb81daeef5b9155e8a5 ] Currently the trampoline_count test doesn't include any fmod_ret bpf programs, fix it to make the test cover all possible trampoline program types. Since fmod_ret bpf programs can't be attached to __set_task_comm function, as it's neither whitelisted for error injection nor a security hook, change it to bpf_modify_return_test. This patch also does some other cleanups such as removing duplicate code, dropping inconsistent comments, etc. Signed-off-by: Yuntao Wang <ytcoode(a)gmail.com> Signed-off-by: Andrii Nakryiko <andrii(a)kernel.org> Acked-by: Yonghong Song <yhs(a)fb.com> Link: https://lore.kernel.org/bpf/20220519150610.601313-1-ytcoode@gmail.com Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- include/linux/bpf.h | 2 +- .../bpf/prog_tests/trampoline_count.c | 134 +++++++----------- .../bpf/progs/test_trampoline_count.c | 16 ++- 3 files changed, 61 insertions(+), 91 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index bdb5298735ce..f084b251fce7 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -672,7 +672,7 @@ struct btf_func_model { #define BPF_TRAMP_F_RET_FENTRY_RET BIT(4) /* Each call __bpf_prog_enter + call bpf_func + call __bpf_prog_exit is ~50 - * bytes on x86. Pick a number to fit into BPF_IMAGE_SIZE / 2 + * bytes on x86. */ #define BPF_MAX_TRAMP_PROGS 38 diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c index 9c795ee52b7b..b0acbda6dbf5 100644 --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c @@ -1,126 +1,94 @@ // SPDX-License-Identifier: GPL-2.0-only #define _GNU_SOURCE -#include <sched.h> -#include <sys/prctl.h> #include <test_progs.h> #define MAX_TRAMP_PROGS 38 struct inst { struct bpf_object *obj; - struct bpf_link *link_fentry; - struct bpf_link *link_fexit; + struct bpf_link *link; }; -static int test_task_rename(void) -{ - int fd, duration = 0, err; - char buf[] = "test_overhead"; - - fd = open("/proc/self/comm", O_WRONLY|O_TRUNC); - if (CHECK(fd < 0, "open /proc", "err %d", errno)) - return -1; - err = write(fd, buf, sizeof(buf)); - if (err < 0) { - CHECK(err < 0, "task rename", "err %d", errno); - close(fd); - return -1; - } - close(fd); - return 0; -} - -static struct bpf_link *load(struct bpf_object *obj, const char *name) +static struct bpf_program *load_prog(char *file, char *name, struct inst *inst) { + struct bpf_object *obj; struct bpf_program *prog; - int duration = 0; + int err; + + obj = bpf_object__open_file(file, NULL); + if (!ASSERT_OK_PTR(obj, "obj_open_file")) + return NULL; + + inst->obj = obj; + + err = bpf_object__load(obj); + if (!ASSERT_OK(err, "obj_load")) + return NULL; prog = bpf_object__find_program_by_name(obj, name); - if (CHECK(!prog, "find_probe", "prog '%s' not found\n", name)) - return ERR_PTR(-EINVAL); - return bpf_program__attach_trace(prog); + if (!ASSERT_OK_PTR(prog, "obj_find_prog")) + return NULL; + + return prog; } /* TODO: use different target function to run in concurrent mode */ void serial_test_trampoline_count(void) { - const char *fentry_name = "prog1"; - const char *fexit_name = "prog2"; - const char *object = "test_trampoline_count.o"; - struct inst inst[MAX_TRAMP_PROGS] = {}; - int err, i = 0, duration = 0; - struct bpf_object *obj; + char *file = "test_trampoline_count.o"; + char *const progs[] = { "fentry_test", "fmod_ret_test", "fexit_test" }; + struct inst inst[MAX_TRAMP_PROGS + 1] = {}; + struct bpf_program *prog; struct bpf_link *link; - char comm[16] = {}; + int prog_fd, err, i; + LIBBPF_OPTS(bpf_test_run_opts, opts); /* attach 'allowed' trampoline programs */ for (i = 0; i < MAX_TRAMP_PROGS; i++) { - obj = bpf_object__open_file(object, NULL); - if (!ASSERT_OK_PTR(obj, "obj_open_file")) { - obj = NULL; + prog = load_prog(file, progs[i % ARRAY_SIZE(progs)], &inst[i]); + if (!prog) goto cleanup; - } - err = bpf_object__load(obj); - if (CHECK(err, "obj_load", "err %d\n", err)) + link = bpf_program__attach(prog); + if (!ASSERT_OK_PTR(link, "attach_prog")) goto cleanup; - inst[i].obj = obj; - obj = NULL; - - if (rand() % 2) { - link = load(inst[i].obj, fentry_name); - if (!ASSERT_OK_PTR(link, "attach_prog")) { - link = NULL; - goto cleanup; - } - inst[i].link_fentry = link; - } else { - link = load(inst[i].obj, fexit_name); - if (!ASSERT_OK_PTR(link, "attach_prog")) { - link = NULL; - goto cleanup; - } - inst[i].link_fexit = link; - } + + inst[i].link = link; } /* and try 1 extra.. */ - obj = bpf_object__open_file(object, NULL); - if (!ASSERT_OK_PTR(obj, "obj_open_file")) { - obj = NULL; + prog = load_prog(file, "fmod_ret_test", &inst[i]); + if (!prog) goto cleanup; - } - - err = bpf_object__load(obj); - if (CHECK(err, "obj_load", "err %d\n", err)) - goto cleanup_extra; /* ..that needs to fail */ - link = load(obj, fentry_name); - err = libbpf_get_error(link); - if (!ASSERT_ERR_PTR(link, "cannot attach over the limit")) { - bpf_link__destroy(link); - goto cleanup_extra; + link = bpf_program__attach(prog); + if (!ASSERT_ERR_PTR(link, "attach_prog")) { + inst[i].link = link; + goto cleanup; } /* with E2BIG error */ - ASSERT_EQ(err, -E2BIG, "proper error check"); - ASSERT_EQ(link, NULL, "ptr_is_null"); + if (!ASSERT_EQ(libbpf_get_error(link), -E2BIG, "E2BIG")) + goto cleanup; + if (!ASSERT_EQ(link, NULL, "ptr_is_null")) + goto cleanup; /* and finaly execute the probe */ - if (CHECK_FAIL(prctl(PR_GET_NAME, comm, 0L, 0L, 0L))) - goto cleanup_extra; - CHECK_FAIL(test_task_rename()); - CHECK_FAIL(prctl(PR_SET_NAME, comm, 0L, 0L, 0L)); + prog_fd = bpf_program__fd(prog); + if (!ASSERT_GE(prog_fd, 0, "bpf_program__fd")) + goto cleanup; + + err = bpf_prog_test_run_opts(prog_fd, &opts); + if (!ASSERT_OK(err, "bpf_prog_test_run_opts")) + goto cleanup; + + ASSERT_EQ(opts.retval & 0xffff, 4, "bpf_modify_return_test.result"); + ASSERT_EQ(opts.retval >> 16, 1, "bpf_modify_return_test.side_effect"); -cleanup_extra: - bpf_object__close(obj); cleanup: - if (i >= MAX_TRAMP_PROGS) - i = MAX_TRAMP_PROGS - 1; for (; i >= 0; i--) { - bpf_link__destroy(inst[i].link_fentry); - bpf_link__destroy(inst[i].link_fexit); + bpf_link__destroy(inst[i].link); bpf_object__close(inst[i].obj); } } diff --git a/tools/testing/selftests/bpf/progs/test_trampoline_count.c b/tools/testing/selftests/bpf/progs/test_trampoline_count.c index f030e469d05b..7765720da7d5 100644 --- a/tools/testing/selftests/bpf/progs/test_trampoline_count.c +++ b/tools/testing/selftests/bpf/progs/test_trampoline_count.c @@ -1,20 +1,22 @@ // SPDX-License-Identifier: GPL-2.0 -#include <stdbool.h> -#include <stddef.h> #include <linux/bpf.h> #include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> -struct task_struct; +SEC("fentry/bpf_modify_return_test") +int BPF_PROG(fentry_test, int a, int *b) +{ + return 0; +} -SEC("fentry/__set_task_comm") -int BPF_PROG(prog1, struct task_struct *tsk, const char *buf, bool exec) +SEC("fmod_ret/bpf_modify_return_test") +int BPF_PROG(fmod_ret_test, int a, int *b, int ret) { return 0; } -SEC("fexit/__set_task_comm") -int BPF_PROG(prog2, struct task_struct *tsk, const char *buf, bool exec) +SEC("fexit/bpf_modify_return_test") +int BPF_PROG(fexit_test, int a, int *b, int ret) { return 0; } -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 133/159] kunit: tool: make parser stop overwriting status of suites w/ no_tests

by Sasha Levin

From: Daniel Latypov <dlatypov(a)google.com> [ Upstream commit dbf0b0d53a2b5afa6ef7372dcedf52302669fc2c ] Consider this invocation $ ./tools/testing/kunit/kunit.py parse <<EOF TAP version 14 1..2 ok 1 - suite # Subtest: no_tests_suite # catastrophic error! not ok 1 - no_tests_suite EOF It will have a 0 exit code even though there's a "not ok". Consider this one: $ ./tools/testing/kunit/kunit.py parse <<EOF TAP version 14 1..2 ok 1 - suite not ok 1 - no_tests_suite EOF It will a non-zero exit code. Why? We have this line in the kunit_parser.py > parent_test = parse_test_header(lines, test) where we have special handling when we see "# Subtest" and we ignore the explicit reported "not ok 1" status! Also, NO_TESTS at a suite-level only results in a non-zero status code where then there's only one suite atm. This change is the minimal one to make sure we don't overwrite it. Signed-off-by: Daniel Latypov <dlatypov(a)google.com> Reviewed-by: David Gow <davidgow(a)google.com> Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/kunit/kunit_parser.py | 7 +++++-- .../test_data/test_is_test_passed-no_tests_no_plan.log | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 05ff334761dd..2f93ed1d7f99 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -789,8 +789,11 @@ def parse_test(lines: LineStream, expected_num: int, log: List[str]) -> Test: # Check for there being no tests if parent_test and len(subtests) == 0: - test.status = TestStatus.NO_TESTS - test.add_error('0 tests run!') + # Don't override a bad status if this test had one reported. + # Assumption: no subtests means CRASHED is from Test.__init__() + if test.status in (TestStatus.TEST_CRASHED, TestStatus.SUCCESS): + test.status = TestStatus.NO_TESTS + test.add_error('0 tests run!') # Add statuses to TestCounts attribute in Test object bubble_up_test_results(test) diff --git a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log index dd873c981108..4f81876ee6f1 100644 --- a/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log +++ b/tools/testing/kunit/test_data/test_is_test_passed-no_tests_no_plan.log @@ -3,5 +3,5 @@ TAP version 14 # Subtest: suite 1..1 # Subtest: case - ok 1 - case # SKIP + ok 1 - case ok 1 - suite -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 107/159] kunit: bail out of test filtering logic quicker if OOM

by Sasha Levin

From: Daniel Latypov <dlatypov(a)google.com> [ Upstream commit a02353f491622e49c7ddedc6a6dc4f1d6ed2150a ] When filtering what tests to run (suites and/or cases) via kunit.filter_glob (e.g. kunit.py run <glob>), we allocate copies of suites. These allocations can fail, and we largely don't handle that. Note: realistically, this probably doesn't matter much. We're not allocating much memory and this happens early in boot, so if we can't do that, then there's likely far bigger problems. This patch makes us immediately bail out from the top-level function (kunit_filter_suites) with -ENOMEM if any of the underlying kmalloc() calls return NULL. Implementation note: we used to return NULL pointers from some functions to indicate either that all suites/tests were filtered out or there was an error allocating the new array. We'll log a short error in this case and not run any tests or print a TAP header. From a kunit.py user's perspective, they'll get a message about missing/invalid TAP output and have to dig into the test.log to see it. Since hitting this error seems so unlikely, it's probably fine to not invent a way to plumb this error message more visibly. See also: https://lore.kernel.org/linux-kselftest/20220329103919.2376818-1-lv.ruyi@zt… Signed-off-by: Daniel Latypov <dlatypov(a)google.com> Reported-by: Zeal Robot <zealci(a)zte.com.cn> Reported-by: Lv Ruyi <lv.ruyi(a)zte.com.cn> Reviewed-by: Brendan Higgins <brendanhiggins(a)google.com> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- lib/kunit/executor.c | 27 ++++++++++++++++++++++----- lib/kunit/executor_test.c | 4 +++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 22640c9ee819..2f73a6a35a7e 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -71,9 +71,13 @@ kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob) /* Use memcpy to workaround copy->name being const. */ copy = kmalloc(sizeof(*copy), GFP_KERNEL); + if (!copy) + return ERR_PTR(-ENOMEM); memcpy(copy, suite, sizeof(*copy)); filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL); + if (!filtered) + return ERR_PTR(-ENOMEM); n = 0; kunit_suite_for_each_test_case(suite, test_case) { @@ -106,14 +110,16 @@ kunit_filter_subsuite(struct kunit_suite * const * const subsuite, filtered = kmalloc_array(n + 1, sizeof(*filtered), GFP_KERNEL); if (!filtered) - return NULL; + return ERR_PTR(-ENOMEM); n = 0; for (i = 0; subsuite[i] != NULL; ++i) { if (!glob_match(filter->suite_glob, subsuite[i]->name)) continue; filtered_suite = kunit_filter_tests(subsuite[i], filter->test_glob); - if (filtered_suite) + if (IS_ERR(filtered_suite)) + return ERR_CAST(filtered_suite); + else if (filtered_suite) filtered[n++] = filtered_suite; } filtered[n] = NULL; @@ -146,7 +152,8 @@ static void kunit_free_suite_set(struct suite_set suite_set) } static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, - const char *filter_glob) + const char *filter_glob, + int *err) { int i; struct kunit_suite * const **copy, * const *filtered_subsuite; @@ -166,6 +173,10 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set, for (i = 0; i < max; ++i) { filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], &filter); + if (IS_ERR(filtered_subsuite)) { + *err = PTR_ERR(filtered_subsuite); + return filtered; + } if (filtered_subsuite) *copy++ = filtered_subsuite; } @@ -236,9 +247,15 @@ int kunit_run_all_tests(void) .start = __kunit_suites_start, .end = __kunit_suites_end, }; + int err; - if (filter_glob_param) - suite_set = kunit_filter_suites(&suite_set, filter_glob_param); + if (filter_glob_param) { + suite_set = kunit_filter_suites(&suite_set, filter_glob_param, &err); + if (err) { + pr_err("kunit executor: error filtering suites: %d\n", err); + return err; + } + } if (!action_param) kunit_exec_run_tests(&suite_set); diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c index 4ed57fd94e42..eac6ff480273 100644 --- a/lib/kunit/executor_test.c +++ b/lib/kunit/executor_test.c @@ -137,14 +137,16 @@ static void filter_suites_test(struct kunit *test) .end = suites + 2, }; struct suite_set filtered = {.start = NULL, .end = NULL}; + int err = 0; /* Emulate two files, each having one suite */ subsuites[0][0] = alloc_fake_suite(test, "suite0", dummy_test_cases); subsuites[1][0] = alloc_fake_suite(test, "suite1", dummy_test_cases); /* Filter out suite1 */ - filtered = kunit_filter_suites(&suite_set, "suite0"); + filtered = kunit_filter_suites(&suite_set, "suite0", &err); kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */ + KUNIT_EXPECT_EQ(test, err, 0); KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t)1); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start); -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 009/159] selftests/bpf: Fix file descriptor leak in load_kallsyms()

by Sasha Levin

From: Yuntao Wang <ytcoode(a)gmail.com> [ Upstream commit 2d0df01974ce2b59b6f7d5bd3ea58d74f12ddf85 ] Currently, if sym_cnt > 0, it just returns and does not close file, fix it. Signed-off-by: Yuntao Wang <ytcoode(a)gmail.com> Signed-off-by: Andrii Nakryiko <andrii(a)kernel.org> Link: https://lore.kernel.org/bpf/20220405145711.49543-1-ytcoode@gmail.com Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/bpf/trace_helpers.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index 3d6217e3aff7..9c4be2cdb21a 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c @@ -25,15 +25,12 @@ static int ksym_cmp(const void *p1, const void *p2) int load_kallsyms(void) { - FILE *f = fopen("/proc/kallsyms", "r"); + FILE *f; char func[256], buf[256]; char symbol; void *addr; int i = 0; - if (!f) - return -ENOENT; - /* * This is called/used from multiplace places, * load symbols just once. @@ -41,6 +38,10 @@ int load_kallsyms(void) if (sym_cnt) return 0; + f = fopen("/proc/kallsyms", "r"); + if (!f) + return -ENOENT; + while (fgets(buf, sizeof(buf), f)) { if (sscanf(buf, "%p %c %s", &addr, &symbol, func) != 3) break; -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 007/159] selftests/bpf: Fix parsing of prog types in UAPI hdr for bpftool sync

by Sasha Levin

From: Quentin Monnet <quentin(a)isovalent.com> [ Upstream commit 4eeebce6ac4ad80ee8243bb847c98e0e55848d47 ] The script for checking that various lists of types in bpftool remain in sync with the UAPI BPF header uses a regex to parse enum bpf_prog_type. If this enum contains a set of values different from the list of program types in bpftool, it complains. This script should have reported the addition, some time ago, of the new BPF_PROG_TYPE_SYSCALL, which was not reported to bpftool's program types list. It failed to do so, because it failed to parse that new type from the enum. This is because the new value, in the BPF header, has an explicative comment on the same line, and the regex does not support that. Let's update the script to support parsing enum values when they have comments on the same line. Signed-off-by: Quentin Monnet <quentin(a)isovalent.com> Signed-off-by: Andrii Nakryiko <andrii(a)kernel.org> Link: https://lore.kernel.org/bpf/20220404140944.64744-1-quentin@isovalent.com Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/bpf/test_bpftool_synctypes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/test_bpftool_synctypes.py b/tools/testing/selftests/bpf/test_bpftool_synctypes.py index 6bf21e47882a..c0e7acd698ed 100755 --- a/tools/testing/selftests/bpf/test_bpftool_synctypes.py +++ b/tools/testing/selftests/bpf/test_bpftool_synctypes.py @@ -180,7 +180,7 @@ class FileExtractor(object): @enum_name: name of the enum to parse """ start_marker = re.compile(f'enum {enum_name} {{\n') - pattern = re.compile('^\s*(BPF_\w+),?$') + pattern = re.compile('^\s*(BPF_\w+),?(\s+/\*.*\*/)?$') end_marker = re.compile('^};') parser = BlockParser(self.reader) parser.search_block(start_marker) -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH AUTOSEL 5.18 006/159] selftests/bpf: Fix vfs_link kprobe definition

by Sasha Levin

From: Nikolay Borisov <nborisov(a)suse.com> [ Upstream commit e299bcd4d16ff86f46c48df1062c8aae0eca1ed8 ] Since commit 6521f8917082 ("namei: prepare for idmapped mounts") vfs_link's prototype was changed, the kprobe definition in profiler selftest in turn wasn't updated. The result is that all argument after the first are now stored in different registers. This means that self-test has been broken ever since. Fix it by updating the kprobe definition accordingly. Signed-off-by: Nikolay Borisov <nborisov(a)suse.com> Signed-off-by: Andrii Nakryiko <andrii(a)kernel.org> Link: https://lore.kernel.org/bpf/20220331140949.1410056-1-nborisov@suse.com Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/bpf/progs/profiler.inc.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h index 4896fdf816f7..92331053dba3 100644 --- a/tools/testing/selftests/bpf/progs/profiler.inc.h +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h @@ -826,8 +826,9 @@ int kprobe_ret__do_filp_open(struct pt_regs* ctx) SEC("kprobe/vfs_link") int BPF_KPROBE(kprobe__vfs_link, - struct dentry* old_dentry, struct inode* dir, - struct dentry* new_dentry, struct inode** delegated_inode) + struct dentry* old_dentry, struct user_namespace *mnt_userns, + struct inode* dir, struct dentry* new_dentry, + struct inode** delegated_inode) { struct bpf_func_stats_ctx stats_ctx; bpf_stats_enter(&stats_ctx, profiler_bpf_vfs_link); -- 2.35.1

2 years, 1 month

1
0
0 0

[PATCH net v2] net/ipv6: Expand and rename accept_unsolicited_na to accept_untracked_na

by Arun Ajith S

RFC 9131 changes default behaviour of handling RX of NA messages when the corresponding entry is absent in the neighbour cache. The current implementation is limited to accept just unsolicited NAs. However, the RFC is more generic where it also accepts solicited NAs. Both types should result in adding a STALE entry for this case. Expand accept_untracked_na behaviour to also accept solicited NAs to be compliant with the RFC and rename the sysctl knob to accept_untracked_na. Fixes: f9a2fb73318e ("net/ipv6: Introduce accept_unsolicited_na knob to implement router-side changes for RFC9131") Signed-off-by: Arun Ajith S <aajith(a)arista.com> --- This change updates the accept_unsolicited_na feature that merged to net-next for v5.19 to be better compliant with the RFC. It also involves renaming the sysctl knob to accept_untracked_na before shipping in a release. Note that the behaviour table has been modifed in the code comments, but dropped from the Documentation. This is because the table documents behaviour that is not unique to the knob, and it is more relevant to understanding the code. The documentation has been updated to be unambiguous even without the table. v2: 1. Changed commit message and subject as suggested. 2. Added Fixes tag. 3. Used en-uk spellings consistently. 4. Added a couple of missing comments. 5. Refactored patch to be smaller by avoiding early return. 6. Made the documentation more clearer. Documentation/networking/ip-sysctl.rst | 23 ++++------- include/linux/ipv6.h | 2 +- include/uapi/linux/ipv6.h | 2 +- net/ipv6/addrconf.c | 6 +-- net/ipv6/ndisc.c | 41 +++++++++++-------- .../net/ndisc_unsolicited_na_test.sh | 23 +++++------ 6 files changed, 49 insertions(+), 48 deletions(-) diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst index b882d4238581..04216564a03c 100644 --- a/Documentation/networking/ip-sysctl.rst +++ b/Documentation/networking/ip-sysctl.rst @@ -2474,21 +2474,16 @@ drop_unsolicited_na - BOOLEAN By default this is turned off. -accept_unsolicited_na - BOOLEAN - Add a new neighbour cache entry in STALE state for routers on receiving an - unsolicited neighbour advertisement with target link-layer address option - specified. This is as per router-side behavior documented in RFC9131. - This has lower precedence than drop_unsolicited_na. +accept_untracked_na - BOOLEAN + Add a new neighbour cache entry in STALE state for routers on receiving a + neighbour advertisement (either solicited or unsolicited) with target + link-layer address option specified if no neighbour entry is already + present for the advertised IPv6 address. Without this knob, NAs received + for untracked addresses (absent in neighbour cache) are silently ignored. + + This is as per router-side behaviour documented in RFC9131. - ==== ====== ====== ============================================== - drop accept fwding behaviour - ---- ------ ------ ---------------------------------------------- - 1 X X Drop NA packet and don't pass up the stack - 0 0 X Pass NA packet up the stack, don't update NC - 0 1 0 Pass NA packet up the stack, don't update NC - 0 1 1 Pass NA packet up the stack, and add a STALE - NC entry - ==== ====== ====== ============================================== + This has lower precedence than drop_unsolicited_na. This will optimize the return path for the initial off-link communication that is initiated by a directly connected host, by ensuring that diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h index 38c8203d52cb..37dfdcfcdd54 100644 --- a/include/linux/ipv6.h +++ b/include/linux/ipv6.h @@ -61,7 +61,7 @@ struct ipv6_devconf { __s32 suppress_frag_ndisc; __s32 accept_ra_mtu; __s32 drop_unsolicited_na; - __s32 accept_unsolicited_na; + __s32 accept_untracked_na; struct ipv6_stable_secret { bool initialized; struct in6_addr secret; diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h index 549ddeaf788b..03cdbe798fe3 100644 --- a/include/uapi/linux/ipv6.h +++ b/include/uapi/linux/ipv6.h @@ -194,7 +194,7 @@ enum { DEVCONF_IOAM6_ID, DEVCONF_IOAM6_ID_WIDE, DEVCONF_NDISC_EVICT_NOCARRIER, - DEVCONF_ACCEPT_UNSOLICITED_NA, + DEVCONF_ACCEPT_UNTRACKED_NA, DEVCONF_MAX }; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index ca0aa744593e..1b1932502e9e 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -5586,7 +5586,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf, array[DEVCONF_IOAM6_ID] = cnf->ioam6_id; array[DEVCONF_IOAM6_ID_WIDE] = cnf->ioam6_id_wide; array[DEVCONF_NDISC_EVICT_NOCARRIER] = cnf->ndisc_evict_nocarrier; - array[DEVCONF_ACCEPT_UNSOLICITED_NA] = cnf->accept_unsolicited_na; + array[DEVCONF_ACCEPT_UNTRACKED_NA] = cnf->accept_untracked_na; } static inline size_t inet6_ifla6_size(void) @@ -7038,8 +7038,8 @@ static const struct ctl_table addrconf_sysctl[] = { .extra2 = (void *)SYSCTL_ONE, }, { - .procname = "accept_unsolicited_na", - .data = &ipv6_devconf.accept_unsolicited_na, + .procname = "accept_untracked_na", + .data = &ipv6_devconf.accept_untracked_na, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c index 254addad0dd3..ed0bbe87e345 100644 --- a/net/ipv6/ndisc.c +++ b/net/ipv6/ndisc.c @@ -979,7 +979,6 @@ static void ndisc_recv_na(struct sk_buff *skb) struct inet6_dev *idev = __in6_dev_get(dev); struct inet6_ifaddr *ifp; struct neighbour *neigh; - bool create_neigh; if (skb->len < sizeof(struct nd_msg)) { ND_PRINTK(2, warn, "NA: packet too short\n"); @@ -1000,7 +999,7 @@ static void ndisc_recv_na(struct sk_buff *skb) /* For some 802.11 wireless deployments (and possibly other networks), * there will be a NA proxy and unsolicitd packets are attacks * and thus should not be accepted. - * drop_unsolicited_na takes precedence over accept_unsolicited_na + * drop_unsolicited_na takes precedence over accept_untracked_na */ if (!msg->icmph.icmp6_solicited && idev && idev->cnf.drop_unsolicited_na) @@ -1041,25 +1040,33 @@ static void ndisc_recv_na(struct sk_buff *skb) in6_ifa_put(ifp); return; } + + neigh = neigh_lookup(&nd_tbl, &msg->target, dev); + /* RFC 9131 updates original Neighbour Discovery RFC 4861. - * An unsolicited NA can now create a neighbour cache entry - * on routers if it has Target LL Address option. + * NAs with Target LL Address option without a corresponding + * entry in the neighbour cache can now create a STALE neighbour + * cache entry on routers. + * + * entry accept fwding solicited behaviour + * ------- ------ ------ --------- ---------------------- + * present X X 0 Set state to STALE + * present X X 1 Set state to REACHABLE + * absent 0 X X Do nothing + * absent 1 0 X Do nothing + * absent 1 1 X Add a new STALE entry * - * drop accept fwding behaviour - * ---- ------ ------ ---------------------------------------------- - * 1 X X Drop NA packet and don't pass up the stack - * 0 0 X Pass NA packet up the stack, don't update NC - * 0 1 0 Pass NA packet up the stack, don't update NC - * 0 1 1 Pass NA packet up the stack, and add a STALE - * NC entry * Note that we don't do a (daddr == all-routers-mcast) check. */ - create_neigh = !msg->icmph.icmp6_solicited && lladdr && - idev && idev->cnf.forwarding && - idev->cnf.accept_unsolicited_na; - neigh = __neigh_lookup(&nd_tbl, &msg->target, dev, create_neigh); + new_state = msg->icmph.icmp6_solicited ? NUD_REACHABLE : NUD_STALE; + if (!neigh && lladdr && + idev && idev->cnf.forwarding && + idev->cnf.accept_untracked_na) { + neigh = neigh_create(&nd_tbl, &msg->target, dev); + new_state = NUD_STALE; + } - if (neigh) { + if (neigh && !IS_ERR(neigh)) { u8 old_flags = neigh->flags; struct net *net = dev_net(dev); @@ -1079,7 +1086,7 @@ static void ndisc_recv_na(struct sk_buff *skb) } ndisc_update(dev, neigh, lladdr, - msg->icmph.icmp6_solicited ? NUD_REACHABLE : NUD_STALE, + new_state, NEIGH_UPDATE_F_WEAK_OVERRIDE| (msg->icmph.icmp6_override ? NEIGH_UPDATE_F_OVERRIDE : 0)| NEIGH_UPDATE_F_OVERRIDE_ISROUTER| diff --git a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh index f508657ee126..86e621b7b9c7 100755 --- a/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh +++ b/tools/testing/selftests/net/ndisc_unsolicited_na_test.sh @@ -1,15 +1,14 @@ #!/bin/bash # SPDX-License-Identifier: GPL-2.0 -# This test is for the accept_unsolicited_na feature to +# This test is for the accept_untracked_na feature to # enable RFC9131 behaviour. The following is the test-matrix. # drop accept fwding behaviour # ---- ------ ------ ---------------------------------------------- -# 1 X X Drop NA packet and don't pass up the stack -# 0 0 X Pass NA packet up the stack, don't update NC -# 0 1 0 Pass NA packet up the stack, don't update NC -# 0 1 1 Pass NA packet up the stack, and add a STALE -# NC entry +# 1 X X Don't update NC +# 0 0 X Don't update NC +# 0 1 0 Don't update NC +# 0 1 1 Add a STALE NC entry ret=0 # Kselftest framework requirement - SKIP code is 4. @@ -72,7 +71,7 @@ setup() set -e local drop_unsolicited_na=$1 - local accept_unsolicited_na=$2 + local accept_untracked_na=$2 local forwarding=$3 # Setup two namespaces and a veth tunnel across them. @@ -93,7 +92,7 @@ setup() ${IP_ROUTER_EXEC} sysctl -qw \ ${ROUTER_CONF}.drop_unsolicited_na=${drop_unsolicited_na} ${IP_ROUTER_EXEC} sysctl -qw \ - ${ROUTER_CONF}.accept_unsolicited_na=${accept_unsolicited_na} + ${ROUTER_CONF}.accept_untracked_na=${accept_untracked_na} ${IP_ROUTER_EXEC} sysctl -qw ${ROUTER_CONF}.disable_ipv6=0 ${IP_ROUTER} addr add ${ROUTER_ADDR_WITH_MASK} dev ${ROUTER_INTF} @@ -144,13 +143,13 @@ link_up() { verify_ndisc() { local drop_unsolicited_na=$1 - local accept_unsolicited_na=$2 + local accept_untracked_na=$2 local forwarding=$3 neigh_show_output=$(${IP_ROUTER} neigh show \ to ${HOST_ADDR} dev ${ROUTER_INTF} nud stale) if [ ${drop_unsolicited_na} -eq 0 ] && \ - [ ${accept_unsolicited_na} -eq 1 ] && \ + [ ${accept_untracked_na} -eq 1 ] && \ [ ${forwarding} -eq 1 ]; then # Neighbour entry expected to be present for 011 case [[ ${neigh_show_output} ]] @@ -179,14 +178,14 @@ test_unsolicited_na_combination() { test_unsolicited_na_common $1 $2 $3 test_msg=("test_unsolicited_na: " "drop_unsolicited_na=$1 " - "accept_unsolicited_na=$2 " + "accept_untracked_na=$2 " "forwarding=$3") log_test $? 0 "${test_msg[*]}" cleanup } test_unsolicited_na_combinations() { - # Args: drop_unsolicited_na accept_unsolicited_na forwarding + # Args: drop_unsolicited_na accept_untracked_na forwarding # Expect entry test_unsolicited_na_combination 0 1 1 -- 2.27.0

2 years, 1 month

1
0
0 0

kbuild problem: ERROR: modpost: missing MODULE_LICENSE() in drivers/iio/afe/iio-rescale.o

by Randy Dunlap

Hi, In March I reported that a randconfig build complained: ERROR: modpost: missing MODULE_LICENSE() in drivers/iio/afe/iio-rescale.o (https://lore.kernel.org/all/16509fb6-e40c-e31b-2c80-264c44b0beb9@infradead.…) I am still seeing this problem so I tried to dig into it a bit. However, I don't see why get_next_modinfo() and friends don't find the MODULE_LICENSE() since it is in the iio-rescale.o file. (BTW, I see this build error on many different $ARCH [around 15 tested] and with 2 different versions of GCC.) Q1: Is modpost checking both vmlinux and iio-rescale.o for modinfo license strings? It looks like it is, because it appears (?) that modpost is looking at drivers/iio/test/iio-test-rescale.o (<<<<< a kunit test, which is builtin in my .config) and at drivers/iio/afe/iio-rescale.o (which is built as a loadable module). Is this confusing modpost? I renamed drivers/iio/afe/iio-rescale.c to afe-rescale.c and changed its Makefile entry accordingly and the MODULE_LICENSE error goes away. Is this a modpost error or is kunit messing things up? thanks for looking. -- ~Randy

2 years, 1 month

3
12
0 0

[PATCH seccomp] selftests/seccomp: Fix compile warning when CC=clang

by YiFei Zhu

clang has -Wconstant-conversion by default, and the constant 0xAAAAAAAAA (9 As) being converted to an int, which is generally 32 bits, results in the compile warning: clang -Wl,-no-as-needed -Wall -isystem ../../../../usr/include/ -lpthread seccomp_bpf.c -lcap -o seccomp_bpf seccomp_bpf.c:812:67: warning: implicit conversion from 'long' to 'int' changes value from 45812984490 to -1431655766 [-Wconstant-conversion] int kill = kill_how == KILL_PROCESS ? SECCOMP_RET_KILL_PROCESS : 0xAAAAAAAAA; ~~~~ ^~~~~~~~~~~ 1 warning generated. -1431655766 is the expected truncation, 0xAAAAAAAA (8 As), so use this directly in the code to avoid the warning. Fixes: 3932fcecd962 ("selftests/seccomp: Add test for unknown SECCOMP_RET kill behavior") Signed-off-by: YiFei Zhu <zhuyifei(a)google.com> --- tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 136df5b76319..4ae6c8991307 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -809,7 +809,7 @@ void kill_thread_or_group(struct __test_metadata *_metadata, .len = (unsigned short)ARRAY_SIZE(filter_thread), .filter = filter_thread, }; - int kill = kill_how == KILL_PROCESS ? SECCOMP_RET_KILL_PROCESS : 0xAAAAAAAAA; + int kill = kill_how == KILL_PROCESS ? SECCOMP_RET_KILL_PROCESS : 0xAAAAAAAA; struct sock_filter filter_process[] = { BPF_STMT(BPF_LD|BPF_W|BPF_ABS, offsetof(struct seccomp_data, nr)), -- 2.36.1.124.g0e6072fb45-goog

2 years, 2 months

2
1
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror May 2022