2022-08-17 18:47:36

by Daniel Xu

[permalink] [raw]
Subject: [PATCH bpf-next v2 4/4] selftests/bpf: Add tests for writing to nf_conn:mark

Add a simple extension to the existing selftest to write to
nf_conn:mark. Also add a failure test for writing to unsupported field.

Signed-off-by: Daniel Xu <[email protected]>
---
tools/testing/selftests/bpf/prog_tests/bpf_nf.c | 1 +
tools/testing/selftests/bpf/progs/test_bpf_nf.c | 6 ++++--
.../testing/selftests/bpf/progs/test_bpf_nf_fail.c | 14 ++++++++++++++
3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
index 544bf90ac2a7..45389c39f211 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
@@ -17,6 +17,7 @@ struct {
{ "set_status_after_insert", "kernel function bpf_ct_set_status args#0 expected pointer to STRUCT nf_conn___init but" },
{ "change_timeout_after_alloc", "kernel function bpf_ct_change_timeout args#0 expected pointer to STRUCT nf_conn but" },
{ "change_status_after_alloc", "kernel function bpf_ct_change_status args#0 expected pointer to STRUCT nf_conn but" },
+ { "write_not_allowlisted_field", "no write support to nf_conn at off" },
};

enum {
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
index 2722441850cc..638b6642d20f 100644
--- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
@@ -175,8 +175,10 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32,
sizeof(opts_def));
if (ct) {
test_exist_lookup = 0;
- if (ct->mark == 42)
- test_exist_lookup_mark = 43;
+ if (ct->mark == 42) {
+ ct->mark++;
+ test_exist_lookup_mark = ct->mark;
+ }
bpf_ct_release(ct);
} else {
test_exist_lookup = opts_def.error;
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
index bf79af15c808..0e4759ab38ff 100644
--- a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
+++ b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
@@ -69,6 +69,20 @@ int lookup_insert(struct __sk_buff *ctx)
return 0;
}

+SEC("?tc")
+int write_not_allowlisted_field(struct __sk_buff *ctx)
+{
+ struct bpf_ct_opts___local opts = {};
+ struct bpf_sock_tuple tup = {};
+ struct nf_conn *ct;
+
+ ct = bpf_skb_ct_lookup(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
+ if (!ct)
+ return 0;
+ ct->status = 0xF00;
+ return 0;
+}
+
SEC("?tc")
int set_timeout_after_insert(struct __sk_buff *ctx)
{
--
2.37.1


2022-08-17 20:02:02

by Kumar Kartikeya Dwivedi

[permalink] [raw]
Subject: Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add tests for writing to nf_conn:mark

On Wed, 17 Aug 2022 at 20:43, Daniel Xu <[email protected]> wrote:
>
> Add a simple extension to the existing selftest to write to
> nf_conn:mark. Also add a failure test for writing to unsupported field.
>
> Signed-off-by: Daniel Xu <[email protected]>
> ---
> tools/testing/selftests/bpf/prog_tests/bpf_nf.c | 1 +
> tools/testing/selftests/bpf/progs/test_bpf_nf.c | 6 ++++--
> .../testing/selftests/bpf/progs/test_bpf_nf_fail.c | 14 ++++++++++++++
> 3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
> index 544bf90ac2a7..45389c39f211 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_nf.c
> @@ -17,6 +17,7 @@ struct {
> { "set_status_after_insert", "kernel function bpf_ct_set_status args#0 expected pointer to STRUCT nf_conn___init but" },
> { "change_timeout_after_alloc", "kernel function bpf_ct_change_timeout args#0 expected pointer to STRUCT nf_conn but" },
> { "change_status_after_alloc", "kernel function bpf_ct_change_status args#0 expected pointer to STRUCT nf_conn but" },
> + { "write_not_allowlisted_field", "no write support to nf_conn at off" },
> };
>
> enum {
> diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> index 2722441850cc..638b6642d20f 100644
> --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c
> @@ -175,8 +175,10 @@ nf_ct_test(struct nf_conn *(*lookup_fn)(void *, struct bpf_sock_tuple *, u32,
> sizeof(opts_def));
> if (ct) {
> test_exist_lookup = 0;
> - if (ct->mark == 42)
> - test_exist_lookup_mark = 43;
> + if (ct->mark == 42) {
> + ct->mark++;

Please also include a test for setting ct->mark on allocated but not
inserted nf_conn. For that you might have to add another check in the
btf_struct_access callback to also consider nf_conn___init equivalent
to nf_conn. We use a container type to deny operations that are not
safe for allocated ct, but the layout is same for both (since it just
embeds struct nf_conn inside it), so the rest of the logic should work
the same.

> + test_exist_lookup_mark = ct->mark;
> + }
> bpf_ct_release(ct);
> } else {
> test_exist_lookup = opts_def.error;
> diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
> index bf79af15c808..0e4759ab38ff 100644
> --- a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
> +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c
> @@ -69,6 +69,20 @@ int lookup_insert(struct __sk_buff *ctx)
> return 0;
> }
>
> +SEC("?tc")
> +int write_not_allowlisted_field(struct __sk_buff *ctx)
> +{
> + struct bpf_ct_opts___local opts = {};
> + struct bpf_sock_tuple tup = {};
> + struct nf_conn *ct;
> +
> + ct = bpf_skb_ct_lookup(ctx, &tup, sizeof(tup.ipv4), &opts, sizeof(opts));
> + if (!ct)
> + return 0;
> + ct->status = 0xF00;
> + return 0;
> +}
> +
> SEC("?tc")
> int set_timeout_after_insert(struct __sk_buff *ctx)
> {
> --
> 2.37.1
>