Re: [PATCH net-next] macsec: introduce default_async_crypto sysctl

Scott Dial
2023-08-28T19:05:23Z

On 8/28/2023 5:42 AM, Sabrina Dubroca wrote:
> 2023-08-24, 13:08:41 -0400, Scott Dial wrote:
>> On 8/24/2023 9:01 AM, Sabrina Dubroca wrote:
>>> 2023-08-23, 16:22:31 -0400, Scott Dial wrote:
>>>> AES-NI's implementation of gcm(aes) requires the FPU, so if it's busy the
>>>> decrypt gets stuck on the cryptd queue, but that queue is not
>>>> order-preserving.
>>>
>>> It should be (per CPU [*]). The queue itself is a linked list, and if we
>>> have requests on the queue we don't let new requests skip the queue.
>>
>> My apologies, I'll be the first to admit that I have not tracked all of the
>> code changes to either the macsec driver or linux-crypto since I first made
>> the commit. This comment that requests are queued forced me to review the
>> code again and it appears that the queueing issue was resolved in v5.2-rc1
>> with commit 1661131a0479, so I no longer believe we need the
>> CRYPTO_ALG_ASYNC since v5.2 and going forward.
>
> Are you sure about this? 1661131a0479 pre-dates your patch by over a
> year.
>
> And AFAICT, that series only moved the existing FPU usable +
> cryptd_aead_queued tests from AESNI's implementation of gcm(aes) to
> common SIMD helpers.

My original issue started with a RHEL7 system, so a backport of the
macsec driver to the 3.10 kernel. I recall building newer kernels and
reproducing the issue, but I don't have my test setup anymore nor any
meaningful notes that would indicate to me what kernels I tested. In any
case, I didn't bisect when the queuing behavior was changed, and maybe I
misread the code, and maybe my test setup was flawed in some other way.

1661131a0479 wasn't obviously just moving code to me, so I didn't trace
back further, but looking at the longterm maintenance 4.x kernels, I can
see that the AES-NI code has the same cryptd_aead_queued check, so I
think you are correct to say that you could revert my change on all of
the maintenance kernels to restore the performance of MACsec w/ AES-NI.

Whether that causes any ordering regressions for any other crypto
accelerations, I have no idea since it would require auditing a lot of
crypto code.

--
Scott Dial
scott@scottdial.com