Re: [PATCH bpf-next 2/2] selftests/bpf: Test that SO_REUSEPORT can be used with sk_assign helper

[Martin KaFai Lau <martin.lau@linux.dev>]

On 5/25/23 1:19 AM, Lorenz Bauer wrote:
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index a105c0cd008a..8a33bcea97de 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -423,6 +423,9 @@ struct nstoken *open_netns(const char *name)
>   
>   void close_netns(struct nstoken *token)
>   {
> +	if (!token)
> +		return;

+1. :)

> +
>   	ASSERT_OK(setns(token->orig_netns_fd, CLONE_NEWNET), "setns");
>   	close(token->orig_netns_fd);
>   	free(token);
> diff --git a/tools/testing/selftests/bpf/prog_tests/assign_reuse.c b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c
> new file mode 100644
> index 000000000000..2cb9bb591e71
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/assign_reuse.c
> @@ -0,0 +1,280 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Isovalent */
> +#include <uapi/linux/if_link.h>
> +#include <test_progs.h>
> +
> +#include <netinet/tcp.h>
> +#include <netinet/udp.h>
> +
> +#include "network_helpers.h"
> +#include "test_assign_reuse.skel.h"
> +
> +#define NS_TEST "assign_reuse"
> +#define LOOPBACK 1
> +#define PORT 4443
> +
> +static int attach_reuseport(int sock_fd, int prog_fd)
> +{
> +	return setsockopt(sock_fd, SOL_SOCKET, SO_ATTACH_REUSEPORT_EBPF,
> +			  &prog_fd, sizeof(prog_fd));
> +}
> +
> +static socklen_t inetaddr_len(const struct sockaddr_storage *addr)
> +{
> +	return addr->ss_family == AF_INET  ? sizeof(struct sockaddr_in) :
> +	       addr->ss_family == AF_INET6 ? sizeof(struct sockaddr_in6) :
> +					     0;
> +}
> +
> +static bool is_ipv6(const char *ip)
> +{
> +	return !!strchr(ip, ':');
> +}
> +
> +static int make_socket(int sotype, const char *ip, int port,
> +		       struct sockaddr_storage *addr)
> +{
> +	struct timeval timeo = { .tv_usec = 500000 /* 500 ms */ };
> +	int family = is_ipv6(ip) ? AF_INET6 : AF_INET;
> +	int ret, fd;
> +
> +	ret = make_sockaddr(family, ip, port, addr, NULL);
> +	if (!ASSERT_OK(ret, "make_sockaddr"))
> +		return -1;
> +	fd = socket(addr->ss_family, sotype, 0);
> +	if (!ASSERT_GE(fd, 0, "socket"))
> +		return -1;
> +	ret = setsockopt(fd, SOL_SOCKET, SO_SNDTIMEO, &timeo, sizeof(timeo));
> +	if (!ASSERT_OK(ret, "sndtimeo")) {
> +		close(fd);
> +		return -1;
> +	}
> +	ret = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
> +	if (!ASSERT_OK(ret, "rcvtimeo")) {
> +		close(fd);
> +		return -1;
> +	}
> +	return fd;
> +}
> +
> +static int create_server(int sotype, const char *ip, int port, int prog_fd)
> +{
> +	struct sockaddr_storage addr = {};
> +	const int one = 1;
> +	int ret, fd;
> +
> +	fd = make_socket(sotype, ip, port, &addr);
> +	if (fd < 0)
> +		return -1;
> +	if (sotype == SOCK_STREAM) {
> +		ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one,
> +				 sizeof(one));
> +		if (!ASSERT_OK(ret, "reuseaddr"))
> +			goto cleanup;
> +	}
> +	if (prog_fd >= 0) {
> +		ret = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &one,
> +				 sizeof(one));
> +		if (!ASSERT_OK(ret, "reuseport"))
> +			goto cleanup;
> +		ret = attach_reuseport(fd, prog_fd);
> +		if (!ASSERT_OK(ret, "attach_reuseport"))
> +			goto cleanup;
> +	}
> +	ret = bind(fd, (void *)&addr, inetaddr_len(&addr));
> +	if (!ASSERT_OK(ret, "bind"))
> +		goto cleanup;
> +	if (sotype == SOCK_STREAM) {
> +		ret = listen(fd, SOMAXCONN);
> +		if (!ASSERT_OK(ret, "listen"))
> +			goto cleanup;
> +	}
> +	return fd;
> +cleanup:
> +	close(fd);
> +	return -1;
> +}
> +
> +static int create_client(int sotype, const char *ip, int port)
> +{
> +	struct sockaddr_storage addr = {};
> +	int ret, fd;
> +
> +	fd = make_socket(sotype, ip, port, &addr);
> +	if (fd < 0)
> +		return -1;
> +	ret = connect(fd, (void *)&addr, inetaddr_len(&addr));
> +	if (ret)
> +		goto cleanup;
> +	return fd;
> +cleanup:
> +	close(fd);
> +	return -1;
> +}

