2019-06-25 19:45:10

by Allan Zhang

[permalink] [raw]
Subject: [PATCH bpf-next v4 0/2] bpf: Allow bpf_skb_event_output for more prog types

Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.

Added socket_filter, cg_skb, sk_skb prog types to generate sw event.

allanzhang (2):
bpf: Allow bpf_skb_event_output for a few prog types
bpf: Add selftests for bpf_perf_event_output

net/core/filter.c | 6 ++
tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
3 files changed, 132 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c

--
2.22.0.410.gd8fdbe21b5-goog


2019-06-25 19:45:51

by Allan Zhang

[permalink] [raw]
Subject: [PATCH bpf-next v4 2/2] bpf: Add selftests for bpf_perf_event_output

Software event output is only enabled by a few prog types.
This test is to ensure that all supported types are enbled for
bpf_perf_event_output sucessfully.

v4:
* Reformating log message
v3:
* Reformating log message
v2:
* Reformating log message

Signed-off-by: allanzhang <[email protected]>
---
tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
.../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
2 files changed, 126 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..901a188e1eea 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -50,7 +50,7 @@
#define MAX_INSNS BPF_MAXINSNS
#define MAX_TEST_INSNS 1000000
#define MAX_FIXUPS 8
-#define MAX_NR_MAPS 18
+#define MAX_NR_MAPS 19
#define MAX_TEST_RUNS 8
#define POINTER_VALUE 0xcafe4all
#define TEST_DATA_LEN 64
@@ -84,6 +84,7 @@ struct bpf_test {
int fixup_map_array_wo[MAX_FIXUPS];
int fixup_map_array_small[MAX_FIXUPS];
int fixup_sk_storage_map[MAX_FIXUPS];
+ int fixup_map_event_output[MAX_FIXUPS];
const char *errstr;
const char *errstr_unpriv;
uint32_t retval, retval_unpriv, insn_processed;
@@ -604,6 +605,28 @@ static int create_sk_storage_map(void)
return fd;
}

+static int create_event_output_map(void)
+{
+ struct bpf_create_map_attr attr = {
+ .name = "test_map",
+ .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = 4,
+ .value_size = 4,
+ .max_entries = 1,
+ };
+ int fd, btf_fd;
+
+ btf_fd = load_btf();
+ if (btf_fd < 0)
+ return -1;
+ attr.btf_fd = btf_fd;
+ fd = bpf_create_map_xattr(&attr);
+ close(attr.btf_fd);
+ if (fd < 0)
+ printf("Failed to create event_output\n");
+ return fd;
+}
+
static char bpf_vlog[UINT_MAX >> 8];

static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
@@ -627,6 +650,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
int *fixup_map_array_wo = test->fixup_map_array_wo;
int *fixup_map_array_small = test->fixup_map_array_small;
int *fixup_sk_storage_map = test->fixup_sk_storage_map;
+ int *fixup_map_event_output = test->fixup_map_event_output;

if (test->fill_helper) {
test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
@@ -788,6 +812,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
fixup_sk_storage_map++;
} while (*fixup_sk_storage_map);
}
+ if (*fixup_map_event_output) {
+ map_fds[18] = create_event_output_map();
+ do {
+ prog[*fixup_map_event_output].imm = map_fds[18];
+ fixup_map_event_output++;
+ } while (*fixup_map_event_output);
+ }
}

static int set_admin(bool admin)
diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
new file mode 100644
index 000000000000..b25eabcfaa56
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/event_output.c
@@ -0,0 +1,94 @@
+/* instructions used to output a skb based software event, produced
+ * from code snippet:
+struct TMP {
+ uint64_t tmp;
+} tt;
+tt.tmp = 5;
+bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
+ &tt, sizeof(tt));
+return 1;
+
+the bpf assembly from llvm is:
+ 0: b7 02 00 00 05 00 00 00 r2 = 5
+ 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
+ 2: bf a4 00 00 00 00 00 00 r4 = r10
+ 3: 07 04 00 00 f8 ff ff ff r4 += -8
+ 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
+ 6: b7 03 00 00 00 00 00 00 r3 = 0
+ 7: b7 05 00 00 08 00 00 00 r5 = 8
+ 8: 85 00 00 00 19 00 00 00 call 25
+ 9: b7 00 00 00 01 00 00 00 r0 = 1
+ 10: 95 00 00 00 00 00 00 00 exit
+
+ The reason I put the code here instead of fill_helpers is that map fixup is
+ against the insns, instead of filled prog.
+*/
+
+#define __PERF_EVENT_INSNS__ \
+ BPF_MOV64_IMM(BPF_REG_2, 5), \
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
+ BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
+ BPF_LD_MAP_FD(BPF_REG_2, 0), \
+ BPF_MOV64_IMM(BPF_REG_3, 0), \
+ BPF_MOV64_IMM(BPF_REG_5, 8), \
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
+ BPF_FUNC_perf_event_output), \
+ BPF_MOV64_IMM(BPF_REG_0, 1), \
+ BPF_EXIT_INSN(),
+{
+ "perfevent for sockops",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCK_OPS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for tc",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SCHED_CLS,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for lwt out",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_LWT_OUT,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for xdp",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_XDP,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for socket filter",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for sk_skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
+{
+ "perfevent for cgroup skb",
+ .insns = { __PERF_EVENT_INSNS__ },
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ .fixup_map_event_output = { 4 },
+ .result = ACCEPT,
+ .retval = 1,
+},
--
2.22.0.410.gd8fdbe21b5-goog

2019-06-25 19:46:37

by Allan Zhang

[permalink] [raw]
Subject: [PATCH bpf-next v4 1/2] bpf: Allow bpf_skb_event_output for a few prog types

Software event output is only enabled by a few prog types right now (TC,
LWT out, XDP, sockops). Many other skb based prog types need
bpf_skb_event_output to produce software event.

Added socket_filter, cg_skb, sk_skb prog types to generate sw event.

Test bpf code is generated from code snippet:

struct TMP {
uint64_t tmp;
} tt;
tt.tmp = 5;
bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
&tt, sizeof(tt));
return 1;

