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

[Eduard Zingerman]

On Thu, 2024-04-11 at 09:03 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeli...@kylinos.cn>
> 
> 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

[...]