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 PATCH net-next v4 8/8] tls: Enable batch async decryption in read_sock
Date: Mon, 23 Mar 2026 15:14:56 +0100	[thread overview]
Message-ID: <acFK4DWAD5h_Oc0f@krikkit> (raw)
In-Reply-To: <20260317-tls-read-sock-v4-8-ab1086ec600f@oracle.com>

2026-03-17, 11:04:21 -0400, Chuck Lever wrote:
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 5ae7e0c026e4437fe442c3a77b0a6d9623816ce1..bc500ba7ce81eb33763c37a8b73473c42dc66044 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -261,6 +261,12 @@ static int tls_decrypt_async_drain(struct tls_sw_context_rx *ctx)
>  	return ret;
>  }
>  
> +/* Submit an AEAD decrypt request.  On success with darg->async set,
> + * the caller must not touch aead_req; the completion handler frees
> + * it.  Every error return clears darg->async and guarantees no
> + * in-flight AEAD operation remains -- callers rely on this to

-EBUSY (which is not a "real" error) will result in calling
tls_decrypt_async_wait, but not other errors?

> + * safely free aead_req and to skip async drain on error paths.
> + */
>  static int tls_do_decryption(struct sock *sk,
>  			     struct scatterlist *sgin,
>  			     struct scatterlist *sgout,
> @@ -2340,6 +2346,13 @@ ssize_t tls_sw_splice_read(struct socket *sock,  loff_t *ppos,
>  	goto splice_read_end;
>  }
>  
> +/* Bound on concurrent async AEAD submissions per read_sock
> + * call.  Chosen to fill typical hardware crypto pipelines
> + * without excessive memory consumption (each in-flight record
> + * holds one cleartext skb plus its AEAD request context).
> + */
> +#define TLS_READ_SOCK_BATCH	16

I suspect that at some point, we'll have a request to make this
configurable (maybe system-wide, maybe by socket?).


>  int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
>  		     sk_read_actor_t read_actor)
>  {
> @@ -2351,6 +2364,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
>  	struct sk_psock *psock;
>  	size_t flushed_at = 0;
>  	bool released = true;
> +	bool async = false;

nit: reverse xmas tree(-ish)

>  	struct tls_msg *tlm;
>  	ssize_t copied = 0;
>  	ssize_t decrypted;
> @@ -2373,25 +2387,61 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
>  	decrypted = 0;
>  	for (;;) {
>  		struct tls_decrypt_arg darg;
> +		int nr_async = 0;
>  
> -		/* Phase 1: Submit -- decrypt one record onto rx_list. */
> +		/* Phase 1: Submit -- decrypt records onto rx_list. */
>  		if (skb_queue_empty(&ctx->rx_list)) {
> -			err = tls_rx_rec_wait(sk, NULL, true, released);
> -			if (err <= 0)
> +			while (nr_async < TLS_READ_SOCK_BATCH) {
> +				if (nr_async == 0) {
> +					err = tls_rx_rec_wait(sk, NULL,
> +							      true,
> +							      released);
> +					if (err <= 0)
> +						goto read_sock_end;
> +				} else {
> +					if (!tls_strp_msg_ready(ctx)) {
> +						tls_strp_check_rcv_quiet(&ctx->strp);
> +						if (!tls_strp_msg_ready(ctx))
> +							break;

This (and tls_rx_rec_wait) looks like tls_strp_check_rcv_quiet should
return the value of msg_ready.

This is also not very different from tls_rx_rec_wait(nonblock=true),
why are you separating the nr_async>0 case and open-coding the core
operations from tls_rx_rec_wait()?

> +					}
> +					if (!tls_strp_msg_load(&ctx->strp,
> +							       released))
> +						break;
> +				}
> +
> +				memset(&darg.inargs, 0, sizeof(darg.inargs));
> +				darg.async = ctx->async_capable;

tls_sw_recvmsg also has:

    if (tlm->control == TLS_RECORD_TYPE_DATA && !bpf_strp_enabled)

before setting darg.async. bpf_strp_enabled isn't relevant since
tls_sw_read_sock aborts when there's a psock, but I think record type
matters here.

> +
> +				err = tls_rx_one_record(sk, NULL, &darg);
> +				if (err < 0)
> +					goto read_sock_end;
> +
> +				async |= darg.async;
> +				released = tls_read_flush_backlog(sk, prot,
> +								  INT_MAX,
> +								  0,
> +								  decrypted,
> +								  &flushed_at);

The level of indentation for phase 1 is getting really unreasonable.

> +				decrypted += strp_msg(darg.skb)->full_len;
> +				tls_rx_rec_release(ctx);
> +				__skb_queue_tail(&ctx->rx_list, darg.skb);
> +				nr_async++;
> +
> +				if (!ctx->async_capable)

Do we want to break out here (stop the current decrypt batch and move
to phase 2) if we've already had to process all pending decrypts
(tls_decrypt_async_wait has been called)?


> +					break;
> +			}
> +		}

-- 
Sabrina

  reply	other threads:[~2026-03-23 14:15 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-17 15:04 [PATCH net-next v4 0/8] TLS read_sock performance scalability Chuck Lever
2026-03-17 15:04 ` [PATCH PATCH net-next v4 1/8] tls: Factor tls_decrypt_async_drain() from recvmsg Chuck Lever
2026-03-17 19:55   ` Breno Leitao
2026-03-19 17:21   ` Sabrina Dubroca
2026-03-20  1:03     ` Chuck Lever
2026-03-17 15:04 ` [PATCH PATCH net-next v4 2/8] tls: Abort the connection on decrypt failure Chuck Lever
2026-03-23 10:22   ` Sabrina Dubroca
2026-03-17 15:04 ` [PATCH PATCH net-next v4 3/8] tls: Fix dangling skb pointer in tls_sw_read_sock() Chuck Lever
2026-03-17 15:04 ` [PATCH PATCH net-next v4 4/8] tls: Factor tls_strp_msg_release() from tls_strp_msg_done() Chuck Lever
2026-03-17 15:04 ` [PATCH PATCH net-next v4 5/8] tls: Suppress spurious saved_data_ready on all receive paths Chuck Lever
2026-03-23 10:32   ` Sabrina Dubroca
2026-03-17 15:04 ` [PATCH PATCH net-next v4 6/8] tls: Flush backlog before waiting for a new record Chuck Lever
2026-03-17 15:04 ` [PATCH PATCH net-next v4 7/8] tls: Restructure tls_sw_read_sock() into submit/deliver phases Chuck Lever
2026-03-23 11:31   ` Sabrina Dubroca
2026-03-17 15:04 ` [PATCH PATCH net-next v4 8/8] tls: Enable batch async decryption in read_sock Chuck Lever
2026-03-23 14:14   ` Sabrina Dubroca [this message]
2026-03-23 15:04     ` Chuck Lever
2026-03-23 23:08       ` Sabrina Dubroca
2026-03-24 13:17         ` Chuck Lever
2026-03-24 22:58           ` Sabrina Dubroca
2026-03-23 15:53     ` Chuck Lever
2026-03-23 21:28   ` Chuck Lever
2026-03-23 21:41     ` Jakub Kicinski
2026-03-23 22:48     ` Sabrina Dubroca
2026-03-24 12:44       ` Chuck Lever

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=acFK4DWAD5h_Oc0f@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.