* [PATCH bpf-next 0/4] selftests/bpf: test_progs: misc fixes
From: Stanislav Fomichev @ 2019-08-14 16:47 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
* make output a bit easier to follow
* add test__skip to indicate skipped tests
* remove global success/error counts (use environment)
* remove asserts from the tests
Cc: Andrii Nakryiko <andriin@fb.com>
Stanislav Fomichev (4):
selftests/bpf: test_progs: change formatting of the condenced output
selftests/bpf: test_progs: test__skip
selftests/bpf: test_progs: remove global fail/success counts
selftests/bpf: test_progs: remove asserts from subtests
.../selftests/bpf/prog_tests/bpf_obj_id.c | 32 +++++--
.../bpf/prog_tests/bpf_verif_scale.c | 10 +-
.../selftests/bpf/prog_tests/flow_dissector.c | 2 +-
.../bpf/prog_tests/get_stack_raw_tp.c | 2 +-
.../selftests/bpf/prog_tests/global_data.c | 10 +-
.../selftests/bpf/prog_tests/l4lb_all.c | 4 +-
.../selftests/bpf/prog_tests/map_lock.c | 28 +++---
.../selftests/bpf/prog_tests/pkt_access.c | 2 +-
.../selftests/bpf/prog_tests/pkt_md_access.c | 2 +-
.../bpf/prog_tests/queue_stack_map.c | 4 +-
.../bpf/prog_tests/reference_tracking.c | 2 +-
.../selftests/bpf/prog_tests/send_signal.c | 1 +
.../selftests/bpf/prog_tests/spinlock.c | 12 ++-
.../bpf/prog_tests/stacktrace_build_id.c | 11 ++-
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 11 ++-
.../selftests/bpf/prog_tests/stacktrace_map.c | 2 +-
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 2 +-
.../bpf/prog_tests/task_fd_query_rawtp.c | 2 +-
.../bpf/prog_tests/task_fd_query_tp.c | 2 +-
.../selftests/bpf/prog_tests/tcp_estats.c | 2 +-
tools/testing/selftests/bpf/prog_tests/xdp.c | 2 +-
.../bpf/prog_tests/xdp_adjust_tail.c | 2 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 +-
tools/testing/selftests/bpf/test_progs.c | 93 +++++++++++--------
tools/testing/selftests/bpf/test_progs.h | 28 +++++-
25 files changed, 165 insertions(+), 107 deletions(-)
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply
* [PATCH bpf-next 1/4] selftests/bpf: test_progs: change formatting of the condenced output
From: Stanislav Fomichev @ 2019-08-14 16:47 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190814164742.208909-1-sdf@google.com>
This makes it visually simpler to follow the output.
Also, highlight with red color failures when outputting to tty.
Before:
#1 attach_probe:FAIL
#2 bpf_obj_id:OK
#3/1 bpf_verif_scale:loop3.o:OK
#3/2 bpf_verif_scale:test_verif_scale1.o:OK
#3/3 bpf_verif_scale:test_verif_scale2.o:OK
#3/4 bpf_verif_scale:test_verif_scale3.o:OK
#3/5 bpf_verif_scale:pyperf50.o:OK
#3/6 bpf_verif_scale:pyperf100.o:OK
#3/7 bpf_verif_scale:pyperf180.o:OK
#3/8 bpf_verif_scale:pyperf600.o:OK
#3/9 bpf_verif_scale:pyperf600_nounroll.o:OK
#3/10 bpf_verif_scale:loop1.o:OK
#3/11 bpf_verif_scale:loop2.o:OK
#3/12 bpf_verif_scale:loop4.o:OK
#3/13 bpf_verif_scale:loop5.o:OK
#3/14 bpf_verif_scale:strobemeta.o:OK
#3/15 bpf_verif_scale:strobemeta_nounroll1.o:OK
#3/16 bpf_verif_scale:strobemeta_nounroll2.o:OK
#3/17 bpf_verif_scale:test_sysctl_loop1.o:OK
#3/18 bpf_verif_scale:test_sysctl_loop2.o:OK
#3/19 bpf_verif_scale:test_xdp_loop.o:OK
#3/20 bpf_verif_scale:test_seg6_loop.o:OK
#3 bpf_verif_scale:OK
#4 flow_dissector:OK
After:
# 1 FAIL attach_probe
# 2 OK bpf_obj_id
# 3/1 OK bpf_verif_scale:loop3.o
# 3/2 OK bpf_verif_scale:test_verif_scale1.o
# 3/3 OK bpf_verif_scale:test_verif_scale2.o
# 3/4 OK bpf_verif_scale:test_verif_scale3.o
# 3/5 OK bpf_verif_scale:pyperf50.o
# 3/6 OK bpf_verif_scale:pyperf100.o
# 3/7 OK bpf_verif_scale:pyperf180.o
# 3/8 OK bpf_verif_scale:pyperf600.o
# 3/9 OK bpf_verif_scale:pyperf600_nounroll.o
# 3/10 OK bpf_verif_scale:loop1.o
# 3/11 OK bpf_verif_scale:loop2.o
# 3/12 OK bpf_verif_scale:loop4.o
# 3/13 OK bpf_verif_scale:loop5.o
# 3/14 OK bpf_verif_scale:strobemeta.o
# 3/15 OK bpf_verif_scale:strobemeta_nounroll1.o
# 3/16 OK bpf_verif_scale:strobemeta_nounroll2.o
# 3/17 OK bpf_verif_scale:test_sysctl_loop1.o
# 3/18 OK bpf_verif_scale:test_sysctl_loop2.o
# 3/19 OK bpf_verif_scale:test_xdp_loop.o
# 3/20 OK bpf_verif_scale:test_seg6_loop.o
# 3 OK bpf_verif_scale
# 4 OK flow_dissector
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/test_progs.c | 29 +++++++++++++++++++-----
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 12895d03d58b..1a7a2a0c0a11 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -56,6 +56,21 @@ static void dump_test_log(const struct prog_test_def *test, bool failed)
fseeko(stdout, 0, SEEK_SET); /* rewind */
}
+static const char *test_status_string(bool success)
+{
+#define COLOR_RED "\033[31m"
+#define COLOR_RESET "\033[m"
+ if (success)
+ return "OK";
+
+ if (isatty(fileno(env.stdout)))
+ return COLOR_RED "FAIL" COLOR_RESET;
+ else
+ return "FAIL";
+#undef COLOR_RED
+#undef COLOR_RESET
+}
+
void test__end_subtest()
{
struct prog_test_def *test = env.test;
@@ -68,9 +83,10 @@ void test__end_subtest()
dump_test_log(test, sub_error_cnt);
- fprintf(env.stdout, "#%d/%d %s:%s\n",
- test->test_num, test->subtest_num,
- test->subtest_name, sub_error_cnt ? "FAIL" : "OK");
+ fprintf(env.stdout, "#%3d/%-3d %4s %s:%s\n",
+ test->test_num, test->subtest_num,
+ test_status_string(test->fail_cnt == 0),
+ test->test_name, test->subtest_name);
}
bool test__start_subtest(const char *name)
@@ -513,9 +529,10 @@ int main(int argc, char **argv)
dump_test_log(test, test->error_cnt);
- fprintf(env.stdout, "#%d %s:%s\n",
- test->test_num, test->test_name,
- test->error_cnt ? "FAIL" : "OK");
+ fprintf(env.stdout, "#%3d %4s %s\n",
+ test->test_num,
+ test_status_string(test->fail_cnt == 0),
+ test->test_name);
}
stdio_restore();
printf("Summary: %d/%d PASSED, %d FAILED\n",
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH bpf-next 2/4] selftests/bpf: test_progs: test__skip
From: Stanislav Fomichev @ 2019-08-14 16:47 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190814164742.208909-1-sdf@google.com>
Export test__skip() to indicate skipped tests and use it in
test_send_signal_nmi().
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/prog_tests/send_signal.c | 1 +
tools/testing/selftests/bpf/test_progs.c | 9 +++++++--
tools/testing/selftests/bpf/test_progs.h | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 1575f0a1f586..40c2c5efdd3e 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -204,6 +204,7 @@ static int test_send_signal_nmi(void)
if (errno == ENOENT) {
printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n",
__func__);
+ test__skip();
return 0;
}
/* Let the test fail with a more informative message */
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 1a7a2a0c0a11..1993f2ce0d23 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -121,6 +121,11 @@ void test__force_log() {
env.test->force_log = true;
}
+void test__skip(void)
+{
+ env.skip_cnt++;
+}
+
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -535,8 +540,8 @@ int main(int argc, char **argv)
test->test_name);
}
stdio_restore();
- printf("Summary: %d/%d PASSED, %d FAILED\n",
- env.succ_cnt, env.sub_succ_cnt, env.fail_cnt);
+ printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
+ env.succ_cnt, env.sub_succ_cnt, env.skip_cnt, env.fail_cnt);
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 37d427f5a1e5..9defd35cb6c0 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -64,6 +64,7 @@ struct test_env {
int succ_cnt; /* successful tests */
int sub_succ_cnt; /* successful sub-tests */
int fail_cnt; /* total failed tests + sub-tests */
+ int skip_cnt; /* skipped tests */
};
extern int error_cnt;
@@ -72,6 +73,7 @@ extern struct test_env env;
extern void test__force_log();
extern bool test__start_subtest(const char *name);
+extern void test__skip(void);
#define MAGIC_BYTES 123
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH bpf-next 3/4] selftests/bpf: test_progs: remove global fail/success counts
From: Stanislav Fomichev @ 2019-08-14 16:47 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190814164742.208909-1-sdf@google.com>
Now that we have a global per-test/per-environment state, there
is no longer the need to have global fail/success counters
(and there is no need to save/get the diff before/after the
test).
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_obj_id.c | 2 +-
.../bpf/prog_tests/bpf_verif_scale.c | 10 +---
.../selftests/bpf/prog_tests/flow_dissector.c | 2 +-
.../bpf/prog_tests/get_stack_raw_tp.c | 2 +-
.../selftests/bpf/prog_tests/global_data.c | 10 ++--
.../selftests/bpf/prog_tests/l4lb_all.c | 4 +-
.../selftests/bpf/prog_tests/map_lock.c | 8 +--
.../selftests/bpf/prog_tests/pkt_access.c | 2 +-
.../selftests/bpf/prog_tests/pkt_md_access.c | 2 +-
.../bpf/prog_tests/queue_stack_map.c | 4 +-
.../bpf/prog_tests/reference_tracking.c | 2 +-
.../selftests/bpf/prog_tests/spinlock.c | 2 +-
.../selftests/bpf/prog_tests/stacktrace_map.c | 2 +-
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 2 +-
.../bpf/prog_tests/task_fd_query_rawtp.c | 2 +-
.../bpf/prog_tests/task_fd_query_tp.c | 2 +-
.../selftests/bpf/prog_tests/tcp_estats.c | 2 +-
tools/testing/selftests/bpf/prog_tests/xdp.c | 2 +-
.../bpf/prog_tests/xdp_adjust_tail.c | 2 +-
.../selftests/bpf/prog_tests/xdp_noinline.c | 4 +-
tools/testing/selftests/bpf/test_progs.c | 55 ++++++++-----------
tools/testing/selftests/bpf/test_progs.h | 26 +++++++--
22 files changed, 75 insertions(+), 74 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 fb5840a62548..f57e0c625de3 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -49,7 +49,7 @@ void test_bpf_obj_id(void)
* to load.
*/
if (err)
- error_cnt++;
+ test__fail();
assert(!err);
/* Insert a magic value to the map */
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 1a1eae356f81..217988243077 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
@@ -28,8 +28,6 @@ static int check_load(const char *file, enum bpf_prog_type type)
attr.prog_flags = BPF_F_TEST_RND_HI32;
err = bpf_prog_load_xattr(&attr, &obj, &prog_fd);
bpf_object__close(obj);
- if (err)
- error_cnt++;
return err;
}
@@ -105,12 +103,8 @@ void test_bpf_verif_scale(void)
continue;
err = check_load(test->file, test->attach_type);
- if (test->fails) { /* expected to fail */
- if (err)
- error_cnt--;
- else
- error_cnt++;
- }
+ if (err && !test->fails)
+ test__fail();
}
if (env.verifier_stats)
diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
index 6892b88ae065..e9d882c05ded 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
@@ -453,7 +453,7 @@ void test_flow_dissector(void)
err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector",
"jmp_table", "last_dissection", &prog_fd, &keys_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
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 3d59b3c841fe..afc60f62e2a8 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
@@ -137,7 +137,7 @@ void test_get_stack_raw_tp(void)
goto close_prog_noerr;
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
if (!IS_ERR_OR_NULL(link))
bpf_link__destroy(link);
diff --git a/tools/testing/selftests/bpf/prog_tests/global_data.c b/tools/testing/selftests/bpf/prog_tests/global_data.c
index d011079fb0bf..db13bfee6bb9 100644
--- a/tools/testing/selftests/bpf/prog_tests/global_data.c
+++ b/tools/testing/selftests/bpf/prog_tests/global_data.c
@@ -8,7 +8,7 @@ static void test_global_data_number(struct bpf_object *obj, __u32 duration)
map_fd = bpf_find_map(__func__, obj, "result_number");
if (map_fd < 0) {
- error_cnt++;
+ test__fail();
return;
}
@@ -45,7 +45,7 @@ static void test_global_data_string(struct bpf_object *obj, __u32 duration)
map_fd = bpf_find_map(__func__, obj, "result_string");
if (map_fd < 0) {
- error_cnt++;
+ test__fail();
return;
}
@@ -82,7 +82,7 @@ static void test_global_data_struct(struct bpf_object *obj, __u32 duration)
map_fd = bpf_find_map(__func__, obj, "result_struct");
if (map_fd < 0) {
- error_cnt++;
+ test__fail();
return;
}
@@ -113,13 +113,13 @@ static void test_global_data_rdonly(struct bpf_object *obj, __u32 duration)
map = bpf_object__find_map_by_name(obj, "test_glo.rodata");
if (!map || !bpf_map__is_internal(map)) {
- error_cnt++;
+ test__fail();
return;
}
map_fd = bpf_map__fd(map);
if (map_fd < 0) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
index 20ddca830e68..724bb40de1f8 100644
--- a/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
+++ b/tools/testing/selftests/bpf/prog_tests/l4lb_all.c
@@ -31,7 +31,7 @@ static void test_l4lb(const char *file)
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
@@ -73,7 +73,7 @@ static void test_l4lb(const char *file)
pkts += stats[i].pkts;
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
- error_cnt++;
+ test__fail();
printf("test_l4lb:FAIL:stats %lld %lld\n", bytes, pkts);
}
out:
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index ee99368c595c..12123ff1f31f 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -10,12 +10,12 @@ static void *parallel_map_access(void *arg)
err = bpf_map_lookup_elem_flags(map_fd, &key, vars, BPF_F_LOCK);
if (err) {
printf("lookup failed\n");
- error_cnt++;
+ test__fail();
goto out;
}
if (vars[0] != 0) {
printf("lookup #%d var[0]=%d\n", i, vars[0]);
- error_cnt++;
+ test__fail();
goto out;
}
rnd = vars[1];
@@ -24,7 +24,7 @@ static void *parallel_map_access(void *arg)
continue;
printf("lookup #%d var[1]=%d var[%d]=%d\n",
i, rnd, j, vars[j]);
- error_cnt++;
+ test__fail();
goto out;
}
}
@@ -69,7 +69,7 @@ void test_map_lock(void)
ret == (void *)&map_fd[i - 4]);
goto close_prog_noerr;
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/pkt_access.c b/tools/testing/selftests/bpf/prog_tests/pkt_access.c
index 4ecfd721a044..9ef4e4ffb379 100644
--- a/tools/testing/selftests/bpf/prog_tests/pkt_access.c
+++ b/tools/testing/selftests/bpf/prog_tests/pkt_access.c
@@ -10,7 +10,7 @@ void test_pkt_access(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c b/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
index ac0d43435806..c354b9d21f4f 100644
--- a/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
+++ b/tools/testing/selftests/bpf/prog_tests/pkt_md_access.c
@@ -10,7 +10,7 @@ void test_pkt_md_access(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
index e60cd5ff1f55..48a8cd144bd1 100644
--- a/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/queue_stack_map.c
@@ -28,7 +28,7 @@ static void test_queue_stack_map_by_type(int type)
err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
@@ -44,7 +44,7 @@ static void test_queue_stack_map_by_type(int type)
for (i = 0; i < MAP_SIZE; i++) {
err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
if (err) {
- error_cnt++;
+ test__fail();
goto out;
}
}
diff --git a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
index 4a4f428d1a78..f6987e3dd28c 100644
--- a/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
+++ b/tools/testing/selftests/bpf/prog_tests/reference_tracking.c
@@ -11,7 +11,7 @@ void test_reference_tracking(void)
obj = bpf_object__open(file);
if (IS_ERR(obj)) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index 114ebe6a438e..e843336713e8 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -23,7 +23,7 @@ void test_spinlock(void)
ret == (void *)&prog_fd);
goto close_prog_noerr;
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
index fc539335c5b3..9dba1cc3da60 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -70,7 +70,7 @@ void test_stacktrace_map(void)
goto disable_pmu_noerr;
disable_pmu:
- error_cnt++;
+ test__fail();
disable_pmu_noerr:
bpf_link__destroy(link);
close_prog:
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
index fbfa8e76cf63..4e7cf2e663f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
@@ -60,7 +60,7 @@ void test_stacktrace_map_raw_tp(void)
goto close_prog_noerr;
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
if (!IS_ERR_OR_NULL(link))
bpf_link__destroy(link);
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
index 958a3d88de99..d9ad1aa8a026 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
@@ -72,7 +72,7 @@ void test_task_fd_query_rawtp(void)
goto close_prog_noerr;
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
index f9b70e81682b..76209f2386c8 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
@@ -68,7 +68,7 @@ static void test_task_fd_query_tp_core(const char *probe_name,
close_pmu:
close(pmu_fd);
close_prog:
- error_cnt++;
+ test__fail();
close_prog_noerr:
bpf_object__close(obj);
}
diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_estats.c b/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
index bb8759d69099..e241e5d7c71f 100644
--- a/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
+++ b/tools/testing/selftests/bpf/prog_tests/tcp_estats.c
@@ -11,7 +11,7 @@ void test_tcp_estats(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
CHECK(err, "", "err %d errno %d\n", err, errno);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp.c b/tools/testing/selftests/bpf/prog_tests/xdp.c
index a74167289545..7c9f89fa1d02 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp.c
@@ -17,7 +17,7 @@ void test_xdp(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
index 922aa0a19764..a479a3303c3b 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c
@@ -11,7 +11,7 @@ void test_xdp_adjust_tail(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
index 15f7c272edb0..10bef9d5ab81 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_noinline.c
@@ -32,7 +32,7 @@ void test_xdp_noinline(void)
err = bpf_prog_load(file, BPF_PROG_TYPE_XDP, &obj, &prog_fd);
if (err) {
- error_cnt++;
+ test__fail();
return;
}
@@ -74,7 +74,7 @@ void test_xdp_noinline(void)
pkts += stats[i].pkts;
}
if (bytes != MAGIC_BYTES * NUM_ITER * 2 || pkts != NUM_ITER * 2) {
- error_cnt++;
+ test__fail();
printf("test_xdp_noinline:FAIL:stats %lld %lld\n",
bytes, pkts);
}
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 1993f2ce0d23..ad90e45768ce 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -8,24 +8,6 @@
/* defined in test_progs.h */
struct test_env env;
-int error_cnt, pass_cnt;
-
-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;
-
- 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)
{
@@ -74,14 +56,14 @@ static const char *test_status_string(bool success)
void test__end_subtest()
{
struct prog_test_def *test = env.test;
- int sub_error_cnt = error_cnt - test->old_error_cnt;
+ int sub_fail_cnt = test->fail_cnt - test->old_fail_cnt;
- if (sub_error_cnt)
- env.fail_cnt++;
+ if (sub_fail_cnt)
+ test->fail_cnt++;
else
env.sub_succ_cnt++;
- dump_test_log(test, sub_error_cnt);
+ dump_test_log(test, sub_fail_cnt);
fprintf(env.stdout, "#%3d/%-3d %4s %s:%s\n",
test->test_num, test->subtest_num,
@@ -111,8 +93,8 @@ bool test__start_subtest(const char *name)
return false;
test->subtest_name = name;
- env.test->old_pass_cnt = pass_cnt;
- env.test->old_error_cnt = error_cnt;
+ env.test->old_succ_cnt = env.test->succ_cnt;
+ env.test->old_fail_cnt = env.test->fail_cnt;
return true;
}
@@ -126,6 +108,19 @@ void test__skip(void)
env.skip_cnt++;
}
+void __test__fail(const char *file, int line)
+{
+ if (env.test->subtest_name)
+ fprintf(stderr, "%s:%s failed at %s:%d, errno=%d\n",
+ env.test->test_name, env.test->subtest_name,
+ file, line, errno);
+ else
+ fprintf(stderr, "%s failed at %s:%d, errno=%d\n",
+ env.test->test_name, file, line, errno);
+
+ env.test->fail_cnt++;
+}
+
struct ipv4_packet pkt_v4 = {
.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
.iph.ihl = 5,
@@ -150,7 +145,7 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name)
map = bpf_object__find_map_by_name(obj, name);
if (!map) {
printf("%s:FAIL:map '%s' not found\n", test, name);
- error_cnt++;
+ test__fail();
return -1;
}
return bpf_map__fd(map);
@@ -509,8 +504,6 @@ int main(int argc, char **argv)
stdio_hijack();
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;
@@ -525,14 +518,12 @@ int main(int argc, char **argv)
test__end_subtest();
test->tested = true;
- test->pass_cnt = pass_cnt - old_pass_cnt;
- test->error_cnt = error_cnt - old_error_cnt;
- if (test->error_cnt)
+ if (test->fail_cnt)
env.fail_cnt++;
else
env.succ_cnt++;
- dump_test_log(test, test->error_cnt);
+ dump_test_log(test, test->fail_cnt);
fprintf(env.stdout, "#%3d %4s %s\n",
test->test_num,
@@ -546,5 +537,5 @@ int main(int argc, char **argv)
free(env.test_selector.num_set);
free(env.subtest_selector.num_set);
- return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
+ return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
}
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 9defd35cb6c0..7b05921784a4 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -38,7 +38,23 @@ typedef __u16 __sum16;
#include "trace_helpers.h"
#include "flow_dissector_load.h"
-struct prog_test_def;
+struct prog_test_def {
+ const char *test_name;
+ int test_num;
+ void (*run_test)(void);
+ bool force_log;
+ bool tested;
+
+ const char *subtest_name;
+ int subtest_num;
+
+ int succ_cnt;
+ int fail_cnt;
+
+ /* store counts before subtest started */
+ int old_succ_cnt;
+ int old_fail_cnt;
+};
struct test_selector {
const char *name;
@@ -67,13 +83,13 @@ struct test_env {
int skip_cnt; /* skipped tests */
};
-extern int error_cnt;
-extern int pass_cnt;
extern struct test_env env;
extern void test__force_log();
extern bool test__start_subtest(const char *name);
extern void test__skip(void);
+#define test__fail() __test__fail(__FILE__, __LINE__)
+extern void __test__fail(const char *file, int line);
#define MAGIC_BYTES 123
@@ -96,11 +112,11 @@ extern struct ipv6_packet pkt_v6;
#define _CHECK(condition, tag, duration, format...) ({ \
int __ret = !!(condition); \
if (__ret) { \
- error_cnt++; \
+ test__fail(); \
printf("%s:FAIL:%s ", __func__, tag); \
printf(format); \
} else { \
- pass_cnt++; \
+ env.test->succ_cnt++; \
printf("%s:PASS:%s %d nsec\n", \
__func__, tag, duration); \
} \
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* [PATCH bpf-next 4/4] selftests/bpf: test_progs: remove asserts from subtests
From: Stanislav Fomichev @ 2019-08-14 16:47 UTC (permalink / raw)
To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev, Andrii Nakryiko
In-Reply-To: <20190814164742.208909-1-sdf@google.com>
Otherwise they can bring the whole process down.
Cc: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/bpf_obj_id.c | 30 ++++++++++++++-----
.../selftests/bpf/prog_tests/map_lock.c | 20 ++++++++-----
.../selftests/bpf/prog_tests/spinlock.c | 10 ++++---
.../bpf/prog_tests/stacktrace_build_id.c | 11 +++++--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 11 +++++--
5 files changed, 57 insertions(+), 25 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 f57e0c625de3..4ec8c4e9e9a1 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_obj_id.c
@@ -48,16 +48,23 @@ void test_bpf_obj_id(void)
/* test_obj_id.o is a dumb prog. It should never fail
* to load.
*/
- if (err)
+ if (err) {
test__fail();
- assert(!err);
+ continue;
+ }
/* Insert a magic value to the map */
map_fds[i] = bpf_find_map(__func__, objs[i], "test_map_id");
- assert(map_fds[i] >= 0);
+ if (map_fds[i] < 0) {
+ test__fail();
+ goto done;
+ }
err = bpf_map_update_elem(map_fds[i], &array_key,
&array_magic_value, 0);
- assert(!err);
+ if (err) {
+ test__fail();
+ goto done;
+ }
/* Check getting map info */
info_len = sizeof(struct bpf_map_info) * 2;
@@ -96,9 +103,15 @@ void test_bpf_obj_id(void)
prog_infos[i].map_ids = ptr_to_u64(map_ids + i);
prog_infos[i].nr_map_ids = 2;
err = clock_gettime(CLOCK_REALTIME, &real_time_ts);
- assert(!err);
+ if (err) {
+ test__fail();
+ goto done;
+ }
err = clock_gettime(CLOCK_BOOTTIME, &boot_time_ts);
- assert(!err);
+ if (err) {
+ test__fail();
+ goto done;
+ }
err = bpf_obj_get_info_by_fd(prog_fds[i], &prog_infos[i],
&info_len);
load_time = (real_time_ts.tv_sec - boot_time_ts.tv_sec)
@@ -224,7 +237,10 @@ void test_bpf_obj_id(void)
nr_id_found++;
err = bpf_map_lookup_elem(map_fd, &array_key, &array_value);
- assert(!err);
+ if (err) {
+ test__fail();
+ goto done;
+ }
err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
CHECK(err || info_len != sizeof(struct bpf_map_info) ||
diff --git a/tools/testing/selftests/bpf/prog_tests/map_lock.c b/tools/testing/selftests/bpf/prog_tests/map_lock.c
index 12123ff1f31f..e7663721fb57 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_lock.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_lock.c
@@ -56,17 +56,21 @@ void test_map_lock(void)
bpf_map_update_elem(map_fd[0], &key, vars, BPF_F_LOCK);
for (i = 0; i < 4; i++)
- assert(pthread_create(&thread_id[i], NULL,
- &spin_lock_thread, &prog_fd) == 0);
+ if (pthread_create(&thread_id[i], NULL,
+ &spin_lock_thread, &prog_fd))
+ goto close_prog;
for (i = 4; i < 6; i++)
- assert(pthread_create(&thread_id[i], NULL,
- ¶llel_map_access, &map_fd[i - 4]) == 0);
+ if (pthread_create(&thread_id[i], NULL,
+ ¶llel_map_access, &map_fd[i - 4]))
+ goto close_prog;
for (i = 0; i < 4; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&prog_fd);
+ if (pthread_join(thread_id[i], &ret) ||
+ ret != (void *)&prog_fd)
+ goto close_prog;
for (i = 4; i < 6; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&map_fd[i - 4]);
+ if (pthread_join(thread_id[i], &ret) ||
+ ret != (void *)&map_fd[i - 4])
+ goto close_prog;
goto close_prog_noerr;
close_prog:
test__fail();
diff --git a/tools/testing/selftests/bpf/prog_tests/spinlock.c b/tools/testing/selftests/bpf/prog_tests/spinlock.c
index e843336713e8..5f32a913f732 100644
--- a/tools/testing/selftests/bpf/prog_tests/spinlock.c
+++ b/tools/testing/selftests/bpf/prog_tests/spinlock.c
@@ -16,11 +16,13 @@ void test_spinlock(void)
goto close_prog;
}
for (i = 0; i < 4; i++)
- assert(pthread_create(&thread_id[i], NULL,
- &spin_lock_thread, &prog_fd) == 0);
+ if (pthread_create(&thread_id[i], NULL,
+ &spin_lock_thread, &prog_fd))
+ goto close_prog;
+
for (i = 0; i < 4; i++)
- assert(pthread_join(thread_id[i], &ret) == 0 &&
- ret == (void *)&prog_fd);
+ if (pthread_join(thread_id[i], &ret) || ret != (void *)&prog_fd)
+ goto close_prog;
goto close_prog_noerr;
close_prog:
test__fail();
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..d74464faebd7 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -51,9 +51,14 @@ void test_stacktrace_build_id(void)
"err %d errno %d\n", err, errno))
goto disable_pmu;
- assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")
- == 0);
- assert(system("./urandom_read") == 0);
+ if (system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")) {
+ test__fail();
+ goto disable_pmu;
+ }
+ if (system("./urandom_read")) {
+ test__fail();
+ goto disable_pmu;
+ }
/* disable stack trace collection */
key = 0;
val = 1;
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..e886911928bc 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
@@ -82,9 +82,14 @@ void test_stacktrace_build_id_nmi(void)
"err %d errno %d\n", err, errno))
goto disable_pmu;
- assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")
- == 0);
- assert(system("taskset 0x1 ./urandom_read 100000") == 0);
+ if (system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")) {
+ test__fail();
+ goto disable_pmu;
+ }
+ if (system("taskset 0x1 ./urandom_read 100000")) {
+ test__fail();
+ goto disable_pmu;
+ }
/* disable stack trace collection */
key = 0;
val = 1;
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* Re: [PATCH net-next v6 6/6] net: mscc: PTP Hardware Clock (PHC) support
From: David Miller @ 2019-08-14 16:49 UTC (permalink / raw)
To: antoine.tenart
Cc: richardcochran, alexandre.belloni, UNGLinuxDriver, netdev,
thomas.petazzoni, allan.nielsen, andrew
In-Reply-To: <20190812144537.14497-7-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Mon, 12 Aug 2019 16:45:37 +0200
> This patch adds support for PTP Hardware Clock (PHC) to the Ocelot
> switch for both PTP 1-step and 2-step modes.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Richard, I really need your review on this patch.
Thank you.
^ permalink raw reply
* fallout from net-next netfilter changes
From: David Miller @ 2019-08-14 16:53 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
This started happening after Jakub's pull of your net-next changes
yesterday:
./include/uapi/linux/netfilter_ipv6/ip6t_LOG.h:5:2: warning: #warning "Please update iptables, this file will be removed soon!" [-Wcpp]
#warning "Please update iptables, this file will be removed soon!"
^~~~~~~
In file included from <command-line>:
./include/uapi/linux/netfilter_ipv4/ipt_LOG.h:5:2: warning: #warning "Please update iptables, this file will be removed soon!" [-Wcpp]
#warning "Please update iptables, this file will be removed soon!"
^~~~~~~
It's probaly from the standard kernel build UAPI header checks.
Please fix this.
^ permalink raw reply
* Re: [PATCH net-next] r8169: fix sporadic transmit timeout issue
From: David Miller @ 2019-08-14 16:54 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev, edumazet, holger
In-Reply-To: <e343933b-1965-4617-3011-6290ed30d4ae@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Mon, 12 Aug 2019 20:47:40 +0200
> Holger reported sporadic transmit timeouts and it turned out that one
> path misses ringing the doorbell. Fix was suggested by Eric.
>
> Fixes: ef14358546b1 ("r8169: make use of xmit_more")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH v4 9/9] Input: add IOC3 serio driver
From: Jonas Gorski @ 2019-08-14 16:57 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
Lee Jones, David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby,
Evgeniy Polyakov, linux-mips, linux-kernel, linux-input,
Network Development, linux-rtc, linux-serial
In-Reply-To: <20190814163733.82f624e342d061866ba8ff87@suse.de>
On Wed, 14 Aug 2019 at 16:37, Thomas Bogendoerfer <tbogendoerfer@suse.de> wrote:
>
> On Wed, 14 Aug 2019 15:20:14 +0200
> Jonas Gorski <jonas.gorski@gmail.com> wrote:
>
> > > + d = devm_kzalloc(&pdev->dev, sizeof(*d), GFP_KERNEL);
> >
> > &pdev->dev => dev
>
> will change.
>
> >
> > > + if (!d)
> > > + return -ENOMEM;
> > > +
> > > + sk = kzalloc(sizeof(*sk), GFP_KERNEL);
> >
> > any reason not to devm_kzalloc this as well? Then you won't need to
> > manually free it in the error cases.
>
> it has different life time than the device, so it may not allocated
> via devm_kzalloc
>
> > > +static int ioc3kbd_remove(struct platform_device *pdev)
> > > +{
> > > + struct ioc3kbd_data *d = platform_get_drvdata(pdev);
> > > +
> > > + devm_free_irq(&pdev->dev, d->irq, d);
> > > + serio_unregister_port(d->kbd);
> > > + serio_unregister_port(d->aux);
> > > + return 0;
> > > +}
> >
> > and on that topic, won't you need to kfree d->kbd and d->aux here?
>
> that's done in serio_release_port() by the serio core.
i see. But in that case, don't the kfree's after the
serio_unregister_port's in the error path of the .probe function cause
a double free?
Regards
Jonas
^ permalink raw reply
* Re: [RFC bpf-next 0/3] tools: bpftool: add subcommand to count map entries
From: Alexei Starovoitov @ 2019-08-14 16:58 UTC (permalink / raw)
To: Edward Cree
Cc: Quentin Monnet, Alexei Starovoitov, Daniel Borkmann, bpf,
Network Development, oss-drivers
In-Reply-To: <bdb4b47b-25fa-eb96-aa8d-dd4f4b012277@solarflare.com>
On Wed, Aug 14, 2019 at 9:45 AM Edward Cree <ecree@solarflare.com> wrote:
>
> On 14/08/2019 10:42, Quentin Monnet wrote:
> > 2019-08-13 18:51 UTC-0700 ~ Alexei Starovoitov
> > <alexei.starovoitov@gmail.com>
> >> The same can be achieved by 'bpftool map dump|grep key|wc -l', no?
> > To some extent (with subtleties for some other map types); and we use a
> > similar command line as a workaround for now. But because of the rate of
> > inserts/deletes in the map, the process often reports a number higher
> > than the max number of entries (we observed up to ~750k when max_entries
> > is 500k), even is the map is only half-full on average during the count.
> > On the worst case (though not frequent), an entry is deleted just before
> > we get the next key from it, and iteration starts all over again. This
> > is not reliable to determine how much space is left in the map.
> >
> > I cannot see a solution that would provide a more accurate count from
> > user space, when the map is under pressure?
> This might be a really dumb suggestion, but: you're wanting to collect a
> summary statistic over an in-kernel data structure in a single syscall,
> because making a series of syscalls to examine every entry is slow and
> racy. Isn't that exactly a job for an in-kernel virtual machine, and
> could you not supply an eBPF program which the kernel runs on each entry
> in the map, thus supporting people who want to calculate something else
> (mean, min and max, whatever) instead of count?
Pretty much my suggestion as well :)
It seems the better fix for your nat threshold is to keep count of
elements in the map in a separate global variable that
bpf program manually increments and decrements.
bpftool will dump it just as regular map of single element.
(I believe it doesn't recognize global variables properly yet)
and BTF will be there to pick exactly that 'count' variable.
^ permalink raw reply
* Re: [RFC bpf-next 0/3] tools: bpftool: add subcommand to count map entries
From: Quentin Monnet @ 2019-08-14 16:58 UTC (permalink / raw)
To: Edward Cree, Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev, oss-drivers
In-Reply-To: <bdb4b47b-25fa-eb96-aa8d-dd4f4b012277@solarflare.com>
2019-08-14 17:45 UTC+0100 ~ Edward Cree <ecree@solarflare.com>
> On 14/08/2019 10:42, Quentin Monnet wrote:
>> 2019-08-13 18:51 UTC-0700 ~ Alexei Starovoitov
>> <alexei.starovoitov@gmail.com>
>>> The same can be achieved by 'bpftool map dump|grep key|wc -l', no?
>> To some extent (with subtleties for some other map types); and we use a
>> similar command line as a workaround for now. But because of the rate of
>> inserts/deletes in the map, the process often reports a number higher
>> than the max number of entries (we observed up to ~750k when max_entries
>> is 500k), even is the map is only half-full on average during the count.
>> On the worst case (though not frequent), an entry is deleted just before
>> we get the next key from it, and iteration starts all over again. This
>> is not reliable to determine how much space is left in the map.
>>
>> I cannot see a solution that would provide a more accurate count from
>> user space, when the map is under pressure?
> This might be a really dumb suggestion, but: you're wanting to collect a
> summary statistic over an in-kernel data structure in a single syscall,
> because making a series of syscalls to examine every entry is slow and
> racy. Isn't that exactly a job for an in-kernel virtual machine, and
> could you not supply an eBPF program which the kernel runs on each entry
> in the map, thus supporting people who want to calculate something else
> (mean, min and max, whatever) instead of count?
>
Hi Edward, I like the approach, thanks for the suggestion.
But I did not mention that we were using offloaded maps: Tracing the
kernel would probably work for programs running on the host, but this is
not a solution we could extend to hardware offload.
Best regards,
Quentin
^ permalink raw reply
* [PATCH] netfilter: nft_bitwise: Adjust parentheses to fix memcmp size argument
From: Nathan Chancellor @ 2019-08-14 16:58 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: David S. Miller, netfilter-devel, coreteam, netdev, linux-kernel,
clang-built-linux, Nathan Chancellor, kbuild test robot
clang warns:
net/netfilter/nft_bitwise.c:138:50: error: size argument in 'memcmp'
call is a comparison [-Werror,-Wmemsize-comparison]
if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
~~~~~~~~~~~~~~~~~~^~
net/netfilter/nft_bitwise.c:138:6: note: did you mean to compare the
result of 'memcmp' instead?
if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
^
)
net/netfilter/nft_bitwise.c:138:32: note: explicitly cast the argument
to size_t to silence this warning
if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
^
(size_t)(
1 error generated.
Adjust the parentheses so that the result of the sizeof is used for the
size argument in memcmp, rather than the result of the comparison (which
would always be true because sizeof is a non-zero number).
Fixes: bd8699e9e292 ("netfilter: nft_bitwise: add offload support")
Link: https://github.com/ClangBuiltLinux/linux/issues/638
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
net/netfilter/nft_bitwise.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nft_bitwise.c b/net/netfilter/nft_bitwise.c
index 1f04ed5c518c..974300178fa9 100644
--- a/net/netfilter/nft_bitwise.c
+++ b/net/netfilter/nft_bitwise.c
@@ -135,8 +135,8 @@ static int nft_bitwise_offload(struct nft_offload_ctx *ctx,
{
const struct nft_bitwise *priv = nft_expr_priv(expr);
- if (memcmp(&priv->xor, &zero, sizeof(priv->xor) ||
- priv->sreg != priv->dreg))
+ if (memcmp(&priv->xor, &zero, sizeof(priv->xor)) ||
+ priv->sreg != priv->dreg)
return -EOPNOTSUPP;
memcpy(&ctx->regs[priv->dreg].mask, &priv->mask, sizeof(priv->mask));
--
2.23.0.rc2
^ permalink raw reply related
* Re: [PATCH net] ipv6: Fix return value of ipv6_mc_may_pull() for malformed packets
From: David Miller @ 2019-08-14 16:58 UTC (permalink / raw)
To: sbrivio; +Cc: gnault, haliu, edumazet, linus.luessing, netdev
In-Reply-To: <dc0d0b1bc3c67e2a1346b0dd1f68428eb956fbb7.1565649789.git.sbrivio@redhat.com>
From: Stefano Brivio <sbrivio@redhat.com>
Date: Tue, 13 Aug 2019 00:46:01 +0200
> Commit ba5ea614622d ("bridge: simplify ip_mc_check_igmp() and
> ipv6_mc_check_mld() calls") replaces direct calls to pskb_may_pull()
> in br_ipv6_multicast_mld2_report() with calls to ipv6_mc_may_pull(),
> that returns -EINVAL on buffers too short to be valid IPv6 packets,
> while maintaining the previous handling of the return code.
>
> This leads to the direct opposite of the intended effect: if the
> packet is malformed, -EINVAL evaluates as true, and we'll happily
> proceed with the processing.
>
> Return 0 if the packet is too short, in the same way as this was
> fixed for IPv4 by commit 083b78a9ed64 ("ip: fix ip_mc_may_pull()
> return value").
>
> I don't have a reproducer for this, unlike the one referred to by
> the IPv4 commit, but this is clearly broken.
>
> Fixes: ba5ea614622d ("bridge: simplify ip_mc_check_igmp() and ipv6_mc_check_mld() calls")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] mcast: ensure L-L IPv6 packets are accepted by bridge
From: Nikolay Aleksandrov @ 2019-08-14 16:58 UTC (permalink / raw)
To: pruddy, Ido Schimmel; +Cc: netdev, roopa, linus.luessing
In-Reply-To: <620d3cfbe58e3ae87ef1d5e7f2aa1588cac3e64a.camel@vyatta.att-mail.com>
On 8/14/19 7:40 PM, Patrick Ruddy wrote:
> Thanks both for the quick replies, answers inline...
>
> On Wed, 2019-08-14 at 02:55 +0300, Nikolay Aleksandrov wrote:
>> On 8/13/19 10:53 PM, Ido Schimmel wrote:
>>> + Bridge maintainers, Linus
>>>
>>
>> Good catch Ido, thanks!
>> First I'd say the subject needs to reflect that this is a bridge change
>> better, please rearrange it like so - bridge: mcast: ...
>> More below,
>>
>>> On Tue, Aug 13, 2019 at 03:18:04PM +0100, Patrick Ruddy wrote:
>>>> At present only all-nodes IPv6 multicast packets are accepted by
>>>> a bridge interface that is not in multicast router mode. Since
>>>> other protocols can be running in the absense of multicast
>>>> forwarding e.g. OSPFv3 IPv6 ND. Change the test to allow
>>>> all of the FFx2::/16 range to be accepted when not in multicast
>>>> router mode. This aligns the code with IPv4 link-local reception
>>>> and RFC4291
>>>
>>> Can you please quote the relevant part from RFC 4291?
>>>
>>>> Signed-off-by: Patrick Ruddy <pruddy@vyatta.att-mail.com>
>>>> ---
>>>> include/net/addrconf.h | 15 +++++++++++++++
>>>> net/bridge/br_multicast.c | 2 +-
>>>> 2 files changed, 16 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
>>>> index becdad576859..05b42867e969 100644
>>>> --- a/include/net/addrconf.h
>>>> +++ b/include/net/addrconf.h
>>>> @@ -434,6 +434,21 @@ static inline void addrconf_addr_solict_mult(const struct in6_addr *addr,
>>>> htonl(0xFF000000) | addr->s6_addr32[3]);
>>>> }
>>>>
>>>> +/*
>>>> + * link local multicast address range ffx2::/16 rfc4291
>>>> + */
>>>> +static inline bool ipv6_addr_is_ll_mcast(const struct in6_addr *addr)
>>>> +{
>>>> +#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
>>>> + __be64 *p = (__be64 *)addr;
>>>> + return ((p[0] & cpu_to_be64(0xff0f000000000000UL))
>>>> + ^ cpu_to_be64(0xff02000000000000UL)) == 0UL;
>>>> +#else
>>>> + return ((addr->s6_addr32[0] & htonl(0xff0f0000)) ^
>>>> + htonl(0xff020000)) == 0;
>>>> +#endif
>>>> +}
>>>> +
>>>> static inline bool ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr)
>>>> {
>>>> #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
>>>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>>>> index 9b379e110129..ed3957381fa2 100644
>>>> --- a/net/bridge/br_multicast.c
>>>> +++ b/net/bridge/br_multicast.c
>>>> @@ -1664,7 +1664,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>>>> err = ipv6_mc_check_mld(skb);
>>>>
>>>> if (err == -ENOMSG) {
>>>> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
>>>> + if (!ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr))
>>>> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>>>
>>> IIUC, you want IPv6 link-local packets to be locally received, but this
>>> also changes how these packets are flooded. RFC 4541 says that packets
>>
>> Indeed, we'll start flooding them all, not just the all hosts address.
>> If that is at all required it'll definitely have to be optional.
>>
>>> addressed to the all hosts address are a special case and should be
>>> forwarded to all ports:
>>>
>>> "In IPv6, the data forwarding rules are more straight forward because MLD is
>>> mandated for addresses with scope 2 (link-scope) or greater. The only exception
>>> is the address FF02::1 which is the all hosts link-scope address for which MLD
>>> messages are never sent. Packets with the all hosts link-scope address should
>>> be forwarded on all ports."
>>>
>>
>> I wonder what is the problem for the host to join such group on behalf of the bridge ?
>> Then you'll receive the traffic at least locally and the RFC says it itself - MLD is mandated
>> for the other link-local addresses.
>> It's very late here and maybe I'm missing something.. :)
>>
> The group is being joined by MLD at the L3 level but the packets are
> not being passed up to the l3 interface becasue there is a MLD querier
> on the network
>
That shouldn't matter if the host has joined the group, there is a specific
check for that. If the host has joined the group and we have an mdst then
we'll hit this code:
mdst = br_mdb_get(br, skb, vid);
if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
br_multicast_querier_exists(br, eth_hdr(skb))) {
if ((mdst && mdst->host_joined) ||
br_multicast_is_router(br)) {
local_rcv = true;
br->dev->stats.multicast++;
}
mcast_hit = true;
} else {
local_rcv become true and the packet is passed up, so what is the problem ?
Have you missed to refresh the group and it has expired in the bridge perhaps ?
> snippet from /proc/net/igmp6
> ...
> 40 sw1 ff0200000000000000000001ff008700 1 00000004 0
> 40 sw1 ff020000000000000000000000000002 1 00000004 0
> 40 sw1 ff020000000000000000000000000001 1 0000000C 0
> 40 sw1 ff010000000000000000000000000001 1 00000008 0
> 41 lo1 ff020000000000000000000000000001 1 0000000C 0
> 41 lo1 ff010000000000000000000000000001 1 00000008 0
> 42 sw1.1 ff020000000000000000000000000006 1 00000004 0
> 42 sw1.1 ff020000000000000000000000000005 1 00000004 0
> 42 sw1.1 ff0200000000000000000001ff000000 2 00000004 0
> 42 sw1.1 ff0200000000000000000001ff008700 1 00000004 0
> 42 sw1.1 ff0200000000000000000001ff000099 1 00000004 0
> 42 sw1.1 ff020000000000000000000000000002 1 00000004 0
> 42 sw1.1 ff020000000000000000000000000001 1 0000000C 0
> 42 sw1.1 ff010000000000000000000000000001 1 00000008 0
> ...
>
> the bridge is sw1 and the l3 intervace is sw1.1
>
> Ido is correct about the flooding - I will update the patch with the
> comments and reissue.
>
> Thanks again
>
> -pr
>>
>>> Maybe you want something like:
>>>
>>
>> I think we can do without the new field, either pass local_rcv into br_multicast_rcv() or
>> set it based on return value. The extra test will have to remain unfortunately, but we
>> can reduce the tests by one if carefully done.
>>
>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>> index 09b1dd8cd853..9f312a73f61c 100644
>>> --- a/net/bridge/br_input.c
>>> +++ b/net/bridge/br_input.c
>>> @@ -132,7 +132,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>> if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
>>> br_multicast_querier_exists(br, eth_hdr(skb))) {
>>> if ((mdst && mdst->host_joined) ||
>>> - br_multicast_is_router(br)) {
>>> + br_multicast_is_router(br) ||
>>> + BR_INPUT_SKB_CB_LOCAL_RECEIVE(skb)) {
>>> local_rcv = true;
>>> br->dev->stats.multicast++;
>>> }
>>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>>> index 9b379e110129..f03cecf6174e 100644
>>> --- a/net/bridge/br_multicast.c
>>> +++ b/net/bridge/br_multicast.c
>>> @@ -1667,6 +1667,9 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>>> if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
>>> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>>>
>>> + if (ipv6_addr_is_ll_mcast(&ipv6_hdr(skb)->daddr))
>>> + BR_INPUT_SKB_CB(skb)->local_receive = 1;
>>> +
>>> if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
>>> err = br_ip6_multicast_mrd_rcv(br, port, skb);
>>>
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index b7a4942ff1b3..d76394ca4059 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -426,6 +426,7 @@ struct br_input_skb_cb {
>>> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>>> u8 igmp;
>>> u8 mrouters_only:1;
>>> + u8 local_receive:1;
>>> #endif
>>> u8 proxyarp_replied:1;
>>> u8 src_port_isolated:1;
>>> @@ -445,8 +446,10 @@ struct br_input_skb_cb {
>>>
>>> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>>> # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (BR_INPUT_SKB_CB(__skb)->mrouters_only)
>>> +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (BR_INPUT_SKB_CB(__skb)->local_receive)
>>> #else
>>> # define BR_INPUT_SKB_CB_MROUTERS_ONLY(__skb) (0)
>>> +# define BR_INPUT_SKB_CB_LOCAL_RECEIVE(__skb) (0)
>>> #endif
>>>
>>> #define br_printk(level, br, format, args...) \
>>>
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] selftests/bpf: test_progs: change formatting of the condenced output
From: Alexei Starovoitov @ 2019-08-14 17:00 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Network Development, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <20190814164742.208909-2-sdf@google.com>
On Wed, Aug 14, 2019 at 9:47 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> This makes it visually simpler to follow the output.
> Also, highlight with red color failures when outputting to tty.
>
> Before:
> #1 attach_probe:FAIL
> #2 bpf_obj_id:OK
> #3/1 bpf_verif_scale:loop3.o:OK
> #3/2 bpf_verif_scale:test_verif_scale1.o:OK
> #3/3 bpf_verif_scale:test_verif_scale2.o:OK
> #3/4 bpf_verif_scale:test_verif_scale3.o:OK
> #3/5 bpf_verif_scale:pyperf50.o:OK
> #3/6 bpf_verif_scale:pyperf100.o:OK
> #3/7 bpf_verif_scale:pyperf180.o:OK
> #3/8 bpf_verif_scale:pyperf600.o:OK
> #3/9 bpf_verif_scale:pyperf600_nounroll.o:OK
> #3/10 bpf_verif_scale:loop1.o:OK
> #3/11 bpf_verif_scale:loop2.o:OK
> #3/12 bpf_verif_scale:loop4.o:OK
> #3/13 bpf_verif_scale:loop5.o:OK
> #3/14 bpf_verif_scale:strobemeta.o:OK
> #3/15 bpf_verif_scale:strobemeta_nounroll1.o:OK
> #3/16 bpf_verif_scale:strobemeta_nounroll2.o:OK
> #3/17 bpf_verif_scale:test_sysctl_loop1.o:OK
> #3/18 bpf_verif_scale:test_sysctl_loop2.o:OK
> #3/19 bpf_verif_scale:test_xdp_loop.o:OK
> #3/20 bpf_verif_scale:test_seg6_loop.o:OK
> #3 bpf_verif_scale:OK
> #4 flow_dissector:OK
>
> After:
> # 1 FAIL attach_probe
> # 2 OK bpf_obj_id
> # 3/1 OK bpf_verif_scale:loop3.o
> # 3/2 OK bpf_verif_scale:test_verif_scale1.o
> # 3/3 OK bpf_verif_scale:test_verif_scale2.o
> # 3/4 OK bpf_verif_scale:test_verif_scale3.o
> # 3/5 OK bpf_verif_scale:pyperf50.o
> # 3/6 OK bpf_verif_scale:pyperf100.o
> # 3/7 OK bpf_verif_scale:pyperf180.o
> # 3/8 OK bpf_verif_scale:pyperf600.o
> # 3/9 OK bpf_verif_scale:pyperf600_nounroll.o
> # 3/10 OK bpf_verif_scale:loop1.o
> # 3/11 OK bpf_verif_scale:loop2.o
> # 3/12 OK bpf_verif_scale:loop4.o
> # 3/13 OK bpf_verif_scale:loop5.o
> # 3/14 OK bpf_verif_scale:strobemeta.o
> # 3/15 OK bpf_verif_scale:strobemeta_nounroll1.o
> # 3/16 OK bpf_verif_scale:strobemeta_nounroll2.o
> # 3/17 OK bpf_verif_scale:test_sysctl_loop1.o
> # 3/18 OK bpf_verif_scale:test_sysctl_loop2.o
> # 3/19 OK bpf_verif_scale:test_xdp_loop.o
> # 3/20 OK bpf_verif_scale:test_seg6_loop.o
> # 3 OK bpf_verif_scale
> # 4 OK flow_dissector
sorry this is nack.
I prefer consistency with test_verifier output.
^ permalink raw reply
* Re: [PATCH AUTOSEL 4.19 04/42] netfilter: conntrack: always store window size un-scaled
From: Sasha Levin @ 2019-08-14 17:01 UTC (permalink / raw)
To: Jakub Jankowski
Cc: Reindl Harald, Thomas Jarosch, linux-kernel, stable,
Florian Westphal, Jozsef Kadlecsik, Pablo Neira Ayuso,
netfilter-devel, coreteam, netdev
In-Reply-To: <alpine.LNX.2.21.1908141316420.1803@kich.toxcorp.com>
On Wed, Aug 14, 2019 at 01:17:30PM +0200, Jakub Jankowski wrote:
>On 2019-08-14, Reindl Harald wrote:
>
>>that's still not in 5.2.8
>
>It will make its way into next 5.2.x release, as it is now in the
>pending queue: https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/queue-5.2
In general, AUTOSEL stuff soak for much longer before they make it to
the queue.
If there's an urgent need for a fix to go in, please make it explicit.
--
Thanks,
Sasha
^ permalink raw reply
* [PATCH net-next v2 0/4] net: bridge: mdb: allow dump/add/del of host-joined entries
From: Nikolay Aleksandrov @ 2019-08-14 17:04 UTC (permalink / raw)
To: netdev; +Cc: davem, roopa, bridge, Nikolay Aleksandrov
In-Reply-To: <81258876-5f03-002c-5aa8-2d6d00e6d99e@cumulusnetworks.com>
Hi,
This set makes the bridge dump host-joined mdb entries, they should be
treated as normal entries since they take a slot and are aging out.
We already have notifications for them but we couldn't dump them until
now so they remained hidden. We dump them similar to how they're
notified, in order to keep user-space compatibility with the dumped
objects (e.g. iproute2 dumps mdbs in a format which can be fed into
add/del commands) we allow host-joined groups also to be added/deleted via
mdb commands. That can later be used for L2 mcast MAC manipulation as
was recently discussed. Note that iproute2 changes are not necessary,
this set will work with the current user-space mdb code.
Patch 01 - a trivial comment move
Patch 02 - factors out the mdb filling code so it can be
re-used for the host-joined entries
Patch 03 - dumps host-joined entries
Patch 04 - allows manipulation of host-joined entries via standard mdb
calls
v2: change patch 04 to avoid double notification and improve host group
manual removal if no ports are present in the group
Thanks,
Nik
Nikolay Aleksandrov (4):
net: bridge: mdb: move vlan comments
net: bridge: mdb: factor out mdb filling
net: bridge: mdb: dump host-joined entries as well
net: bridge: mdb: allow add/delete for host-joined groups
net/bridge/br_mdb.c | 173 +++++++++++++++++++++++++-------------
net/bridge/br_multicast.c | 30 +++++--
net/bridge/br_private.h | 2 +
3 files changed, 141 insertions(+), 64 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] selftests/bpf: test_progs: change formatting of the condenced output
From: Stanislav Fomichev @ 2019-08-14 17:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, Network Development, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
In-Reply-To: <CAADnVQJk=qSLR1A=1poPY85wNqiye3dMvXZOZ+1OFZSA78VARg@mail.gmail.com>
On 08/14, Alexei Starovoitov wrote:
> On Wed, Aug 14, 2019 at 9:47 AM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > This makes it visually simpler to follow the output.
> > Also, highlight with red color failures when outputting to tty.
> >
> > Before:
> > #1 attach_probe:FAIL
> > #2 bpf_obj_id:OK
> > #3/1 bpf_verif_scale:loop3.o:OK
> > #3/2 bpf_verif_scale:test_verif_scale1.o:OK
> > #3/3 bpf_verif_scale:test_verif_scale2.o:OK
> > #3/4 bpf_verif_scale:test_verif_scale3.o:OK
> > #3/5 bpf_verif_scale:pyperf50.o:OK
> > #3/6 bpf_verif_scale:pyperf100.o:OK
> > #3/7 bpf_verif_scale:pyperf180.o:OK
> > #3/8 bpf_verif_scale:pyperf600.o:OK
> > #3/9 bpf_verif_scale:pyperf600_nounroll.o:OK
> > #3/10 bpf_verif_scale:loop1.o:OK
> > #3/11 bpf_verif_scale:loop2.o:OK
> > #3/12 bpf_verif_scale:loop4.o:OK
> > #3/13 bpf_verif_scale:loop5.o:OK
> > #3/14 bpf_verif_scale:strobemeta.o:OK
> > #3/15 bpf_verif_scale:strobemeta_nounroll1.o:OK
> > #3/16 bpf_verif_scale:strobemeta_nounroll2.o:OK
> > #3/17 bpf_verif_scale:test_sysctl_loop1.o:OK
> > #3/18 bpf_verif_scale:test_sysctl_loop2.o:OK
> > #3/19 bpf_verif_scale:test_xdp_loop.o:OK
> > #3/20 bpf_verif_scale:test_seg6_loop.o:OK
> > #3 bpf_verif_scale:OK
> > #4 flow_dissector:OK
> >
> > After:
> > # 1 FAIL attach_probe
> > # 2 OK bpf_obj_id
> > # 3/1 OK bpf_verif_scale:loop3.o
> > # 3/2 OK bpf_verif_scale:test_verif_scale1.o
> > # 3/3 OK bpf_verif_scale:test_verif_scale2.o
> > # 3/4 OK bpf_verif_scale:test_verif_scale3.o
> > # 3/5 OK bpf_verif_scale:pyperf50.o
> > # 3/6 OK bpf_verif_scale:pyperf100.o
> > # 3/7 OK bpf_verif_scale:pyperf180.o
> > # 3/8 OK bpf_verif_scale:pyperf600.o
> > # 3/9 OK bpf_verif_scale:pyperf600_nounroll.o
> > # 3/10 OK bpf_verif_scale:loop1.o
> > # 3/11 OK bpf_verif_scale:loop2.o
> > # 3/12 OK bpf_verif_scale:loop4.o
> > # 3/13 OK bpf_verif_scale:loop5.o
> > # 3/14 OK bpf_verif_scale:strobemeta.o
> > # 3/15 OK bpf_verif_scale:strobemeta_nounroll1.o
> > # 3/16 OK bpf_verif_scale:strobemeta_nounroll2.o
> > # 3/17 OK bpf_verif_scale:test_sysctl_loop1.o
> > # 3/18 OK bpf_verif_scale:test_sysctl_loop2.o
> > # 3/19 OK bpf_verif_scale:test_xdp_loop.o
> > # 3/20 OK bpf_verif_scale:test_seg6_loop.o
> > # 3 OK bpf_verif_scale
> > # 4 OK flow_dissector
>
> sorry this is nack.
> I prefer consistency with test_verifier output.
No problem, let me know how you feel about the other patches
in the series, can drop this one.
^ permalink raw reply
* Re: [RFC bpf-next 0/3] tools: bpftool: add subcommand to count map entries
From: Quentin Monnet @ 2019-08-14 17:12 UTC (permalink / raw)
To: Alexei Starovoitov, Edward Cree
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Network Development,
oss-drivers
In-Reply-To: <CAADnVQJE2DCU0J2_d4Z-1cmXZsb_q2FODcbC1S24C0f=_b2ffg@mail.gmail.com>
2019-08-14 09:58 UTC-0700 ~ Alexei Starovoitov
<alexei.starovoitov@gmail.com>
> On Wed, Aug 14, 2019 at 9:45 AM Edward Cree <ecree@solarflare.com> wrote:
>>
>> On 14/08/2019 10:42, Quentin Monnet wrote:
>>> 2019-08-13 18:51 UTC-0700 ~ Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com>
>>>> The same can be achieved by 'bpftool map dump|grep key|wc -l', no?
>>> To some extent (with subtleties for some other map types); and we use a
>>> similar command line as a workaround for now. But because of the rate of
>>> inserts/deletes in the map, the process often reports a number higher
>>> than the max number of entries (we observed up to ~750k when max_entries
>>> is 500k), even is the map is only half-full on average during the count.
>>> On the worst case (though not frequent), an entry is deleted just before
>>> we get the next key from it, and iteration starts all over again. This
>>> is not reliable to determine how much space is left in the map.
>>>
>>> I cannot see a solution that would provide a more accurate count from
>>> user space, when the map is under pressure?
>> This might be a really dumb suggestion, but: you're wanting to collect a
>> summary statistic over an in-kernel data structure in a single syscall,
>> because making a series of syscalls to examine every entry is slow and
>> racy. Isn't that exactly a job for an in-kernel virtual machine, and
>> could you not supply an eBPF program which the kernel runs on each entry
>> in the map, thus supporting people who want to calculate something else
>> (mean, min and max, whatever) instead of count?
>
> Pretty much my suggestion as well :)
>
> It seems the better fix for your nat threshold is to keep count of
> elements in the map in a separate global variable that
> bpf program manually increments and decrements.
> bpftool will dump it just as regular map of single element.
> (I believe it doesn't recognize global variables properly yet)
> and BTF will be there to pick exactly that 'count' variable.
>
It would be with an offloaded map, but yes, I suppose we could keep
track of the numbers in a separate map. We'll have a look into this.
Thanks to both of you for the suggestions.
Quentin
^ permalink raw reply
* Re: [RFC bpf-next 0/3] tools: bpftool: add subcommand to count map entries
From: Edward Cree @ 2019-08-14 17:14 UTC (permalink / raw)
To: Quentin Monnet, Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev, oss-drivers
In-Reply-To: <18f887ec-99fd-20ae-f5d6-a1f4117b2d77@netronome.com>
On 14/08/2019 17:58, Quentin Monnet wrote:
> 2019-08-14 17:45 UTC+0100 ~ Edward Cree <ecree@solarflare.com>
>> This might be a really dumb suggestion, but: you're wanting to collect a
>> summary statistic over an in-kernel data structure in a single syscall,
>> because making a series of syscalls to examine every entry is slow and
>> racy. Isn't that exactly a job for an in-kernel virtual machine, and
>> could you not supply an eBPF program which the kernel runs on each entry
>> in the map, thus supporting people who want to calculate something else
>> (mean, min and max, whatever) instead of count?
>>
> Hi Edward, I like the approach, thanks for the suggestion.
>
> But I did not mention that we were using offloaded maps: Tracing the
> kernel would probably work for programs running on the host, but this is
> not a solution we could extend to hardware offload.
I don't see where "tracing" comes into it; this is a new program type and
a new map op under the bpf() syscall.
Could the user-supplied BPF program not then be passed down to the device
for it to run against its offloaded maps?
^ permalink raw reply
* Re: fallout from net-next netfilter changes
From: Florian Westphal @ 2019-08-14 17:18 UTC (permalink / raw)
To: David Miller; +Cc: pablo, netfilter-devel, netdev
In-Reply-To: <20190814.125330.1934256694306164517.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote:
> This started happening after Jakub's pull of your net-next changes
> yesterday:
>
> ./include/uapi/linux/netfilter_ipv6/ip6t_LOG.h:5:2: warning: #warning "Please update iptables, this file will be removed soon!" [-Wcpp]
> #warning "Please update iptables, this file will be removed soon!"
> ^~~~~~~
> In file included from <command-line>:
> ./include/uapi/linux/netfilter_ipv4/ipt_LOG.h:5:2: warning: #warning "Please update iptables, this file will be removed soon!" [-Wcpp]
> #warning "Please update iptables, this file will be removed soon!"
> ^~~~~~~
>
> It's probaly from the standard kernel build UAPI header checks.
A patch that removes those #warning from the kernel is sitting in
the netfilter patchwork queue already.
^ permalink raw reply
* [PATCH net-next] selftests: Fix get_ifidx and callers in nettest.c
From: David Ahern @ 2019-08-14 17:11 UTC (permalink / raw)
To: davem; +Cc: netdev, dan.carpenter, David Ahern
From: David Ahern <dsahern@gmail.com>
Dan reported:
The patch acda655fefae: "selftests: Add nettest" from Aug 1, 2019,
leads to the following static checker warning:
./tools/testing/selftests/net/nettest.c:1690 main()
warn: unsigned 'tmp' is never less than zero.
./tools/testing/selftests/net/nettest.c
1680 case '1':
1681 args.has_expected_raddr = 1;
1682 if (convert_addr(&args, optarg,
1683 ADDR_TYPE_EXPECTED_REMOTE))
1684 return 1;
1685
1686 break;
1687 case '2':
1688 if (str_to_uint(optarg, 0, 0x7ffffff, &tmp) != 0) {
1689 tmp = get_ifidx(optarg);
1690 if (tmp < 0) {
"tmp" is unsigned so it can't be negative. Also all the callers assume
that get_ifidx() returns negatives on error but it looks like it really
returns zero on error so it's a bit unclear to me.
Update get_ifidx to return -1 on errors and cleanup callers of it.
Fixes: acda655fefae ("selftests: Add nettest")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/nettest.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/net/nettest.c b/tools/testing/selftests/net/nettest.c
index 83515e5ea4dc..c08f4db8330d 100644
--- a/tools/testing/selftests/net/nettest.c
+++ b/tools/testing/selftests/net/nettest.c
@@ -266,7 +266,7 @@ static int get_ifidx(const char *ifname)
int sd, rc;
if (!ifname || *ifname == '\0')
- return 0;
+ return -1;
memset(&ifdata, 0, sizeof(ifdata));
@@ -275,14 +275,14 @@ static int get_ifidx(const char *ifname)
sd = socket(PF_INET, SOCK_DGRAM, IPPROTO_IP);
if (sd < 0) {
log_err_errno("socket failed");
- return 0;
+ return -1;
}
rc = ioctl(sd, SIOCGIFINDEX, (char *)&ifdata);
close(sd);
if (rc != 0) {
log_err_errno("ioctl(SIOCGIFINDEX) failed");
- return 0;
+ return -1;
}
return ifdata.ifr_ifindex;
@@ -419,20 +419,20 @@ static int set_multicast_if(int sd, int ifindex)
return rc;
}
-static int set_membership(int sd, uint32_t grp, uint32_t addr, const char *dev)
+static int set_membership(int sd, uint32_t grp, uint32_t addr, int ifindex)
{
uint32_t if_addr = addr;
struct ip_mreqn mreq;
int rc;
- if (addr == htonl(INADDR_ANY) && !dev) {
+ if (addr == htonl(INADDR_ANY) && !ifindex) {
log_error("Either local address or device needs to be given for multicast membership\n");
return -1;
}
mreq.imr_multiaddr.s_addr = grp;
mreq.imr_address.s_addr = if_addr;
- mreq.imr_ifindex = dev ? get_ifidx(dev) : 0;
+ mreq.imr_ifindex = ifindex;
rc = setsockopt(sd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &mreq, sizeof(mreq));
if (rc < 0) {
@@ -1048,7 +1048,7 @@ static int msock_init(struct sock_args *args, int server)
if (server &&
set_membership(sd, args->grp.s_addr,
- args->local_addr.in.s_addr, args->dev))
+ args->local_addr.in.s_addr, args->ifindex))
goto out_err;
return sd;
@@ -1685,15 +1685,16 @@ int main(int argc, char *argv[])
break;
case '2':
- if (str_to_uint(optarg, 0, 0x7ffffff, &tmp) != 0) {
- tmp = get_ifidx(optarg);
- if (tmp < 0) {
+ if (str_to_uint(optarg, 0, INT_MAX, &tmp) == 0) {
+ args.expected_ifindex = (int)tmp;
+ } else {
+ args.expected_ifindex = get_ifidx(optarg);
+ if (args.expected_ifindex < 0) {
fprintf(stderr,
- "Invalid device index\n");
+ "Invalid expected device\n");
return 1;
}
}
- args.expected_ifindex = (int)tmp;
break;
case 'q':
quiet = 1;
--
2.11.0
^ permalink raw reply related
* Re: [net PATCH] net: tls, fix sk_write_space NULL write when tx disabled
From: Jakub Kicinski @ 2019-08-14 17:08 UTC (permalink / raw)
To: John Fastabend; +Cc: davem, ying.xue, netdev, andreyknvl
In-Reply-To: <156576071416.1402.5907777786031481705.stgit@ubuntu3-kvm1>
On Wed, 14 Aug 2019 05:31:54 +0000, John Fastabend wrote:
> The ctx->sk_write_space pointer is only set when TLS tx mode is enabled.
> When running without TX mode its a null pointer but we still set the
> sk sk_write_space pointer on close().
>
> Fix the close path to only overwrite sk->sk_write_space when the current
> pointer is to the tls_write_space function indicating the tls module should
> clean it up properly as well.
>
> Reported-by: Hillf Danton <hdanton@sina.com>
> Cc: Ying Xue <ying.xue@windriver.com>
> Cc: Andrey Konovalov <andreyknvl@google.com>
> Fixes: 57c722e932cfb ("net/tls: swap sk_write_space on close")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks!
^ permalink raw reply
* Re: [RFC PATCH bpf-next 00/14] xdp_flow: Flow offload to XDP
From: Stanislav Fomichev @ 2019-08-14 17:07 UTC (permalink / raw)
To: Toshiaki Makita
Cc: Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, David S. Miller, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, netdev, bpf, William Tu
In-Reply-To: <20190813120558.6151-1-toshiaki.makita1@gmail.com>
On 08/13, Toshiaki Makita wrote:
> * Implementation
>
> xdp_flow makes use of UMH to load an eBPF program for XDP, similar to
> bpfilter. The difference is that xdp_flow does not generate the eBPF
> program dynamically but a prebuilt program is embedded in UMH. This is
> mainly because flow insertion is considerably frequent. If we generate
> and load an eBPF program on each insertion of a flow, the latency of the
> first packet of ping in above test will incease, which I want to avoid.
Can this be instead implemented with a new hook that will be called
for TC events? This hook can write to perf event buffer and control
plane will insert/remove/modify flow tables in the BPF maps (contol
plane will also install xdp program).
Why do we need UMH? What am I missing?
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: net_failover: Fix typo in a filepath
From: David Miller @ 2019-08-14 17:24 UTC (permalink / raw)
To: efremov; +Cc: linux-kernel, joe, sridhar.samudrala, netdev
In-Reply-To: <20190813060530.13138-1-efremov@linux.com>
From: Denis Efremov <efremov@linux.com>
Date: Tue, 13 Aug 2019 09:05:30 +0300
> Replace "driver" with "drivers" in the filepath to net_failover.c
>
> Cc: Sridhar Samudrala <sridhar.samudrala@intel.com>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Fixes: cfc80d9a1163 ("net: Introduce net_failover driver")
> Signed-off-by: Denis Efremov <efremov@linux.com>
Applied.
^ 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