Some tests, like tc_tunnel or tc_edt, sporadically fail in CI with the following logs: (network_helpers.c:309: errno: Operation now in progress) \ Failed to connect to server send_and_test_data:FAIL:connect to server unexpected error: -115 This is due to SO_RCVTIMEO and SO_SNDTIMEO being set on the client socket (see settimeo() in client_socket()), allowing connect() to return an error and to set errno to EINPROGRESS instead of blocking until connection result is known. Increasing the timeout value for those tests is likely not a good solution (and it has already been done by commit 2790db208b44 ("selftests/bpf: Improve tc_tunnel test reliability")): they involve subtests that expect the connection to fail, and so increasing the timeout value would increase overall test execution duration again (not only the connection, but any socket operation). Another solution, as documented in man 2 connect, is to poll the socket for POLLOUT once connect has returned EINPROGRESS, and to get the actual connection result through getsockopt: this allows to keep the overall timeout values low for the general traffic, while letting a chance to the connection to succeed even if CI runners are loaded. When connect() returns EINPROGRESS, poll the socket for POLLOUT and check the connection result via getsockopt(SO_ERROR). This new handling conforms to the configured timeout: the polling loop will only run for the amount of time still available, accounting for the time used by the initial connect() call. Fixes: 99126abec5e5 ("bpf: selftests: A few improvements to network_helpers.c") Signed-off-by: Alexis Lothoré (eBPF Foundation) --- tools/testing/selftests/bpf/network_helpers.c | 70 +++++++++++++++++++++++++-- tools/testing/selftests/bpf/testing_helpers.h | 9 ++++ 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index db935a9d9fc1..e3157724adec 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -294,7 +295,10 @@ int client_socket(int family, int type, int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen, const struct network_helper_opts *opts) { - int fd; + __u64 start_ms, duration_ms; + __u64 remaining_ms; + socklen_t errlen; + int fd, err, ret; if (!opts) opts = &default_opts; @@ -305,13 +309,69 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add return -1; } - if (connect(fd, (const struct sockaddr *)addr, addrlen)) { + start_ms = get_time_ms(); + err = connect(fd, (const struct sockaddr *)addr, addrlen); + + if (!err) + return fd; + + if (errno != EINPROGRESS) { log_err("Failed to connect to server"); - save_errno_close(fd); - return -1; + goto close; } - return fd; + duration_ms = get_time_ms() - start_ms; + remaining_ms = duration_ms < opts->timeout_ms ? + opts->timeout_ms - duration_ms : + 0; + if (!remaining_ms) { + errno = ETIMEDOUT; + log_err("Can not poll connection, already in timeout"); + goto close; + } + + while (remaining_ms) { + struct pollfd pfd = { .fd = fd, .events = POLLOUT }; + + start_ms = get_time_ms(); + ret = poll(&pfd, 1, remaining_ms); + + if (ret == 0) { + errno = ETIMEDOUT; + log_err("Connection timeout while polling"); + goto close; + } else if (ret < 0 && errno == EINTR) { + duration_ms = get_time_ms() - start_ms; + remaining_ms = duration_ms < remaining_ms ? + remaining_ms - duration_ms : + 0; + if (!remaining_ms) { + errno = ETIMEDOUT; + log_err("Connection timeout after signal"); + goto close; + } + } else if (ret < 0) { + log_err("Failed to poll connect status"); + goto close; + } + + errlen = sizeof(err); + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &errlen) < 0) { + log_err("Failed to getsockopt"); + goto close; + } + + if (err) { + errno = err; + log_err("Eventually failed to connect to server"); + goto close; + } + return fd; + } + +close: + save_errno_close(fd); + return -1; } int connect_to_addr_str(int family, int type, const char *addr_str, __u16 port, diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h index 2edc6fb7fc52..7440cfcd1025 100644 --- a/tools/testing/selftests/bpf/testing_helpers.h +++ b/tools/testing/selftests/bpf/testing_helpers.h @@ -52,6 +52,15 @@ static inline __u64 get_time_ns(void) return (u64)t.tv_sec * 1000000000 + t.tv_nsec; } +static inline __u64 get_time_ms(void) +{ + struct timespec t; + + clock_gettime(CLOCK_MONOTONIC, &t); + + return (u64)t.tv_sec * 1000 + t.tv_nsec / 1000000; +} + struct bpf_insn; /* Request BPF program instructions after all rewrites are applied, * e.g. verifier.c:convert_ctx_access() is done. -- 2.55.0