I believe a lot of the above can be done by start_reuseport_server() and 
connect_to_fd() from network_helpers.c.

[ ... ]

> +void serial_test_assign_reuse(void)

Remove "serial_" .


> +{
> +	struct nstoken *tok = NULL;
> +
> +	SYS(out, "ip netns add %s", NS_TEST);
> +	SYS(cleanup, "ip -net %s link set dev lo up", NS_TEST);
> +
> +	tok = open_netns(NS_TEST);
> +	if (!ASSERT_OK_PTR(tok, "netns token"))
> +		return;
> +
> +	if (test__start_subtest("tcpv4"))
> +		run_assign_reuse(SOCK_STREAM, "127.0.0.1", PORT);
> +	if (test__start_subtest("tcpv6"))
> +		run_assign_reuse(SOCK_STREAM, "::1", PORT);
> +	if (test__start_subtest("udpv4"))
> +		run_assign_reuse(SOCK_DGRAM, "127.0.0.1", PORT);
> +	if (test__start_subtest("udpv6"))
> +		run_assign_reuse(SOCK_DGRAM, "::1", PORT);
> +
> +cleanup:
> +	close_netns(tok);
> +	SYS_NOFAIL("ip netns delete %s", NS_TEST);
> +out:
> +	return;
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_assign_reuse.c b/tools/testing/selftests/bpf/progs/test_assign_reuse.c
> new file mode 100644
> index 000000000000..c83e870b2612
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_assign_reuse.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Isovalent */
> +#include <stdbool.h>
> +#include <linux/bpf.h>
> +#include <linux/if_ether.h>
> +#include <linux/in.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +#include <linux/udp.h>
> +#include <bpf/bpf_endian.h>
> +#include <bpf/bpf_helpers.h>
> +#include <linux/pkt_cls.h>
> +
> +char LICENSE[] SEC("license") = "GPL";
> +
> +__u64 sk_cookie_seen;
> +__u64 reuseport_executed;
> +union {
> +	struct tcphdr tcp;
> +	struct udphdr udp;
> +} headers;
> +
> +const volatile __u16 dest_port;
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_SOCKMAP);
> +	__uint(max_entries, 1);
> +	__type(key, __u32);
> +	__type(value, __u64);
> +} sk_map SEC(".maps");
> +
> +SEC("sk_reuseport")
> +int reuse_accept(struct sk_reuseport_md *ctx)
> +{
> +	reuseport_executed++;
> +
> +	if (ctx->ip_protocol == IPPROTO_TCP) {
> +		if (ctx->data + sizeof(headers.tcp) > ctx->data_end)
> +			return SK_DROP;
> +
> +		if (__builtin_memcmp(&headers.tcp, ctx->data, sizeof(headers.tcp)) != 0)
> +			return SK_DROP;
> +	} else if (ctx->ip_protocol == IPPROTO_UDP) {
> +		if (ctx->data + sizeof(headers.udp) > ctx->data_end)
> +			return SK_DROP;
> +
> +		if (__builtin_memcmp(&headers.udp, ctx->data, sizeof(headers.udp)) != 0)
> +			return SK_DROP;
> +	} else {
> +		return SK_DROP;
> +	}
> +
> +	sk_cookie_seen = bpf_get_socket_cookie(ctx->sk);
> +	return SK_PASS;
> +}
> +
> +SEC("sk_reuseport")
> +int reuse_drop(struct sk_reuseport_md *ctx)
> +{
> +	reuseport_executed++;
> +	sk_cookie_seen = 0;
> +	return SK_DROP;
> +}
> +
> +static inline int

nit. inline is no longer a must. Same for a few functions below.

> +assign_sk(struct __sk_buff *skb)
> +{
> +	int zero = 0, ret = 0;
> +	struct bpf_sock *sk;
> +
> +	sk = bpf_map_lookup_elem(&sk_map, &zero);
> +	if (!sk)
> +		return TC_ACT_SHOT;
> +	ret = bpf_sk_assign(skb, sk, 0);
> +	bpf_sk_release(sk);
> +	return ret ? TC_ACT_SHOT : TC_ACT_OK;
> +}
> +