| Message ID | 20220513224827.662254-5-mathew.j.martineau@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: mptcp: Support for mptcp_sock | expand |
Mat Martineau <mathew.j.martineau@linux.intel.com> 于2022年5月14日周六 06:48写道： > > From: Geliang Tang <geliang.tang@suse.com> > > This patch extends the MPTCP test base, to test the new helper > bpf_skc_to_mptcp_sock(). > > Define struct mptcp_sock in bpf_tcp_helpers.h, use bpf_skc_to_mptcp_sock > to get the msk socket in progs/mptcp_sock.c and store the infos in > socket_storage_map. > > Get the infos from socket_storage_map in prog_tests/mptcp.c. Add a new > function verify_msk() to verify the infos of MPTCP socket, and rename > verify_sk() to verify_tsk() to verify TCP socket only. > > v2: Add CONFIG_MPTCP check for clearer error messages > v4: > - use ASSERT_* instead of CHECK_FAIL (Andrii) > - drop bpf_mptcp_helpers.h (Andrii) > > Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net> > Signed-off-by: Geliang Tang <geliang.tang@suse.com> > Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > --- > tools/testing/selftests/bpf/bpf_tcp_helpers.h | 5 +++ > .../testing/selftests/bpf/prog_tests/mptcp.c | 45 ++++++++++++++----- > .../testing/selftests/bpf/progs/mptcp_sock.c | 23 ++++++++-- > 3 files changed, 58 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h > index 22e0c8849a17..90fecafc493d 100644 > --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h > +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h > @@ -226,4 +226,9 @@ static __always_inline bool tcp_cc_eq(const char *a, const char *b) > extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym; > extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym; > > +struct mptcp_sock { > + struct inet_connection_sock sk; > + > +} __attribute__((preserve_access_index)); > + > #endif > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c > index cb0389ca8690..02e7fd8918e6 100644 > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > @@ -11,14 +11,12 @@ struct mptcp_storage { > __u32 is_mptcp; > }; > > -static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp) > +static int verify_tsk(int map_fd, int client_fd) > { > + char *msg = "plain TCP socket"; > int err, cfd = client_fd; > struct mptcp_storage val; > > - if (is_mptcp == 1) > - return 0; > - > err = bpf_map_lookup_elem(map_fd, &cfd, &val); > if (!ASSERT_OK(err, "bpf_map_lookup_elem")) > return err; > @@ -38,6 +36,31 @@ static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp) > return err; > } > > +static int verify_msk(int map_fd, int client_fd) > +{ > + char *msg = "MPTCP subflow socket"; > + int err, cfd = client_fd; > + struct mptcp_storage val; > + > + err = bpf_map_lookup_elem(map_fd, &cfd, &val); > + if (!ASSERT_OK(err, "bpf_map_lookup_elem")) > + return err; > + > + if (val.invoked != 1) { > + log_err("%s: unexpected invoked count %d != 1", > + msg, val.invoked); > + err++; > + } > + > + if (val.is_mptcp != 1) { > + log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1", > + msg, val.is_mptcp); > + err++; > + } > + > + return err; > +} > + > static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > { > int client_fd, prog_fd, map_fd, err; > @@ -88,8 +111,8 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > goto out; > } > > - err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) : > - verify_sk(map_fd, client_fd, "plain TCP socket", 0); > + err += is_mptcp ? verify_msk(map_fd, client_fd) : > + verify_tsk(map_fd, client_fd); > > close(client_fd); > '''' > @@ -103,25 +126,25 @@ void test_base(void) > int server_fd, cgroup_fd; > > cgroup_fd = test__join_cgroup("/mptcp"); > - if (CHECK_FAIL(cgroup_fd < 0)) > + if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) > return; > > /* without MPTCP */ > server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0); > - if (CHECK_FAIL(server_fd < 0)) > + if (!ASSERT_GE(server_fd, 0, "start_server")) > goto with_mptcp; > > - CHECK_FAIL(run_test(cgroup_fd, server_fd, false)); > + ASSERT_OK(run_test(cgroup_fd, server_fd, false), "run_test tcp"); > > close(server_fd); > > with_mptcp: > /* with MPTCP */ > server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); > - if (CHECK_FAIL(server_fd < 0)) > + if (!ASSERT_GE(server_fd, 0, "start_mptcp_server")) > goto close_cgroup_fd; > > - CHECK_FAIL(run_test(cgroup_fd, server_fd, true)); > + ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp"); ''' Sorry Mat, this code using ASSERT_* instead of CHECK_FAIL should be squash into patch #3, it shouldn't in this patch. I'll send a v5 to MPTCP ML to fix this. Thanks, -Geliang > > close(server_fd); > > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c > index bc09dba0b078..3feb7ff578e2 100644 > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c > @@ -7,6 +7,7 @@ > #include "bpf_tcp_helpers.h" > > char _license[] SEC("license") = "GPL"; > +extern bool CONFIG_MPTCP __kconfig; > > struct mptcp_storage { > __u32 invoked; > @@ -24,6 +25,7 @@ SEC("sockops") > int _sockops(struct bpf_sock_ops *ctx) > { > struct mptcp_storage *storage; > + struct mptcp_sock *msk; > int op = (int)ctx->op; > struct tcp_sock *tsk; > struct bpf_sock *sk; > @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx) > return 1; > > is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0; > - storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > - BPF_SK_STORAGE_GET_F_CREATE); > - if (!storage) > - return 1; > + if (!is_mptcp) { > + storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > + BPF_SK_STORAGE_GET_F_CREATE); > + if (!storage) > + return 1; > + } else { > + if (!CONFIG_MPTCP) > + return 1; > + > + msk = bpf_skc_to_mptcp_sock(sk); > + if (!msk) > + return 1; > > + storage = bpf_sk_storage_get(&socket_storage_map, msk, 0, > + BPF_SK_STORAGE_GET_F_CREATE); > + if (!storage) > + return 1; > + } > storage->invoked++; > storage->is_mptcp = is_mptcp; > > -- > 2.36.1 > >
On Fri, May 13, 2022 at 3:48 PM Mat Martineau <mathew.j.martineau@linux.intel.com> wrote: > > From: Geliang Tang <geliang.tang@suse.com> > > This patch extends the MPTCP test base, to test the new helper > bpf_skc_to_mptcp_sock(). > > Define struct mptcp_sock in bpf_tcp_helpers.h, use bpf_skc_to_mptcp_sock > to get the msk socket in progs/mptcp_sock.c and store the infos in > socket_storage_map. > > Get the infos from socket_storage_map in prog_tests/mptcp.c. Add a new > function verify_msk() to verify the infos of MPTCP socket, and rename > verify_sk() to verify_tsk() to verify TCP socket only. > > v2: Add CONFIG_MPTCP check for clearer error messages > v4: > - use ASSERT_* instead of CHECK_FAIL (Andrii) > - drop bpf_mptcp_helpers.h (Andrii) > > Acked-by: Matthieu Baerts <matthieu.baerts@tessares.net> > Signed-off-by: Geliang Tang <geliang.tang@suse.com> > Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com> > --- > tools/testing/selftests/bpf/bpf_tcp_helpers.h | 5 +++ > .../testing/selftests/bpf/prog_tests/mptcp.c | 45 ++++++++++++++----- > .../testing/selftests/bpf/progs/mptcp_sock.c | 23 ++++++++-- > 3 files changed, 58 insertions(+), 15 deletions(-) > [...] > +static int verify_msk(int map_fd, int client_fd) > +{ > + char *msg = "MPTCP subflow socket"; > + int err, cfd = client_fd; > + struct mptcp_storage val; > + > + err = bpf_map_lookup_elem(map_fd, &cfd, &val); > + if (!ASSERT_OK(err, "bpf_map_lookup_elem")) > + return err; > + > + if (val.invoked != 1) { > + log_err("%s: unexpected invoked count %d != 1", > + msg, val.invoked); > + err++; > + } > + > + if (val.is_mptcp != 1) { > + log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1", > + msg, val.is_mptcp); > + err++; > + } any reason to not use ASSERT_NEQ ? > + > + return err; > +} > + > static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > { > int client_fd, prog_fd, map_fd, err; [...]
On Fri, May 13, 2022 at 03:48:24PM -0700, Mat Martineau wrote: [ ... ] > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c > index bc09dba0b078..3feb7ff578e2 100644 > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c > @@ -7,6 +7,7 @@ > #include "bpf_tcp_helpers.h" > > char _license[] SEC("license") = "GPL"; > +extern bool CONFIG_MPTCP __kconfig; > > struct mptcp_storage { > __u32 invoked; > @@ -24,6 +25,7 @@ SEC("sockops") > int _sockops(struct bpf_sock_ops *ctx) > { > struct mptcp_storage *storage; > + struct mptcp_sock *msk; > int op = (int)ctx->op; > struct tcp_sock *tsk; > struct bpf_sock *sk; > @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx) > return 1; > > is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0; > - storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > - BPF_SK_STORAGE_GET_F_CREATE); > - if (!storage) > - return 1; > + if (!is_mptcp) { > + storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > + BPF_SK_STORAGE_GET_F_CREATE); > + if (!storage) > + return 1; > + } else { > + if (!CONFIG_MPTCP) hmm... how is it possible ? The above just tested "!is_mptcp". > + return 1; > + > + msk = bpf_skc_to_mptcp_sock(sk); > + if (!msk) > + return 1; > > + storage = bpf_sk_storage_get(&socket_storage_map, msk, 0, > + BPF_SK_STORAGE_GET_F_CREATE); > + if (!storage) > + return 1; > + } > storage->invoked++; > storage->is_mptcp = is_mptcp; > > -- > 2.36.1 >
Martin KaFai Lau <kafai@fb.com> 于2022年5月17日周二 09:21写道： > > On Fri, May 13, 2022 at 03:48:24PM -0700, Mat Martineau wrote: > [ ... ] > > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c > > index bc09dba0b078..3feb7ff578e2 100644 > > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c > > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c > > @@ -7,6 +7,7 @@ > > #include "bpf_tcp_helpers.h" > > > > char _license[] SEC("license") = "GPL"; > > +extern bool CONFIG_MPTCP __kconfig; > > > > struct mptcp_storage { > > __u32 invoked; > > @@ -24,6 +25,7 @@ SEC("sockops") > > int _sockops(struct bpf_sock_ops *ctx) > > { > > struct mptcp_storage *storage; > > + struct mptcp_sock *msk; > > int op = (int)ctx->op; > > struct tcp_sock *tsk; > > struct bpf_sock *sk; > > @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx) > > return 1; > > > > is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0; > > - storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > > - BPF_SK_STORAGE_GET_F_CREATE); > > - if (!storage) > > - return 1; > > + if (!is_mptcp) { > > + storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, > > + BPF_SK_STORAGE_GET_F_CREATE); > > + if (!storage) > > + return 1; > > + } else { > > + if (!CONFIG_MPTCP) > hmm... how is it possible ? The above just tested "!is_mptcp". Will drop this in v5, thanks. > > > + return 1; > > + > > + msk = bpf_skc_to_mptcp_sock(sk); > > + if (!msk) > > + return 1; > > > > + storage = bpf_sk_storage_get(&socket_storage_map, msk, 0, > > + BPF_SK_STORAGE_GET_F_CREATE); > > + if (!storage) > > + return 1; > > + } > > storage->invoked++; > > storage->is_mptcp = is_mptcp; > > > > -- > > 2.36.1 > > >
diff --git a/tools/testing/selftests/bpf/bpf_tcp_helpers.h b/tools/testing/selftests/bpf/bpf_tcp_helpers.h index 22e0c8849a17..90fecafc493d 100644 --- a/tools/testing/selftests/bpf/bpf_tcp_helpers.h +++ b/tools/testing/selftests/bpf/bpf_tcp_helpers.h @@ -226,4 +226,9 @@ static __always_inline bool tcp_cc_eq(const char *a, const char *b) extern __u32 tcp_slow_start(struct tcp_sock *tp, __u32 acked) __ksym; extern void tcp_cong_avoid_ai(struct tcp_sock *tp, __u32 w, __u32 acked) __ksym; +struct mptcp_sock { + struct inet_connection_sock sk; + +} __attribute__((preserve_access_index)); + #endif diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c index cb0389ca8690..02e7fd8918e6 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -11,14 +11,12 @@ struct mptcp_storage { __u32 is_mptcp; }; -static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp) +static int verify_tsk(int map_fd, int client_fd) { + char *msg = "plain TCP socket"; int err, cfd = client_fd; struct mptcp_storage val; - if (is_mptcp == 1) - return 0; - err = bpf_map_lookup_elem(map_fd, &cfd, &val); if (!ASSERT_OK(err, "bpf_map_lookup_elem")) return err; @@ -38,6 +36,31 @@ static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp) return err; } +static int verify_msk(int map_fd, int client_fd) +{ + char *msg = "MPTCP subflow socket"; + int err, cfd = client_fd; + struct mptcp_storage val; + + err = bpf_map_lookup_elem(map_fd, &cfd, &val); + if (!ASSERT_OK(err, "bpf_map_lookup_elem")) + return err; + + if (val.invoked != 1) { + log_err("%s: unexpected invoked count %d != 1", + msg, val.invoked); + err++; + } + + if (val.is_mptcp != 1) { + log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != 1", + msg, val.is_mptcp); + err++; + } + + return err; +} + static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) { int client_fd, prog_fd, map_fd, err; @@ -88,8 +111,8 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) goto out; } - err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) : - verify_sk(map_fd, client_fd, "plain TCP socket", 0); + err += is_mptcp ? verify_msk(map_fd, client_fd) : + verify_tsk(map_fd, client_fd); close(client_fd); @@ -103,25 +126,25 @@ void test_base(void) int server_fd, cgroup_fd; cgroup_fd = test__join_cgroup("/mptcp"); - if (CHECK_FAIL(cgroup_fd < 0)) + if (!ASSERT_GE(cgroup_fd, 0, "test__join_cgroup")) return; /* without MPTCP */ server_fd = start_server(AF_INET, SOCK_STREAM, NULL, 0, 0); - if (CHECK_FAIL(server_fd < 0)) + if (!ASSERT_GE(server_fd, 0, "start_server")) goto with_mptcp; - CHECK_FAIL(run_test(cgroup_fd, server_fd, false)); + ASSERT_OK(run_test(cgroup_fd, server_fd, false), "run_test tcp"); close(server_fd); with_mptcp: /* with MPTCP */ server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); - if (CHECK_FAIL(server_fd < 0)) + if (!ASSERT_GE(server_fd, 0, "start_mptcp_server")) goto close_cgroup_fd; - CHECK_FAIL(run_test(cgroup_fd, server_fd, true)); + ASSERT_OK(run_test(cgroup_fd, server_fd, true), "run_test mptcp"); close(server_fd); diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c index bc09dba0b078..3feb7ff578e2 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c @@ -7,6 +7,7 @@ #include "bpf_tcp_helpers.h" char _license[] SEC("license") = "GPL"; +extern bool CONFIG_MPTCP __kconfig; struct mptcp_storage { __u32 invoked; @@ -24,6 +25,7 @@ SEC("sockops") int _sockops(struct bpf_sock_ops *ctx) { struct mptcp_storage *storage; + struct mptcp_sock *msk; int op = (int)ctx->op; struct tcp_sock *tsk; struct bpf_sock *sk; @@ -41,11 +43,24 @@ int _sockops(struct bpf_sock_ops *ctx) return 1; is_mptcp = bpf_core_field_exists(tsk->is_mptcp) ? tsk->is_mptcp : 0; - storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, - BPF_SK_STORAGE_GET_F_CREATE); - if (!storage) - return 1; + if (!is_mptcp) { + storage = bpf_sk_storage_get(&socket_storage_map, sk, 0, + BPF_SK_STORAGE_GET_F_CREATE); + if (!storage) + return 1; + } else { + if (!CONFIG_MPTCP) + return 1; + + msk = bpf_skc_to_mptcp_sock(sk); + if (!msk) + return 1; + storage = bpf_sk_storage_get(&socket_storage_map, msk, 0, + BPF_SK_STORAGE_GET_F_CREATE); + if (!storage) + return 1; + } storage->invoked++; storage->is_mptcp = is_mptcp;