2019-07-08 22:43:43

by Krzesimir Nowak

[permalink] [raw]
Subject: [bpf-next v3 10/12] bpf: Implement bpf_prog_test_run for perf event programs

As an input, test run for perf event program takes struct
bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
data_in. For an output, it basically ignores ctx_out and data_out.

The implementation sets an instance of struct bpf_perf_event_data_kern
in such a way that the BPF program reading data from context will
receive what we passed to the bpf prog test run in ctx_in. Also BPF
program can call bpf_perf_prog_read_value to receive what was passed
in data_in.

Changes since v2:
- drop the changes in perf event verifier test - they are not needed
anymore after reworked ctx size handling

Signed-off-by: Krzesimir Nowak <[email protected]>
---
kernel/trace/bpf_trace.c | 60 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1255d14576..b870fc2314d0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -19,6 +19,8 @@
#include "trace_probe.h"
#include "trace.h"

+#include <trace/events/bpf_test_run.h>
+
#define bpf_event_rcu_dereference(p) \
rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))

@@ -1160,7 +1162,65 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
.convert_ctx_access = pe_prog_convert_ctx_access,
};

+static int pe_prog_test_run(struct bpf_prog *prog,
+ const union bpf_attr *kattr,
+ union bpf_attr __user *uattr)
+{
+ struct bpf_perf_event_data_kern real_ctx = {0, };
+ struct perf_sample_data sample_data = {0, };
+ struct bpf_perf_event_data *fake_ctx;
+ struct bpf_perf_event_value *value;
+ struct perf_event event = {0, };
+ u32 retval = 0, duration = 0;
+ int err;
+
+ if (kattr->test.data_size_out || kattr->test.data_out)
+ return -EINVAL;
+ if (kattr->test.ctx_size_out || kattr->test.ctx_out)
+ return -EINVAL;
+
+ fake_ctx = bpf_receive_ctx(kattr, sizeof(struct bpf_perf_event_data));
+ if (IS_ERR(fake_ctx))
+ return PTR_ERR(fake_ctx);
+
+ value = bpf_receive_data(kattr, sizeof(struct bpf_perf_event_value));
+ if (IS_ERR(value)) {
+ kfree(fake_ctx);
+ return PTR_ERR(value);
+ }
+
+ real_ctx.regs = &fake_ctx->regs;
+ real_ctx.data = &sample_data;
+ real_ctx.event = &event;
+ perf_sample_data_init(&sample_data, fake_ctx->addr,
+ fake_ctx->sample_period);
+ event.cpu = smp_processor_id();
+ event.oncpu = -1;
+ event.state = PERF_EVENT_STATE_OFF;
+ local64_set(&event.count, value->counter);
+ event.total_time_enabled = value->enabled;
+ event.total_time_running = value->running;
+ /* make self as a leader - it is used only for checking the
+ * state field
+ */
+ event.group_leader = &event;
+ err = bpf_test_run(prog, &real_ctx, kattr->test.repeat,
+ BPF_TEST_RUN_PLAIN, &retval, &duration);
+ if (err) {
+ kfree(value);
+ kfree(fake_ctx);
+ return err;
+ }
+
+ err = bpf_test_finish(uattr, retval, duration);
+ trace_bpf_test_finish(&err);
+ kfree(value);
+ kfree(fake_ctx);
+ return err;
+}
+
const struct bpf_prog_ops perf_event_prog_ops = {
+ .test_run = pe_prog_test_run,
};

static DEFINE_MUTEX(bpf_event_mutex);
--
2.20.1


2019-07-11 21:07:35

by Stanislav Fomichev

[permalink] [raw]
Subject: Re: [bpf-next v3 10/12] bpf: Implement bpf_prog_test_run for perf event programs

On 07/08, Krzesimir Nowak wrote:
> As an input, test run for perf event program takes struct
> bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
> data_in. For an output, it basically ignores ctx_out and data_out.
>
> The implementation sets an instance of struct bpf_perf_event_data_kern
> in such a way that the BPF program reading data from context will
> receive what we passed to the bpf prog test run in ctx_in. Also BPF
> program can call bpf_perf_prog_read_value to receive what was passed
> in data_in.
>
> Changes since v2:
> - drop the changes in perf event verifier test - they are not needed
> anymore after reworked ctx size handling
>
> Signed-off-by: Krzesimir Nowak <[email protected]>
> ---
> kernel/trace/bpf_trace.c | 60 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index ca1255d14576..b870fc2314d0 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -19,6 +19,8 @@
> #include "trace_probe.h"
> #include "trace.h"
>
> +#include <trace/events/bpf_test_run.h>
> +
> #define bpf_event_rcu_dereference(p) \
> rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
>
> @@ -1160,7 +1162,65 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
> .convert_ctx_access = pe_prog_convert_ctx_access,
> };
>
> +static int pe_prog_test_run(struct bpf_prog *prog,
> + const union bpf_attr *kattr,
> + union bpf_attr __user *uattr)
> +{
> + struct bpf_perf_event_data_kern real_ctx = {0, };
> + struct perf_sample_data sample_data = {0, };
> + struct bpf_perf_event_data *fake_ctx;
> + struct bpf_perf_event_value *value;
> + struct perf_event event = {0, };
> + u32 retval = 0, duration = 0;
> + int err;
> +
> + if (kattr->test.data_size_out || kattr->test.data_out)
> + return -EINVAL;
> + if (kattr->test.ctx_size_out || kattr->test.ctx_out)
> + return -EINVAL;
> +
> + fake_ctx = bpf_receive_ctx(kattr, sizeof(struct bpf_perf_event_data));
> + if (IS_ERR(fake_ctx))
> + return PTR_ERR(fake_ctx);
> +
> + value = bpf_receive_data(kattr, sizeof(struct bpf_perf_event_value));
> + if (IS_ERR(value)) {
> + kfree(fake_ctx);
> + return PTR_ERR(value);
> + }
nit: maybe use bpf_test_ prefix for receive_ctx/data:
* bpf_test_receive_ctx
* bpf_test_receive_data

? To signify that they are used for tests only.

> +
> + real_ctx.regs = &fake_ctx->regs;
> + real_ctx.data = &sample_data;
> + real_ctx.event = &event;
> + perf_sample_data_init(&sample_data, fake_ctx->addr,
> + fake_ctx->sample_period);
> + event.cpu = smp_processor_id();
> + event.oncpu = -1;
> + event.state = PERF_EVENT_STATE_OFF;
> + local64_set(&event.count, value->counter);
> + event.total_time_enabled = value->enabled;
> + event.total_time_running = value->running;
> + /* make self as a leader - it is used only for checking the
> + * state field
> + */
> + event.group_leader = &event;
> + err = bpf_test_run(prog, &real_ctx, kattr->test.repeat,
> + BPF_TEST_RUN_PLAIN, &retval, &duration);
> + if (err) {
> + kfree(value);
> + kfree(fake_ctx);
> + return err;
> + }
> +
> + err = bpf_test_finish(uattr, retval, duration);
> + trace_bpf_test_finish(&err);
Can probably do:

err = bpf_test_run(...)
if (!err) {
err = bpf_test_finish(uattr, retval, duration);
trace_bpf_test_finish(&err);
}
kfree(..);
kfree(..);
return err;

So you don't have to copy-paste the error handling.

> + kfree(value);
> + kfree(fake_ctx);
> + return err;
> +}
> +
> const struct bpf_prog_ops perf_event_prog_ops = {
> + .test_run = pe_prog_test_run,
> };
>
> static DEFINE_MUTEX(bpf_event_mutex);
> --
> 2.20.1
>