* Re: [PATCH bpf-next 1/8] bpf: consolidate shared test timing code
@ 2021-02-16 13:48 kernel test robot
0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2021-02-16 13:48 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 6012 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20210216105713.45052-2-lmb@cloudflare.com>
References: <20210216105713.45052-2-lmb@cloudflare.com>
TO: Lorenz Bauer <lmb@cloudflare.com>
TO: ast(a)kernel.org
TO: daniel(a)iogearbox.net
TO: andrii(a)kernel.org
TO: jakub(a)cloudflare.com
CC: kernel-team(a)cloudflare.com
CC: bpf(a)vger.kernel.org
CC: netdev(a)vger.kernel.org
CC: Lorenz Bauer <lmb@cloudflare.com>
Hi Lorenz,
I love your patch! Perhaps something to improve:
[auto build test WARNING on bpf-next/master]
url: https://github.com/0day-ci/linux/commits/Lorenz-Bauer/PROG_TEST_RUN-support-for-sk_lookup-programs/20210216-190705
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
:::::: branch date: 3 hours ago
:::::: commit date: 3 hours ago
config: i386-randconfig-m021-20210215 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
net/bpf/test_run.c:774 bpf_prog_test_run_flow_dissector() error: uninitialized symbol 'retval'.
Old smatch warnings:
include/linux/u64_stats_sync.h:128 u64_stats_update_begin() warn: statement has no effect 31
vim +/retval +774 net/bpf/test_run.c
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 714
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 715 int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 716 const union bpf_attr *kattr,
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 717 union bpf_attr __user *uattr)
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 718 {
938a0856b8ac67 Lorenz Bauer 2021-02-16 719 struct test_timer t = { NO_PREEMPT };
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 720 u32 size = kattr->test.data_size_in;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 721 struct bpf_flow_dissector ctx = {};
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 722 u32 repeat = kattr->test.repeat;
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 723 struct bpf_flow_keys *user_ctx;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 724 struct bpf_flow_keys flow_keys;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 725 const struct ethhdr *eth;
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 726 unsigned int flags = 0;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 727 u32 retval, duration;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 728 void *data;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 729 int ret;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 730
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 731 if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 732 return -EINVAL;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 733
1b4d60ec162f82 Song Liu 2020-09-25 734 if (kattr->test.flags || kattr->test.cpu)
1b4d60ec162f82 Song Liu 2020-09-25 735 return -EINVAL;
1b4d60ec162f82 Song Liu 2020-09-25 736
7b8a1304323b35 Stanislav Fomichev 2019-04-22 737 if (size < ETH_HLEN)
7b8a1304323b35 Stanislav Fomichev 2019-04-22 738 return -EINVAL;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 739
7b8a1304323b35 Stanislav Fomichev 2019-04-22 740 data = bpf_test_init(kattr, size, 0, 0);
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 741 if (IS_ERR(data))
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 742 return PTR_ERR(data);
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 743
7b8a1304323b35 Stanislav Fomichev 2019-04-22 744 eth = (struct ethhdr *)data;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 745
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 746 if (!repeat)
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 747 repeat = 1;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 748
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 749 user_ctx = bpf_ctx_init(kattr, sizeof(struct bpf_flow_keys));
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 750 if (IS_ERR(user_ctx)) {
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 751 kfree(data);
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 752 return PTR_ERR(user_ctx);
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 753 }
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 754 if (user_ctx) {
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 755 ret = verify_user_bpf_flow_keys(user_ctx);
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 756 if (ret)
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 757 goto out;
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 758 flags = user_ctx->flags;
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 759 }
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 760
7b8a1304323b35 Stanislav Fomichev 2019-04-22 761 ctx.flow_keys = &flow_keys;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 762 ctx.data = data;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 763 ctx.data_end = (__u8 *)data + size;
7b8a1304323b35 Stanislav Fomichev 2019-04-22 764
938a0856b8ac67 Lorenz Bauer 2021-02-16 765 while (t_check(&t, repeat, &ret, &duration)) {
7b8a1304323b35 Stanislav Fomichev 2019-04-22 766 retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
b2ca4e1cfa7d3d Stanislav Fomichev 2019-07-25 767 size, flags);
a439184d515fbf Stanislav Fomichev 2019-02-19 768 }
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 769
938a0856b8ac67 Lorenz Bauer 2021-02-16 770 if (ret < 0)
938a0856b8ac67 Lorenz Bauer 2021-02-16 771 goto out;
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 772
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 773 ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
b7a1848e8398b8 Stanislav Fomichev 2019-01-28 @774 retval, duration);
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38029 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH bpf-next 1/8] bpf: consolidate shared test timing code
2021-02-16 10:57 [PATCH bpf-next 0/8] PROG_TEST_RUN support for sk_lookup programs Lorenz Bauer
@ 2021-02-16 10:57 ` Lorenz Bauer
0 siblings, 0 replies; 2+ messages in thread
From: Lorenz Bauer @ 2021-02-16 10:57 UTC (permalink / raw)
To: ast, daniel, andrii, jakub; +Cc: kernel-team, bpf, netdev, Lorenz Bauer
Share the timing / signal interruption logic between different
implementations of PROG_TEST_RUN. There is a change in behaviour
as well. We check the loop exit condition before checking for
pending signals. This resolves an edge case where a signal
arrives during the last iteration. Instead of aborting with
EINTR we return the successful result to user space.
Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
---
net/bpf/test_run.c | 137 +++++++++++++++++++++++++--------------------
1 file changed, 76 insertions(+), 61 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 58bcb8c849d5..33bd2f67e259 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -16,14 +16,82 @@
#define CREATE_TRACE_POINTS
#include <trace/events/bpf_test_run.h>
+struct test_timer {
+ enum { NO_PREEMPT, NO_MIGRATE } mode;
+ u32 i;
+ u64 time_start, time_spent;
+};
+
+static inline void t_enter(struct test_timer *t)
+{
+ rcu_read_lock();
+ if (t->mode == NO_PREEMPT)
+ preempt_disable();
+ else
+ migrate_disable();
+
+ t->time_start = ktime_get_ns();
+}
+
+static inline void t_leave(struct test_timer *t)
+{
+ t->time_spent += ktime_get_ns() - t->time_start;
+ t->time_start = 0;
+
+ if (t->mode == NO_PREEMPT)
+ preempt_enable();
+ else
+ migrate_enable();
+ rcu_read_unlock();
+}
+
+static inline bool t_check(struct test_timer *t, u32 repeat, int *err, u32 *duration)
+{
+ if (!t->time_start) {
+ /* Enter protected section before first iteration. */
+ t_enter(t);
+ return true;
+ }
+
+ t->i++;
+ if (t->i >= repeat) {
+ /* Leave the protected section after the last iteration. */
+ t_leave(t);
+ do_div(t->time_spent, t->i);
+ *duration = t->time_spent > U32_MAX ? U32_MAX : (u32)t->time_spent;
+ *err = 0;
+ goto reset;
+ }
+
+ if (signal_pending(current)) {
+ /* During iteration: we've been cancelled, abort. */
+ t_leave(t);
+ *err = -EINTR;
+ goto reset;
+ }
+
+ if (need_resched()) {
+ /* During iteration: we need to reschedule between runs. */
+ t_leave(t);
+ cond_resched();
+ t_enter(t);
+ }
+
+ /* Do another round. */
+ return true;
+
+reset:
+ t->time_spent = t->i = 0;
+ return false;
+}
+
static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
u32 *retval, u32 *time, bool xdp)
{
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
+ struct test_timer t = { NO_MIGRATE };
enum bpf_cgroup_storage_type stype;
- u64 time_start, time_spent = 0;
- int ret = 0;
- u32 i;
+ int ret;
for_each_cgroup_storage_type(stype) {
storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
@@ -38,40 +106,14 @@ static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
if (!repeat)
repeat = 1;
- rcu_read_lock();
- migrate_disable();
- time_start = ktime_get_ns();
- for (i = 0; i < repeat; i++) {
+ while (t_check(&t, repeat, &ret, time)) {
bpf_cgroup_storage_set(storage);
if (xdp)
*retval = bpf_prog_run_xdp(prog, ctx);
else
*retval = BPF_PROG_RUN(prog, ctx);
-
- if (signal_pending(current)) {
- ret = -EINTR;
- break;
- }
-
- if (need_resched()) {
- time_spent += ktime_get_ns() - time_start;
- migrate_enable();
- rcu_read_unlock();
-
- cond_resched();
-
- rcu_read_lock();
- migrate_disable();
- time_start = ktime_get_ns();
- }
}
- time_spent += ktime_get_ns() - time_start;
- migrate_enable();
- rcu_read_unlock();
-
- do_div(time_spent, repeat);
- *time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
for_each_cgroup_storage_type(stype)
bpf_cgroup_storage_free(storage[stype]);
@@ -674,18 +716,17 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr)
{
+ struct test_timer t = { NO_PREEMPT };
u32 size = kattr->test.data_size_in;
struct bpf_flow_dissector ctx = {};
u32 repeat = kattr->test.repeat;
struct bpf_flow_keys *user_ctx;
struct bpf_flow_keys flow_keys;
- u64 time_start, time_spent = 0;
const struct ethhdr *eth;
unsigned int flags = 0;
u32 retval, duration;
void *data;
int ret;
- u32 i;
if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
return -EINVAL;
@@ -721,39 +762,13 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
ctx.data = data;
ctx.data_end = (__u8 *)data + size;
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- for (i = 0; i < repeat; i++) {
+ while (t_check(&t, repeat, &ret, &duration)) {
retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
size, flags);
-
- if (signal_pending(current)) {
- preempt_enable();
- rcu_read_unlock();
-
- ret = -EINTR;
- goto out;
- }
-
- if (need_resched()) {
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
-
- cond_resched();
-
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- }
}
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
- do_div(time_spent, repeat);
- duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
+ if (ret < 0)
+ goto out;
ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
retval, duration);
--
2.27.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-16 13:48 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 13:48 [PATCH bpf-next 1/8] bpf: consolidate shared test timing code kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-02-16 10:57 [PATCH bpf-next 0/8] PROG_TEST_RUN support for sk_lookup programs Lorenz Bauer
2021-02-16 10:57 ` [PATCH bpf-next 1/8] bpf: consolidate shared test timing code Lorenz Bauer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.