Re: [RFCv3 05/15] tcp: authopt: Add crypto initialization

On Wed, Aug 25, 2021 at 1:08 AM Herbert Xu [off-list ref] wrote:

On Tue, Aug 24, 2021 at 04:34:58PM -0700, Eric Dumazet wrote:

quoted

On 8/24/21 2:34 PM, Leonard Crestez wrote:

quoted

The crypto_shash API is used in order to compute packet signatures. The
API comes with several unfortunate limitations:

1) Allocating a crypto_shash can sleep and must be done in user context.
2) Packet signatures must be computed in softirq context
3) Packet signatures use dynamic "traffic keys" which require exclusive
access to crypto_shash for crypto_setkey.

The solution is to allocate one crypto_shash for each possible cpu for
each algorithm at setsockopt time. The per-cpu tfm is then borrowed from
softirq context, signatures are computed and the tfm is returned.

I could not see the per-cpu stuff that you mention in the changelog.

Perhaps it's time we moved the key information from the tfm into
the request structure for hashes? Or at least provide a way for
the key to be in the request structure in addition to the tfm as
the tfm model still works for IPsec.  Ard/Eric, what do you think
about that?

What is the typical size of a ' tfm' and associated data ?

per-cpu tfm might still make sense, if we had proper NUMA affinities.
AFAIK, currently we can not provide a numa node to crypto allocations.

So using construct like this ends up allocating all data on one single NUMA node

for_each_possible_cpu(cpu) {
    tfm = crypto_alloc_shash(algo->name, 0, 0);
    if (IS_ERR(tfm))
        return PTR_ERR(tfm);
    p_tfm = per_cpu_ptr(algo->tfms, cpu);
    *p_tfm = tfm;
}