Re: [PATCH PATCH net-next v4 8/8] tls: Enable batch async decryption in read_sock

[Chuck Lever <cel@kernel.org>]

On 3/23/26 7:08 PM, Sabrina Dubroca wrote:
> 2026-03-23, 11:04:16 -0400, Chuck Lever wrote:
>>
>> On Mon, Mar 23, 2026, at 10:14 AM, Sabrina Dubroca wrote:
>>> 2026-03-17, 11:04:21 -0400, Chuck Lever wrote:
>>>> +/* 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?).
>>
>> I appreciate your careful and close review. The series has
>> improved significantly.
>>
>> I will admit that the current value (16) is arbitrary. I agree
>> that someone might want to modify this value. At this point,
>> however, the constant is straightforward and it is still quite
>> easy to promote to a tunable later if that proves to be needed.
> 
> Agreed.
> 
>> The right interface for this depends on kTLS consumer needs
>> that aren't clear (to me) yet.
> 
> In this case (read_sock), the kTLS consumer is NVMe/TCP etc, and
> specifically users of those features with crypto acceleration
> cards. I'm not familiar with either.
> 
>> But let me know if you have a
>> preferred API mechanism or a specific use case in mind, or if
>> there is a netdev policy that should guide the introduction
>> of a suitable API for this purpose.
> 
> Nothing specific, I just thought I'd mention it since I was replying
> to the patch anyway. I think at this stage "it seems easy to promote
> to a tunable later" is enough consideration (just to avoid getting
> trapped in some API (or lack thereof) and unable to change it, but I
> agree that it shouldn't be a problem here).

I looked into this a little more yesterday before noticing that
async crypto was disabled for TLS 1.3.

The hardware crypto engines do not surface a concurrency limit to
their consumers, but their ring sizes are typically much larger than
the batch limit of 16 I've chosen here. We can't rely on them to
tell kTLS where to set that limit.

However, the loop was structured to continue until the batch limit
was hit or reading gets -EBUSY. Either way 16 seems to be a safe
but fairly conservative setting. For the larger rings it might cause
a decrypt pipeline bubble.

So the true purpose of the low limit is to constrain the amount of
memory tls_read_sock sets aside for decryption in progress. That's
going to be about 256KB per socket. Yes, that could be made larger
or contingent upon TCP socket buffer sizes, for example.


-- 
Chuck Lever