[PATCH bpf v6] selftests/bpf: allocate a larger timeout for connection

[PATCH bpf v6] selftests/bpf: allocate a larger timeout for connection

[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 ETIMEDOUT.
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")): some tests
expect some data transfer to fail, and so the timeout value would
increase overall test execution duration again (not only the connection,
but any socket operation).

Another solution is to allocate a timeout budget specific to the
connection: we can apply a larger timeout only for connections, and once
the connection is established, set back the timeout configured through
opts->timeout_ms; this would allow connection to succeed under heavy CI
load, while keeping timeout reasonable for the rest of the test traffic.

Create a dedicated connect helper that sets a large timeout for the
connect step, tries to connect, and if successful, finally sets the
timeout value to the one configured by the test.

Fixes: 99126abec5e5 ("bpf: selftests: A few improvements to network_helpers.c")
Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Hello,
this is the v5 of the series aiming to reduce the flakyness of
tc_tunnel/tc_edt tests in CI. This revision refactor a bit the new
connect timeout logic, and makes sure to apply the new logic to the
other connect() call in connect_fd_to_fd, as suggested by Daniel.
---
Changes in v6:
- Factor settimeo/connect/settimeo in a dedicated helper
- Apply the same strategy to connect_fd_to_fd
- drop comment-style updated patch, applied separately by Daniel
- Link to v5: https://patch.msgid.link/20260814-tc_tunnel_flaky-v5-0-5b93d030c42c@bootlin.com

Changes in v5:
- reformat multi-line comments
- collect Ihor's Acked-by
- Link to v4: https://patch.msgid.link/20260813-tc_tunnel_flaky-v4-1-3534df3fe930@bootlin.com

Changes in v4:
- dropped the polling loop in favor of a larger, connect-specific timeout
- drop timeout configuration from tc_edt test
- Link to v3: https://patch.msgid.link/20260811-tc_tunnel_flaky-v3-0-876f4e0bc603@bootlin.com

Changes in v3:
- set errno before logging errors
- respect time budget set by
- respect opts->timeout_ms when polling: only poll for the remaining
  time not already consume by connect()
- keep polling if poll returns with EINTR
- reorder early returns and add intermediate variables to clarify code
  flow
- Link to v2: https://patch.msgid.link/20260803-tc_tunnel_flaky-v2-1-657b287dfa75@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

To: Alexei Starovoitov <ast@kernel.org>
To: Daniel Borkmann <daniel@iogearbox.net>
To: Andrii Nakryiko <andrii@kernel.org>
To: Eduard Zingerman <eddyz87@gmail.com>
To: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: Martin KaFai Lau <martin.lau@linux.dev>
To: Song Liu <song@kernel.org>
To: Yonghong Song <yonghong.song@linux.dev>
To: Jiri Olsa <jolsa@kernel.org>
To: Emil Tsalapatis <emil@etsalapatis.com>
To: Ihor Solodrai <ihor.solodrai@linux.dev>
To: Shuah Khan <shuah@kernel.org>
Cc: ebpf@linuxfoundation.org
Cc: Bastien Curutchet <bastien.curutchet@bootlin.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: bpf@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 tools/testing/selftests/bpf/network_helpers.c | 48 +++++++++++++++++++++------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index cdf2d7d3ab32..9f6abc4dd273 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -49,6 +49,8 @@
 			errno = __save;					\
 })
 
+#define CONNECT_MIN_TIMEOUT_MS	5000
+
 struct ipv4_packet pkt_v4 = {
 	.eth.h_proto = __bpf_constant_htons(ETH_P_IP),
 	.iph.ihl = 5,
@@ -291,6 +293,37 @@ int client_socket(int family, int type,
 	return -1;
 }
 
+static int connect_with_timeout(int fd, const struct sockaddr_storage *addr,
+				socklen_t addrlen, int timeout_ms)
+{
+	int connect_timeout_ms = MAX(timeout_ms, CONNECT_MIN_TIMEOUT_MS);
+
+	/*
+	 * Override timeout configuration with a larger value for the
+	 * connection
+	 */
+	if (settimeo(fd, connect_timeout_ms)) {
+		log_err("Failed to set connect timeout");
+		return -1;
+	}
+
+	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
+		log_err("Failed to connect");
+		return -1;
+	}
+
+	/*
+	 * If the timeout configured by the test is different from the
+	 * connect timeout, restore it
+	 */
+	if (timeout_ms != connect_timeout_ms && settimeo(fd, timeout_ms)) {
+		log_err("Failed to set timeout for connected socket");
+		return -1;
+	}
+
+	return 0;
+}
+
 int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen,
 		    const struct network_helper_opts *opts)
 {
@@ -305,8 +338,7 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
 		return -1;
 	}
 
-	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
-		log_err("Failed to connect to server");
+	if (connect_with_timeout(fd, addr, addrlen, opts->timeout_ms)) {
 		save_errno_close(fd);
 		return -1;
 	}
@@ -376,20 +408,14 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
 	struct sockaddr_storage addr;
 	socklen_t len = sizeof(addr);
 
-	if (settimeo(client_fd, timeout_ms))
-		return -1;
-
 	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
 		log_err("Failed to get server addr");
 		return -1;
 	}
 