the bpf assembly from llvm is:
0: b7 02 00 00 05 00 00 00 r2 = 5
1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
2: bf a4 00 00 00 00 00 00 r4 = r10
3: 07 04 00 00 f8 ff ff ff r4 += -8
4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
6: b7 03 00 00 00 00 00 00 r3 = 0
7: b7 05 00 00 08 00 00 00 r5 = 8
8: 85 00 00 00 19 00 00 00 call 25
9: b7 00 00 00 01 00 00 00 r0 = 1
10: 95 00 00 00 00 00 00 00 exit

Signed-off-by: allanzhang <[email protected]>

v4:
* Reformating log message
v3:
* Reformating log message
v2:
* Reformating log message

---
net/core/filter.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..b75fcf412628 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5958,6 +5958,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_proto;
case BPF_FUNC_get_socket_uid:
return &bpf_get_socket_uid_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
default:
return bpf_base_func_proto(func_id);
}
@@ -5978,6 +5980,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_storage_get_proto;
case BPF_FUNC_sk_storage_delete:
return &bpf_sk_storage_delete_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
@@ -6226,6 +6230,8 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sk_redirect_map_proto;
case BPF_FUNC_sk_redirect_hash:
return &bpf_sk_redirect_hash_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_skb_event_output_proto;
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sk_lookup_tcp_proto;
--
2.22.0.410.gd8fdbe21b5-goog

2019-06-25 20:07:06

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 0/2] bpf: Allow bpf_skb_event_output for more prog types

On Tue, Jun 25, 2019 at 12:45 PM allanzhang <[email protected]> wrote:
>
> Software event output is only enabled by a few prog types right now (TC,
> LWT out, XDP, sockops). Many other skb based prog types need
> bpf_skb_event_output to produce software event.
>
> Added socket_filter, cg_skb, sk_skb prog types to generate sw event.
>
> allanzhang (2):
> bpf: Allow bpf_skb_event_output for a few prog types
> bpf: Add selftests for bpf_perf_event_output

I am not sure whether this is caused by delay in the mailing list or something
else. But it appears to me that you are ignoring some of the feedback. Please
pay more attention to these feedback.

Please include changes "v1, xxx, v2, xxx, .." in the cover letter, but not the
commit log itself. In other words, include that in 0/2, but not in 1/2 or 2/2.

Thanks,
Song
>
> net/core/filter.c | 6 ++
> tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
> .../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
> 3 files changed, 132 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>

2019-06-25 20:12:27

by Song Liu

[permalink] [raw]
Subject: Re: [PATCH bpf-next v4 2/2] bpf: Add selftests for bpf_perf_event_output

On Tue, Jun 25, 2019 at 12:45 PM allanzhang <[email protected]> wrote:
>
> Software event output is only enabled by a few prog types.
> This test is to ensure that all supported types are enbled for
> bpf_perf_event_output sucessfully.

Please fix these typos highlighted by Daniel.
enbled
sucessfully

