All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sabrina Dubroca <sd@queasysnail.net>
To: Chuck Lever <cel@kernel.org>
Cc: john.fastabend@gmail.com, kuba@kernel.org,
	netdev@vger.kernel.org, kernel-tls-handshake@lists.linux.dev,
	Chuck Lever <chuck.lever@oracle.com>,
	Hannes Reinecke <hare@suse.de>
Subject: Re: [PATCH net-next v5 6/6] tls: Flush backlog before waiting for a new record
Date: Tue, 24 Mar 2026 17:18:10 +0100	[thread overview]
Message-ID: <acK5Qj-F0wGFLUhj@krikkit> (raw)
In-Reply-To: <20260324-tls-read-sock-v5-6-5408befe5774@oracle.com>

2026-03-24, 08:53:28 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> While lock_sock is held, incoming TCP segments land on
> sk->sk_backlog rather than sk->sk_receive_queue.
> tls_rx_rec_wait() inspects only sk_receive_queue, so
> backlog data remains invisible. For non-blocking callers
> (read_sock, and recvmsg or splice_read with MSG_DONTWAIT)
> this causes a spurious -EAGAIN. For blocking callers it
> forces an unnecessary sleep/wakeup cycle.
> 
> Flush the backlog inside tls_rx_rec_wait() before checking
> sk_receive_queue so the strparser can parse newly-arrived
> segments immediately.
> 
> Fixes: 20ffc7adf53a ("net/tls: missing received data after fast remote close")

How did you pick that Fixes tag? That commit mentions FIN/connection
closing, which doesn't seem related to the local backlog.

And it's quite possible there was a similar problem when kTLS was
using the generic strparser, but the code has changed so much with
84c61fe1a75b ("tls: rx: do not use the standard strparser") and the
work around that, that blaming something older probably doesn't make
too much sense.

> Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/tls/tls_sw.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 8fb2f2a93846..84c4ae0330d1 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1372,6 +1372,7 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock,
>  		if (ret < 0)
>  			return ret;
>  
> +		sk_flush_backlog(sk);

Do we need to update released when this returns true, like callers of
tls_read_flush_backlog() do? I also wonder if we'd want to update the
caller's flushed_at to avoid bypassing the "smart checks" in
tls_read_flush_backlog().

>  		if (!skb_queue_empty(&sk->sk_receive_queue)) {
>  			/* Defer notification to the exit point;
>  			 * this thread will consume the record

-- 
Sabrina

  reply	other threads:[~2026-03-24 16:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-24 12:53 [PATCH net-next v5 0/6] TLS read_sock performance scalability Chuck Lever
2026-03-24 12:53 ` [PATCH net-next v5 1/6] tls: Purge async_hold in tls_decrypt_async_wait() Chuck Lever
2026-03-26 10:32   ` Hannes Reinecke
2026-03-24 12:53 ` [PATCH net-next v5 2/6] tls: Abort the connection on decrypt failure Chuck Lever
2026-03-24 12:53 ` [PATCH net-next v5 3/6] tls: Fix dangling skb pointer in tls_sw_read_sock() Chuck Lever
2026-03-24 12:53 ` [PATCH net-next v5 4/6] tls: Factor tls_strp_msg_release() from tls_strp_msg_done() Chuck Lever
2026-03-24 12:53 ` [PATCH net-next v5 5/6] tls: Suppress spurious saved_data_ready on all receive paths Chuck Lever
2026-03-24 12:53 ` [PATCH net-next v5 6/6] tls: Flush backlog before waiting for a new record Chuck Lever
2026-03-24 16:18   ` Sabrina Dubroca [this message]
2026-03-24 19:07     ` Chuck Lever
2026-03-26  8:59       ` Sabrina Dubroca
2026-03-26  9:10 ` [PATCH net-next v5 0/6] TLS read_sock performance scalability patchwork-bot+netdevbpf

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=acK5Qj-F0wGFLUhj@krikkit \
    --to=sd@queasysnail.net \
    --cc=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=hare@suse.de \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-tls-handshake@lists.linux.dev \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.