Re: [PATCH v2 bpf-next 02/11] bpf: tcp: Introduce BPF_SOCK_OPS_RCVQ_CB.
From: bot+bpf-ci@kernel.org
Date: 2026-05-22 08:22:25
Also in:
bpf
quoted hunk ↗ jump to hunk
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index aec171ccb6ef..31130e1b63ea 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h
[ ... ]
quoted hunk ↗ jump to hunk
@@ -7031,8 +7034,16 @@ enum { * options first before the BPF program does. */ BPF_SOCK_OPS_WRITE_HDR_OPT_CB_FLAG = (1<<6), + /* Call bpf when TCP payload is queued to sk->sk_receive_queue + * and after recvmsg(). The bpf prog will be called under + * sock_ops->op == BPF_SOCK_OPS_RCVQ_CB. + * + * It can be used to adjust sk->sk_rcvlowat and suppress + * unnecessary wakeups before sufficient data is available. + */ + BPF_SOCK_OPS_RCVQ_CB_FLAG = (1<<7), /* Mask of all currently supported cb flags */ - BPF_SOCK_OPS_ALL_CB_FLAGS = 0x7F, + BPF_SOCK_OPS_ALL_CB_FLAGS = 0xFF, };
The BPF subsystem requires multi-line comments to have the opening /* on its own line. This comment starts with text on the same line as /*: /* Call bpf when TCP payload is queued to sk->sk_receive_queue Should it be formatted as: /* * Call bpf when TCP payload is queued to sk->sk_receive_queue * and after recvmsg(). The bpf prog will be called under * sock_ops->op == BPF_SOCK_OPS_RCVQ_CB. * * It can be used to adjust sk->sk_rcvlowat and suppress * unnecessary wakeups before sufficient data is available. */ [ ... ]
quoted hunk ↗ jump to hunk
@@ -7176,6 +7187,11 @@ enum { * sendmsg timestamp with corresponding * tskey. */ + BPF_SOCK_OPS_RCVQ_CB, /* Called when TCP payload is queued to + * sk->sk_receive_queue and after recvmsg() + * to allow adjusting sk->sk_rcvlowat and + * to suppress early wakeups. + */ };
The multi-line comment here also starts with text on the same line as /*: BPF_SOCK_OPS_RCVQ_CB, /* Called when TCP payload is queued to Should it be reformatted with the opening /* on its own line?
quoted hunk ↗ jump to hunk
diff --git a/net/core/filter.c b/net/core/filter.c index 9590877b0714..4a50fe2cd863 100644 --- a/net/core/filter.c +++ b/net/core/filter.c@@ -6002,7 +6002,8 @@ BPF_CALL_2(bpf_sock_ops_cb_flags_set, struct bpf_sock_ops_kern *, bpf_sock, struct sock *sk = bpf_sock->sk; int val = argval & BPF_SOCK_OPS_ALL_CB_FLAGS; - if (!is_locked_tcp_sock_ops(bpf_sock)) + if (!is_locked_tcp_sock_ops(bpf_sock) && + bpf_sock->op != BPF_SOCK_OPS_RCVQ_CB) return -EOPNOTSUPP; if (!IS_ENABLED(CONFIG_INET) || !sk_fullsock(sk)) return -EINVAL; tcp_sk(sk)->bpf_sock_ops_cb_flags = val; return argval & (~BPF_SOCK_OPS_ALL_CB_FLAGS);
The patch allows bpf_sock_ops_cb_flags_set() to modify tcp_sk(sk)->bpf_sock_ops_cb_flags without holding the socket lock when bpf_sock->op == BPF_SOCK_OPS_RCVQ_CB. However, the same field can also be modified through bpf_setsockopt() which requires the socket lock. Can there be a race between concurrent modifications? When a BPF program in BPF_SOCK_OPS_RCVQ_CB context (unlocked) calls bpf_sock_ops_cb_flags_set(): bpf_sock_ops_cb_flags_set() tcp_sk(sk)->bpf_sock_ops_cb_flags = val; Another BPF program in a locked context could simultaneously call bpf_setsockopt(): do_tcp_setsockopt() sock_owned_by_me(sk); ... tp->bpf_sock_ops_cb_flags = flags; Both perform full writes (= val), not atomic operations. The tcp_sock.bpf_sock_ops_cb_flags field is a plain u8, not atomic_t, and no READ_ONCE/WRITE_ONCE is used. While byte writes are atomic on most architectures, the lack of synchronization means one write can completely overwrite the other. Would this lead to incorrect callback flag state if two updates race?
quoted hunk ↗ jump to hunk
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 37142e6d911a..3b8f392d8c69 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h
[ ... ] --- 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/26275674572