Thread (35 messages) 35 messages, 5 authors, 4d ago

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help