Re: [PATCH v4 bpf-next 04/10] bpf: tcp: Guard fast-path bpf_tcp_ops_call() under per-socket flag.
flat view
From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-10-07 02:48:23
Also in:
bpf
On Tue, Oct 6, 2026 at 4:25 PM Amery Hung [off-list ref] wrote:
quoted
+++ b/net/ipv4/tcp_input.c@@ -210,6 +210,11 @@ static void bpf_skops_established(struct sock *sk, int bpf_op, static void bpf_tcp_ops_parse_hdr(struct sock *sk, struct sk_buff *skb) { + const struct tcp_sock *tp; + + if (!cgroup_bpf_enabled(CGROUP_TCP_SOCK_OPS)) + return; + switch (sk->sk_state) { case TCP_SYN_RECV: case TCP_SYN_SENT:@@ -217,7 +222,12 @@ static void bpf_tcp_ops_parse_hdr(struct sock *sk, struct sk_buff *skb) return; } - bpf_tcp_ops_call(parse_hdr, sk, skb); + tp = tcp_sk(sk); + + if ((tp->rx_opt.saw_unknown && + BPF_TCP_OPS_TEST_FLAG(tp, PARSE_HDR_OPT_UNKNOWN)) || + BPF_TCP_OPS_TEST_FLAG(tp, PARSE_HDR_OPT_ALL))nit: ^ missing a space
this indentation is correct.
quoted
+ __bpf_tcp_ops_call(parse_hdr, sk, skb); } static __cold void tcp_gro_dev_warn(const struct sock *sk, const struct sk_buff *skb,diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 3ac465513bf5..8770f3084efe 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c@@ -581,8 +581,8 @@ static void bpf_skops_write_hdr_opt(struct sock *sk, struct sk_buff *skb, * writer's bytes). The writer finds the append point by scanning from * first_opt_off + nr_written to the first NOP. */ - bpf_tcp_ops_call(write_hdr_opt, sk, skb, req, syn_skb, synack_type, - first_opt_off + nr_written); + bpf_tcp_ops_call_flag(write_hdr_opt, WRITE_HDR_OPT, sk, skb, req, + syn_skb, synack_type, first_opt_off + nr_written); } #else static u32 bpf_skops_hdr_opt_len(struct sock *sk, struct sk_buff *skb,@@ -613,11 +613,13 @@ static u32 bpf_tcp_ops_hdr_opt_len(struct sock *sk, struct sk_buff *skb, { unsigned int remaining_out = remaining, reserved; - if (!remaining) - return 0; + if (!cgroup_bpf_enabled(CGROUP_TCP_SOCK_OPS) || + !BPF_TCP_OPS_TEST_FLAG(tcp_sk(sk), WRITE_HDR_OPT)||nit: missing a space before ||
oops, due to last minute edit :/
quoted
+ !remaining) + return remaining; /* bpf_tcp_ops_reserve_hdr_opt() reserves space via remaining_out */ - bpf_tcp_ops_call(hdr_opt_len, sk, skb, req, syn_skb, synack_type, &remaining_out); + __bpf_tcp_ops_call(hdr_opt_len, sk, skb, req, syn_skb, synack_type, &remaining_out); reserved = remaining - remaining_out; if (!reserved)@@ -1193,8 +1195,9 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb struct tcp_key *key) { struct tcp_sock *tp = tcp_sk(sk); - unsigned int size = 0; unsigned int eff_sacks; + unsigned int remaining; + unsigned int size = 0; opts->options = 0; opts->bpf_opt_len = 0;@@ -1224,10 +1227,10 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb * left. */ if (sk_is_mptcp(sk)) { - unsigned int remaining = MAX_TCP_OPTION_SPACE - size; bool has_ts = opts->options & OPTION_TS; int opt_size; + remaining = MAX_TCP_OPTION_SPACE - size; opts->mptcp.drop_ts = 0; opt_size = mptcp_established_options(sk, skb, remaining, has_ts,@@ -1244,7 +1247,8 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack; if (unlikely(eff_sacks)) { - const unsigned int remaining = MAX_TCP_OPTION_SPACE - size; + remaining = MAX_TCP_OPTION_SPACE - size; + if (likely(remaining >= TCPOLEN_SACK_BASE_ALIGNED + TCPOLEN_SACK_PERBLOCK)) { opts->num_sack_blocks =@@ -1277,22 +1281,18 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb if (unlikely(BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG))) { - unsigned int remaining = MAX_TCP_OPTION_SPACE - size; - + remaining = MAX_TCP_OPTION_SPACE - size; remaining = bpf_skops_hdr_opt_len(sk, skb, NULL, NULL, 0, opts, remaining); size = MAX_TCP_OPTION_SPACE - remaining; } - if (cgroup_bpf_enabled(CGROUP_TCP_SOCK_OPS)) { - unsigned int remaining = MAX_TCP_OPTION_SPACE - size; - - remaining = bpf_tcp_ops_hdr_opt_len(sk, skb, NULL, NULL, 0, opts, - remaining); + remaining = MAX_TCP_OPTION_SPACE - size; + remaining = bpf_tcp_ops_hdr_opt_len(sk, skb, NULL, NULL, 0, opts, + remaining); - size = MAX_TCP_OPTION_SPACE - remaining; - } + size = MAX_TCP_OPTION_SPACE - remaining;This static-key check is not redundant unless bpf_tcp_ops_hdr_opt_len() is inlined. In my build, tcp_established_options() unconditionally calls the out-of-line helper, so sockets pay the call/prologue cost even when no bpf_tcp_ops is attached.
Interesting, I guess you don't use FDO ?
On my FDO build, even tcp_established_options() is inlined
to __tcp_transmit_skb() and both bpf_tcp_ops_hdr_opt_len()
and bpf_tcp_ops_hdr_opt_len() are NOPs as expected.
ffffffff8224a511: 41 bc 28 00 00 00 movl $0x28, %r12d
# remaining = MAX_TCP_OPTION_SPACE (40)
ffffffff8224a517: 45 29 ec subl %r13d, %r12d
# remaining -= size
ffffffff8224a51a: 0f 1f 44 00 00 nopl (%rax,%rax)
# bpf_skops_hdr_opt_len()
ffffffff8224a51f: 44 89 64 24 78 movl %r12d, 0x78(%rsp)
# remaining_out = remaining
ffffffff8224a524: 0f 1f 44 00 00 nopl (%rax,%rax)
# bpf_tcp_ops_hdr_opt_len()
I will add a static inline function and move both
cgroup_bpf_enabled() there and reuse it in 3 places.
(same for bpf_tcp_ops_parse_hdr())
Then, the unnecessary movl will also disappear.