* Re: general protection fault in tls_trim_both_msgs
From: syzbot @ 2019-07-28 3:46 UTC (permalink / raw)
To: ast, aviadye, borisp, bpf, corbet, daniel, davejwatson, davem,
jakub.kicinski, john.fastabend, kafai, linux-doc, linux-kernel,
netdev, songliubraving, syzkaller-bugs, yhs
In-Reply-To: <0000000000002b4896058e7abf78@google.com>
syzbot has bisected this bug to:
commit 32857cf57f920cdc03b5095f08febec94cf9c36b
Author: John Fastabend <john.fastabend@gmail.com>
Date: Fri Jul 19 17:29:18 2019 +0000
net/tls: fix transition through disconnect with close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=155064d8600000
start commit: fde50b96 Add linux-next specific files for 20190726
git tree: linux-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=175064d8600000
console output: https://syzkaller.appspot.com/x/log.txt?x=135064d8600000
kernel config: https://syzkaller.appspot.com/x/.config?x=4b58274564b354c1
dashboard link: https://syzkaller.appspot.com/bug?extid=0e0fedcad708d12d3032
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14779d64600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1587c842600000
Reported-by: syzbot+0e0fedcad708d12d3032@syzkaller.appspotmail.com
Fixes: 32857cf57f92 ("net/tls: fix transition through disconnect with
close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* [PATCH v3 bpf-next 6/9] selftests/bpf: abstract away test log output
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
This patch changes how test output is printed out. By default, if test
had no errors, the only output will be a single line with test number,
name, and verdict at the end, e.g.:
#31 xdp:OK
If test had any errors, all log output captured during test execution
will be output after test completes.
It's possible to force output of log with `-v` (`--verbose`) option, in
which case output won't be buffered and will be output immediately.
To support this, individual tests are required to use helper methods for
logging: `test__printf()` and `test__vprintf()`.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../selftests/bpf/prog_tests/bpf_obj_id.c | 6 +-
.../bpf/prog_tests/bpf_verif_scale.c | 31 ++--
.../bpf/prog_tests/get_stack_raw_tp.c | 4 +-
.../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../selftests/bpf/prog_tests/map_lock.c | 10 +-
.../selftests/bpf/prog_tests/send_signal.c | 8 +-
.../selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 +-
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 3 +-
tools/testing/selftests/bpf/test_progs.c | 145 ++++++++++++++----
tools/testing/selftests/bpf/test_progs.h | 37 ++++-
12 files changed, 183 insertions(+), 73 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
index cb827383db4d..fb5840a62548 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -106,8 +106,8 @@ void test_bpf_obj_id(void)
if (CHECK(err ||
prog_infos[i].type != BPF_PROG_TYPE_SOCKET_FILTER ||
info_len != sizeof(struct bpf_prog_info) ||
- (jit_enabled && !prog_infos[i].jited_prog_len) ||
- (jit_enabled &&
+ (env.jit_enabled && !prog_infos[i].jited_prog_len) ||
+ (env.jit_enabled &&
!memcmp(jited_insns, zeros, sizeof(zeros))) ||
!prog_infos[i].xlated_prog_len ||
!memcmp(xlated_insns, zeros, sizeof(zeros)) ||
@@ -121,7 +121,7 @@ void test_bpf_obj_id(void)
err, errno, i,
prog_infos[i].type, BPF_PROG_TYPE_SOCKET_FILTER,
info_len, sizeof(struct bpf_prog_info),
- jit_enabled,
+ env.jit_enabled,
prog_infos[i].jited_prog_len,
prog_infos[i].xlated_prog_len,
!!memcmp(jited_insns, zeros, sizeof(zeros)),
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index ceddb8cc86f4..b59017279e0b 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -4,12 +4,15 @@
static int libbpf_debug_print(enum libbpf_print_level level,
const char *format, va_list args)
{
- if (level != LIBBPF_DEBUG)
- return vfprintf(stderr, format, args);
+ if (level != LIBBPF_DEBUG) {
+ test__vprintf(format, args);
+ return 0;
+ }
if (!strstr(format, "verifier log"))
return 0;
- return vfprintf(stderr, "%s", args);
+ test__vprintf("%s", args);
+ return 0;
}
static int check_load(const char *file, enum bpf_prog_type type)
@@ -73,32 +76,38 @@ void test_bpf_verif_scale(void)
libbpf_print_fn_t old_print_fn = NULL;
int err, i;
- if (verifier_stats)
+ if (env.verifier_stats) {
+ test__force_log();
old_print_fn = libbpf_set_print(libbpf_debug_print);
+ }
err = check_load("./loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT);
- printf("test_scale:loop3:%s\n", err ? (error_cnt--, "OK") : "FAIL");
+ test__printf("test_scale:loop3:%s\n",
+ err ? (error_cnt--, "OK") : "FAIL");
for (i = 0; i < ARRAY_SIZE(sched_cls); i++) {
err = check_load(sched_cls[i], BPF_PROG_TYPE_SCHED_CLS);
- printf("test_scale:%s:%s\n", sched_cls[i], err ? "FAIL" : "OK");
+ test__printf("test_scale:%s:%s\n", sched_cls[i],
+ err ? "FAIL" : "OK");
}
for (i = 0; i < ARRAY_SIZE(raw_tp); i++) {
err = check_load(raw_tp[i], BPF_PROG_TYPE_RAW_TRACEPOINT);
- printf("test_scale:%s:%s\n", raw_tp[i], err ? "FAIL" : "OK");
+ test__printf("test_scale:%s:%s\n", raw_tp[i],
+ err ? "FAIL" : "OK");
}
for (i = 0; i < ARRAY_SIZE(cg_sysctl); i++) {
err = check_load(cg_sysctl[i], BPF_PROG_TYPE_CGROUP_SYSCTL);
- printf("test_scale:%s:%s\n", cg_sysctl[i], err ? "FAIL" : "OK");
+ test__printf("test_scale:%s:%s\n", cg_sysctl[i],
+ err ? "FAIL" : "OK");
}
err = check_load("./test_xdp_loop.o", BPF_PROG_TYPE_XDP);
- printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
+ test__printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
err = check_load("./test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL);
- printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
+ test__printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
- if (verifier_stats)
+ if (env.verifier_stats)
libbpf_set_print(old_print_fn);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
index 9d73a8f932ac..3d59b3c841fe 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c
@@ -41,7 +41,7 @@ static void get_stack_print_output(void *ctx, int cpu, void *data, __u32 size)
* just assume it is good if the stack is not empty.
* This could be improved in the future.
*/
- if (jit_enabled) {
+ if (env.jit_enabled) {
found = num_stack > 0;
} else {
for (i = 0; i < num_stack; i++) {
@@ -58,7 +58,7 @@ static void get_stack_print_output(void *ctx, int cpu, void *data, __u32 size)
}
} else {
num_stack = e->kern_stack_size / sizeof(__u64);
- if (jit_enabled) {
+ if (env.jit_enabled) {
good_kern_stack = num_stack > 0;
} else {
for (i = 0; i < num_stack; i++) {
diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
index 20ddca830e68..5ce572c03a5f 100644
--- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
+++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
@@ -74,7 +74,7 @@ static void test_l4lb(const char *file)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
+ test__printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index ee99368c595c..2e78217ed3fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -9,12 +9,12 @@ static void *parallel_map_access(void *arg)
for (i = 0; i < 10000; i++) {
err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
if (err) {
- printf("lookup failed\n");
+ test__printf("lookup failed\n");
error_cnt++;
goto out;
}
if (vars[0] != 0) {
- printf("lookup #%d var[0]=%d\n", i, vars[0]);
+ test__printf("lookup #%d var[0]=%d\n", i, vars[0]);
error_cnt++;
goto out;
}
@@ -22,8 +22,8 @@ static void *parallel_map_access(void *arg)
for (j = 2; j < 17; j++) {
if (vars[j] == rnd)
continue;
- printf("lookup #%d var[1]=%d var[%d]=%d\n",
- i, rnd, j, vars[j]);
+ test__printf("lookup #%d var[1]=%d var[%d]=%d\n",
+ i, rnd, j, vars[j]);
error_cnt++;
goto out;
}
@@ -43,7 +43,7 @@ void test_map_lock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- printf("test_map_lock:bpf_prog_load errno %d\n", errno);
+ test__printf("test_map_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
map_fd[0] = bpf_find_map(__func__, obj, "hash_map");
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 54218ee3c004..d950f4558897 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -202,8 +202,8 @@ static int test_send_signal_nmi(void)
-1 /* cpu */, -1 /* group_fd */, 0 /* flags */);
if (pmu_fd == -1) {
if (errno == ENOENT) {
- printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
- __func__);
+ test__printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
+ __func__);
return 0;
}
/* Let the test fail with a more informative message */
@@ -222,8 +222,4 @@ void test_send_signal(void)
ret |= test_send_signal_tracepoint();
ret |= test_send_signal_perf();
ret |= test_send_signal_nmi();
- if (!ret)
- printf("test_send_signal:OK\n");
- else
- printf("test_send_signal:FAIL\n");
}
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index 114ebe6a438e..deb2db5b85b0 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -12,7 +12,7 @@ void test_spinlock(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (err) {
- printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
+ test__printf("test_spin_lock:bpf_prog_load errno %d\n", errno);
goto close_prog;
}
for (i = 0; i < 4; i++)
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index ac44fda84833..356d2c017a9c 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -109,8 +109,8 @@ void test_stacktrace_build_id(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
index 9557b7dfb782..f44f2c159714 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
@@ -140,8 +140,8 @@ void test_stacktrace_build_id_nmi(void)
if (build_id_matches < 1 && retry--) {
bpf_link__destroy(link);
bpf_object__close(obj);
- printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
- __func__);
+ test__printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
+ __func__);
goto retry;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
index 09e6b46f5515..b5404494b8aa 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
@@ -75,7 +75,8 @@ void test_xdp_noinline(void)
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
error_cnt++;
- printf("test_xdp_noinline:FAIL:stats %lld %lld\n", bytes, pkts);
+ test__printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
+ bytes, pkts);
}
out:
bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 94b6951b90b3..1b7470d3da22 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -6,9 +6,85 @@
#include <argp.h>
#include <string.h>
+/* defined in test_progs.h */
+struct test_env env = {
+ .test_num_selector = -1,
+};
int error_cnt, pass_cnt;
-bool jit_enabled;
-bool verifier_stats = false;
+
+struct prog_test_def {
+ const char *test_name;
+ int test_num;
+ void (*run_test)(void);
+ bool force_log;
+ int pass_cnt;
+ int error_cnt;
+ bool tested;
+};
+
+void test__force_log() {
+ env.test->force_log = true;
+}
+
+void test__vprintf(const char *fmt, va_list args)
+{
+ size_t rem_sz;
+ int ret = 0;
+
+ if (env.verbose || (env.test && env.test->force_log)) {
+ vfprintf(stderr, fmt, args);
+ return;
+ }
+
+try_again:
+ rem_sz = env.log_cap - env.log_cnt;
+ if (rem_sz) {
+ va_list ap;
+
+ va_copy(ap, args);
+ /* we reserved extra byte for \0 at the end */
+ ret = vsnprintf(env.log_buf + env.log_cnt, rem_sz + 1, fmt, ap);
+ va_end(ap);
+
+ if (ret < 0) {
+ env.log_buf[env.log_cnt] = '\0';
+ fprintf(stderr, "failed to log w/ fmt '%s'\n", fmt);
+ return;
+ }
+ }
+
+ if (!rem_sz || ret > rem_sz) {
+ size_t new_sz = env.log_cap * 3 / 2;
+ char *new_buf;
+
+ if (new_sz < 4096)
+ new_sz = 4096;
+ if (new_sz < ret + env.log_cnt)
+ new_sz = ret + env.log_cnt;
+
+ /* +1 for guaranteed space for terminating \0 */
+ new_buf = realloc(env.log_buf, new_sz + 1);
+ if (!new_buf) {
+ fprintf(stderr, "failed to realloc log buffer: %d\n",
+ errno);
+ return;
+ }
+ env.log_buf = new_buf;
+ env.log_cap = new_sz;
+ goto try_again;
+ }
+
+ env.log_cnt += ret;
+}
+
+void test__printf(const char *fmt, ...)
+{
+ va_list args;
+
+ va_start(args, fmt);
+ test__vprintf(fmt, args);
+ va_end(args);
+}
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
@@ -163,20 +239,15 @@ void *spin_lock_thread(void *arg)
#include <prog_tests/tests.h>
#undef DEFINE_TEST
-struct prog_test_def {
- const char *test_name;
- int test_num;
- void (*run_test)(void);
-};
-
static struct prog_test_def prog_test_defs[] = {
-#define DEFINE_TEST(name) { \
- .test_name = #name, \
- .run_test = &test_##name, \
+#define DEFINE_TEST(name) { \
+ .test_name = #name, \
+ .run_test = &test_##name, \
},
#include <prog_tests/tests.h>
#undef DEFINE_TEST
};
+const int prog_test_cnt = ARRAY_SIZE(prog_test_defs);
const char *argp_program_version = "test_progs 0.1";
const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
@@ -186,7 +257,6 @@ enum ARG_KEYS {
ARG_TEST_NUM = 'n',
ARG_TEST_NAME = 't',
ARG_VERIFIER_STATS = 's',
-
ARG_VERBOSE = 'v',
};
@@ -202,24 +272,13 @@ static const struct argp_option opts[] = {
{},
};
-struct test_env {
- int test_num_selector;
- const char *test_name_selector;
- bool verifier_stats;
- bool verbose;
- bool very_verbose;
-};
-
-static struct test_env env = {
- .test_num_selector = -1,
-};
-
static int libbpf_print_fn(enum libbpf_print_level level,
const char *format, va_list args)
{
if (!env.very_verbose && level == LIBBPF_DEBUG)
return 0;
- return vfprintf(stderr, format, args);
+ test__vprintf(format, args);
+ return 0;
}
static error_t parse_arg(int key, char *arg, struct argp_state *state)
@@ -267,7 +326,6 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
return 0;
}
-
int main(int argc, char **argv)
{
static const struct argp argp = {
@@ -275,7 +333,6 @@ int main(int argc, char **argv)
.parser = parse_arg,
.doc = argp_program_doc,
};
- struct prog_test_def *test;
int err, i;
err = argp_parse(&argp, argc, argv, 0, NULL, &env);
@@ -286,13 +343,14 @@ int main(int argc, char **argv)
srand(time(NULL));
- jit_enabled = is_jit_enabled();
+ env.jit_enabled = is_jit_enabled();
- verifier_stats = env.verifier_stats;
-
- for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
- test = &prog_test_defs[i];
+ for (i = 0; i < prog_test_cnt; i++) {
+ struct prog_test_def *test = &prog_test_defs[i];
+ int old_pass_cnt = pass_cnt;
+ int old_error_cnt = error_cnt;
+ env.test = test;
test->test_num = i + 1;
if (env.test_num_selector >= 0 &&
@@ -303,8 +361,29 @@ int main(int argc, char **argv)
continue;
test->run_test();
+ test->tested = true;
+ test->pass_cnt = pass_cnt - old_pass_cnt;
+ test->error_cnt = error_cnt - old_error_cnt;
+ if (test->error_cnt)
+ env.fail_cnt++;
+ else
+ env.succ_cnt++;
+
+ if (env.verbose || test->force_log || test->error_cnt) {
+ if (env.log_cnt) {
+ fprintf(stdout, "%s", env.log_buf);
+ if (env.log_buf[env.log_cnt - 1] != '\n')
+ fprintf(stdout, "\n");
+ }
+ }
+ env.log_cnt = 0;
+
+ printf("#%d %s:%s\n", test->test_num, test->test_name,
+ test->error_cnt ? "FAIL" : "OK");
}
+ printf("Summary: %d PASSED, %d FAILED\n", env.succ_cnt, env.fail_cnt);
+
+ free(env.log_buf);
- printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 49e0f7d85643..62f55a4231e9 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -38,9 +38,33 @@ typedef __u16 __sum16;
#include "trace_helpers.h"
#include "flow_dissector_load.h"
-extern int error_cnt, pass_cnt;
-extern bool jit_enabled;
-extern bool verifier_stats;
+struct prog_test_def;
+
+struct test_env {
+ int test_num_selector;
+ const char *test_name_selector;
+ bool verifier_stats;
+ bool verbose;
+ bool very_verbose;
+
+ bool jit_enabled;
+
+ struct prog_test_def *test;
+ char *log_buf;
+ size_t log_cnt;
+ size_t log_cap;
+
+ int succ_cnt;
+ int fail_cnt;
+};
+
+extern int error_cnt;
+extern int pass_cnt;
+extern struct test_env env;
+
+extern void test__printf(const char *fmt, ...);
+extern void test__vprintf(const char *fmt, va_list args);
+extern void test__force_log();
#define MAGIC_BYTES 123
@@ -64,11 +88,12 @@ extern struct ipv6_packet pkt_v6;
int __ret = !!(condition); \
if (__ret) { \
error_cnt++; \
- printf("%s:FAIL:%s ", __func__, tag); \
- printf(format); \
+ test__printf("%s:FAIL:%s ", __func__, tag); \
+ test__printf(format); \
} else { \
pass_cnt++; \
- printf("%s:PASS:%s %d nsec\n", __func__, tag, duration);\
+ test__printf("%s:PASS:%s %d nsec\n", \
+ __func__, tag, duration); \
} \
__ret; \
})
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 9/9] selftests/bpf: convert send_signal.c to use subtests
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Convert send_signal set of tests to be exposed as three sub-tests.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/prog_tests/send_signal.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index d950f4558897..461b423d0584 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -219,7 +219,10 @@ void test_send_signal(void)
{
int ret = 0;
- ret |= test_send_signal_tracepoint();
- ret |= test_send_signal_perf();
- ret |= test_send_signal_nmi();
+ if (test__start_subtest("send_signal_tracepoint"))
+ ret |= test_send_signal_tracepoint();
+ if (test__start_subtest("send_signal_perf"))
+ ret |= test_send_signal_perf();
+ if (test__start_subtest("send_signal_nmi"))
+ ret |= test_send_signal_nmi();
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 8/9] selftests/bpf: convert bpf_verif_scale.c to sub-tests API
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Expose each BPF verifier scale test as individual sub-test to allow
independent results output and test selection.
Test run results now look like this:
$ sudo ./test_progs -t verif/
#3/1 loop3.o:OK
#3/2 test_verif_scale1.o:OK
#3/3 test_verif_scale2.o:OK
#3/4 test_verif_scale3.o:OK
#3/5 pyperf50.o:OK
#3/6 pyperf100.o:OK
#3/7 pyperf180.o:OK
#3/8 pyperf600.o:OK
#3/9 pyperf600_nounroll.o:OK
#3/10 loop1.o:OK
#3/11 loop2.o:OK
#3/12 strobemeta.o:OK
#3/13 strobemeta_nounroll1.o:OK
#3/14 strobemeta_nounroll2.o:OK
#3/15 test_sysctl_loop1.o:OK
#3/16 test_sysctl_loop2.o:OK
#3/17 test_xdp_loop.o:OK
#3/18 test_seg6_loop.o:OK
#3 bpf_verif_scale:OK
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../bpf/prog_tests/bpf_verif_scale.c | 77 ++++++++++---------
1 file changed, 40 insertions(+), 37 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index b59017279e0b..b4be96162ff4 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -33,14 +33,25 @@ static int check_load(const char *file, enum bpf_prog_type type)
return err;
}
+struct scale_test_def {
+ const char *file;
+ enum bpf_prog_type attach_type;
+ bool fails;
+};
+
void test_bpf_verif_scale(void)
{
- const char *sched_cls[] = {
- "./test_verif_scale1.o", "./test_verif_scale2.o", "./test_verif_scale3.o",
- };
- const char *raw_tp[] = {
+ struct scale_test_def tests[] = {
+ { "loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT, true /* fails */ },
+
+ { "test_verif_scale1.o", BPF_PROG_TYPE_SCHED_CLS },
+ { "test_verif_scale2.o", BPF_PROG_TYPE_SCHED_CLS },
+ { "test_verif_scale3.o", BPF_PROG_TYPE_SCHED_CLS },
+
/* full unroll by llvm */
- "./pyperf50.o", "./pyperf100.o", "./pyperf180.o",
+ { "pyperf50.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "pyperf100.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "pyperf180.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. llvm will unroll loop ~150 times.
* C loop count -> 600.
@@ -48,7 +59,7 @@ void test_bpf_verif_scale(void)
* 16k insns in loop body.
* Total of 5 such loops. Total program size ~82k insns.
*/
- "./pyperf600.o",
+ { "pyperf600.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* no unroll at all.
* C loop count -> 600.
@@ -56,22 +67,26 @@ void test_bpf_verif_scale(void)
* ~110 insns in loop body.
* Total of 5 such loops. Total program size ~1500 insns.
*/
- "./pyperf600_nounroll.o",
+ { "pyperf600_nounroll.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
- "./loop1.o", "./loop2.o",
+ { "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* partial unroll. 19k insn in a loop.
* Total program size 20.8k insn.
* ~350k processed_insns
*/
- "./strobemeta.o",
+ { "strobemeta.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
/* no unroll, tiny loops */
- "./strobemeta_nounroll1.o",
- "./strobemeta_nounroll2.o",
- };
- const char *cg_sysctl[] = {
- "./test_sysctl_loop1.o", "./test_sysctl_loop2.o",
+ { "strobemeta_nounroll1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+ { "strobemeta_nounroll2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
+
+ { "test_sysctl_loop1.o", BPF_PROG_TYPE_CGROUP_SYSCTL },
+ { "test_sysctl_loop2.o", BPF_PROG_TYPE_CGROUP_SYSCTL },
+
+ { "test_xdp_loop.o", BPF_PROG_TYPE_XDP },
+ { "test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL },
};
libbpf_print_fn_t old_print_fn = NULL;
int err, i;
@@ -81,33 +96,21 @@ void test_bpf_verif_scale(void)
old_print_fn = libbpf_set_print(libbpf_debug_print);
}
- err = check_load("./loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT);
- test__printf("test_scale:loop3:%s\n",
- err ? (error_cnt--, "OK") : "FAIL");
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ const struct scale_test_def *test = &tests[i];
- for (i = 0; i < ARRAY_SIZE(sched_cls); i++) {
- err = check_load(sched_cls[i], BPF_PROG_TYPE_SCHED_CLS);
- test__printf("test_scale:%s:%s\n", sched_cls[i],
- err ? "FAIL" : "OK");
- }
+ if (!test__start_subtest(test->file))
+ continue;
- for (i = 0; i < ARRAY_SIZE(raw_tp); i++) {
- err = check_load(raw_tp[i], BPF_PROG_TYPE_RAW_TRACEPOINT);
- test__printf("test_scale:%s:%s\n", raw_tp[i],
- err ? "FAIL" : "OK");
+ err = check_load(test->file, test->attach_type);
+ if (test->fails) { /* expected to fail */
+ if (err)
+ error_cnt--;
+ else
+ error_cnt++;
+ }
}
- for (i = 0; i < ARRAY_SIZE(cg_sysctl); i++) {
- err = check_load(cg_sysctl[i], BPF_PROG_TYPE_CGROUP_SYSCTL);
- test__printf("test_scale:%s:%s\n", cg_sysctl[i],
- err ? "FAIL" : "OK");
- }
- err = check_load("./test_xdp_loop.o", BPF_PROG_TYPE_XDP);
- test__printf("test_scale:test_xdp_loop:%s\n", err ? "FAIL" : "OK");
-
- err = check_load("./test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL);
- test__printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
-
if (env.verifier_stats)
libbpf_set_print(old_print_fn);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 7/9] selftests/bpf: add sub-tests support for test_progs
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Allow tests to have their own set of sub-tests. Also add ability to do
test/subtest selection using `-t <test-name>/<subtest-name>` and `-n
<test-nums-set>/<subtest-nums-set>`, as an extension of existing -t/-n
selector options. For the <test-num-set> format: it's a comma-separated
list of either individual test numbers (1-based), or range of test
numbers. E.g., all of the following are valid sets of test numbers:
- 10
- 1,2,3
- 1-3
- 5-10,1,3-4
'/<subtest' part is optional, but has the same format. E.g., to select
test #3 and its sub-tests #10 through #15, use: -t 3/10-15.
Similarly, to select tests by name, use `-t verif/strobe`:
$ sudo ./test_progs -t verif/strobe
#3/12 strobemeta.o:OK
#3/13 strobemeta_nounroll1.o:OK
#3/14 strobemeta_nounroll2.o:OK
#3 bpf_verif_scale:OK
Summary: 1/3 PASSED, 0 FAILED
Example of using subtest API is in the next patch, converting
bpf_verif_scale.c tests to use sub-tests.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/test_progs.c | 198 ++++++++++++++++++++---
tools/testing/selftests/bpf/test_progs.h | 16 +-
2 files changed, 185 insertions(+), 29 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 1b7470d3da22..546d99b3ec34 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -7,9 +7,7 @@
#include <string.h>
/* defined in test_progs.h */
-struct test_env env = {
- .test_num_selector = -1,
-};
+struct test_env env;
int error_cnt, pass_cnt;
struct prog_test_def {
@@ -20,8 +18,82 @@ struct prog_test_def {
int pass_cnt;
int error_cnt;
bool tested;
+
+ const char *subtest_name;
+ int subtest_num;
+
+ /* store counts before subtest started */
+ int old_pass_cnt;
+ int old_error_cnt;
};
+static bool should_run(struct test_selector *sel, int num, const char *name)
+{
+ if (sel->name && sel->name[0] && !strstr(name, sel->name))
+ return false;
+
+ if (!sel->num_set)
+ return true;
+
+ return num < sel->num_set_len && sel->num_set[num];
+}
+
+static void dump_test_log(const struct prog_test_def *test, bool failed)
+{
+ if (env.verbose || test->force_log || failed) {
+ if (env.log_cnt) {
+ fprintf(stdout, "%s", env.log_buf);
+ if (env.log_buf[env.log_cnt - 1] != '\n')
+ fprintf(stdout, "\n");
+ }
+ env.log_cnt = 0;
+ }
+}
+
+void test__end_subtest()
+{
+ struct prog_test_def *test = env.test;
+ int sub_error_cnt = error_cnt - test->old_error_cnt;
+
+ if (sub_error_cnt)
+ env.fail_cnt++;
+ else
+ env.sub_succ_cnt++;
+
+ dump_test_log(test, sub_error_cnt);
+
+ printf("#%d/%d %s:%s\n",
+ test->test_num, test->subtest_num,
+ test->subtest_name, sub_error_cnt ? "FAIL" : "OK");
+}
+
+bool test__start_subtest(const char *name)
+{
+ struct prog_test_def *test = env.test;
+
+ if (test->subtest_name) {
+ test__end_subtest();
+ test->subtest_name = NULL;
+ }
+
+ test->subtest_num++;
+
+ if (!name || !name[0]) {
+ fprintf(stderr, "Subtest #%d didn't provide sub-test name!\n",
+ test->subtest_num);
+ return false;
+ }
+
+ if (!should_run(&env.subtest_selector, test->subtest_num, name))
+ return false;
+
+ test->subtest_name = name;
+ env.test->old_pass_cnt = pass_cnt;
+ env.test->old_error_cnt = error_cnt;
+
+ return true;
+}
+
void test__force_log() {
env.test->force_log = true;
}
@@ -281,24 +353,103 @@ static int libbpf_print_fn(enum libbpf_print_level level,
return 0;
}
+int parse_num_list(const char *s, struct test_selector *sel)
+{
+ int i, set_len = 0, num, start = 0, end = -1;
+ bool *set = NULL, *tmp, parsing_end = false;
+ char *next;
+
+ while (s[0]) {
+ errno = 0;
+ num = strtol(s, &next, 10);
+ if (errno)
+ return -errno;
+
+ if (parsing_end)
+ end = num;
+ else
+ start = num;
+
+ if (!parsing_end && *next == '-') {
+ s = next + 1;
+ parsing_end = true;
+ continue;
+ } else if (*next == ',') {
+ parsing_end = false;
+ s = next + 1;
+ end = num;
+ } else if (*next == '\0') {
+ parsing_end = false;
+ s = next;
+ end = num;
+ } else {
+ return -EINVAL;
+ }
+
+ if (start > end)
+ return -EINVAL;
+
+ if (end + 1 > set_len) {
+ set_len = end + 1;
+ tmp = realloc(set, set_len);
+ if (!tmp) {
+ free(set);
+ return -ENOMEM;
+ }
+ set = tmp;
+ }
+ for (i = start; i <= end; i++) {
+ set[i] = true;
+ }
+
+ }
+
+ if (!set)
+ return -EINVAL;
+
+ sel->num_set = set;
+ sel->num_set_len = set_len;
+
+ return 0;
+}
+
static error_t parse_arg(int key, char *arg, struct argp_state *state)
{
struct test_env *env = state->input;
switch (key) {
case ARG_TEST_NUM: {
- int test_num;
+ char *subtest_str = strchr(arg, '/');
- errno = 0;
- test_num = strtol(arg, NULL, 10);
- if (errno)
- return -errno;
- env->test_num_selector = test_num;
+ if (subtest_str) {
+ *subtest_str = '\0';
+ if (parse_num_list(subtest_str + 1,
+ &env->subtest_selector)) {
+ fprintf(stderr,
+ "Failed to parse subtest numbers.\n");
+ return -EINVAL;
+ }
+ }
+ if (parse_num_list(arg, &env->test_selector)) {
+ fprintf(stderr, "Failed to parse test numbers.\n");
+ return -EINVAL;
+ }
break;
}
- case ARG_TEST_NAME:
- env->test_name_selector = arg;
+ case ARG_TEST_NAME: {
+ char *subtest_str = strchr(arg, '/');
+
+ if (subtest_str) {
+ *subtest_str = '\0';
+ env->subtest_selector.name = strdup(subtest_str + 1);
+ if (!env->subtest_selector.name)
+ return -ENOMEM;
+ }
+ env->test_selector.name = strdup(arg);
+ if (!env->test_selector.name)
+ return -ENOMEM;
break;
+ }
case ARG_VERIFIER_STATS:
env->verifier_stats = true;
break;
@@ -353,14 +504,15 @@ int main(int argc, char **argv)
env.test = test;
test->test_num = i + 1;
- if (env.test_num_selector >= 0 &&
- test->test_num != env.test_num_selector)
- continue;
- if (env.test_name_selector &&
- !strstr(test->test_name, env.test_name_selector))
+ if (!should_run(&env.test_selector,
+ test->test_num, test->test_name))
continue;
test->run_test();
+ /* ensure last sub-test is finalized properly */
+ if (test->subtest_name)
+ test__end_subtest();
+
test->tested = true;
test->pass_cnt = pass_cnt - old_pass_cnt;
test->error_cnt = error_cnt - old_error_cnt;
@@ -369,21 +521,17 @@ int main(int argc, char **argv)
else
env.succ_cnt++;
- if (env.verbose || test->force_log || test->error_cnt) {
- if (env.log_cnt) {
- fprintf(stdout, "%s", env.log_buf);
- if (env.log_buf[env.log_cnt - 1] != '\n')
- fprintf(stdout, "\n");
- }
- }
- env.log_cnt = 0;
+ dump_test_log(test, test->error_cnt);
printf("#%d %s:%s\n", test->test_num, test->test_name,
test->error_cnt ? "FAIL" : "OK");
}
- printf("Summary: %d PASSED, %d FAILED\n", env.succ_cnt, env.fail_cnt);
+ printf("Summary: %d/%d PASSED, %d FAILED\n",
+ env.succ_cnt, env.sub_succ_cnt, env.fail_cnt);
free(env.log_buf);
+ free(env.test_selector.num_set);
+ free(env.subtest_selector.num_set);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 62f55a4231e9..afd14962456f 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -40,9 +40,15 @@ typedef __u16 __sum16;
struct prog_test_def;
+struct test_selector {
+ const char *name;
+ bool *num_set;
+ int num_set_len;
+};
+
struct test_env {
- int test_num_selector;
- const char *test_name_selector;
+ struct test_selector test_selector;
+ struct test_selector subtest_selector;
bool verifier_stats;
bool verbose;
bool very_verbose;
@@ -54,8 +60,9 @@ struct test_env {
size_t log_cnt;
size_t log_cap;
- int succ_cnt;
- int fail_cnt;
+ int succ_cnt; /* successful tests */
+ int sub_succ_cnt; /* successful sub-tests */
+ int fail_cnt; /* total failed tests + sub-tests */
};
extern int error_cnt;
@@ -65,6 +72,7 @@ extern struct test_env env;
extern void test__printf(const char *fmt, ...);
extern void test__vprintf(const char *fmt, va_list args);
extern void test__force_log();
+extern bool test__start_subtest(const char *name);
#define MAGIC_BYTES 123
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 3/9] selftests/bpf: add test selectors by number and name to test_progs
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Add ability to specify either test number or test name substring to
narrow down a set of test to run.
Usage:
sudo ./test_progs -n 1
sudo ./test_progs -t attach_probe
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/test_progs.c | 43 +++++++++++++++++++++---
1 file changed, 39 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index eea88ba59225..6e04b9f83777 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -4,6 +4,7 @@
#include "test_progs.h"
#include "bpf_rlimit.h"
#include <argp.h>
+#include <string.h>
int error_cnt, pass_cnt;
bool jit_enabled;
@@ -164,6 +165,7 @@ void *spin_lock_thread(void *arg)
struct prog_test_def {
const char *test_name;
+ int test_num;
void (*run_test)(void);
};
@@ -181,26 +183,49 @@ const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
const char argp_program_doc[] = "BPF selftests test runner";
enum ARG_KEYS {
+ ARG_TEST_NUM = 'n',
+ ARG_TEST_NAME = 't',
ARG_VERIFIER_STATS = 's',
};
static const struct argp_option opts[] = {
+ { "num", ARG_TEST_NUM, "NUM", 0,
+ "Run test number NUM only " },
+ { "name", ARG_TEST_NAME, "NAME", 0,
+ "Run tests with names containing NAME" },
{ "verifier-stats", ARG_VERIFIER_STATS, NULL, 0,
"Output verifier statistics", },
{},
};
struct test_env {
+ int test_num_selector;
+ const char *test_name_selector;
bool verifier_stats;
};
-static struct test_env env = {};
+static struct test_env env = {
+ .test_num_selector = -1,
+};
static error_t parse_arg(int key, char *arg, struct argp_state *state)
{
struct test_env *env = state->input;
switch (key) {
+ case ARG_TEST_NUM: {
+ int test_num;
+
+ errno = 0;
+ test_num = strtol(arg, NULL, 10);
+ if (errno)
+ return -errno;
+ env->test_num_selector = test_num;
+ break;
+ }
+ case ARG_TEST_NAME:
+ env->test_name_selector = arg;
+ break;
case ARG_VERIFIER_STATS:
env->verifier_stats = true;
break;
@@ -223,7 +248,7 @@ int main(int argc, char **argv)
.parser = parse_arg,
.doc = argp_program_doc,
};
- const struct prog_test_def *def;
+ struct prog_test_def *test;
int err, i;
err = argp_parse(&argp, argc, argv, 0, NULL, &env);
@@ -237,8 +262,18 @@ int main(int argc, char **argv)
verifier_stats = env.verifier_stats;
for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
- def = &prog_test_defs[i];
- def->run_test();
+ test = &prog_test_defs[i];
+
+ test->test_num = i + 1;
+
+ if (env.test_num_selector >= 0 &&
+ test->test_num != env.test_num_selector)
+ continue;
+ if (env.test_name_selector &&
+ !strstr(test->test_name, env.test_name_selector))
+ continue;
+
+ test->run_test();
}
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 5/9] selftest/bpf: centralize libbpf logging management for test_progs
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Make test_progs test runner own libbpf logging. Also introduce two
levels of verbosity: -v and -vv. First one will be used in subsequent
patches to enable test log output always. Second one increases verbosity
level of libbpf logging further to include debug output as well.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
.../bpf/prog_tests/bpf_verif_scale.c | 6 +++-
.../bpf/prog_tests/reference_tracking.c | 15 +++-------
tools/testing/selftests/bpf/test_progs.c | 29 +++++++++++++++++++
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
index e1b55261526f..ceddb8cc86f4 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -70,10 +70,11 @@ void test_bpf_verif_scale(void)
const char *cg_sysctl[] = {
"./test_sysctl_loop1.o", "./test_sysctl_loop2.o",
};
+ libbpf_print_fn_t old_print_fn = NULL;
int err, i;
if (verifier_stats)
- libbpf_set_print(libbpf_debug_print);
+ old_print_fn = libbpf_set_print(libbpf_debug_print);
err = check_load("./loop3.o", BPF_PROG_TYPE_RAW_TRACEPOINT);
printf("test_scale:loop3:%s\n", err ? (error_cnt--, "OK") : "FAIL");
@@ -97,4 +98,7 @@ void test_bpf_verif_scale(void)
err = check_load("./test_seg6_loop.o", BPF_PROG_TYPE_LWT_SEG6LOCAL);
printf("test_scale:test_seg6_loop:%s\n", err ? "FAIL" : "OK");
+
+ if (verifier_stats)
+ libbpf_set_print(old_print_fn);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
index 5633be43828f..4a4f428d1a78 100644
--- a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
+++ b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
@@ -1,15 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
-static int libbpf_debug_print(enum libbpf_print_level level,
- const char *format, va_list args)
-{
- if (level == LIBBPF_DEBUG)
- return 0;
-
- return vfprintf(stderr, format, args);
-}
-
void test_reference_tracking(void)
{
const char *file = "./test_sk_lookup_kern.o";
@@ -36,9 +27,11 @@ void test_reference_tracking(void)
/* Expect verifier failure if test name has 'fail' */
if (strstr(title, "fail") != NULL) {
- libbpf_set_print(NULL);
+ libbpf_print_fn_t old_print_fn;
+
+ old_print_fn = libbpf_set_print(NULL);
err = !bpf_program__load(prog, "GPL", 0);
- libbpf_set_print(libbpf_debug_print);
+ libbpf_set_print(old_print_fn);
} else {
err = bpf_program__load(prog, "GPL", 0);
}
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 6e04b9f83777..94b6951b90b3 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -186,6 +186,8 @@ enum ARG_KEYS {
ARG_TEST_NUM = 'n',
ARG_TEST_NAME = 't',
ARG_VERIFIER_STATS = 's',
+
+ ARG_VERBOSE = 'v',
};
static const struct argp_option opts[] = {
@@ -195,6 +197,8 @@ static const struct argp_option opts[] = {
"Run tests with names containing NAME" },
{ "verifier-stats", ARG_VERIFIER_STATS, NULL, 0,
"Output verifier statistics", },
+ { "verbose", ARG_VERBOSE, "LEVEL", OPTION_ARG_OPTIONAL,
+ "Verbose output (use -vv for extra verbose output)" },
{},
};
@@ -202,12 +206,22 @@ struct test_env {
int test_num_selector;
const char *test_name_selector;
bool verifier_stats;
+ bool verbose;
+ bool very_verbose;
};
static struct test_env env = {
.test_num_selector = -1,
};
+static int libbpf_print_fn(enum libbpf_print_level level,
+ const char *format, va_list args)
+{
+ if (!env.very_verbose && level == LIBBPF_DEBUG)
+ return 0;
+ return vfprintf(stderr, format, args);
+}
+
static error_t parse_arg(int key, char *arg, struct argp_state *state)
{
struct test_env *env = state->input;
@@ -229,6 +243,19 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
case ARG_VERIFIER_STATS:
env->verifier_stats = true;
break;
+ case ARG_VERBOSE:
+ if (arg) {
+ if (strcmp(arg, "v") == 0) {
+ env->very_verbose = true;
+ } else {
+ fprintf(stderr,
+ "Unrecognized verbosity setting ('%s'), only -v and -vv are supported\n",
+ arg);
+ return -EINVAL;
+ }
+ }
+ env->verbose = true;
+ break;
case ARGP_KEY_ARG:
argp_usage(state);
break;
@@ -255,6 +282,8 @@ int main(int argc, char **argv)
if (err)
return err;
+ libbpf_set_print(libbpf_print_fn);
+
srand(time(NULL));
jit_enabled = is_jit_enabled();
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 2/9] selftests/bpf: revamp test_progs to allow more control
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Refactor test_progs to allow better control on what's being run.
Also use argp to do argument parsing, so that it's easier to keep adding
more options.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/Makefile | 8 +--
tools/testing/selftests/bpf/test_progs.c | 84 +++++++++++++++++++++---
2 files changed, 77 insertions(+), 15 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index bb66cc4a7f34..3bd0f4a0336a 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -239,14 +239,8 @@ $(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
$(shell ( cd prog_tests/; \
echo '/* Generated header, do not edit */'; \
- echo '#ifdef DECLARE'; \
ls *.c 2> /dev/null | \
- sed -e 's@\([^\.]*\)\.c@extern void test_\1(void);@'; \
- echo '#endif'; \
- echo '#ifdef CALL'; \
- ls *.c 2> /dev/null | \
- sed -e 's@\([^\.]*\)\.c@test_\1();@'; \
- echo '#endif' \
+ sed -e 's@\([^\.]*\)\.c@DEFINE_TEST(\1)@'; \
) > $(PROG_TESTS_H))
MAP_TESTS_DIR = $(OUTPUT)/map_tests
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index dae0819b1141..eea88ba59225 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -3,6 +3,7 @@
*/
#include "test_progs.h"
#include "bpf_rlimit.h"
+#include <argp.h>
int error_cnt, pass_cnt;
bool jit_enabled;
@@ -156,22 +157,89 @@ void *spin_lock_thread(void *arg)
pthread_exit(arg);
}
-#define DECLARE
+/* extern declarations for test funcs */
+#define DEFINE_TEST(name) extern void test_##name();
#include <prog_tests/tests.h>
-#undef DECLARE
+#undef DEFINE_TEST
-int main(int ac, char **av)
+struct prog_test_def {
+ const char *test_name;
+ void (*run_test)(void);
+};
+
+static struct prog_test_def prog_test_defs[] = {
+#define DEFINE_TEST(name) { \
+ .test_name = #name, \
+ .run_test = &test_##name, \
+},
+#include <prog_tests/tests.h>
+#undef DEFINE_TEST
+};
+
+const char *argp_program_version = "test_progs 0.1";
+const char *argp_program_bug_address = "<bpf@vger.kernel.org>";
+const char argp_program_doc[] = "BPF selftests test runner";
+
+enum ARG_KEYS {
+ ARG_VERIFIER_STATS = 's',
+};
+
+static const struct argp_option opts[] = {
+ { "verifier-stats", ARG_VERIFIER_STATS, NULL, 0,
+ "Output verifier statistics", },
+ {},
+};
+
+struct test_env {
+ bool verifier_stats;
+};
+
+static struct test_env env = {};
+
+static error_t parse_arg(int key, char *arg, struct argp_state *state)
{
+ struct test_env *env = state->input;
+
+ switch (key) {
+ case ARG_VERIFIER_STATS:
+ env->verifier_stats = true;
+ break;
+ case ARGP_KEY_ARG:
+ argp_usage(state);
+ break;
+ case ARGP_KEY_END:
+ break;
+ default:
+ return ARGP_ERR_UNKNOWN;
+ }
+ return 0;
+}
+
+
+int main(int argc, char **argv)
+{
+ static const struct argp argp = {
+ .options = opts,
+ .parser = parse_arg,
+ .doc = argp_program_doc,
+ };
+ const struct prog_test_def *def;
+ int err, i;
+
+ err = argp_parse(&argp, argc, argv, 0, NULL, &env);
+ if (err)
+ return err;
+
srand(time(NULL));
jit_enabled = is_jit_enabled();
- if (ac == 2 && strcmp(av[1], "-s") == 0)
- verifier_stats = true;
+ verifier_stats = env.verifier_stats;
-#define CALL
-#include <prog_tests/tests.h>
-#undef CALL
+ for (i = 0; i < ARRAY_SIZE(prog_test_defs); i++) {
+ def = &prog_test_defs[i];
+ def->run_test();
+ }
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 4/9] libbpf: return previous print callback from libbpf_set_print
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
By returning previously set print callback from libbpf_set_print, it's
possible to restore it, eventually. This is useful when running many
independent test with one default print function, but overriding log
verbosity for particular subset of tests.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 5 ++++-
tools/lib/bpf/libbpf.h | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 8741c39adb1c..ead915aec349 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -74,9 +74,12 @@ static int __base_pr(enum libbpf_print_level level, const char *format,
static libbpf_print_fn_t __libbpf_pr = __base_pr;
-void libbpf_set_print(libbpf_print_fn_t fn)
+libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn)
{
+ libbpf_print_fn_t old_print_fn = __libbpf_pr;
+
__libbpf_pr = fn;
+ return old_print_fn;
}
__printf(2, 3)
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5cbf459ece0b..8a9d462a6f6d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -57,7 +57,7 @@ enum libbpf_print_level {
typedef int (*libbpf_print_fn_t)(enum libbpf_print_level level,
const char *, va_list ap);
-LIBBPF_API void libbpf_set_print(libbpf_print_fn_t fn);
+LIBBPF_API libbpf_print_fn_t libbpf_set_print(libbpf_print_fn_t fn);
/* Hide internal to user */
struct bpf_object;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 1/9] selftests/bpf: prevent headers to be compiled as C code
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
In-Reply-To: <20190728032531.2358749-1-andriin@fb.com>
Apprently listing header as a normal dependency for a binary output
makes it go through compilation as if it was C code. This currently
works without a problem, but in subsequent commits causes problems for
differently generated test.h for test_progs. Marking those headers as
order-only dependency solves the issue.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/testing/selftests/bpf/Makefile | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 11c9c62c3362..bb66cc4a7f34 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -235,7 +235,7 @@ PROG_TESTS_H := $(PROG_TESTS_DIR)/tests.h
PROG_TESTS_FILES := $(wildcard prog_tests/*.c)
test_progs.c: $(PROG_TESTS_H)
$(OUTPUT)/test_progs: CFLAGS += $(TEST_PROGS_CFLAGS)
-$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_H) $(PROG_TESTS_FILES)
+$(OUTPUT)/test_progs: test_progs.c $(PROG_TESTS_FILES) | $(PROG_TESTS_H)
$(PROG_TESTS_H): $(PROG_TESTS_FILES) | $(PROG_TESTS_DIR)
$(shell ( cd prog_tests/; \
echo '/* Generated header, do not edit */'; \
@@ -256,7 +256,7 @@ MAP_TESTS_H := $(MAP_TESTS_DIR)/tests.h
MAP_TESTS_FILES := $(wildcard map_tests/*.c)
test_maps.c: $(MAP_TESTS_H)
$(OUTPUT)/test_maps: CFLAGS += $(TEST_MAPS_CFLAGS)
-$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_H) $(MAP_TESTS_FILES)
+$(OUTPUT)/test_maps: test_maps.c $(MAP_TESTS_FILES) | $(MAP_TESTS_H)
$(MAP_TESTS_H): $(MAP_TESTS_FILES) | $(MAP_TESTS_DIR)
$(shell ( cd map_tests/; \
echo '/* Generated header, do not edit */'; \
@@ -277,7 +277,7 @@ VERIFIER_TESTS_H := $(VERIFIER_TESTS_DIR)/tests.h
VERIFIER_TEST_FILES := $(wildcard verifier/*.c)
test_verifier.c: $(VERIFIER_TESTS_H)
$(OUTPUT)/test_verifier: CFLAGS += $(TEST_VERIFIER_CFLAGS)
-$(OUTPUT)/test_verifier: test_verifier.c $(VERIFIER_TESTS_H)
+$(OUTPUT)/test_verifier: test_verifier.c | $(VERIFIER_TEST_FILES) $(VERIFIER_TESTS_H)
$(VERIFIER_TESTS_H): $(VERIFIER_TEST_FILES) | $(VERIFIER_TESTS_DIR)
$(shell ( cd verifier/; \
echo '/* Generated header, do not edit */'; \
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28 3:25 UTC (permalink / raw)
To: bpf, netdev, ast, daniel, sdf
Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko
This patch set makes a number of changes to test_progs selftest, which is
a collection of many other tests (and sometimes sub-tests as well), to provide
better testing experience and allow to start convering many individual test
programs under selftests/bpf into a single and convenient test runner.
Patch #1 fixes issue with Makefile, which makes prog_tests/test.h compiled as
a C code. This fix allows to change how test.h is generated, providing ability
to have more control on what and how tests are run.
Patch #2 changes how test.h is auto-generated, which allows to have test
definitions, instead of just running test functions. This gives ability to do
more complicated test run policies.
Patch #3 adds `-t <test-name>` and `-n <test-num>` selectors to run only
subset of tests.
Patch #4 changes libbpf_set_print() to return previously set print callback,
allowing to temporarily replace current print callback and then set it back.
This is necessary for some tests that want more control over libbpf logging.
Patch #5 sets up and takes over libbpf logging from individual tests to
test_prog runner, adding -vv verbosity to capture debug output from libbpf.
This is useful when debugging failing tests.
Patch #6 furthers test output management and buffers it by default, emitting
log output only if test fails. This give succinct and clean default test
output. It's possible to bypass this behavior with -v flag, which will turn
off test output buffering.
Patch #7 adds support for sub-tests. It also enhances -t and -n selectors to
both support ability to specify sub-test selectors, as well as enhancing
number selector to accept sets of test, instead of just individual test
number.
Patch #8 converts bpf_verif_scale.c test to use sub-test APIs.
Patch #9 converts send_signal.c tests to use sub-test APIs.
v2->v3:
- fix buffered output rare unitialized value bug (Alexei);
- fix buffered output va_list reuse bug (Alexei);
- fix buffered output truncation due to interleaving zero terminators;
v1->v2:
- drop libbpf_swap_print, instead return previous function from
libbpf_set_print (Stanislav);
Andrii Nakryiko (9):
selftests/bpf: prevent headers to be compiled as C code
selftests/bpf: revamp test_progs to allow more control
selftests/bpf: add test selectors by number and name to test_progs
libbpf: return previous print callback from libbpf_set_print
selftest/bpf: centralize libbpf logging management for test_progs
selftests/bpf: abstract away test log output
selftests/bpf: add sub-tests support for test_progs
selftests/bpf: convert bpf_verif_scale.c to sub-tests API
selftests/bpf: convert send_signal.c to use subtests
tools/lib/bpf/libbpf.c | 5 +-
tools/lib/bpf/libbpf.h | 2 +-
tools/testing/selftests/bpf/Makefile | 14 +-
.../selftests/bpf/prog_tests/bpf_obj_id.c | 6 +-
.../bpf/prog_tests/bpf_verif_scale.c | 90 ++--
.../bpf/prog_tests/get_stack_raw_tp.c | 4 +-
.../selftests/bpf/prog_tests/l4lb_all.c | 2 +-
.../selftests/bpf/prog_tests/map_lock.c | 10 +-
.../bpf/prog_tests/reference_tracking.c | 15 +-
.../selftests/bpf/prog_tests/send_signal.c | 17 +-
.../selftests/bpf/prog_tests/spinlock.c | 2 +-
.../bpf/prog_tests/stacktrace_build_id.c | 4 +-
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 4 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 3 +-
tools/testing/selftests/bpf/test_progs.c | 383 +++++++++++++++++-
tools/testing/selftests/bpf/test_progs.h | 45 +-
16 files changed, 502 insertions(+), 104 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28 2:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
Daniel Borkmann, Stanislav Fomichev, Kernel Team
In-Reply-To: <CAEf4BzY3snLh5=qhFo6RNL1RQMcmVhkCiB2s4p57jQcovp5TWw@mail.gmail.com>
On Sat, Jul 27, 2019 at 7:16 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Jul 27, 2019 at 6:12 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
> > >
> > > This patch set makes a number of changes to test_progs selftest, which is
> > > a collection of many other tests (and sometimes sub-tests as well), to provide
> > > better testing experience and allow to start convering many individual test
> > > programs under selftests/bpf into a single and convenient test runner.
> >
> > I really like the patches, but something isn't right:
>
> Argh... Uninitialized `int ret` in test__vprintf(). Should be
> initialized to zero, otherwise in some corner cases when log buffer is
> completely full and ret's initial value is sufficiently large negative
> number, it can underflow env.log_cnt, silently skipping one log
> output, and then crashing on next one. You've somehow encountered a
> fascinating series of conditions that I've never stumbled upon running
> my code dozens of times. Fixing, sorry about that!
Ok, I doubt it was that specific bug (even though it is a bug). Turns
out that you can't call vprintf with the same va_list twice, as it
consumes it, so I have to do va_copy() if I might call vprintf
multiple times. So fixing that now.
>
> > #16 raw_tp_writable_reject_nbd_invalid:OK
> > #17 raw_tp_writable_test_run:OK
> > #18 reference_tracking:OK
> > [ 87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
> > 00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
> > [ 87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
> > 8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
> > 4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
> > [ 87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
> > gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
> > exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
> > sig=11 res=1
> > Segmentation fault (core dumped)
> >
> > Under gdb fault is different:
> > #23 stacktrace_build_id:OK
> > Detaching after fork from child process 2276.
> > Detaching after fork from child process 2278.
> > [ 149.013116] perf: interrupt took too long (6799 > 6713), lowering
> > kernel.perf_event_max_sample_rate to 29000
> > [ 149.014634] perf: interrupt took too long (8511 > 8498), lowering
> > kernel.perf_event_max_sample_rate to 23000
> > [ 149.017038] perf: interrupt took too long (10649 > 10638), lowering
> > kernel.perf_event_max_sample_rate to 18000
> > [ 149.021901] perf: interrupt took too long (13322 > 13311), lowering
> > kernel.perf_event_max_sample_rate to 15000
> > [ 149.042946] perf: interrupt took too long (16660 > 16652), lowering
> > kernel.perf_event_max_sample_rate to 12000
> > Detaching after fork from child process 2279.
> > #24 stacktrace_build_id_nmi:OK
> > #25 stacktrace_map:OK
> > #26 stacktrace_map_raw_tp:OK
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> > (gdb) bt
> > #0 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> > #1 0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
> > #2 0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
> > nsec\n", args=0x7fffffffe878) at test_progs.c:114
> > #3 0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
> > "%s:PASS:%s %d nsec\n") at test_progs.c:147
> > #4 0x000000000042222d in test_task_fd_query_rawtp () at
> > prog_tests/task_fd_query_rawtp.c:19
> > #5 0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
> > out>) at test_progs.c:501
> > (gdb) info threads
> > Id Target Id Frame
> > * 1 Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
> > 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Andrii Nakryiko @ 2019-07-28 2:16 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
Daniel Borkmann, Stanislav Fomichev, Kernel Team
In-Reply-To: <CAADnVQJoS8Yjt-qFHg4XEkiF_3H8nRx15xZfJDSy8YcRT_UKrg@mail.gmail.com>
On Sat, Jul 27, 2019 at 6:12 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > This patch set makes a number of changes to test_progs selftest, which is
> > a collection of many other tests (and sometimes sub-tests as well), to provide
> > better testing experience and allow to start convering many individual test
> > programs under selftests/bpf into a single and convenient test runner.
>
> I really like the patches, but something isn't right:
Argh... Uninitialized `int ret` in test__vprintf(). Should be
initialized to zero, otherwise in some corner cases when log buffer is
completely full and ret's initial value is sufficiently large negative
number, it can underflow env.log_cnt, silently skipping one log
output, and then crashing on next one. You've somehow encountered a
fascinating series of conditions that I've never stumbled upon running
my code dozens of times. Fixing, sorry about that!
> #16 raw_tp_writable_reject_nbd_invalid:OK
> #17 raw_tp_writable_test_run:OK
> #18 reference_tracking:OK
> [ 87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
> 00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
> [ 87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
> 8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
> 4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
> [ 87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
> gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
> exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
> sig=11 res=1
> Segmentation fault (core dumped)
>
> Under gdb fault is different:
> #23 stacktrace_build_id:OK
> Detaching after fork from child process 2276.
> Detaching after fork from child process 2278.
> [ 149.013116] perf: interrupt took too long (6799 > 6713), lowering
> kernel.perf_event_max_sample_rate to 29000
> [ 149.014634] perf: interrupt took too long (8511 > 8498), lowering
> kernel.perf_event_max_sample_rate to 23000
> [ 149.017038] perf: interrupt took too long (10649 > 10638), lowering
> kernel.perf_event_max_sample_rate to 18000
> [ 149.021901] perf: interrupt took too long (13322 > 13311), lowering
> kernel.perf_event_max_sample_rate to 15000
> [ 149.042946] perf: interrupt took too long (16660 > 16652), lowering
> kernel.perf_event_max_sample_rate to 12000
> Detaching after fork from child process 2279.
> #24 stacktrace_build_id_nmi:OK
> #25 stacktrace_map:OK
> #26 stacktrace_map_raw_tp:OK
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> (gdb) bt
> #0 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
> #1 0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
> #2 0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
> nsec\n", args=0x7fffffffe878) at test_progs.c:114
> #3 0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
> "%s:PASS:%s %d nsec\n") at test_progs.c:147
> #4 0x000000000042222d in test_task_fd_query_rawtp () at
> prog_tests/task_fd_query_rawtp.c:19
> #5 0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
> out>) at test_progs.c:501
> (gdb) info threads
> Id Target Id Frame
> * 1 Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
> 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
^ permalink raw reply
* Re: [PATCH net-next v3 0/2] qed*: Support for NVM config attributes.
From: David Miller @ 2019-07-28 2:13 UTC (permalink / raw)
To: skalluru; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: Sat, 27 Jul 2019 18:55:47 -0700
> The patch series adds support for managing the NVM config attributes.
> Patch (1) adds functionality to update config attributes via MFW.
> Patch (2) adds driver interface for updating the config attributes.
>
> Changes from previous versions:
> -------------------------------
> v3: Removed unused variable.
> v2: Removed unused API.
>
> Please consider applying this series to "net-next".
I don't see where an existing ethtool method hooks into and calls this
new NVM code.
^ permalink raw reply
* Re: [PATCH V3 net-next 06/10] net: hns3: add debug messages to identify eth down cause
From: David Miller @ 2019-07-28 2:03 UTC (permalink / raw)
To: tanhuazhong
Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm, saeedm,
liuyonglong, lipeng321
In-Reply-To: <1564206372-42467-7-git-send-email-tanhuazhong@huawei.com>
From: Huazhong Tan <tanhuazhong@huawei.com>
Date: Sat, 27 Jul 2019 13:46:08 +0800
> From: Yonglong Liu <liuyonglong@huawei.com>
>
> Some times just see the eth interface have been down/up via
> dmesg, but can not know why the eth down. So adds some debug
> messages to identify the cause for this.
>
> Signed-off-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Peng Li <lipeng321@huawei.com>
> Signed-off-by: Huazhong Tan <tanhuazhong@huawei.com>
> ---
> drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 18 ++++++++++++++++++
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 19 +++++++++++++++++++
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 11 +++++++++++
> 3 files changed, 48 insertions(+)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> index 4d58c53..973c57b 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
> @@ -459,6 +459,9 @@ static int hns3_nic_net_open(struct net_device *netdev)
> h->ae_algo->ops->set_timer_task(priv->ae_handle, true);
>
> hns3_config_xps(priv);
> +
> + netif_info(h, drv, netdev, "net open\n");
These will pollute everyone's kernel logs for normal operations.
This is not reasonable at all, sorry.
Furthermore, even if it was appropriate, "netif_info()" is not "debug".
^ permalink raw reply
* [PATCH net-next v3 2/2] qed: Add driver API for flashing the config attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28 1:55 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>
The patch adds driver interface for reading the NVM config request and
update the attributes on nvm config flash partition.
This API can be used by ethtool flash update command (i.e., ethtool -f) to
update config attributes in the NVM flash parition.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 65 ++++++++++++++++++++++++++++++
include/linux/qed/qed_if.h | 1 +
2 files changed, 66 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 829dd60..54f00d2 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -67,6 +67,8 @@
#define QED_ROCE_QPS (8192)
#define QED_ROCE_DPIS (8)
#define QED_RDMA_SRQS QED_ROCE_QPS
+#define QED_NVM_CFG_SET_FLAGS 0xE
+#define QED_NVM_CFG_SET_PF_FLAGS 0x1E
static char version[] =
"QLogic FastLinQ 4xxxx Core Module qed " DRV_MODULE_VERSION "\n";
@@ -2227,6 +2229,66 @@ static int qed_nvm_flash_image_validate(struct qed_dev *cdev,
return 0;
}
+/* Binary file format -
+ * /----------------------------------------------------------------------\
+ * 0B | 0x5 [command index] |
+ * 4B | Entity ID | Reserved | Number of config attributes |
+ * 8B | Config ID | Length | Value |
+ * | |
+ * \----------------------------------------------------------------------/
+ * There can be several Cfg_id-Length-Value sets as specified by 'Number of...'.
+ * Entity ID - A non zero entity value for which the config need to be updated.
+ */
+static int qed_nvm_flash_cfg_write(struct qed_dev *cdev, const u8 **data)
+{
+ struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+ u8 entity_id, len, buf[32];
+ struct qed_ptt *ptt;
+ u16 cfg_id, count;
+ int rc = 0, i;
+ u32 flags;
+
+ ptt = qed_ptt_acquire(hwfn);
+ if (!ptt)
+ return -EAGAIN;
+
+ /* NVM CFG ID attribute header */
+ *data += 4;
+ entity_id = **data;
+ *data += 2;
+ count = *((u16 *)*data);
+ *data += 2;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "Read config ids: entity id %02x num _attrs = %0d\n",
+ entity_id, count);
+ /* NVM CFG ID attributes */
+ for (i = 0; i < count; i++) {
+ cfg_id = *((u16 *)*data);
+ *data += 2;
+ len = **data;
+ (*data)++;
+ memcpy(buf, *data, len);
+ *data += len;
+
+ flags = entity_id ? QED_NVM_CFG_SET_PF_FLAGS :
+ QED_NVM_CFG_SET_FLAGS;
+
+ DP_VERBOSE(cdev, NETIF_MSG_DRV,
+ "cfg_id = %d len = %d\n", cfg_id, len);
+ rc = qed_mcp_nvm_set_cfg(hwfn, ptt, cfg_id, entity_id, flags,
+ buf, len);
+ if (rc) {
+ DP_ERR(cdev, "Error %d configuring %d\n", rc, cfg_id);
+ break;
+ }
+ }
+
+ qed_ptt_release(hwfn, ptt);
+
+ return rc;
+}
+
static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
{
const struct firmware *image;
@@ -2268,6 +2330,9 @@ static int qed_nvm_flash(struct qed_dev *cdev, const char *name)
rc = qed_nvm_flash_image_access(cdev, &data,
&check_resp);
break;
+ case QED_NVM_FLASH_CMD_NVM_CFG_ID:
+ rc = qed_nvm_flash_cfg_write(cdev, &data);
+ break;
default:
DP_ERR(cdev, "Unknown command %08x\n", cmd_type);
rc = -EINVAL;
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index eef02e6..23805ea 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -804,6 +804,7 @@ enum qed_nvm_flash_cmd {
QED_NVM_FLASH_CMD_FILE_DATA = 0x2,
QED_NVM_FLASH_CMD_FILE_START = 0x3,
QED_NVM_FLASH_CMD_NVM_CHANGE = 0x4,
+ QED_NVM_FLASH_CMD_NVM_CFG_ID = 0x5,
QED_NVM_FLASH_CMD_NVM_MAX,
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 1/2] qed: Add API for configuring NVM attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28 1:55 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190728015549.27051-1-skalluru@marvell.com>
The patch adds API for configuring the NVM config attributes using
Management FW (MFW) interfaces.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 17 ++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 32 +++++++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 20 +++++++++++++++++++
3 files changed, 69 insertions(+)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index e054f6c..557a12e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -12580,6 +12580,8 @@ struct public_drv_mb {
#define DRV_MSG_CODE_BW_UPDATE_ACK 0x32000000
#define DRV_MSG_CODE_NIG_DRAIN 0x30000000
#define DRV_MSG_CODE_S_TAG_UPDATE_ACK 0x3b000000
+#define DRV_MSG_CODE_GET_NVM_CFG_OPTION 0x003e0000
+#define DRV_MSG_CODE_SET_NVM_CFG_OPTION 0x003f0000
#define DRV_MSG_CODE_INITIATE_PF_FLR 0x02010000
#define DRV_MSG_CODE_VF_DISABLED_DONE 0xc0000000
#define DRV_MSG_CODE_CFG_VF_MSIX 0xc0010000
@@ -12748,6 +12750,21 @@ struct public_drv_mb {
#define DRV_MB_PARAM_FEATURE_SUPPORT_PORT_EEE 0x00000002
#define DRV_MB_PARAM_FEATURE_SUPPORT_FUNC_VLINK 0x00010000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_SHIFT 0
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_MASK 0x0000FFFF
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_SHIFT 16
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_MASK 0x00010000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_SHIFT 17
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_MASK 0x00020000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_SHIFT 18
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_MASK 0x00040000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_SHIFT 19
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_MASK 0x00080000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_SHIFT 20
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_MASK 0x00100000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_SHIFT 24
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_MASK 0x0f000000
+
u32 fw_mb_header;
#define FW_MSG_CODE_MASK 0xffff0000
#define FW_MSG_CODE_UNSUPPORTED 0x00000000
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 758702c..89462c4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -3750,3 +3750,35 @@ int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
return 0;
}
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+ u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+ u32 len)
+{
+ u32 mb_param = 0, resp, param;
+
+ QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+ if (flags & QED_NVM_CFG_OPTION_ALL)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_ALL, 1);
+ if (flags & QED_NVM_CFG_OPTION_INIT)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+ if (flags & QED_NVM_CFG_OPTION_COMMIT)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT, 1);
+ if (flags & QED_NVM_CFG_OPTION_FREE)
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+ if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+ QED_MFW_SET_FIELD(mb_param,
+ DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+ entity_id);
+ }
+
+ return qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
+ DRV_MSG_CODE_SET_NVM_CFG_OPTION,
+ mb_param, &resp, ¶m, len, (u32 *)p_buf);
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index e4f8fe4..83649a8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -251,6 +251,12 @@ struct qed_mfw_tlv_generic {
struct qed_mfw_tlv_iscsi iscsi;
};
+#define QED_NVM_CFG_OPTION_ALL BIT(0)
+#define QED_NVM_CFG_OPTION_INIT BIT(1)
+#define QED_NVM_CFG_OPTION_COMMIT BIT(2)
+#define QED_NVM_CFG_OPTION_FREE BIT(3)
+#define QED_NVM_CFG_OPTION_ENTITY_SEL BIT(4)
+
/**
* @brief - returns the link params of the hw function
*
@@ -1202,4 +1208,18 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
*/
int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
+/**
+ * @brief Set NVM config attribute value.
+ *
+ * @param p_hwfn
+ * @param p_ptt
+ * @param option_id
+ * @param entity_id
+ * @param flags
+ * @param p_buf
+ * @param len
+ */
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+ u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+ u32 len);
#endif
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next v3 0/2] qed*: Support for NVM config attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28 1:55 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
The patch series adds support for managing the NVM config attributes.
Patch (1) adds functionality to update config attributes via MFW.
Patch (2) adds driver interface for updating the config attributes.
Changes from previous versions:
-------------------------------
v3: Removed unused variable.
v2: Removed unused API.
Please consider applying this series to "net-next".
Sudarsana Reddy Kalluru (2):
qed: Add API for configuring NVM attributes.
qed: Add driver API for flashing the config attributes.
drivers/net/ethernet/qlogic/qed/qed_hsi.h | 17 ++++++++
drivers/net/ethernet/qlogic/qed/qed_main.c | 65 ++++++++++++++++++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 32 +++++++++++++++
drivers/net/ethernet/qlogic/qed/qed_mcp.h | 20 +++++++++
include/linux/qed/qed_if.h | 1 +
5 files changed, 135 insertions(+)
--
1.8.3.1
^ permalink raw reply
* RE: [PATCH net-next v2 1/2] qed: Add API for configuring NVM attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-28 1:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org, Michal Kalderon, Ariel Elior
In-Reply-To: <20190727.140029.1705855810720310694.davem@davemloft.net>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of David Miller
> Sent: Sunday, July 28, 2019 2:30 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>;
> Ariel Elior <aelior@marvell.com>
> Subject: Re: [PATCH net-next v2 1/2] qed: Add API for configuring NVM
> attributes.
>
> From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Date: Fri, 26 Jul 2019 08:52:14 -0700
>
> > +int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt
> *p_ptt,
> > + u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
> > + u32 len)
> > +{
> > + u32 mb_param = 0, resp, param;
> > + int rc;
> ...
> > + rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
> > + DRV_MSG_CODE_SET_NVM_CFG_OPTION,
> > + mb_param, &resp, ¶m, len, (u32
> *)p_buf);
> > +
> > + return rc;
>
> 'rc' is completely unnecessary, please just return the function result directly.
>
> Thank you.
Thanks for your comments. Will send the updated patch.
^ permalink raw reply
* Re: [PATCH net-next] mvpp2: document HW checksum behaviour
From: Matteo Croce @ 2019-07-28 1:36 UTC (permalink / raw)
To: Antoine Tenart, Marcin Wojtas, Stefan Chulski, Maxime Chevallier
Cc: netdev, LKML, David S . Miller
In-Reply-To: <20190726125715.GB5031@kwain>
On Fri, Jul 26, 2019 at 2:57 PM Antoine Tenart
<antoine.tenart@bootlin.com> wrote:
>
> Hi Matteo,
>
> On Fri, Jul 26, 2019 at 01:15:46AM +0200, Matteo Croce wrote:
> > The hardware can only offload checksum calculation on first port due to
> > the Tx FIFO size limitation. Document this in a comment.
> >
> > Fixes: 576193f2d579 ("net: mvpp2: jumbo frames support")
> > Signed-off-by: Matteo Croce <mcroce@redhat.com>
>
> Looks good. Please note there's a similar code path in the probe. You
> could also add a comment there (or move this check/comment in a common
> place).
>
> Thanks!
> Antoine
>
Hi Antoine,
I was making a v2, when I looked at the mvpp2_port_probe() which does:
--------------------------------%<------------------------------
features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_TSO;
if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) {
dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM);
}
dev->vlan_features |= features;
-------------------------------->%------------------------------
Is it ok to remove NETIF_F_IP*_CSUM from dev->features and
dev->hw_features but keep it in dev->vlan_features?
Regards,
--
Matteo Croce
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] Revamp test_progs as a test running framework
From: Alexei Starovoitov @ 2019-07-28 1:12 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Network Development, Alexei Starovoitov, Daniel Borkmann,
Stanislav Fomichev, Andrii Nakryiko, Kernel Team
In-Reply-To: <20190727190150.649137-1-andriin@fb.com>
On Sat, Jul 27, 2019 at 12:02 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> This patch set makes a number of changes to test_progs selftest, which is
> a collection of many other tests (and sometimes sub-tests as well), to provide
> better testing experience and allow to start convering many individual test
> programs under selftests/bpf into a single and convenient test runner.
I really like the patches, but something isn't right:
#16 raw_tp_writable_reject_nbd_invalid:OK
#17 raw_tp_writable_test_run:OK
#18 reference_tracking:OK
[ 87.715996] test_progs[2200]: segfault at 2f ip 00007f56aeea347b sp
00007ffce9720980 error 4 in libc-2.23.so[7f56aee5b000+198000]
[ 87.717316] Code: ff ff 44 89 8d 30 fb ff ff e8 01 74 fd ff 44 8b
8d 30 fb ff ff 4c 8b 85 28 fb ff ff e9 fd eb ff ff 31 c0 48 83 c9 ff
4c 89 df <f2> ae c7 85 28 fb ff ff 00 00 00 00 48 89 c8 48 f7 d0 4c 8f
[ 87.719493] audit: type=1701 audit(1564276195.971:5): auid=0 uid=0
gid=0 ses=1 subj=kernel pid=2200 comm="test_progs"
exe="/data/users/ast/net-next/tools/testing/selftests/bpf/test_progs"
sig=11 res=1
Segmentation fault (core dumped)
Under gdb fault is different:
#23 stacktrace_build_id:OK
Detaching after fork from child process 2276.
Detaching after fork from child process 2278.
[ 149.013116] perf: interrupt took too long (6799 > 6713), lowering
kernel.perf_event_max_sample_rate to 29000
[ 149.014634] perf: interrupt took too long (8511 > 8498), lowering
kernel.perf_event_max_sample_rate to 23000
[ 149.017038] perf: interrupt took too long (10649 > 10638), lowering
kernel.perf_event_max_sample_rate to 18000
[ 149.021901] perf: interrupt took too long (13322 > 13311), lowering
kernel.perf_event_max_sample_rate to 15000
[ 149.042946] perf: interrupt took too long (16660 > 16652), lowering
kernel.perf_event_max_sample_rate to 12000
Detaching after fork from child process 2279.
#24 stacktrace_build_id_nmi:OK
#25 stacktrace_map:OK
#26 stacktrace_map_raw_tp:OK
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
(gdb) bt
#0 0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
#1 0x00007ffff72655a9 in vsnprintf () from /usr/lib/libc.so.6
#2 0x0000000000403100 in test__vprintf (fmt=0x426754 "%s:PASS:%s %d
nsec\n", args=0x7fffffffe878) at test_progs.c:114
#3 0x000000000040325c in test__printf (fmt=fmt@entry=0x426754
"%s:PASS:%s %d nsec\n") at test_progs.c:147
#4 0x000000000042222d in test_task_fd_query_rawtp () at
prog_tests/task_fd_query_rawtp.c:19
#5 0x0000000000402c76 in main (argc=<optimized out>, argv=<optimized
out>) at test_progs.c:501
(gdb) info threads
Id Target Id Frame
* 1 Thread 0x7ffff7fea700 (LWP 2245) "test_progs"
0x00007ffff723f47b in vfprintf () from /usr/lib/libc.so.6
^ permalink raw reply
* [PATCH v3] net: dsa: qca8k: enable port flow control
From: xiaofeis @ 2019-07-28 0:57 UTC (permalink / raw)
To: davem
Cc: vkoul, netdev, andrew, linux-arm-msm, bjorn.andersson,
vivien.didelot, f.fainelli, niklas.cassel, xiazha, xiaofeis
Set phy device advertising to enable MAC flow control.
Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
---
Changes since V2:
drivers/net/dsa/qca8k.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 232e8cc..e429e92 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -2,7 +2,7 @@
/*
* Copyright (C) 2009 Felix Fietkau <nbd@nbd.name>
* Copyright (C) 2011-2012 Gabor Juhos <juhosg@openwrt.org>
- * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2015, 2019, The Linux Foundation. All rights reserved.
* Copyright (c) 2016 John Crispin <john@phrozen.org>
*/
@@ -935,6 +935,8 @@
qca8k_port_set_status(priv, port, 1);
priv->port_sts[port].enabled = 1;
+ phy_support_asym_pause(phy);
+
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net v2] mvpp2: refactor MTU change code
From: Matteo Croce @ 2019-07-28 0:46 UTC (permalink / raw)
To: netdev
Cc: Antoine Tenart, Maxime Chevallier, Marcin Wojtas, Stefan Chulski,
LKML, David Miller
The MTU change code can call napi_disable() with the device already down,
leading to a deadlock. Also, lot of code is duplicated unnecessarily.
Rework mvpp2_change_mtu() to avoid the deadlock and remove duplicated code.
Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
.../net/ethernet/marvell/mvpp2/mvpp2_main.c | 41 ++++++-------------
1 file changed, 13 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index b6591ea0c6d6..68fa2d563f0d 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3700,6 +3700,7 @@ static int mvpp2_set_mac_address(struct net_device *dev, void *p)
static int mvpp2_change_mtu(struct net_device *dev, int mtu)
{
struct mvpp2_port *port = netdev_priv(dev);
+ bool running = netif_running(dev);
int err;
if (!IS_ALIGNED(MVPP2_RX_PKT_SIZE(mtu), 8)) {
@@ -3708,40 +3709,24 @@ static int mvpp2_change_mtu(struct net_device *dev, int mtu)
mtu = ALIGN(MVPP2_RX_PKT_SIZE(mtu), 8);
}
- if (!netif_running(dev)) {
- err = mvpp2_bm_update_mtu(dev, mtu);
- if (!err) {
- port->pkt_size = MVPP2_RX_PKT_SIZE(mtu);
- return 0;
- }
-
- /* Reconfigure BM to the original MTU */
- err = mvpp2_bm_update_mtu(dev, dev->mtu);
- if (err)
- goto log_error;
- }
-
- mvpp2_stop_dev(port);
+ if (running)
+ mvpp2_stop_dev(port);
err = mvpp2_bm_update_mtu(dev, mtu);
- if (!err) {
+ if (err) {
+ netdev_err(dev, "failed to change MTU\n");
+ /* Reconfigure BM to the original MTU */
+ mvpp2_bm_update_mtu(dev, dev->mtu);
+ } else {
port->pkt_size = MVPP2_RX_PKT_SIZE(mtu);
- goto out_start;
}
- /* Reconfigure BM to the original MTU */
- err = mvpp2_bm_update_mtu(dev, dev->mtu);
- if (err)
- goto log_error;
-
-out_start:
- mvpp2_start_dev(port);
- mvpp2_egress_enable(port);
- mvpp2_ingress_enable(port);
+ if (running) {
+ mvpp2_start_dev(port);
+ mvpp2_egress_enable(port);
+ mvpp2_ingress_enable(port);
+ }
- return 0;
-log_error:
- netdev_err(dev, "failed to change MTU\n");
return err;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] rocker: fix memory leaks of fib_work on two error return paths
From: David Ahern @ 2019-07-28 0:32 UTC (permalink / raw)
To: Colin King, Jiri Pirko, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20190727233726.3121-1-colin.king@canonical.com>
On 7/27/19 5:37 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently there are two error return paths that leak memory allocated
> to fib_work. Fix this by kfree'ing fib_work before returning.
>
> Addresses-Coverity: ("Resource leak")
> Fixes: 19a9d136f198 ("ipv4: Flag fib_info with a fib_nh using IPv6 gateway")
> Fixes: dbcc4fa718ee ("rocker: Fail attempts to use routes with nexthop objects")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/rocker/rocker_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Thanks for the patch,
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next 02/10] libbpf: implement BPF CO-RE offset relocation algorithm
From: Song Liu @ 2019-07-28 0:24 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Yonghong Song, Kernel Team
In-Reply-To: <CAEf4BzbwzPsPonkqvGwS-FOWtWYQHQP=PwdVuVEkuEevrUKHWQ@mail.gmail.com>
> On Jul 27, 2019, at 12:09 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Jul 27, 2019 at 11:59 AM Song Liu <songliubraving@fb.com> wrote:
>>
>>
>>
>>> On Jul 26, 2019, at 11:11 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>>>
>>> On Thu, Jul 25, 2019 at 12:32 PM Song Liu <songliubraving@fb.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Jul 24, 2019, at 12:27 PM, Andrii Nakryiko <andriin@fb.com> wrote:
>>>>>
>>>>> This patch implements the core logic for BPF CO-RE offsets relocations.
>>>>> All the details are described in code comments.
>>>>
>>>> Some description in the change log is still useful. Please at least
>>>> copy-paste key comments here.
>>>
>>> OK, will add some more.
>>>
>>>>
>>>> And, this is looooong. I think it is totally possible to split it into
>>>> multiple smaller patches.
>>>
>>> I don't really know how to split it further without hurting reviewing
>>> by artificially splitting related code into separate patches. Remove
>>> any single function and algorithm will be incomplete.
>>>
>>> Let me give you some high-level overview of how pieces are put
>>> together. There are 9 non-trivial functions, let's go over their
>>> purpose in the orderd in which they are defined in file:
>>>
>>> 1. bpf_core_spec_parse()
>>>
>>> This one take bpf_offset_reloc's type_id and accessor string
>>> ("0:1:2:3") and parses it into more convenient bpf_core_spec
>>> datastructure, which has calculated offset and high-level spec
>>> "steps": either named field or array access.
>>>
>>> 2. bpf_core_find_cands()
>>>
>>> Given local type name, finds all possible target BTF types with same
>>> name (modulo "flavor" differences, ___flavor suffix is just ignored).
>>>
>>> 3. bpf_core_fields_are_compat()
>>>
>>> Given local and target field match, checks that their types are
>>> compatible (so that we don't accidentally match, e.g., int against
>>> struct).
>>>
>>> 4. bpf_core_match_member()
>>>
>>> Given named local field, find corresponding field in target struct. To
>>> understand why it's not trivial, here's an example:
>>>
>>> Local type:
>>>
>>> struct s___local {
>>> int a;
>>> };
>>>
>>> Target type:
>>>
>>> struct s___target {
>>> struct {
>>> union {
>>> int a;
>>> };
>>> };
>>> };
>>>
>>> For both cases you can access a as s.a, but in local case, field a is
>>> immediately inside s___local, while for s___target, you have to
>>> traverse two levels deeper into anonymous fields to get to an `a`
>>> inside anonymous union.
>>>
>>> So this function find that `a` by doing exhaustive search across all
>>> named field and anonymous struct/unions. But otherwise it's pretty
>>> straightforward recursive function.
>>>
>>> bpf_core_spec_match()
>>>
>>> Just goes over high-level spec steps in local spec and tries to figure
>>> out both high-level and low-level steps for targe type. Consider the
>>> above example. For both structs accessing s.a is one high-level step,
>>> but for s___local it's single low-level step (just another :0 in spec
>>> string), while for s___target it's three low-level steps: ":0:0:0",
>>> one step for each BTF type we need to traverse.
>>>
>>> Array access is simpler, it's always one high-level and one low-level step.
>>>
>>> bpf_core_reloc_insn()
>>>
>>> Once we match local and target specs and have local and target
>>> offsets, do the relocations - check that instruction has expected
>>> local offset and replace it with target offset.
>>>
>>> bpf_core_find_kernel_btf()
>>>
>>> This is the only function that can be moved into separate patch, but
>>> it's also very simple. It just iterates over few known possible
>>> locations for vmlinux image and once found, tries to parse .BTF out of
>>> it, to be used as target BTF.
>>>
>>> bpf_core_reloc_offset()
>>>
>>> It combines all the above functions to perform single relocation.
>>> Parse spec, get candidates, for each candidate try to find matching
>>> target spec. All candidates that matched are cached for given local
>>> root type.
>>
>> Thanks for these explanation. They are really helpful.
>>
>> I think some example explaining each step of bpf_core_reloc_offset()
>> will be very helpful. Something like:
>>
>> Example:
>>
>> struct s {
>> int a;
>> struct {
>> int b;
>> bool c;
>> };
>> };
>>
>> To get offset for c, we do:
>>
>> bpf_core_reloc_offset() {
>>
>> /* input data: xxx */
>>
>> /* first step: bpf_core_spec_parse() */
>>
>> /* data after first step */
>>
>> /* second step: bpf_core_find_cands() */
>>
>> /* candidate A and B after second step */
>>
>> ...
>> }
>>
>> Well, it requires quite some work to document this way. Please let me
>> know if you feel this is an overkill.
>
> Yeah :) And it's not just work, but I think it's bad if comments
> become too specific and document very low-level steps, because code
> might evolve and comments can quickly get out of sync and just add to
> confusion. Which is why I tried to document high-level ideas, leaving
> it up to the source code to be the ultimate reference of minutia
> details.
Fair enough.
>
>>
>>>
>>> bpf_core_reloc_offsets()
>>>
>>> High-level coordination. Iterate over all per-program .BTF.ext offset
>>> reloc sections, each relocation within them. Find corresponding
>>> program and try to apply relocations one by one.
>>>
>>>
>>> I think the only non-obvious part here is to understand that
>>> relocation records local raw spec with every single anonymous type
>>> traversal, which is not that useful when we try to match it against
>>> target type, which can have very different composition, but still the
>>> same field access pattern, from C language standpoint (which hides all
>>> those anonymous type traversals from programmer).
>>>
>>> But it should be pretty clear now, plus also check tests, they have
>>> lots of cases showing what's compatible and what's not.
>>
>> I see. I will review the tests.
>>
>>>>>
>>>>> static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
>>>>> - __u32 id)
>>>>> + __u32 id,
>>>>> + __u32 *res_id)
>>>>> {
>>>>> const struct btf_type *t = btf__type_by_id(btf, id);
>>>>
>>>> Maybe have a local "__u32 rid;"
>>>>
>>>>>
>>>>> + if (res_id)
>>>>> + *res_id = id;
>>>>> +
>>>>
>>>> and do "rid = id;" here
>>>>
>>>>> while (true) {
>>>>> switch (BTF_INFO_KIND(t->info)) {
>>>>> case BTF_KIND_VOLATILE:
>>>>> case BTF_KIND_CONST:
>>>>> case BTF_KIND_RESTRICT:
>>>>> case BTF_KIND_TYPEDEF:
>>>>> + if (res_id)
>>>>> + *res_id = t->type;
>>>> and here
>>>>
>>>>> t = btf__type_by_id(btf, t->type);
>>>>> break;
>>>>> default:
>>>> and "*res_id = rid;" right before return?
>>>
>>> Sure, but why?
>>
>> I think it is cleaner that way. But feel free to ignore if you
>> think otherwise.
>>
>>>
>>>>
>>>>> @@ -1041,7 +1049,7 @@ static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
>>>>> static bool get_map_field_int(const char *map_name, const struct btf *btf,
>>>>> const struct btf_type *def,
>>>>> const struct btf_member *m, __u32 *res) {
>>>
>>> [...]
>>>
>>>>> +struct bpf_core_spec {
>>>>> + const struct btf *btf;
>>>>> + /* high-level spec: named fields and array indicies only */
>>>>
>>>> typo: indices
>>>
>>> thanks!
>>>
>>>>
>>>>> + struct bpf_core_accessor spec[BPF_CORE_SPEC_MAX_LEN];
>>>>> + /* high-level spec length */
>>>>> + int len;
>>>>> + /* raw, low-level spec: 1-to-1 with accessor spec string */
>>>>> + int raw_spec[BPF_CORE_SPEC_MAX_LEN];
>>>>> + /* raw spec length */
>>>>> + int raw_len;
>>>>> + /* field byte offset represented by spec */
>>>>> + __u32 offset;
>>>>> +};
>>>
>>> [...]
>>>
>>>>> + *
>>>>> + * int x = &s->a[3]; // access string = '0:1:2:3'
>>>>> + *
>>>>> + * Low-level spec has 1:1 mapping with each element of access string (it's
>>>>> + * just a parsed access string representation): [0, 1, 2, 3].
>>>>> + *
>>>>> + * High-level spec will capture only 3 points:
>>>>> + * - intial zero-index access by pointer (&s->... is the same as &s[0]...);
>>>>> + * - field 'a' access (corresponds to '2' in low-level spec);
>>>>> + * - array element #3 access (corresponds to '3' in low-level spec).
>>>>> + *
>>>>> + */
>>>>
>>>> IIUC, high-level points are subset of low-level points. How about we introduce
>>>> "anonymous" high-level points, so that high-level points and low-level points
>>>> are 1:1 mapping?
>>>
>>> No, that will just hurt and complicate things. See above explanation
>>> about why we need high-level points (it's what you as C programmer try
>>> to achieve vs low-level spec is what C-language does in reality, with
>>> all the anonymous struct/union traversal).
>>>
>>> What's wrong with this separation? Think about it as recording
>>> "intent" (high-level spec) vs "mechanics" (low-level spec, how exactly
>>> to achieve that intent, in excruciating details).
>>
>> There is nothing wrong with separation. I just personally think it is
>> cleaner the other way. That's why I raised the question.
>>
>> I will go with your assessment, as you looked into this much more than
>> I did. :-)
>
> For me it's a machine view of the problem (raw spec) vs human view of
> the problem (high-level spec, which resembles how you think about this
> in C code). I'll keep it separate unless it proves to be problematic
> going forward.
[...]
>>>
>>> No. spec->offset is carefully maintained across multiple low-level
>>> steps, as we traverse down embedded structs/unions.
>>>
>>> Think about, e.g.:
>>>
>>> struct s {
>>> int a;
>>> struct {
>>> int b;
>>> };
>>> };
>>>
>>> Imagine you are trying to match s.b access. With what you propose
>>> you'll end up with offset 0, but it should be 4.
>>
>> Hmm... this is just for i == 0, right? Which line updated spec->offset
>> after "memset(spec, 0, sizeof(*spec));"?
>
> Ah, I missed that you are referring to the special i == 0 case. I can
> do assignment, yes, you are right. I'll probably also extract it out
> of the loop to make it less confusing.
Yes, please.
>
>>>>> + continue;
>>>>> + }
>>>>
>>>> Maybe pull i == 0 case out of the for loop?
As I mentioned earlier. ;-)
>>>>
>>>>> +
>>>>> + if (btf_is_composite(t)) {
>>>
>>> [...]
>>>
>>>>> +
>>>>> + if (spec->len == 0)
>>>>> + return -EINVAL;
>>>>
>>>> Can this ever happen?
>>>
>>> Not really, because I already check raw_len == 0 and exit with error.
>>> I'll remove.
>>>
>>>>
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>
[...]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox