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

KaFai Wan
2026-04-18T02:20:12Z

On Fri, 2026-04-17 at 09:25 -0700, Martin KaFai Lau wrote:
> On Fri, Apr 17, 2026 at 05:20:35PM +0800, KaFai Wan wrote:
> > diff --git a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> > b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> > index 56685fc03c7e..7b9dbbb84316 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/tcp_hdr_options.c
> > @@ -461,7 +461,7 @@ static void misc(void)
> >   const unsigned int nr_data = 2;
> >   struct bpf_link *link;
> >   struct sk_fds sk_fds;
> > - int i, ret;
> > + int i, ret, true_val = 1;
> >
> >   lport_linum_map_fd = bpf_map__fd(misc_skel->maps.lport_linum_map);
> >
> > @@ -477,6 +477,10 @@ static void misc(void)
> >   return;
> >   }
> >
> > + ret = setsockopt(sk_fds.active_fd, SOL_TCP, TCP_NODELAY, &true_val, sizeof(true_val));
>
> Same comment as in v2. Why this setsockopt is needed?

Sorry I miss this. It's from the review of v1, my first version would break the syscall setsockopt
and other CB besides HDR_OPT_LEN/WRITE_HDR_OPT. So in the test I check setsockopt() and
bpf_setsockopt() in PASSIVE_ESTABLISHED_CB to make sure patch#1 would not break user space and other
CB.

> The setsockopt in userspace is unnecessary.

Is bpf_setsockopt() in PASSIVE_ESTABLISHED_CB also unnecessary? I'll respin if they are unnecessary.

> In the future,
> we may need to understand why it is needed here in the first place.

Okay, I'll remember that. Thanks for the review and guidance.

--
Thanks,
KaFai