> diff --git a/tools/testing/selftests/bpf/network_helpers.c
> b/tools/testing/selftests/bpf/network_helpers.c
> index b82f572641b7..380b776f182d 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
[ ... ]
> @@ -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;
> + }
Does the 5-second floor apply to callers that deliberately set a small
timeout because they expect the connect to fail?
Looking at cgroup_skb_sk_lookup.c, the test expects a connection to time
out after 1 second:
/* Client outside of test cgroup should fail to connect by timeout. */
err = connect_fd_to_fd(out_sk, serv_sk, 1000);
if (CHECK(!err || errno != EINPROGRESS, "connect_fd_to_fd", ...))
The call chain is:
test_cgroup_skb_sk_lookup()
-> run_cgroup_bpf_test()
-> run_lookup_test()
-> connect_fd_to_fd(out_sk, serv_sk, 1000)
-> connect_with_timeout(..., 1000)
With MAX(1000, 5000) returning 5000, settimeo(fd, 5000) is installed,
and the deliberately-dropped connect() now blocks for 5 seconds instead
of 1 second.
The test still passes because the cgroup_skb program drops the SYNs
permanently, so at 5 seconds the socket is still in SYN_SENT and
connect() returns EINPROGRESS. But the test now waits 5 times longer than
intended.
The commit message mentions that some tests expect data transfer to fail
and that increasing timeout would increase execution duration, which is
what happens here for connects expected to time out.
Would it make sense to honour the caller's timeout_ms when explicitly set,
and only apply the 5-second floor when timeout_ms is 0 or use a default?
[ ... ]
---
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/32172718494
