[bpf-next,v11,0/9] use network helpers, part 8

From: Geliang Tang <tanggeliang@kylinos.cn>

v11:
 - new patches 2, 4, 6.
 - drop expect_errno from network_helper_opts as Eduard and Martin
   suggested.
 - drop sockmap_ktls patches from this set.
 - add a new helper connect_fd_to_addr_str.

v10:
 - a new patch 10 is added.
 - patches 1-6, 8-9 unchanged, only commit logs updated.
 - "err = -errno" is used in patches 7, 11, 12 to get the real error
   number before checking value of "err".

v9:
 - new patches 5-7, new struct member expect_errno for network_helper_opts.
 - patches 1-4, 8-9 unchanged.
 - update patches 10-11 to make sure all tests pass.

v8:
 - only patch 8 updated, to fix errors reported by CI.

v7:
 - address Martin's comments in v6. (thanks)
 - use MAX(opts->backlog, 0) instead of opts->backlog.
 - use connect_to_fd_opts instead connect_to_fd.
 - more ASSERT_* to check errors.

v6:
 - update patch 6 as Daniel suggested. (thanks)

v5:
 - keep make_server and make_client as Eduard suggested.

v4:
 - a new patch to use make_sockaddr in sockmap_ktls.
 - a new patch to close fd in error path in drop_on_reuseport.
 - drop make_server() in patch 7.
 - drop make_client() too in patch 9.

v3:
 - a new patch to add backlog for network_helper_opts.
 - use start_server_str in sockmap_ktls now, not start_server.

v2:
 - address Eduard's comments in v1. (thanks)
 - fix errors reported by CI.

This patch set uses network helpers in sk_lookup, and drop the local
helpers inetaddr_len() and make_socket().

Geliang Tang (9):
  selftests/bpf: Add backlog for network_helper_opts
  selftests/bpf: Add ASSERT_OK_FD macro
  selftests/bpf: Close fd in error path in drop_on_reuseport
  selftests/bpf: Use start_server_str in sk_lookup
  selftests/bpf: Use start_server_addr in sk_lookup
  selftests/bpf: Use connect_fd_to_fd in sk_lookup
  selftests/bpf: Add connect_fd_to_addr_str helper
  selftests/bpf: Use connect_fd_to_addr_str in sk_lookup
  selftests/bpf: Drop make_socket in sk_lookup

 tools/testing/selftests/bpf/network_helpers.c |  23 ++-
 tools/testing/selftests/bpf/network_helpers.h |   7 +
 .../selftests/bpf/prog_tests/sk_lookup.c      | 156 ++++++------------
 tools/testing/selftests/bpf/test_progs.h      |   8 +
 4 files changed, 92 insertions(+), 102 deletions(-)

On 7/9/24 2:16 AM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In the error path when update_lookup_map() fails in drop_on_reuseport in
> prog_tests/sk_lookup.c, "server1", the fd of server 1, should be closed.
> This patch fixes this by using "goto close_srv1" lable instead of "detach"
> to close "server1" in this case.
> 

A reference to the Fixes tag will be useful

Fixes: 0ab5539f8584 ("selftests/bpf: Tests for BPF_SK_LOOKUP attach point")

On 7/9/24 2:16 AM, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Similar to connect_fd_to_fd() helper to connect from a client fd to a
> server fd, this patch adds a new helper connect_fd_to_addr_str() to connect
> from a client fd to a server address. It accepts the server address string
> "addr_str", together with the server family, type and port, as parameters
> instead of using a "server_fd" like connect_fd_to_fd().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>   tools/testing/selftests/bpf/network_helpers.c | 21 +++++++++++++++++++
>   tools/testing/selftests/bpf/network_helpers.h |  3 +++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index e0cba4178e41..9758e707b859 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -388,6 +388,27 @@ int connect_fd_to_fd(int client_fd, int server_fd, int timeout_ms)
>   	return 0;
>   }
>   
> +int connect_fd_to_addr_str(int client_fd, int family, int type,

Similar to the comment in the earlier revision on the existing 
connect_to_fd_opts. The "int type" is redundant of "int client_fd".

and where is the "int type" arg actually used in this new function?

Beside, is it more useful for patch 8 to add connect_to_addr_str() which calls 
socket()/client_socket(), connect(), and then return the client_fd instead?

Something like this?

int connect_to_addr_str(int family, int type, const char *addr_str, __u16 port,
			const struct network_helper_opts *opts)

Patch 1-6 is applied with the mentioned minor changes. Thanks.


> +			   const char *addr_str, __u16 port,
> +			   const struct network_helper_opts *opts)
> +{
> +	struct sockaddr_storage addr;
> +	socklen_t len;
> +
> +	if (!opts)
> +		opts = &default_opts;
> +
> +	if (settimeo(client_fd, opts->timeout_ms))
> +		return -1;
> +
> +	if (make_sockaddr(family, addr_str, port, &addr, &len)) {
> +		log_err("Failed to make server addr");
> +		return -1;
> +	}
> +
> +	return connect_fd_to_addr(client_fd, &addr, len, false);
> +}
> +
>   int make_sockaddr(int family, const char *addr_str, __u16 port,
>   		  struct sockaddr_storage *addr, socklen_t *len)
>   {