[PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress

[PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress

[Alexis Lothoré (eBPF Foundation)]

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).

Fixes: 99126abec5e5 ("bpf: selftests: A few improvements to network_helpers.c")
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Changes in v2:
- drop unneeded initialization
- add back error message for immediate connection failure, and slightly
  reword the async connection failure error message
- Link to v1: https://patch.msgid.link/20260710-tc_tunnel_flaky-v1-1-42aab5399a49@bootlin.com
---
I manage to reproduce the issue locally by running `./test_progs -a
tc_tunnel` in a qemu machine, while making all my CPUs busy with
stress-ng on host side; the issue happens pretty quickly. I have not
been able to reproduce the issue anymore with this fix.
---
 tools/testing/selftests/bpf/network_helpers.c | 41 ++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index db935a9d9fc1..52e7b72f8a77 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -14,6 +14,7 @@
 #include <sys/types.h>
 #include <sys/un.h>
 #include <sys/eventfd.h>
+#include <sys/poll.h>
 
 #include <linux/err.h>
 #include <linux/in.h>
@@ -40,6 +41,8 @@
 #define IPPROTO_MPTCP 262
 #endif
 
+#define CONNECTION_IN_PROGRESS_TIMEOUT_MS	3000
+
 #define clean_errno() (errno == 0 ? "None" : strerror(errno))
 #define log_err(MSG, ...) ({						\
 			int __save = errno;				\
@@ -294,7 +297,8 @@ 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;
+	socklen_t errlen;
+	int fd, err;
 
 	if (!opts)
 		opts = &default_opts;
@@ -305,13 +309,42 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
 		return -1;
 	}
 
