| Message ID | 5730eabdd6e90d1534d4f5df47e55a76f04197ca.1720767414.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | use network helpers, part 9 | expand |
On 7/12/24 12:04 AM, 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 | 10 ++---- > 3 files changed, 5 insertions(+), 37 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..04e82a486102 100644 > --- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c > +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c > @@ -9,9 +9,6 @@ > > static int run_test(int cgroup_fd, int server_fd, bool classid) > { > - struct network_helper_opts opts = { > - .must_fail = true, > - }; > struct connect4_dropper *skel; > int fd, err = 0; > > @@ -32,11 +29,10 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) > goto out; > } > > - fd = connect_to_fd_opts(server_fd, &opts); > - if (fd < 0) > - err = -1; > - else > + fd = connect_to_fd_opts(server_fd, NULL); > + if (fd >= 0) > close(fd); > + err = errno != EPERM ? -1 : 0; Still not right. Can errno be used when fd >= 0? That makes the fd >= 0 case flaky. fd >= 0 should always be treated as error here (i.e. err = -1). just always set err = -1 in the fd >= 0 case. No need to be smart here to save a few lines. Please take time to review the patches before posting to the list. 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..04e82a486102 100644 --- a/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_v1v2.c @@ -9,9 +9,6 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) { - struct network_helper_opts opts = { - .must_fail = true, - }; struct connect4_dropper *skel; int fd, err = 0; @@ -32,11 +29,10 @@ static int run_test(int cgroup_fd, int server_fd, bool classid) goto out; } - fd = connect_to_fd_opts(server_fd, &opts); - if (fd < 0) - err = -1; - else + fd = connect_to_fd_opts(server_fd, NULL); + if (fd >= 0) close(fd); + err = errno != EPERM ? -1 : 0; out: connect4_dropper__destroy(skel); return err;