Message ID | 20220502211235.142250-7-mathew.j.martineau@linux.intel.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | BPF |
Headers | show |
Series | bpf: mptcp: Support for mptcp_sock and is_mptcp | expand |
On Mon, 2 May 2022, Mat Martineau wrote: > From: Geliang Tang <geliang.tang@suse.com> > > This patch verifies the struct member token of struct mptcp_sock. Add a > new function get_msk_token() to parse the msk token from the output of > the command 'ip mptcp monitor', and verify it in verify_msk(). > Daniel, Andrii, The z15 CI build failed on this commit, not due to any endianness issue but because it appears the z15 CI VM has an older iproute2 version (5.5.0) than the x86_64 VM (where the build succeeded). Do you need us (MPTCP) to change anything? Thanks! > 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> > --- > .../testing/selftests/bpf/bpf_mptcp_helpers.h | 1 + > .../testing/selftests/bpf/prog_tests/mptcp.c | 66 +++++++++++++++++++ > .../testing/selftests/bpf/progs/mptcp_sock.c | 5 ++ > 3 files changed, 72 insertions(+) -- Mat Martineau Intel
On Mon, May 2, 2022 at 3:14 PM Mat Martineau <mathew.j.martineau@linux.intel.com> wrote: > > On Mon, 2 May 2022, Mat Martineau wrote: > > > From: Geliang Tang <geliang.tang@suse.com> > > > > This patch verifies the struct member token of struct mptcp_sock. Add a > > new function get_msk_token() to parse the msk token from the output of > > the command 'ip mptcp monitor', and verify it in verify_msk(). > > > > Daniel, Andrii, > > The z15 CI build failed on this commit, not due to any endianness issue > but because it appears the z15 CI VM has an older iproute2 version (5.5.0) > than the x86_64 VM (where the build succeeded). > > Do you need us (MPTCP) to change anything? > I'll defer to Ilya (cc'ed). > Thanks! > > > 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> > > --- > > .../testing/selftests/bpf/bpf_mptcp_helpers.h | 1 + > > .../testing/selftests/bpf/prog_tests/mptcp.c | 66 +++++++++++++++++++ > > .../testing/selftests/bpf/progs/mptcp_sock.c | 5 ++ > > 3 files changed, 72 insertions(+) > > -- > Mat Martineau > Intel
On Mon, 2 May 2022, Mat Martineau wrote: > From: Geliang Tang <geliang.tang@suse.com> > > This patch verifies the struct member token of struct mptcp_sock. Add a > new function get_msk_token() to parse the msk token from the output of > the command 'ip mptcp monitor', and verify it in verify_msk(). > > 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> > --- > .../testing/selftests/bpf/bpf_mptcp_helpers.h | 1 + > .../testing/selftests/bpf/prog_tests/mptcp.c | 66 +++++++++++++++++++ > .../testing/selftests/bpf/progs/mptcp_sock.c | 5 ++ > 3 files changed, 72 insertions(+) > > diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > index 18da4cc65e89..87e15810997d 100644 > --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > @@ -9,6 +9,7 @@ > struct mptcp_sock { > struct inet_connection_sock sk; > > + __u32 token; > } __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 4b40bbdaf91f..c5d96ba81e04 100644 > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > @@ -8,8 +8,11 @@ > struct mptcp_storage { > __u32 invoked; > __u32 is_mptcp; > + __u32 token; > }; > > +static char monitor_log_path[64]; > + > static int verify_tsk(int map_fd, int client_fd) > { > char *msg = "plain TCP socket"; > @@ -36,11 +39,58 @@ static int verify_tsk(int map_fd, int client_fd) > return err; > } > > +/* > + * Parse the token from the output of 'ip mptcp monitor': > + * > + * [ CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ... > + * [ CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ... > + */ > +static __u32 get_msk_token(void) > +{ > + char *prefix = "[ CREATED] token="; > + char buf[BUFSIZ] = {}; > + __u32 token = 0; > + ssize_t len; > + int fd; > + > + sync(); > + > + fd = open(monitor_log_path, O_RDONLY); > + if (CHECK_FAIL(fd < 0)) { > + log_err("Failed to open %s", monitor_log_path); > + return token; > + } > + > + len = read(fd, buf, sizeof(buf)); > + if (CHECK_FAIL(len < 0)) { > + log_err("Failed to read %s", monitor_log_path); > + goto err; > + } > + > + if (strncmp(buf, prefix, strlen(prefix))) { > + log_err("Invalid prefix %s", buf); > + goto err; > + } > + > + token = strtol(buf + strlen(prefix), NULL, 16); > + > +err: > + close(fd); > + return token; > +} > + > static int verify_msk(int map_fd, int client_fd) > { > char *msg = "MPTCP subflow socket"; > int err = 0, cfd = client_fd; > struct mptcp_storage val; > + __u32 token; > + > + token = get_msk_token(); > + if (token <= 0) { > + log_err("Unexpected token %x", token); > + return -1; > + } > > if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) { > perror("Failed to read socket storage"); > @@ -59,6 +109,12 @@ static int verify_msk(int map_fd, int client_fd) > err++; > } > > + if (val.token != token) { > + log_err("Unexpected mptcp_sock.token %x != %x", > + val.token, token); > + err++; > + } > + > return err; > } > > @@ -124,6 +180,7 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > > void test_base(void) > { > + char cmd[256], tmp_dir[] = "/tmp/XXXXXX"; > int server_fd, cgroup_fd; > > cgroup_fd = test__join_cgroup("/mptcp"); > @@ -141,6 +198,13 @@ void test_base(void) > > with_mptcp: > /* with MPTCP */ Geliang, could you add a check here that skips this test (instead of failing) if the 'ip mptcp monitor' command is not supported? Checking the exit status of "ip mptcp help 2>&1 | grep monitor" should work. Thanks, Mat > + if (CHECK_FAIL(!mkdtemp(tmp_dir))) > + goto close_cgroup_fd; > + snprintf(monitor_log_path, sizeof(monitor_log_path), > + "%s/ip_mptcp_monitor", tmp_dir); > + snprintf(cmd, sizeof(cmd), "ip mptcp monitor > %s &", monitor_log_path); > + if (CHECK_FAIL(system(cmd))) > + goto close_cgroup_fd; > server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); > if (CHECK_FAIL(server_fd < 0)) > goto close_cgroup_fd; > @@ -148,6 +212,8 @@ void test_base(void) > CHECK_FAIL(run_test(cgroup_fd, server_fd, true)); > > close(server_fd); > + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir); > + system(cmd); > > close_cgroup_fd: > close(cgroup_fd); > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c > index 7b6a25e37de8..c58c191d8416 100644 > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c > @@ -12,6 +12,7 @@ extern bool CONFIG_MPTCP __kconfig; > struct mptcp_storage { > __u32 invoked; > __u32 is_mptcp; > + __u32 token; > }; > > struct { > @@ -46,6 +47,8 @@ int _sockops(struct bpf_sock_ops *ctx) > BPF_SK_STORAGE_GET_F_CREATE); > if (!storage) > return 1; > + > + storage->token = 0; > } else { > if (!CONFIG_MPTCP) > return 1; > @@ -58,6 +61,8 @@ int _sockops(struct bpf_sock_ops *ctx) > BPF_SK_STORAGE_GET_F_CREATE); > if (!storage) > return 1; > + > + storage->token = msk->token; > } > storage->invoked++; > storage->is_mptcp = tcp_sk->is_mptcp; > -- > 2.36.0 > > -- Mat Martineau Intel
On Tue, May 10, 2022 at 2:59 PM Mat Martineau <mathew.j.martineau@linux.intel.com> wrote: > > On Mon, 2 May 2022, Mat Martineau wrote: > > > From: Geliang Tang <geliang.tang@suse.com> > > > > This patch verifies the struct member token of struct mptcp_sock. Add a > > new function get_msk_token() to parse the msk token from the output of > > the command 'ip mptcp monitor', and verify it in verify_msk(). > > > > 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> > > --- > > .../testing/selftests/bpf/bpf_mptcp_helpers.h | 1 + > > .../testing/selftests/bpf/prog_tests/mptcp.c | 66 +++++++++++++++++++ > > .../testing/selftests/bpf/progs/mptcp_sock.c | 5 ++ > > 3 files changed, 72 insertions(+) > > > > diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > > index 18da4cc65e89..87e15810997d 100644 > > --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > > +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h > > @@ -9,6 +9,7 @@ > > struct mptcp_sock { > > struct inet_connection_sock sk; > > > > + __u32 token; > > } __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 4b40bbdaf91f..c5d96ba81e04 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c > > +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c > > @@ -8,8 +8,11 @@ > > struct mptcp_storage { > > __u32 invoked; > > __u32 is_mptcp; > > + __u32 token; > > }; > > > > +static char monitor_log_path[64]; > > + > > static int verify_tsk(int map_fd, int client_fd) > > { > > char *msg = "plain TCP socket"; > > @@ -36,11 +39,58 @@ static int verify_tsk(int map_fd, int client_fd) > > return err; > > } > > > > +/* > > + * Parse the token from the output of 'ip mptcp monitor': > > + * > > + * [ CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ... > > + * [ CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ... > > + */ > > +static __u32 get_msk_token(void) > > +{ > > + char *prefix = "[ CREATED] token="; > > + char buf[BUFSIZ] = {}; > > + __u32 token = 0; > > + ssize_t len; > > + int fd; > > + > > + sync(); > > + > > + fd = open(monitor_log_path, O_RDONLY); > > + if (CHECK_FAIL(fd < 0)) { > > + log_err("Failed to open %s", monitor_log_path); > > + return token; > > + } > > + > > + len = read(fd, buf, sizeof(buf)); > > + if (CHECK_FAIL(len < 0)) { > > + log_err("Failed to read %s", monitor_log_path); > > + goto err; > > + } > > + > > + if (strncmp(buf, prefix, strlen(prefix))) { > > + log_err("Invalid prefix %s", buf); > > + goto err; > > + } > > + > > + token = strtol(buf + strlen(prefix), NULL, 16); > > + > > +err: > > + close(fd); > > + return token; > > +} > > + > > static int verify_msk(int map_fd, int client_fd) > > { > > char *msg = "MPTCP subflow socket"; > > int err = 0, cfd = client_fd; > > struct mptcp_storage val; > > + __u32 token; > > + > > + token = get_msk_token(); > > + if (token <= 0) { > > + log_err("Unexpected token %x", token); > > + return -1; > > + } > > > > if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) { > > perror("Failed to read socket storage"); > > @@ -59,6 +109,12 @@ static int verify_msk(int map_fd, int client_fd) > > err++; > > } > > > > + if (val.token != token) { > > + log_err("Unexpected mptcp_sock.token %x != %x", > > + val.token, token); > > + err++; > > + } > > + > > return err; > > } > > > > @@ -124,6 +180,7 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) > > > > void test_base(void) > > { > > + char cmd[256], tmp_dir[] = "/tmp/XXXXXX"; > > int server_fd, cgroup_fd; > > > > cgroup_fd = test__join_cgroup("/mptcp"); > > @@ -141,6 +198,13 @@ void test_base(void) > > > > with_mptcp: > > /* with MPTCP */ > > Geliang, could you add a check here that skips this test (instead of > failing) if the 'ip mptcp monitor' command is not supported? > > Checking the exit status of "ip mptcp help 2>&1 | grep monitor" should > work. > Ilya actually already generated updated image, and after [0] it should be used in CI runs. But we'll know for sure with your next MPTCP submission. [0] https://github.com/libbpf/ci/pull/16 > Thanks, > > Mat > > > + if (CHECK_FAIL(!mkdtemp(tmp_dir))) > > + goto close_cgroup_fd; > > + snprintf(monitor_log_path, sizeof(monitor_log_path), > > + "%s/ip_mptcp_monitor", tmp_dir); > > + snprintf(cmd, sizeof(cmd), "ip mptcp monitor > %s &", monitor_log_path); > > + if (CHECK_FAIL(system(cmd))) > > + goto close_cgroup_fd; > > server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); > > if (CHECK_FAIL(server_fd < 0)) > > goto close_cgroup_fd; > > @@ -148,6 +212,8 @@ void test_base(void) > > CHECK_FAIL(run_test(cgroup_fd, server_fd, true)); > > > > close(server_fd); > > + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir); > > + system(cmd); > > > > close_cgroup_fd: > > close(cgroup_fd); > > diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c > > index 7b6a25e37de8..c58c191d8416 100644 > > --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c > > +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c > > @@ -12,6 +12,7 @@ extern bool CONFIG_MPTCP __kconfig; > > struct mptcp_storage { > > __u32 invoked; > > __u32 is_mptcp; > > + __u32 token; > > }; > > > > struct { > > @@ -46,6 +47,8 @@ int _sockops(struct bpf_sock_ops *ctx) > > BPF_SK_STORAGE_GET_F_CREATE); > > if (!storage) > > return 1; > > + > > + storage->token = 0; > > } else { > > if (!CONFIG_MPTCP) > > return 1; > > @@ -58,6 +61,8 @@ int _sockops(struct bpf_sock_ops *ctx) > > BPF_SK_STORAGE_GET_F_CREATE); > > if (!storage) > > return 1; > > + > > + storage->token = msk->token; > > } > > storage->invoked++; > > storage->is_mptcp = tcp_sk->is_mptcp; > > -- > > 2.36.0 > > > > > > -- > Mat Martineau > Intel
diff --git a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h index 18da4cc65e89..87e15810997d 100644 --- a/tools/testing/selftests/bpf/bpf_mptcp_helpers.h +++ b/tools/testing/selftests/bpf/bpf_mptcp_helpers.h @@ -9,6 +9,7 @@ struct mptcp_sock { struct inet_connection_sock sk; + __u32 token; } __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 4b40bbdaf91f..c5d96ba81e04 100644 --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c @@ -8,8 +8,11 @@ struct mptcp_storage { __u32 invoked; __u32 is_mptcp; + __u32 token; }; +static char monitor_log_path[64]; + static int verify_tsk(int map_fd, int client_fd) { char *msg = "plain TCP socket"; @@ -36,11 +39,58 @@ static int verify_tsk(int map_fd, int client_fd) return err; } +/* + * Parse the token from the output of 'ip mptcp monitor': + * + * [ CREATED] token=3ca933d3 remid=0 locid=0 saddr4=127.0.0.1 ... + * [ CREATED] token=2ab57040 remid=0 locid=0 saddr4=127.0.0.1 ... + */ +static __u32 get_msk_token(void) +{ + char *prefix = "[ CREATED] token="; + char buf[BUFSIZ] = {}; + __u32 token = 0; + ssize_t len; + int fd; + + sync(); + + fd = open(monitor_log_path, O_RDONLY); + if (CHECK_FAIL(fd < 0)) { + log_err("Failed to open %s", monitor_log_path); + return token; + } + + len = read(fd, buf, sizeof(buf)); + if (CHECK_FAIL(len < 0)) { + log_err("Failed to read %s", monitor_log_path); + goto err; + } + + if (strncmp(buf, prefix, strlen(prefix))) { + log_err("Invalid prefix %s", buf); + goto err; + } + + token = strtol(buf + strlen(prefix), NULL, 16); + +err: + close(fd); + return token; +} + static int verify_msk(int map_fd, int client_fd) { char *msg = "MPTCP subflow socket"; int err = 0, cfd = client_fd; struct mptcp_storage val; + __u32 token; + + token = get_msk_token(); + if (token <= 0) { + log_err("Unexpected token %x", token); + return -1; + } if (CHECK_FAIL(bpf_map_lookup_elem(map_fd, &cfd, &val) < 0)) { perror("Failed to read socket storage"); @@ -59,6 +109,12 @@ static int verify_msk(int map_fd, int client_fd) err++; } + if (val.token != token) { + log_err("Unexpected mptcp_sock.token %x != %x", + val.token, token); + err++; + } + return err; } @@ -124,6 +180,7 @@ static int run_test(int cgroup_fd, int server_fd, bool is_mptcp) void test_base(void) { + char cmd[256], tmp_dir[] = "/tmp/XXXXXX"; int server_fd, cgroup_fd; cgroup_fd = test__join_cgroup("/mptcp"); @@ -141,6 +198,13 @@ void test_base(void) with_mptcp: /* with MPTCP */ + if (CHECK_FAIL(!mkdtemp(tmp_dir))) + goto close_cgroup_fd; + snprintf(monitor_log_path, sizeof(monitor_log_path), + "%s/ip_mptcp_monitor", tmp_dir); + snprintf(cmd, sizeof(cmd), "ip mptcp monitor > %s &", monitor_log_path); + if (CHECK_FAIL(system(cmd))) + goto close_cgroup_fd; server_fd = start_mptcp_server(AF_INET, NULL, 0, 0); if (CHECK_FAIL(server_fd < 0)) goto close_cgroup_fd; @@ -148,6 +212,8 @@ void test_base(void) CHECK_FAIL(run_test(cgroup_fd, server_fd, true)); close(server_fd); + snprintf(cmd, sizeof(cmd), "rm -rf %s", tmp_dir); + system(cmd); close_cgroup_fd: close(cgroup_fd); diff --git a/tools/testing/selftests/bpf/progs/mptcp_sock.c b/tools/testing/selftests/bpf/progs/mptcp_sock.c index 7b6a25e37de8..c58c191d8416 100644 --- a/tools/testing/selftests/bpf/progs/mptcp_sock.c +++ b/tools/testing/selftests/bpf/progs/mptcp_sock.c @@ -12,6 +12,7 @@ extern bool CONFIG_MPTCP __kconfig; struct mptcp_storage { __u32 invoked; __u32 is_mptcp; + __u32 token; }; struct { @@ -46,6 +47,8 @@ int _sockops(struct bpf_sock_ops *ctx) BPF_SK_STORAGE_GET_F_CREATE); if (!storage) return 1; + + storage->token = 0; } else { if (!CONFIG_MPTCP) return 1; @@ -58,6 +61,8 @@ int _sockops(struct bpf_sock_ops *ctx) BPF_SK_STORAGE_GET_F_CREATE); if (!storage) return 1; + + storage->token = msk->token; } storage->invoked++; storage->is_mptcp = tcp_sk->is_mptcp;