-	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
+	err = connect(fd, (const struct sockaddr *)addr, addrlen);
+	if (err && errno == EINPROGRESS) {
+		struct pollfd pfd = { .fd = fd, .events = POLLOUT };
+
+		err = poll(&pfd, 1, CONNECTION_IN_PROGRESS_TIMEOUT_MS);
+
+		if (err <= 0) {
+			if (err == 0) {
+				log_err("Connection timeout");
+				errno = ETIMEDOUT;
+			} else {
+				log_err("Failed to poll connection 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) {
+			log_err("Eventually failed to connect to server");
+			errno = err;
+			goto close;
+		}
+	} else if (err) {
 		log_err("Failed to connect to server");
-		save_errno_close(fd);
-		return -1;
+		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,

---
base-commit: 2efc18d4bb9ad28d240b69bd324937f1ec12e93d
change-id: 20260710-tc_tunnel_flaky-27e9a191bd03

Best regards,
--  
Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

Re: [PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `log_err()` prints the wrong error string because `errno` is updated after the logging call in the asynchronous connection failure paths.
--

--- Patch [1]: [PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress ---
commit 533fe7390970bae0c12d4d1a4c61075dda1b64ff
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

selftests/bpf: keep polling connection that is still in progress

This commit reduces flaky connection failures in CI by polling sockets that
initially return EINPROGRESS. It uses poll() to wait up to a few seconds
for the socket to become writable. If the connection completes, it checks
the final result via getsockopt() without requiring large timeout increases.

> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index b82f572641b7d..2f2719a28f026 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c

[ ... ]

> @@ -305,13 +309,42 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
>  		return -1;
>  	}
>  
> -	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
> +	err = connect(fd, (const struct sockaddr *)addr, addrlen);
> +	if (err && errno == EINPROGRESS) {
> +		struct pollfd pfd = { .fd = fd, .events = POLLOUT };
> +
> +		err = poll(&pfd, 1, CONNECTION_IN_PROGRESS_TIMEOUT_MS);
> +
> +		if (err <= 0) {
> +			if (err == 0) {
> +				log_err("Connection timeout");
> +				errno = ETIMEDOUT;

[Severity: Medium]
Will this log the wrong error string?

Because log_err() automatically incorporates the current errno into its
output, and errno is still set to EINPROGRESS here, this will log an
unexpected error message instead of ETIMEDOUT. Should errno be set to
ETIMEDOUT before calling log_err()?

> +			} else {
> +				log_err("Failed to poll connection 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) {
> +			log_err("Eventually failed to connect to server");
> +			errno = err;

[Severity: Medium]
Does this code also suffer from the same issue?

Since errno is still EINPROGRESS when log_err() is called, this will print the
wrong error reason. Should errno be updated to the socket error in err before
calling log_err()?

> +			goto close;
> +		}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-tc_tunnel_flaky-v2-1-657b287dfa75@bootlin.com?part=1

Re: [PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3985 bytes --]

> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index b82f572641b7..2f2719a28f02 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c

[ ... ]

> @@ -40,6 +41,8 @@
>  #define IPPROTO_MPTCP 262
>  #endif
>
> +#define CONNECTION_IN_PROGRESS_TIMEOUT_MS	3000
> +
>  #define clean_errno() (errno == 0 ? "None" : strerror(errno))
>  #define log_err(MSG, ...) ({						\
>  			int __save = errno;				\

[ ... ]

> @@ -294,7 +297,8 @@ 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;
> +	socklen_t errlen;
> +	int fd, err;
>
>  	if (!opts)
>  		opts = &default_opts;
> @@ -305,13 +309,42 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
>  		return -1;
>  	}
>
> -	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
> +	err = connect(fd, (const struct sockaddr *)addr, addrlen);
> +	if (err && errno == EINPROGRESS) {
> +		struct pollfd pfd = { .fd = fd, .events = POLLOUT };
> +
> +		err = poll(&pfd, 1, CONNECTION_IN_PROGRESS_TIMEOUT_MS);

Does the fixed 3000 ms timeout override opts->timeout_ms correctly?

client_socket() in this file passes opts->timeout_ms to settimeo(), which
converts it into SO_RCVTIMEO and SO_SNDTIMEO:

tools/testing/selftests/bpf/network_helpers.c:settimeo() {
	...
	if (timeout_ms > 0) {
		timeout.tv_sec = timeout_ms / 1000;
		timeout.tv_usec = (timeout_ms % 1000) * 1000;
	} else {
		timeout.tv_sec = 3;
		timeout.tv_usec = 0;
	}
	...
}

tools/testing/selftests/bpf/network_helpers.c:client_socket() {
	...
	if (settimeo(fd, opts->timeout_ms))
		goto error_close;
	...
}

That opts->timeout_ms is the caller's requested timeout contract. But the
new poll() discards it and uses CONNECTION_IN_PROGRESS_TIMEOUT_MS instead,
which is always 3000.

This changes observable timing for callers in both directions:

Callers that request a SHORT timeout now wait longer than they asked for.
prog_tests/assign_reuse.c calls connect_to_fd(fd_sv, 100) at lines 42 and
66, requesting a 100 ms budget. A stalled connect now blocks for 100 ms
(SO_SNDTIMEO) plus 3000 ms in poll() - a 30x overrun.
prog_tests/test_tc_tunnel.c sets .timeout_ms = 1000 at line 171.

The commit message explicitly argues against lengthening timeouts:
"increasing the timeout value would increase overall test execution
duration again", yet the patch lengthens these callers' timeouts from
their requested values.

Callers that request a LONG timeout get a shorter window than they asked
for. prog_tests/tc_redirect.c uses TIMEOUT_MILLIS (10000) at lines 428,
561, and 658. prog_tests/xdp_synproxy.c uses 10000 at line 135. On a
loaded runner the connect is now abandoned at SO_SNDTIMEO + 3 s rather
than the requested 10 s.

Would deriving the poll timeout from opts->timeout_ms preserve the single
timeout contract that callers already use?

>
> +		if (err <= 0) {
> +			if (err == 0) {
> +				log_err("Connection timeout");
> +				errno = ETIMEDOUT;
> +			} else {
> +				log_err("Failed to poll connection 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) {
> +			log_err("Eventually failed to connect to server");
> +			errno = err;
> +			goto close;
> +		}
> +	} else if (err) {
>  		log_err("Failed to connect to server");
> -		save_errno_close(fd);
> -		return -1;
> +		goto close;
>  	}
>
>  	return fd;
> +close:
> +	save_errno_close(fd);
> +	return -1;
>  }


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30795380743

Re: [PATCH bpf v2] selftests/bpf: keep polling connection that is still in progress

[Ihor Solodrai]

On 8/3/26 12:36 AM, Alexis Lothoré (eBPF Foundation) wrote:
> 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).
> 
> Fixes: 99126abec5e5 ("bpf: selftests: A few improvements to network_helpers.c")
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
> ---
> Changes in v2:
> - drop unneeded initialization
> - add back error message for immediate connection failure, and slightly
>   reword the async connection failure error message
> - Link to v1: https://patch.msgid.link/20260710-tc_tunnel_flaky-v1-1-42aab5399a49@bootlin.com
> ---
> I manage to reproduce the issue locally by running `./test_progs -a
> tc_tunnel` in a qemu machine, while making all my CPUs busy with
> stress-ng on host side; the issue happens pretty quickly. I have not
> been able to reproduce the issue anymore with this fix.
> ---
>  tools/testing/selftests/bpf/network_helpers.c | 41 ++++++++++++++++++++++++---
>  1 file changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index db935a9d9fc1..52e7b72f8a77 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -14,6 +14,7 @@
>  #include <sys/types.h>
>  #include <sys/un.h>
>  #include <sys/eventfd.h>
> +#include <sys/poll.h>
>  
>  #include <linux/err.h>
>  #include <linux/in.h>
> @@ -40,6 +41,8 @@
>  #define IPPROTO_MPTCP 262
>  #endif
>  
> +#define CONNECTION_IN_PROGRESS_TIMEOUT_MS	3000
> +
>  #define clean_errno() (errno == 0 ? "None" : strerror(errno))
>  #define log_err(MSG, ...) ({						\
>  			int __save = errno;				\
> @@ -294,7 +297,8 @@ 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;
> +	socklen_t errlen;
> +	int fd, err;
>  
>  	if (!opts)
>  		opts = &default_opts;
> @@ -305,13 +309,42 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
>  		return -1;
>  	}
>  
> -	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
> +	err = connect(fd, (const struct sockaddr *)addr, addrlen);
> +	if (err && errno == EINPROGRESS) {
> +		struct pollfd pfd = { .fd = fd, .events = POLLOUT };
> +
> +		err = poll(&pfd, 1, CONNECTION_IN_PROGRESS_TIMEOUT_MS);

Hi Alexis, thanks for the patch.

When I ran tc_* selftests in parallel it looked like they hanged.

I think what's happening is that this 3s timeout is additive, making
some tests to wait for too long. For example tc_tunnel has 1s timeout,
but with this change it actually becomes 4s. AI says a successful
tc_tunnel run makes 50+ connections, so it adds up to minutes.

We should probably be using opts->timeout_ms as an absolute budget set
by the caller, and pass it (or remainder?) to the poll().

> +
> +		if (err <= 0) {
> +			if (err == 0) {
> +				log_err("Connection timeout");
> +				errno = ETIMEDOUT;
> +			} else {
> +				log_err("Failed to poll connection status");
> +			}
> +			goto close;
> +		}

Also poll() can return EINTR here. I think we should retry EINTR,
taking into account the absolute deadline.

pw-bot: cr

> +
> +		errlen = sizeof(err);
> +		if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &err, &errlen) < 0) {
> +			log_err("Failed to getsockopt");
> +			goto close;
> +		}
> +
> +		if (err) {
> +			log_err("Eventually failed to connect to server");
> +			errno = err;
> +			goto close;
> +		}
> +	} else if (err) {
>  		log_err("Failed to connect to server");
> -		save_errno_close(fd);
> -		return -1;
> +		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,
> 
> ---
> base-commit: 2efc18d4bb9ad28d240b69bd324937f1ec12e93d
> change-id: 20260710-tc_tunnel_flaky-27e9a191bd03
> 
> Best regards,
> --  
> Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>