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