Re: [PATCH PATCH net-next v4 7/8] tls: Restructure tls_sw_read_sock() into submit/deliver phases

[Sabrina Dubroca <sd@queasysnail.net>]

2026-03-17, 11:04:20 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Pipelining multiple AEAD operations requires separating decryption
> from delivery so that several records can be submitted before any
> are passed to the read_actor callback. The main loop in
> tls_sw_read_sock() is split into two explicit phases: a submit
> phase that decrypts one record onto ctx->rx_list, and a deliver
> phase that drains rx_list and passes each cleartext skb to the
> read_actor callback.
> 
> With a single record per submit phase, behavior is identical to the
> previous code. A subsequent patch will extend the submit phase to
> pipeline multiple AEAD operations.
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  net/tls/tls_sw.c | 70 +++++++++++++++++++++++++++++---------------------------
>  1 file changed, 36 insertions(+), 34 deletions(-)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 5b154afbd7ac2ddd51b46d8d6bef0a7a41f0a841..5ae7e0c026e4437fe442c3a77b0a6d9623816ce1 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2346,8 +2346,8 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
>  	struct tls_context *tls_ctx = tls_get_ctx(sk);
>  	struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
>  	struct tls_prot_info *prot = &tls_ctx->prot_info;
> -	struct strp_msg *rxm = NULL;
>  	struct sk_buff *skb = NULL;
> +	struct strp_msg *rxm;

nit: networking tries to follow the "reverse xmas tree" ordering (a
bit broken in ktls because of context structs).

[or just leave this alone because setting it to NULL doesn't hurt?]


[...]
> +		/* Phase 2: Deliver -- drain rx_list to read_actor */
> +		while ((skb = __skb_dequeue(&ctx->rx_list)) != NULL) {
>  			rxm = strp_msg(skb);
>  			tlm = tls_msg(skb);
> -			decrypted += rxm->full_len;
>  
[...]
> +			copied += used;
> +			if (used < rxm->full_len) {
> +				rxm->offset += used;
> +				rxm->full_len -= used;
> +				if (!desc->count)
> +					goto read_sock_requeue;
> +			} else {
> +				consume_skb(skb);
> +				skb = NULL;
> +			}
>  		}
> +		/* Drain all of rx_list before honoring !desc->count */
> +		if (!desc->count)
> +			break;

I'm not really familiar with the read_sock users, why is it ok to
ignore desc->count reaching 0 while we're in the rx_list loop?

>  	}

-- 
Sabrina