Re: [PATCH net v5 3/3] selftests/bpf: Add mptcp test with sockmap

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/mptcp.c b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> index f8eb7f9d4fd2..b976fe626343 100644
> --- a/tools/testing/selftests/bpf/prog_tests/mptcp.c
> +++ b/tools/testing/selftests/bpf/prog_tests/mptcp.c
> @@ -6,11 +6,14 @@
>   #include <netinet/in.h>
>   #include <test_progs.h>
>   #include <unistd.h>
> +#include <error.h>

I changed to errno.h to be specific. I think you only need the values of 
an errno here.
>   #include "cgroup_helpers.h"
>   #include "network_helpers.h"
> +#include "socket_helpers.h"
>   #include "mptcp_sock.skel.h"
>   #include "mptcpify.skel.h"
>   #include "mptcp_subflow.skel.h"
> +#include "mptcp_sockmap.skel.h"
>   
>   #define NS_TEST "mptcp_ns"
>   #define ADDR_1	"10.0.1.1"
> @@ -436,6 +439,142 @@ static void test_subflow(void)
>   	close(cgroup_fd);
>   }
>   
> +/* Test sockmap on MPTCP server handling non-mp-capable clients. */
> +static void test_sockmap_with_mptcp_fallback(struct mptcp_sockmap *skel)
> +{
> +	int listen_fd = -1, client_fd1 = -1, client_fd2 = -1;
> +	int server_fd1 = -1, server_fd2 = -1, sent, recvd;
> +	char snd[9] = "123456789";
> +	char rcv[10];
> +
> +	/* start server with MPTCP enabled */
> +	listen_fd = start_mptcp_server(AF_INET, NULL, 0, 0);
> +	if (!ASSERT_OK_FD(listen_fd, "sockmap-fb:start_mptcp_server"))
> +		return;
> +
> +	skel->bss->trace_port = ntohs(get_socket_local_port(listen_fd));
> +	skel->bss->sk_index = 0;
> +	/* create client without MPTCP enabled */
> +	client_fd1 = connect_to_fd_opts(listen_fd, NULL);
> +	if (!ASSERT_OK_FD(client_fd1, "sockmap-fb:connect_to_fd"))
> +		goto end;
> +
> +	server_fd1 = xaccept_nonblock(listen_fd, NULL, NULL);
> +	skel->bss->sk_index = 1;
> +	client_fd2 = connect_to_fd_opts(listen_fd, NULL);
> +	if (!ASSERT_OK_FD(client_fd2, "sockmap-fb:connect_to_fd"))
> +		goto end;
> +
> +	server_fd2 = xaccept_nonblock(listen_fd, NULL, NULL);
> +	/* test normal redirect behavior: data sent by client_fd1 can be
> +	 * received by client_fd2
> +	 */
> +	skel->bss->redirect_idx = 1;
> +	sent = xsend(client_fd1, snd, sizeof(snd), 0);
> +	if (!ASSERT_EQ(sent, sizeof(snd), "sockmap-fb:xsend(client_fd1)"))
> +		goto end;
> +
> +	/* try to recv more bytes to avoid truncation check */
> +	recvd = recv_timeout(client_fd2, rcv, sizeof(rcv), MSG_DONTWAIT, 2);

I removed the socket_helpers.h usage. The _nonblock, _timeout, and
MSG_DONTWAIT are unnecessary. I replaced them with the regular accept,
send, and recv. All fds from network_helpers.c have a default 3s
timeout instead of 30s in xaccept_nonblock. This matches how most of
the selftests/bpf are doing it as well.

I also touched up the commit message in patch 2 based on Matt's comment.

Applied. Thanks.

> +	server_fd = xaccept_nonblock(listen_fd, NULL, NULL);
> +	err = bpf_map_update_elem(bpf_map__fd(skel->maps.sock_map),
> +				  &zero, &server_fd, BPF_NOEXIST);