Re: [PATCH v4 1/4] crypto: Introduce crypto_pool

[Dmitry Safonov]

On 1/20/23 08:49, Herbert Xu wrote:
> On Thu, Jan 19, 2023 at 06:03:40PM +0000, Dmitry Safonov wrote:
>>
>> - net/ipv4/ah4.c could benefit from it: currently it allocates
>> crypto_alloc_ahash() per every connection, allocating user-specified
>> hash algorithm with ahash = crypto_alloc_ahash(x->aalg->alg_name, 0, 0),
>> which are not shared between each other and it doesn't provide
>> pre-allocated temporary/scratch buffer to calculate hash, so it uses
>> GFP_ATOMIC in ah_alloc_tmp()
>> - net/ipv6/ah6.c is copy'n'paste of the above
>> - net/ipv4/esp4.c and net/ipv6/esp6.c are more-or-less also copy'n'paste
>> with crypto_alloc_aead() instead of crypto_alloc_ahash()
> 
> No they should definitely not switch over to the pool model.  In
> fact, these provide the correct model that you should follow.
> 
> The correct model is to allocate the tfm on the control/slow path,
> and allocate requests on the fast path (or reuse existing memory,
> e.g., from the skb).

Ok, I see. Do you think, it's worth having a pool of tfms?

If not, I can proceed with TCP-AO patches set and implement pool of
ahash tfms that will be used only for TCP-MD5 and TCP-AO, does that
sound good to you?

I see that ahash tfm allocation doesn't eat a lot of memory, rather
little more than 100 bytes, but even so, I don't see why not saving some
memory "for free", especially if one can have thousands of keys over
different sockets. Where there's not much complexity in sharing tfms &
scratch buffers?

Thanks,
          Dmitry