-	if (connect(client_fd, (const struct sockaddr *)&addr, len)) {
-		log_err("Failed to connect to server");
-		return -1;
-	}
-
-	return 0;
+	return connect_with_timeout(client_fd,
+				    &addr, len,
+				    timeout_ms);
 }
 
 int make_sockaddr(int family, const char *addr_str, __u16 port,

---
base-commit: 23ff631b3b8b1452dfe933ee21f96321a9c5e209
change-id: 20260710-tc_tunnel_flaky-27e9a191bd03

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

Re: [PATCH bpf v6] selftests/bpf: allocate a larger timeout for connection

[bot+bpf-ci]

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

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

[ ... ]

> @@ -291,6 +293,37 @@ int client_socket(int family, int type,
>  	return -1;
>  }
>
> +static int connect_with_timeout(int fd, const struct sockaddr_storage *addr,
> +				socklen_t addrlen, int timeout_ms)
> +{
> +	int connect_timeout_ms = MAX(timeout_ms, CONNECT_MIN_TIMEOUT_MS);
> +
> +	/*
> +	 * Override timeout configuration with a larger value for the
> +	 * connection
> +	 */
> +	if (settimeo(fd, connect_timeout_ms)) {
> +		log_err("Failed to set connect timeout");
> +		return -1;
> +	}
> +
> +	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
> +		log_err("Failed to connect");
> +		return -1;
> +	}
> +
> +	/*
> +	 * If the timeout configured by the test is different from the
> +	 * connect timeout, restore it
> +	 */

This isn't a bug, but do the two block comments here add anything over
the code they sit above? The first sits above settimeo(fd,
connect_timeout_ms) where connect_timeout_ms was computed with MAX(...,
CONNECT_MIN_TIMEOUT_MS) on the line before. The second transliterates
the condition on the line below (timeout_ms != connect_timeout_ms means
"restore it"). Could they be trimmed to a single line about why a larger
connect budget is needed?

> +	if (timeout_ms != connect_timeout_ms && settimeo(fd, timeout_ms)) {
> +		log_err("Failed to set timeout for connected socket");
> +		return -1;
> +	}
> +
> +	return 0;
> +}

[ ... ]

> @@ -376,20 +408,14 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
>  	struct sockaddr_storage addr;
>  	socklen_t len = sizeof(addr);
>
> -	if (settimeo(client_fd, timeout_ms))
> -		return -1;
> -
>  	if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
>  		log_err("Failed to get server addr");
>  		return -1;
>  	}
>
> -	if (connect(client_fd, (const struct sockaddr *)&addr, len)) {
> -		log_err("Failed to connect to server");
> -		return -1;
> -	}
> -
> -	return 0;
> +	return connect_with_timeout(client_fd,
> +				    &addr, len,
> +				    timeout_ms);
                            ^^^^

This isn't a bug, but could this call fit on one line? With the leading
tab it comes to about 71 columns as a single statement, well inside the
80-column limit. Elsewhere the file only wraps arguments when the line
doesn't fit.

>  }


---
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/32062499118

Re: [PATCH bpf v6] selftests/bpf: allocate a larger timeout for connection

[Alexis Lothoré]

On Mon Aug 17, 2026 at 10:14 PM CEST, bot+bpf-ci wrote:
>> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
>> index b82f572641b7..1fda1fa7281c 100644
>> --- a/tools/testing/selftests/bpf/network_helpers.c
>> +++ b/tools/testing/selftests/bpf/network_helpers.c

[...]

>> +static int connect_with_timeout(int fd, const struct sockaddr_storage *addr,
>> +				socklen_t addrlen, int timeout_ms)
>> +{
>> +	int connect_timeout_ms = MAX(timeout_ms, CONNECT_MIN_TIMEOUT_MS);
>> +
>> +	/*
>> +	 * Override timeout configuration with a larger value for the
>> +	 * connection
>> +	 */
>> +	if (settimeo(fd, connect_timeout_ms)) {
>> +		log_err("Failed to set connect timeout");
>> +		return -1;
>> +	}
>> +
>> +	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
>> +		log_err("Failed to connect");
>> +		return -1;
>> +	}
>> +
>> +	/*
>> +	 * If the timeout configured by the test is different from the
>> +	 * connect timeout, restore it
>> +	 */
>
> This isn't a bug, but do the two block comments here add anything over
> the code they sit above? The first sits above settimeo(fd,
> connect_timeout_ms) where connect_timeout_ms was computed with MAX(...,
> CONNECT_MIN_TIMEOUT_MS) on the line before. The second transliterates
> the condition on the line below (timeout_ms != connect_timeout_ms means
> "restore it"). Could they be trimmed to a single line about why a larger
> connect budget is needed?

This is slowly turning into bikeshedding (and if the comment wasn't
there, another LLM run could possibly request it to clarify the
intent...) so I'll keep it as-is.

[...]

>> -	if (connect(client_fd, (const struct sockaddr *)&addr, len)) {
>> -		log_err("Failed to connect to server");
>> -		return -1;
>> -	}
>> -
>> -	return 0;
>> +	return connect_with_timeout(client_fd,
>> +				    &addr, len,
>> +				    timeout_ms);
>                             ^^^^
>
> This isn't a bug, but could this call fit on one line? With the leading
> tab it comes to about 71 columns as a single statement, well inside the
> 80-column limit. Elsewhere the file only wraps arguments when the line
> doesn't fit.

True. Will be fixed.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com