BPF List
 help / color / mirror / Atom feed
* [PATCH bpf v4] selftests/bpf: allocate a larger timeout for connection
@ 2026-08-13  9:38 Alexis Lothoré (eBPF Foundation)
  2026-08-13 10:33 ` bot+bpf-ci
  0 siblings, 1 reply; 3+ messages in thread
From: Alexis Lothoré (eBPF Foundation) @ 2026-08-13  9:38 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, Shuah Khan
  Cc: ebpf, Bastien Curutchet, Thomas Petazzoni, bpf, linux-kselftest,
	linux-kernel, Alexis Lothoré (eBPF Foundation),
	Ihor Solodrai

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.

Set a larger SO_SNDTIMEO/SO_RCVTIMEO for the connection step, and reset
it back to the timeout configured by the test once the connection has
succeeded.

Fixes: 99126abec5e5 ("bpf: selftests: A few improvements to network_helpers.c")
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
Hello,
this is the v4 of the series aiming to reduce the flakyness of
tc_tunnel/tc_edt tests in CI. This revision takes a step back, based on
Ihor's tests,  and drops the poll loop in favor of a bare, larger
timeout value applied only for the connection step. The main downside
of this new mechanism is a slight increase of the duration for tests
expecting a connection failure. In my testing setup (x86-based Qemu on
my work laptop), I observed a ~10s increase (on a ~5m18 base for the
whole test_progs set).
---
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 | 34 ++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index db935a9d9fc1..89336201572f 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,12 @@ int client_socket(int family, int type,
 	return -1;
 }
 
+static int connect_timeout_ms(const struct network_helper_opts *opts)
+{
+	/* Enforce a minimum connect timeout value */
+	return MAX(opts->timeout_ms, CONNECT_MIN_TIMEOUT_MS);
+}
+
 int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t addrlen,
 		    const struct network_helper_opts *opts)
 {
@@ -305,13 +313,33 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
 		return -1;
 	}
 
+	/* Override timeout configuration with a larger value for the
+	 * connection
+	 */
+	if (settimeo(fd, connect_timeout_ms(opts))) {
+		log_err("Failed to set connect timeout");
+		goto close;
+	}
+
 	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
-		log_err("Failed to connect to server");
-		save_errno_close(fd);
-		return -1;
+		log_err("Failed to connect");
+		goto close;
+	}
+
+	/* If the timeout configured by the test is different from the
+	 * connect timeout, restore it
+	 */
+	if (opts->timeout_ms != CONNECT_MIN_TIMEOUT_MS &&
+	    settimeo(fd, opts->timeout_ms)) {
+		log_err("Failed to set timeout for connected socket");
+		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: 673b1f0272d7dedeec78dee2b47040481e4332ba
change-id: 20260710-tc_tunnel_flaky-27e9a191bd03

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf v4] selftests/bpf: allocate a larger timeout for connection
  2026-08-13  9:38 [PATCH bpf v4] selftests/bpf: allocate a larger timeout for connection Alexis Lothoré (eBPF Foundation)
@ 2026-08-13 10:33 ` bot+bpf-ci
  2026-08-13 11:23   ` Alexis Lothoré
  0 siblings, 1 reply; 3+ messages in thread
From: bot+bpf-ci @ 2026-08-13 10:33 UTC (permalink / raw)
  To: alexis.lothore, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, shuah
  Cc: ebpf, bastien.curutchet, thomas.petazzoni, bpf, linux-kselftest,
	linux-kernel, alexis.lothore, ihor.solodrai, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

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

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

[ ... ]

> @@ -305,13 +313,33 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
>  		return -1;
>  	}
>
> +	/* Override timeout configuration with a larger value for the
> +	 * connection
> +	 */
> +	if (settimeo(fd, connect_timeout_ms(opts))) {

A subsystem pattern flags this as potentially concerning:

Both multi-line comments added by this change start their text on the
same line as the opening '/*'. The loaded BPF subsystem guide requires
the opening '/*' to sit on its own line for files under
tools/testing/selftests/bpf/.

The guide specifies: "Multi-line comments MUST have the opening /* on
its own line, with the comment text starting on the next line."

However, this file contains 8 pre-existing multi-line comments (at
lines 455, 896, 1064, 1070, 1076, 1110, 1181, 1286) and every one puts
text on the opening '/*' line; there are zero instances of the form the
guide prescribes. Across tools/testing/selftests/bpf/ the ratio is
roughly 1986 to 689 in favour of the style used here.

Should these comments match the guide's requirement, or is the local
convention the right choice for this file?

> +		log_err("Failed to set connect timeout");
> +		goto close;
> +	}
> +
>  	if (connect(fd, (const struct sockaddr *)addr, addrlen)) {
> -		log_err("Failed to connect to server");
> -		save_errno_close(fd);
> -		return -1;
> +		log_err("Failed to connect");
> +		goto close;
> +	}
> +
> +	/* If the timeout configured by the test is different from the
> +	 * connect timeout, restore it
> +	 */
> +	if (opts->timeout_ms != CONNECT_MIN_TIMEOUT_MS &&
> +	    settimeo(fd, opts->timeout_ms)) {
> +		log_err("Failed to set timeout for connected socket");
> +		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/31688637156

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf v4] selftests/bpf: allocate a larger timeout for connection
  2026-08-13 10:33 ` bot+bpf-ci
