| Message ID | 009c7c136773462cc71c73ed8da84d58df2e1e3f.1720664658.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | use network helpers, part 9 | expand |
On 7/10/24 7:52 PM, Geliang Tang wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > The struct member "must_fail" of network_helper_opts() is only used in > cgroup_v1v2 tests, it makes sense to drop it from network_helper_opts. > > Return value (fd) of connect_to_fd_opts() and the expect errno (EPERM) > can be checked in cgroup_v1v2.c directly, no need to check them in > connect_fd_to_addr() in network_helpers.c. > > This also makes connect_fd_to_addr() function useless. It can be replaced > by connect(). > > Suggested-by: Martin KaFai Lau <martin.lau@kernel.org> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn> > --- > tools/testing/selftests/bpf/network_helpers.c | 31 ++----------------- > tools/testing/selftests/bpf/network_helpers.h | 1 - > .../selftests/bpf/prog_tests/cgroup_v1v2.c | 6 ++-- > 3 files changed, 4 insertions(+), 34 deletions(-) > > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c > index 15e0e0bb7553..48c27c810db7 100644 > --- a/tools/testing/selftests/bpf/network_helpers.c > +++ b/tools/testing/selftests/bpf/network_helpers.c > @@ -277,33 +277,6 @@ int client_socket(int family, int type, > return -1; > } > > -static int connect_fd_to_addr(int fd, > - const struct sockaddr_storage *addr, > - socklen_t addrlen, const bool must_fail) > -{ > - int ret; > - > - errno = 0; > - ret = connect(fd, (const struct sockaddr *)addr, addrlen); > - if (must_fail) { > - if (!ret) { > - log_err("Unexpected success to connect to server"); > - return -1; > - } > - if (errno != EPERM) { > - log_err("Unexpected error from connect to server"); > - return -1; > - } > - } else { > - if (ret) { > - log_err("Failed to connect to server"); > - return -1; > - } > - } > - > - return 0; > -} > - > int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen, > const struct network_helper_opts *opts) > { > @@ -318,7 +291,7 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add > return -1; > } > > - if (connect_fd_to_addr(fd, addr, addrlen, opts->must_fail)) > + if (connect(fd, (const struct sockaddr *)addr, addrlen)) > goto error_close; > > return fd; > @@ -383,7 +356,7 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms) > return -1; > } > > - if (connect_fd_to_addr(client_fd, &addr, len, false)) > + if (connect(client_fd, (const struct sockaddr *)&addr, len)) > return -1; > > return 0; > diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h > index 5b548c0c60de..f39eeb5a4594 100644 > --- a/tools/testing/selftests/bpf/network_helpers.h > +++ b/tools/testing/selftests/bpf/network_helpers.h > @@ -23,7 +23,6 @@ typedef __u16 __sum16; > > struct network_helper_opts { > int timeout_ms; > - bool must_fail; > int proto; > /* +ve: Passed to listen() as-is. > * 0: Default when the test does not set > diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c > index addf720428f7..eda67b629c67 100644 > --- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c > +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c > @@ -9,9 +9,7 @@ > > static int run_test(int cgroup_fd, int server_fd, bool classid) > { > - struct network_helper_opts opts = { > - .must_fail = true, > - }; > + struct network_helper_opts opts = {}; > struct connect4_dropper *skel; > int fd, err = 0; > > @@ -33,7 +31,7 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) > } > > fd = connect_to_fd_opts(server_fd, &opts); > - if (fd < 0) > + if (fd < 0 && errno != EPERM) The if test is incorrect. > err = -1; > else > close(fd); Beside close(-1) which is not good, it does not treat an OK_FD as an error and that is the whole test trying to catch. pw-bot: cr
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index 15e0e0bb7553..48c27c810db7 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -277,33 +277,6 @@ int client_socket(int family, int type, return -1; } -static int connect_fd_to_addr(int fd, - const struct sockaddr_storage *addr, - socklen_t addrlen, const bool must_fail) -{ - int ret; - - errno = 0; - ret = connect(fd, (const struct sockaddr *)addr, addrlen); - if (must_fail) { - if (!ret) { - log_err("Unexpected success to connect to server"); - return -1; - } - if (errno != EPERM) { - log_err("Unexpected error from connect to server"); - return -1; - } - } else { - if (ret) { - log_err("Failed to connect to server"); - return -1; - } - } - - return 0; -} - int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen, const struct network_helper_opts *opts) { @@ -318,7 +291,7 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add return -1; } - if (connect_fd_to_addr(fd, addr, addrlen, opts->must_fail)) + if (connect(fd, (const struct sockaddr *)addr, addrlen)) goto error_close; return fd; @@ -383,7 +356,7 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms) return -1; } - if (connect_fd_to_addr(client_fd, &addr, len, false)) + if (connect(client_fd, (const struct sockaddr *)&addr, len)) return -1; return 0; diff --git a/tools/testing/selftests/bpf/network_helpers.h b/tools/testing/selftests/bpf/network_helpers.h index 5b548c0c60de..f39eeb5a4594 100644 --- a/tools/testing/selftests/bpf/network_helpers.h +++ b/tools/testing/selftests/bpf/network_helpers.h @@ -23,7 +23,6 @@ typedef __u16 __sum16; struct network_helper_opts { int timeout_ms; - bool must_fail; int proto; /* +ve: Passed to listen() as-is. * 0: Default when the test does not set diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c index addf720428f7..eda67b629c67 100644 --- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c @@ -9,9 +9,7 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) { - struct network_helper_opts opts = { - .must_fail = true, - }; + struct network_helper_opts opts = {}; struct connect4_dropper *skel; int fd, err = 0; @@ -33,7 +31,7 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) } fd = connect_to_fd_opts(server_fd, &opts); - if (fd < 0) + if (fd < 0 && errno != EPERM) err = -1; else close(fd);