>
> v4:
> * Reformating log message
> v3:
> * Reformating log message
> v2:
> * Reformating log message
>
> Signed-off-by: allanzhang <[email protected]>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 33 ++++++-
> .../selftests/bpf/verifier/event_output.c | 94 +++++++++++++++++++
> 2 files changed, 126 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/event_output.c
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index c5514daf8865..901a188e1eea 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -50,7 +50,7 @@
> #define MAX_INSNS BPF_MAXINSNS
> #define MAX_TEST_INSNS 1000000
> #define MAX_FIXUPS 8
> -#define MAX_NR_MAPS 18
> +#define MAX_NR_MAPS 19
> #define MAX_TEST_RUNS 8
> #define POINTER_VALUE 0xcafe4all
> #define TEST_DATA_LEN 64
> @@ -84,6 +84,7 @@ struct bpf_test {
> int fixup_map_array_wo[MAX_FIXUPS];
> int fixup_map_array_small[MAX_FIXUPS];
> int fixup_sk_storage_map[MAX_FIXUPS];
> + int fixup_map_event_output[MAX_FIXUPS];
> const char *errstr;
> const char *errstr_unpriv;
> uint32_t retval, retval_unpriv, insn_processed;
> @@ -604,6 +605,28 @@ static int create_sk_storage_map(void)
> return fd;
> }
>
> +static int create_event_output_map(void)
> +{
> + struct bpf_create_map_attr attr = {
> + .name = "test_map",
> + .map_type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = 4,
> + .value_size = 4,
> + .max_entries = 1,
> + };
> + int fd, btf_fd;
> +
> + btf_fd = load_btf();
> + if (btf_fd < 0)
> + return -1;
> + attr.btf_fd = btf_fd;
> + fd = bpf_create_map_xattr(&attr);
> + close(attr.btf_fd);
> + if (fd < 0)
> + printf("Failed to create event_output\n");
> + return fd;
> +}
> +
> static char bpf_vlog[UINT_MAX >> 8];
>
> static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> @@ -627,6 +650,7 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> int *fixup_map_array_wo = test->fixup_map_array_wo;
> int *fixup_map_array_small = test->fixup_map_array_small;
> int *fixup_sk_storage_map = test->fixup_sk_storage_map;
> + int *fixup_map_event_output = test->fixup_map_event_output;
>
> if (test->fill_helper) {
> test->fill_insns = calloc(MAX_TEST_INSNS, sizeof(struct bpf_insn));
> @@ -788,6 +812,13 @@ static void do_test_fixup(struct bpf_test *test, enum bpf_prog_type prog_type,
> fixup_sk_storage_map++;
> } while (*fixup_sk_storage_map);
> }
> + if (*fixup_map_event_output) {
> + map_fds[18] = create_event_output_map();
> + do {
> + prog[*fixup_map_event_output].imm = map_fds[18];
> + fixup_map_event_output++;
> + } while (*fixup_map_event_output);
> + }
> }
>
> static int set_admin(bool admin)
> diff --git a/tools/testing/selftests/bpf/verifier/event_output.c b/tools/testing/selftests/bpf/verifier/event_output.c
> new file mode 100644
> index 000000000000..b25eabcfaa56
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/verifier/event_output.c
> @@ -0,0 +1,94 @@
> +/* instructions used to output a skb based software event, produced
> + * from code snippet:
> +struct TMP {
> + uint64_t tmp;
> +} tt;
> +tt.tmp = 5;
> +bpf_perf_event_output(skb, &connection_tracking_event_map, 0,
> + &tt, sizeof(tt));
> +return 1;
> +
> +the bpf assembly from llvm is:
> + 0: b7 02 00 00 05 00 00 00 r2 = 5
> + 1: 7b 2a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r2
> + 2: bf a4 00 00 00 00 00 00 r4 = r10
> + 3: 07 04 00 00 f8 ff ff ff r4 += -8
> + 4: 18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0ll
> + 6: b7 03 00 00 00 00 00 00 r3 = 0
> + 7: b7 05 00 00 08 00 00 00 r5 = 8
> + 8: 85 00 00 00 19 00 00 00 call 25
> + 9: b7 00 00 00 01 00 00 00 r0 = 1
> + 10: 95 00 00 00 00 00 00 00 exit
> +
> + The reason I put the code here instead of fill_helpers is that map fixup is
> + against the insns, instead of filled prog.
> +*/

Please prefix every line in the comment section with space_star_space: " * ".
This makes it obvious that this is a comment.

Thanks,
Song

> +
> +#define __PERF_EVENT_INSNS__ \
> + BPF_MOV64_IMM(BPF_REG_2, 5), \
> + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -8), \
> + BPF_MOV64_REG(BPF_REG_4, BPF_REG_10), \
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -8), \
> + BPF_LD_MAP_FD(BPF_REG_2, 0), \
> + BPF_MOV64_IMM(BPF_REG_3, 0), \
> + BPF_MOV64_IMM(BPF_REG_5, 8), \
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, \
> + BPF_FUNC_perf_event_output), \
> + BPF_MOV64_IMM(BPF_REG_0, 1), \
> + BPF_EXIT_INSN(),
> +{
> + "perfevent for sockops",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for tc",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for lwt out",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_LWT_OUT,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for xdp",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_XDP,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for socket filter",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SOCKET_FILTER,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for sk_skb",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_SK_SKB,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> +{
> + "perfevent for cgroup skb",
> + .insns = { __PERF_EVENT_INSNS__ },
> + .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
> + .fixup_map_event_output = { 4 },
> + .result = ACCEPT,
> + .retval = 1,
> +},
> --
> 2.22.0.410.gd8fdbe21b5-goog
>