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

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

[ ... ]

> @@ -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;
> +	}

Does the EINTR branch need to continue the loop? When poll() returns
EINTR, the code recomputes remaining_ms but then falls through to
getsockopt(SO_ERROR). If the connection is still in progress, SO_ERROR
will be 0 and connect_to_addr() returns the fd even though the socket
is not yet connected.

Also, no path inside the while loop reaches the loop condition again -
every branch either does 'goto close' or 'return fd'. So after the EINTR
budget recalculation runs, the code never re-enters poll() to use that
budget.

Would adding 'continue;' at the end of the EINTR branch fix this? That
way poll() gets called again with the updated remaining_ms, and the
connection completes properly after a signal interruption.

Test_progs installs a SIGINT handler without SA_RESTART in
test_progs.c, so EINTR is reachable here.

> +
> +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/31503700726