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

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

On Tue Aug 11, 2026 at 5:30 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..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.

Argh, I accidentally got rid of the continue that was in the EINTR
branch while reorganizing a bit all the branches. And it then defeat the
whole purpose of the EINTR branch... Will be updated.
>
> 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




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