Netdev List
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: mptcp@lists.linux.dev,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org
Cc: daniel@iogearbox.net, andrii@kernel.org, eddyz87@gmail.com,
	memxor@gmail.com, john.fastabend@gmail.com, jakub@cloudflare.com,
	jiayuan.chen@linux.dev, davem@davemloft.net, edumazet@kernel.org,
	kuba@kernel.org, pabeni@redhat.com, horms@kernel.org,
	kuniyu@google.com, willemb@google.com, matttbe@kernel.org,
	mptcp@lists.linux.dev, mason@kernel.org, josef@toxicpanda.com
Subject: Re: [PATCH bpf v2 03/15] bpf, sockmap: Don't let an SK_PASS verdict cover the next message
Date: Mon, 02 Nov 2026 11:51:58 -0700	[thread overview]
Message-ID: <tnw1v9.qyjuvdc9kq45@gmail.com> (raw)
In-Reply-To: <20261102072112.4160195-4-alexei.starovoitov@gmail.com>

On Mon, 02 Nov 2026 00:21, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>From: Alexei Starovoitov <ast@kernel.org>
>
>tcp_bpf_send_verdict() keeps the verdict of the sk_msg prog in psock->eval
>and runs the prog only when eval is __SK_NONE. For __SK_PASS it resets eval
>only after tcp_bpf_push() returns 0. But the push can release the socket
>lock in sk_stream_wait_memory(), and it can fail. Then another
>tcp_bpf_sendmsg() sends its message and does not run the prog.
>
>With bpf_msg_apply_bytes() the verdict covers apply_bytes bytes. After a
>failed push, apply_bytes still includes the sent bytes. Thus the verdict
>covers more bytes than the prog requested.
>
>Reset eval before the push when apply_bytes is 0. If this push fails, the
>prog now runs for the rest of the message. The same already occurs when a
>message does not fit into one sk_msg.
>
>When the push fails, subtract the sent bytes from apply_bytes. When
>apply_bytes is not 0, eval stays __SK_PASS during the push, and the prog
>does not run for another message, as today. The verdict must still cover
>the bytes that a failed push does not send. But such a message also lowers
>apply_bytes. Thus, if apply_bytes is 0 after the subtraction, end the
>verdict, as after a push that returns 0.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

>
>Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
>Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>---
> net/ipv4/tcp_bpf.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
>index d90bcc911b36..b016aa419fb2 100644
>--- a/net/ipv4/tcp_bpf.c
>+++ b/net/ipv4/tcp_bpf.c
>@@ -457,8 +457,22 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
> 
> 	switch (psock->eval) {
> 	case __SK_PASS:
>+		/* The push may sleep without the socket lock, and it may fail */
>+		if (!psock->apply_bytes)
>+			psock->eval = __SK_NONE;
>+		origsize = msg->sg.size;
> 		ret = tcp_bpf_push(sk, msg, tosend, flags, true);
> 		if (unlikely(ret)) {
>+			/* A part of it may have gone out */
>+			sk_msg_apply_bytes(psock, origsize - msg->sg.size);
>+			/* Another sender can use up the rest during the push */

Interesting. We've never hit this with http parsers. We do use this to
skip payloads some times.

Thanks.

  reply	other threads:[~2026-11-02 18:54 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02  7:20 [PATCH bpf v2 00/15] bpf, sockmap: Fixes for verdicts, redirects, corks and recvmsg() Alexei Starovoitov
2026-11-02  7:20 ` [PATCH bpf v2 01/15] bpf, sockmap: Write sg.start when recvmsg() faults Alexei Starovoitov
2026-11-02  8:32   ` Jiayuan Chen
2026-11-02  7:20 ` [PATCH bpf v2 02/15] bpf, sockmap: Don't let an SK_DROP verdict stick Alexei Starovoitov
2026-11-02  8:45   ` Jiayuan Chen
2026-11-02 18:47   ` John Fastabend
2026-11-02  7:21 ` [PATCH bpf v2 03/15] bpf, sockmap: Don't let an SK_PASS verdict cover the next message Alexei Starovoitov
2026-11-02 18:51   ` John Fastabend [this message]
2026-11-02  7:21 ` [PATCH bpf v2 04/15] bpf, sockmap: Subtract all dropped bytes from the return value Alexei Starovoitov
2026-11-02 11:19   ` Jiayuan Chen
2026-11-02 19:12   ` John Fastabend
2026-11-02  7:21 ` [PATCH bpf v2 05/15] bpf, sockmap: Clear the resume state of a stopped psock Alexei Starovoitov
2026-11-02 11:40   ` Jiayuan Chen
2026-11-02 19:16     ` John Fastabend
2026-11-02  7:21 ` [PATCH bpf v2 06/15] bpf, sockmap: Give no progs to a TCP listener Alexei Starovoitov
2026-11-02 12:00   ` Jiayuan Chen
2026-11-02 19:31   ` John Fastabend
2026-11-02  7:21 ` [PATCH bpf v2 07/15] bpf, sockmap: Check the target of an egress redirect before each send Alexei Starovoitov
2026-11-02 12:32   ` Jiayuan Chen
2026-11-02 16:18     ` Alexei Starovoitov
2026-11-02 19:36   ` John Fastabend
2026-11-02  7:21 ` [PATCH bpf v2 08/15] net: Leave sk_stream_wait_memory() once the socket is dead Alexei Starovoitov
2026-11-02 12:54   ` Jiayuan Chen
2026-11-02  7:21 ` [PATCH bpf v2 09/15] bpf, sockmap: Check the TCP state in recvmsg() Alexei Starovoitov
2026-11-02 15:15   ` Jiayuan Chen
2026-11-02 16:57   ` Matthieu Baerts
2026-11-02  7:21 ` [PATCH bpf v2 10/15] bpf, sockmap: Reset the copy position when corking Alexei Starovoitov
2026-11-02  7:21 ` [PATCH bpf v2 11/15] bpf, sockmap: Don't copy 0 bytes into a full cork Alexei Starovoitov
2026-11-02  7:21 ` [PATCH bpf v2 12/15] bpf, sockmap: Clear the redirect target before the sk_msg prog runs Alexei Starovoitov
2026-11-02  7:21 ` [PATCH bpf v2 13/15] bpf, sockmap: Check that the redirected message is the cork Alexei Starovoitov
2026-11-02  7:21 ` [PATCH bpf v2 14/15] bpf, sockmap: Keep the rest of the cork after a redirect Alexei Starovoitov
2026-11-02  7:21 ` [PATCH bpf v2 15/15] bpf, sockmap: Clear the end mark of elements added by push and pop Alexei Starovoitov
2026-11-02 20:11 ` [PATCH bpf v2 00/15] bpf, sockmap: Fixes for verdicts, redirects, corks and recvmsg() John Fastabend

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=tnw1v9.qyjuvdc9kq45@gmail.com \
    --to=john.fastabend@gmail.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eddyz87@gmail.com \
    --cc=edumazet@kernel.org \
    --cc=horms@kernel.org \
    --cc=jakub@cloudflare.com \
    --cc=jiayuan.chen@linux.dev \
    --cc=josef@toxicpanda.com \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=mason@kernel.org \
    --cc=matttbe@kernel.org \
    --cc=memxor@gmail.com \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=willemb@google.com \
    /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