Re: [PATCH bpf-next v2 05/14] selftests/bpf: Use connect_to_addr in sk_assign

[Eduard Zingerman <eddyz87@xxxxxxxxx>]

On Thu, 2024-04-11 at 09:03 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@xxxxxxxxxx>
> 
> This patch uses public helper connect_to_addr() exported in
> network_helpers.h instead of the local defined function connect_to_server()
> in prog_tests/sk_assign.c. This can avoid duplicate code.
> 
> The code that sets SO_SNDTIMEO timeout as timeo_sec (3s) can be dropped,
> since connect_to_addr() sets default timeout as 3s.

Hi Geliang,

Thank you for this cleanup patch-set!
I think there is a mistake regarding connect_to_addr(),
as it does not set any timeouts (while start_server_addr() does):

-------------------------------------------------------- >8 ---

int connect_to_addr(...)
{
	...
	fd = socket(addr->ss_family, type, 0);
	if (fd < 0) { ... }
	if (connect_fd_to_addr(fd, addr, addrlen, false))
		goto error_close;
	return fd;

error_close:
	...
}

static int connect_fd_to_addr(int fd,
			      const struct sockaddr_storage *addr,
			      socklen_t addrlen, const bool must_fail)
{
	...
	ret = connect(fd, (const struct sockaddr *)addr, addrlen);
	if (must_fail) { ... error handling ... }
    else { ... error handling ... }
	return 0;
}

--- 8< --------------------------------------------------------

Maybe add a flavor like connect_to_addr_opt() with an additional
flag or options parameter?

Thanks,
Eduard

[...]