Re: [PATCH bpf v2 2/2] selftests/bpf: Test TCP_NODELAY in TCP hdr opt callbacks

Martin KaFai Lau
2026-04-16T19:06:29Z

On Thu, Apr 16, 2026 at 07:23:08PM +0800, KaFai Wan wrote:
> index 56685fc03c7e..2d738c0c4259 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> @@ -513,6 +513,59 @@ static void misc(void)
> bpf_link__destroy(link);
> }
>
> +static void hdr_sockopt(void)
> +{
> + const char send_msg[] = "MISC!!!";
> + char recv_msg[sizeof(send_msg)];
> + const unsigned int nr_data = 2;
> + struct bpf_link *link;
> + struct sk_fds sk_fds;
> + int i, ret, true_val = 1;
> +
> + lport_linum_map_fd = bpf_map__fd(misc_skel->maps.lport_linum_map);
> +
> + link = bpf_program__attach_cgroup(misc_skel->progs.misc_hdr_sockopt, cg_fd);
> + if (!ASSERT_OK_PTR(link, "attach_cgroup(misc_hdr_sockopt)"))
> + return;
> +
> + if (sk_fds_connect(&sk_fds, false)) {
> + bpf_link__destroy(link);
> + return;
> + }
> +
> + ret = setsockopt(sk_fds.active_fd, SOL_TCP, TCP_NODELAY, &true_val, sizeof(true_val));
> + if (!ASSERT_OK(ret, "setsockopt(TCP_NODELAY) active"))
> + goto check_linum;
> +
> + ret = setsockopt(sk_fds.passive_fd, SOL_TCP, TCP_NODELAY, &true_val, sizeof(true_val));

Why are these two setsockopt(TCP_NODELAY) calls needed?

Instead of creating a new "void hdr_sockopt(void)", can the test be done in the
existing "void misc(void)" by doing bpf_setsockopt(TCP_NODELAY) in the
misc_estab() bpf prog?

The PASSIVE_ESTABLISHED_CB can do the bpf_setsockopt(TCP_NODELAY, 0)
if it wants to keep the same expectation on Nagle. The
BPF_SOCK_OPS_HDR_OPT_LEN_CB and BPF_SOCK_OPS_WRITE_HDR_OPT_CB
can do bpf_setsockopt(TCP_NODELAY, 1) to test recursion and
the error return value.

> void test_tcp_hdr_options(void)
> diff --git a/tools/testing/selftests/bpf/progs/test_misc_tcp_hdr_options.c b/tools/testing/selftests/bpf/progs/test_misc_tcp_hdr_options.c
> index d487153a839d..a8cf7c4e7ed2 100644
> --- a/tools/testing/selftests/bpf/progs/test_misc_tcp_hdr_options.c
> +++ b/tools/testing/selftests/bpf/progs/test_misc_tcp_hdr_options.c
> @@ -28,6 +28,12 @@ unsigned int nr_data = 0;
> unsigned int nr_syn = 0;
> unsigned int nr_fin = 0;
> unsigned int nr_hwtstamp = 0;
> +unsigned int nr_hdr_sockopt_estab = 0;
> +unsigned int nr_hdr_sockopt_estab_err = 0;
> +unsigned int nr_hdr_sockopt_len = 0;
> +unsigned int nr_hdr_sockopt_len_err = 0;
> +unsigned int nr_hdr_sockopt_write = 0;
> +unsigned int nr_hdr_sockopt_write_err = 0;

nr_hdr_sockopt_estab, nr_hdr_sockopt_len, and nr_hdr_sockopt_write
are unnecessary. These tests have already been covered in some ways.

Mostly a nit. The new counters are used in both connections. Note the
existing nr_xxx is exclusively used in either active or passive,
so there is no parallel counting in practice.

Instead of counting, just use a bool nodelay_est_ok,
nodelay_hdr_len_err, nodelay_write_err and assert them
to be true in userspace.