From: bot+bpf-ci@kernel.org
To: kuniyu@google.com,ast@kernel.org,daniel@iogearbox.net,andrii@kernel.org,martin.lau@linux.dev,eddyz87@gmail.com,memxor@gmail.com
Cc: ameryhung@gmail.com,yonghong.song@linux.dev,john.fastabend@gmail.com,sdf@fomichev.me,edumazet@kernel.org,ncardwell@google.com,willemb@google.com,ukyab@berkeley.edu,cleger@meta.com,kuniyu@google.com,kuni1840@gmail.com,bpf@vger.kernel.org,netdev@vger.kernel.org,ast@kernel.org,andrii@kernel.org,daniel@iogearbox.net,martin.lau@kernel.org,eddyz87@gmail.com,yonghong.song@linux.dev,mason@kernel.org,ihor.solodrai@linux.dev
Subject: Re: [PATCH v3 bpf-next 4/9] bpf: tcp: Guard fast-path bpf_tcp_ops_call() under per-socket flag.
Date: Mon, 5 Oct 2026 16:27:14 +0000 (UTC) [thread overview]
Message-ID: <0f2dd53b9e73dcd22f0241f1895682fd088338f9358ca4fce1421e00db842319@mail.kernel.org> (raw)
In-Reply-To: <20261005154533.4147685-5-kuniyu@google.com>
[-- Attachment #1: Type: text/plain, Size: 2673 bytes --]
> bpf: tcp: Guard fast-path bpf_tcp_ops_call() under per-socket flag.
TL;DR: Documentation only, not a bug: the bpf_tcp_ops kernel-doc for rtt,
parse_hdr, hdr_opt_len and write_hdr_opt doesn't say sockets must now opt in
via BPF_TCP_OPS_FLAG_*, and the UAPI flags are undocumented.
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index a12766e38ad4..0f2ceda58319 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -3142,7 +3142,8 @@ static inline void tcp_bpf_rtt(struct sock *sk, long mrtt, u32 srtt)
> {
> if (BPF_SOCK_OPS_TEST_FLAG(tcp_sk(sk), BPF_SOCK_OPS_RTT_CB_FLAG))
> tcp_call_bpf_2arg(sk, BPF_SOCK_OPS_RTT_CB, mrtt, srtt);
> - bpf_tcp_ops_call(rtt, sk, mrtt, srtt);
> + if (BPF_TCP_OPS_TEST_FLAG(tcp_sk(sk), RTT))
> + bpf_tcp_ops_call(rtt, sk, mrtt, srtt);
> }
This isn't a bug, but the kernel-doc in struct bpf_tcp_ops no longer matches
what the code does. It still describes rtt() like this:
/* Called on every RTT sample.
* @mrtt: the measured RTT, in microseconds.
* @srtt: the updated smoothed RTT.
*/
void (*rtt)(struct sock *sk, long mrtt, u32 srtt);
but it now runs only when the socket has BPF_TCP_OPS_FLAG_RTT set.
The same applies to the other three callbacks this patch gates:
- parse_hdr() is documented as parsing the options of an incoming skb on an
established connection. It now runs only with
BPF_TCP_OPS_FLAG_PARSE_HDR_OPT_ALL set, or with
BPF_TCP_OPS_FLAG_PARSE_HDR_OPT_UNKNOWN set and rx_opt.saw_unknown true.
- hdr_opt_len() and write_hdr_opt() now run only with
BPF_TCP_OPS_FLAG_WRITE_HDR_OPT set.
None of these comments say that the socket has to opt in, for example by
calling bpf_tcp_ops_set_flags() from the listen() or connect() callback. The
BPF_TCP_OPS_FLAG_* enum in include/uapi/linux/bpf.h has no comments either,
whereas the legacy BPF_SOCK_OPS_*_CB_FLAG values document which callback each
flag enables.
As a result, a program that attaches a bpf_tcp_ops with only .rtt, .parse_hdr,
.hdr_opt_len or .write_hdr_opt, as the struct comments describe, will see
those callbacks never run, with no error. Before this patch they ran
unconditionally.
None of the later patches in the series (up to 3cb5919af2b2) update these
comments.
Could the rtt, parse_hdr, hdr_opt_len and write_hdr_opt comments say which
BPF_TCP_OPS_FLAG_* has to be set on the socket for the callback to run, and
could the UAPI enum values be documented as well?
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37336989320
next prev parent reply other threads:[~2026-10-05 16:27 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-05 15:40 [PATCH v3 bpf-next 0/9] bpf: Add bpf_tcp_ops hooks for TCP AutoLOWAT Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 1/9] bpf: tcp: Convert deny-list for bpf_{get,set}sockopt() to allow-list Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 2/9] bpf: tcp: Add a new per-socket flag and kfunc for bpf_tcp_ops Kuniyuki Iwashima
2026-10-05 16:27 ` bot+bpf-ci
2026-10-05 17:26 ` Kuniyuki Iwashima
2026-10-05 18:30 ` Stanislav Fomichev
2026-10-05 18:37 ` Kuniyuki Iwashima
2026-10-05 22:47 ` Stanislav Fomichev
2026-10-05 23:31 ` Kuniyuki Iwashima
2026-10-05 21:19 ` Amery Hung
2026-10-05 21:23 ` Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 3/9] selftest: bpf: Use bpf_tcp_ops_set_flags() in bpf_tcp_ops_hdr.c Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 4/9] bpf: tcp: Guard fast-path bpf_tcp_ops_call() under per-socket flag Kuniyuki Iwashima
2026-10-05 16:27 ` bot+bpf-ci [this message]
2026-10-05 19:10 ` Amery Hung
2026-10-05 19:13 ` Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 5/9] bpf: tcp: Introduce bpf_tcp_ops.{enqueue,dequeue}_rcvq() Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 6/9] tcp: Split out __tcp_set_rcvlowat() Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 7/9] bpf: mptcp: Don't support BPF_TCP_OPS_FLAG_RCVQ Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 8/9] bpf: tcp: Add kfunc to adjust sk->sk_rcvlowat Kuniyuki Iwashima
2026-10-05 15:40 ` [PATCH v3 bpf-next 9/9] selftest: bpf: Add test for bpf_tcp_ops.{enqueue,dequeue}_rcvq() Kuniyuki Iwashima
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0f2dd53b9e73dcd22f0241f1895682fd088338f9358ca4fce1421e00db842319@mail.kernel.org \
--to=bot+bpf-ci@kernel.org \
--cc=ameryhung@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=cleger@meta.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=edumazet@kernel.org \
--cc=ihor.solodrai@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=kuni1840@gmail.com \
--cc=kuniyu@google.com \
--cc=martin.lau@kernel.org \
--cc=martin.lau@linux.dev \
--cc=mason@kernel.org \
--cc=memxor@gmail.com \
--cc=ncardwell@google.com \
--cc=netdev@vger.kernel.org \
--cc=sdf@fomichev.me \
--cc=ukyab@berkeley.edu \
--cc=willemb@google.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox