diff mbox series

[bpf-next,v5,4/5] bpf, selftests: add sk_msg helpers load and attach test

Message ID 159033909665.12355.6166415847337547879.stgit@john-Precision-5820-Tower
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series bpf: Add sk_msg and networking helpers | expand

Commit Message

John Fastabend May 24, 2020, 4:51 p.m. UTC
The test itself is not particularly useful but it encodes a common
pattern we have.

Namely do a sk storage lookup then depending on data here decide if
we need to do more work or alternatively allow packet to PASS. Then
if we need to do more work consult task_struct for more information
about the running task. Finally based on this additional information
drop or pass the data. In this case the suspicious check is not so
realisitic but it encodes the general pattern and uses the helpers
so we test the workflow.

This is a load test to ensure verifier correctly handles this case.

Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c       |   35 +++++++++++++++
 .../selftests/bpf/progs/test_skmsg_load_helpers.c  |   47 ++++++++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c

Comments

Andrii Nakryiko May 26, 2020, 6:07 p.m. UTC | #1
On Sun, May 24, 2020 at 9:51 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> The test itself is not particularly useful but it encodes a common
> pattern we have.
>
> Namely do a sk storage lookup then depending on data here decide if
> we need to do more work or alternatively allow packet to PASS. Then
> if we need to do more work consult task_struct for more information
> about the running task. Finally based on this additional information
> drop or pass the data. In this case the suspicious check is not so
> realisitic but it encodes the general pattern and uses the helpers
> so we test the workflow.
>
> This is a load test to ensure verifier correctly handles this case.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---

Other than perror and CHECK_FAIL nag below, looks good:

Acked-by: Andrii Nakryiko <andriin@fb.com>

>  .../selftests/bpf/prog_tests/sockmap_basic.c       |   35 +++++++++++++++
>  .../selftests/bpf/progs/test_skmsg_load_helpers.c  |   47 ++++++++++++++++++++
>  2 files changed, 82 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index aa43e0b..96e7b7f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> @@ -1,7 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // Copyright (c) 2020 Cloudflare
> +#include <error.h>
>
>  #include "test_progs.h"
> +#include "test_skmsg_load_helpers.skel.h"
>
>  #define TCP_REPAIR             19      /* TCP sock is under repair right now */
>
> @@ -70,10 +72,43 @@ static void test_sockmap_create_update_free(enum bpf_map_type map_type)
>         close(s);
>  }
>
> +static void test_skmsg_helpers(enum bpf_map_type map_type)
> +{
> +       struct test_skmsg_load_helpers *skel;
> +       int err, map, verdict;
> +
> +       skel = test_skmsg_load_helpers__open_and_load();
> +       if (CHECK_FAIL(!skel)) {
> +               perror("test_skmsg_load_helpers__open_and_load");

All test_progs tests use CHECK() macro to test and emit error message
on error, so no need to do silent CHECK_FAIL() and then perror(). Same
below in few places. I don't think you need to send v6 just for this,
but please follow up with a clean up.

> +               return;
> +       }
> +
> +       verdict = bpf_program__fd(skel->progs.prog_msg_verdict);
> +       map = bpf_map__fd(skel->maps.sock_map);
> +
> +       err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
> +       if (CHECK_FAIL(err)) {
> +               perror("bpf_prog_attach");
> +               goto out;
> +       }
> +
> +       err = bpf_prog_detach2(verdict, map, BPF_SK_MSG_VERDICT);
> +       if (CHECK_FAIL(err)) {
> +               perror("bpf_prog_detach2");
> +               goto out;
> +       }
> +out:
> +       test_skmsg_load_helpers__destroy(skel);
> +}
> +

[...]
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index aa43e0b..96e7b7f 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1,7 +1,9 @@ 
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (c) 2020 Cloudflare
+#include <error.h>
 
 #include "test_progs.h"
+#include "test_skmsg_load_helpers.skel.h"
 
 #define TCP_REPAIR		19	/* TCP sock is under repair right now */
 
@@ -70,10 +72,43 @@  static void test_sockmap_create_update_free(enum bpf_map_type map_type)
 	close(s);
 }
 
+static void test_skmsg_helpers(enum bpf_map_type map_type)
+{
+	struct test_skmsg_load_helpers *skel;
+	int err, map, verdict;
+
+	skel = test_skmsg_load_helpers__open_and_load();
+	if (CHECK_FAIL(!skel)) {
+		perror("test_skmsg_load_helpers__open_and_load");
+		return;
+	}
+
+	verdict = bpf_program__fd(skel->progs.prog_msg_verdict);
+	map = bpf_map__fd(skel->maps.sock_map);
+
+	err = bpf_prog_attach(verdict, map, BPF_SK_MSG_VERDICT, 0);
+	if (CHECK_FAIL(err)) {
+		perror("bpf_prog_attach");
+		goto out;
+	}
+
+	err = bpf_prog_detach2(verdict, map, BPF_SK_MSG_VERDICT);
+	if (CHECK_FAIL(err)) {
+		perror("bpf_prog_detach2");
+		goto out;
+	}
+out:
+	test_skmsg_load_helpers__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
 		test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKMAP);
 	if (test__start_subtest("sockhash create_update_free"))
 		test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKHASH);
+	if (test__start_subtest("sockmap sk_msg load helpers"))
+		test_skmsg_helpers(BPF_MAP_TYPE_SOCKMAP);
+	if (test__start_subtest("sockhash sk_msg load helpers"))
+		test_skmsg_helpers(BPF_MAP_TYPE_SOCKHASH);
 }
diff --git a/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
new file mode 100644
index 0000000..45e8fc7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_skmsg_load_helpers.c
@@ -0,0 +1,47 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2020 Isovalent, Inc.
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKMAP);
+	__uint(max_entries, 2);
+	__type(key, __u32);
+	__type(value, __u64);
+} sock_map SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKHASH);
+	__uint(max_entries, 2);
+	__type(key, __u32);
+	__type(value, __u64);
+} sock_hash SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SK_STORAGE);
+	__uint(map_flags, BPF_F_NO_PREALLOC);
+	__type(key, __u32);
+	__type(value, __u64);
+} socket_storage SEC(".maps");
+
+SEC("sk_msg")
+int prog_msg_verdict(struct sk_msg_md *msg)
+{
+	struct task_struct *task = (struct task_struct *)bpf_get_current_task();
+	int verdict = SK_PASS;
+	__u32 pid, tpid;
+	__u64 *sk_stg;
+
+	pid = bpf_get_current_pid_tgid() >> 32;
+	sk_stg = bpf_sk_storage_get(&socket_storage, msg->sk, 0, BPF_SK_STORAGE_GET_F_CREATE);
+	if (!sk_stg)
+		return SK_DROP;
+	*sk_stg = pid;
+	bpf_probe_read_kernel(&tpid , sizeof(tpid), &task->tgid);
+	if (pid != tpid)
+		verdict = SK_DROP;
+	bpf_sk_storage_delete(&socket_storage, (void *)msg->sk);
+	return verdict;
+}
+
+char _license[] SEC("license") = "GPL";