lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 14 Sep 2020 11:07:30 -0700 From: Song Liu <song@...nel.org> To: Nicolas Rybowski <nicolas.rybowski@...sares.net> Cc: Shuah Khan <shuah@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, Andrii Nakryiko <andriin@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...omium.org>, Matthieu Baerts <matthieu.baerts@...sares.net>, open list <linux-kernel@...r.kernel.org>, linux-kselftest@...r.kernel.org, Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org> Subject: Re: [PATCH bpf-next v2 4/5] bpf: selftests: add MPTCP test base On Fri, Sep 11, 2020 at 8:02 AM Nicolas Rybowski <nicolas.rybowski@...sares.net> wrote: > > This patch adds a base for MPTCP specific tests. > > It is currently limited to the is_mptcp field in case of plain TCP > connection because for the moment there is no easy way to get the subflow > sk from a msk in userspace. This implies that we cannot lookup the > sk_storage attached to the subflow sk in the sockops program. > > Acked-by: Matthieu Baerts <matthieu.baerts@...sares.net> > Signed-off-by: Nicolas Rybowski <nicolas.rybowski@...sares.net> Acked-by: Song Liu <songliubraving@...com> With some nitpicks below. > --- > > Notes: > v1 -> v2: > - new patch: mandatory selftests (Alexei) > [...] > int timeout_ms); > diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c > new file mode 100644 > index 000000000000..0e65d64868e9 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > @@ -0,0 +1,119 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <test_progs.h> > +#include "cgroup_helpers.h" > +#include "network_helpers.h" > + > +struct mptcp_storage { > + __u32 invoked; > + __u32 is_mptcp; > +}; > + > +static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 is_mptcp) > +{ > + int err = 0, cfd = client_fd; > + struct mptcp_storage val; > + > + /* Currently there is no easy way to get back the subflow sk from the MPTCP > + * sk, thus we cannot access here the sk_storage associated to the subflow > + * sk. Also, there is no sk_storage associated with the MPTCP sk since it > + * does not trigger sockops events. > + * We silently pass this situation at the moment. > + */ > + if (is_mptcp == 1) > + return 0; > + > + if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) { > + perror("Failed to read socket storage"); Maybe simplify this with CHECK(), which contains a customized error message? Same for some other calls. > + return -1; > + } > + > + if (val.invoked != 1) { > + log_err("%s: unexpected invoked count %d != %d", > + msg, val.invoked, 1); > + err++; > + } > + > + if (val.is_mptcp != is_mptcp) { > + log_err("%s: unexpected bpf_tcp_sock.is_mptcp %d != %d", > + msg, val.is_mptcp, is_mptcp); > + err++; > + } > + > + return err; > +} > + > +static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) [...] > + > + client_fd = is_mptcp ? connect_to_mptcp_fd(server_fd, 0) : > + connect_to_fd(server_fd, 0); > + if (client_fd < 0) { > + err = -1; > + goto close_client_fd; This should be "goto close_bpf_object;", and we don't really need the label close_client_fd. > + } > + > + err += is_mptcp ? verify_sk(map_fd, client_fd, "MPTCP subflow socket", 1) : It doesn't really change the logic, but I guess we only need "err = xxx"? > + verify_sk(map_fd, client_fd, "plain TCP socket", 0); > + > +close_client_fd: > + close(client_fd); > + > +close_bpf_object: > + bpf_object__close(obj); > + return err; > +} > +
Powered by blists - more mailing lists