@ 2026-08-13 11:23   ` Alexis Lothoré
  0 siblings, 0 replies; 3+ messages in thread
From: Alexis Lothoré @ 2026-08-13 11:23 UTC (permalink / raw)
  To: bot+bpf-ci, alexis.lothore, ast, daniel, andrii, eddyz87, memxor,
	martin.lau, song, yonghong.song, jolsa, emil, shuah
  Cc: ebpf, bastien.curutchet, thomas.petazzoni, bpf, linux-kselftest,
	linux-kernel, ihor.solodrai, martin.lau, clm

On Thu Aug 13, 2026 at 12:33 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..dd33bcdf75d7 100644
>> --- a/tools/testing/selftests/bpf/network_helpers.c
>> +++ b/tools/testing/selftests/bpf/network_helpers.c
>
> [ ... ]
>
>> @@ -305,13 +313,33 @@ int connect_to_addr(int type, const struct sockaddr_storage *addr, socklen_t add
>>  		return -1;
>>  	}
>>
>> +	/* Override timeout configuration with a larger value for the
>> +	 * connection
>> +	 */
>> +	if (settimeo(fd, connect_timeout_ms(opts))) {
>
> A subsystem pattern flags this as potentially concerning:
>
> Both multi-line comments added by this change start their text on the
> same line as the opening '/*'. The loaded BPF subsystem guide requires
> the opening '/*' to sit on its own line for files under
> tools/testing/selftests/bpf/.
>
> The guide specifies: "Multi-line comments MUST have the opening /* on
> its own line, with the comment text starting on the next line."
>
> However, this file contains 8 pre-existing multi-line comments (at
> lines 455, 896, 1064, 1070, 1076, 1110, 1181, 1286) and every one puts
> text on the opening '/*' line; there are zero instances of the form the
> guide prescribes. Across tools/testing/selftests/bpf/ the ratio is
> roughly 1986 to 689 in favour of the style used here.
>
> Should these comments match the guide's requirement, or is the local
> convention the right choice for this file?

If the patch needs another revision, I'll use the opportunity to update
all the comments in the file before fixing this one.

Alexis

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-13 11:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  9:38 [PATCH bpf v4] selftests/bpf: allocate a larger timeout for connection Alexis Lothoré (eBPF Foundation)
2026-08-13 10:33 ` bot+bpf-ci
2026-08-13 11:23   ` Alexis Lothoré

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox