Page MenuHomeFreeBSD

ossl: Add a VAES-based AES-GCM implementation for amd64
ClosedPublic

Authored by markj on Apr 24 2023, 9:54 PM.
Tags
None
Referenced Files
F102675755: D39783.diff
Fri, Nov 15, 5:24 PM
Unknown Object (File)
Sun, Nov 3, 10:10 AM
Unknown Object (File)
Thu, Oct 24, 8:55 AM
Unknown Object (File)
Sep 25 2024, 2:40 PM
Unknown Object (File)
Sep 19 2024, 11:47 AM
Unknown Object (File)
Sep 18 2024, 11:15 PM
Unknown Object (File)
Sep 18 2024, 9:56 AM
Unknown Object (File)
Sep 18 2024, 5:12 AM
Subscribers

Details

Summary

aes-gcm-avx512.S is generated from OpenSSL and implements AES-GCM.
ossl_x86.c detects whether the CPU implements the required AVX512
instructions; if not, the ossl(4) module does not provide an AES-GCM
implementation. The VAES implementation appears to increase throughput
for all buffer sizes in both directions, up to 2x for sufficiently large
buffers.

The "process" implementation is in two parts: a generic OCF layer in
ossl_aes.c that calls a set of MD functions to do the heavy lifting.
The intent there is to make it possible to add other implementations for
other platforms, e.g., to reduce the diff required for D37421.

I did not include a fallback to legacy AES-NI here. I did implement it
just for comparison purposes, but since we already have aesni(4) I don't
see a compelling reason to include it unless there's a notion of
switching to ossl(4) in the long term.

Sponsored by: Stormshield
Sponsored by: Klara, Inc.

Test Plan

I wrote a little kernel benchmark to compare aesni(4) and ossl(4)-acclerated
AES-GCM. The latter is better in most cases and roughly equal in the rest.
See: https://people.freebsd.org/~markj/kubench-aesni-vs-avx512.txt

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 51162
Build 48053: arc lint + arc unit

Event Timeline

markj requested review of this revision.Apr 24 2023, 9:54 PM

FWIW, I would prefer to use deprecate aesni(4) in favor of ossl(4) mostly because ossl(4) will be maintained in the future (e.g. the updates for AVX-512 as you are pulling in here). Unless aesni(4) outperforms ossl(4), I think we should move forward with deprecating aesni(4) and preferring ossl(4) instead. If I read the last part of your log correctly, it sounds like we probably should be moving to ossl(4) for AES-GCM in general on x86?

In D39783#906084, @jhb wrote:

FWIW, I would prefer to use deprecate aesni(4) in favor of ossl(4) mostly because ossl(4) will be maintained in the future (e.g. the updates for AVX-512 as you are pulling in here). Unless aesni(4) outperforms ossl(4), I think we should move forward with deprecating aesni(4) and preferring ossl(4) instead. If I read the last part of your log correctly, it sounds like we probably should be moving to ossl(4) for AES-GCM in general on x86?

Yes, I suspect we can add OpenSSL's AES-NI-based implementation without hurting anything. I omitted it here to avoid cluttering the diff more than necessary. I could bring it back and upload it as a separate patch on top of this one, but my main goal is to realize the throughput gains of the AVX512 implementation.

Adding in all the other variants of AES-GCM for x86 as a separate followup is fine with me. Hopefully it also makes it easier to do this for the armv7 review? I'd also like to move the AES-GCM bits using OpenSSL assembly out of armv8crypto and into ossl instead (it would be nice to retire armv8crypto and instead just use ossl similar to retiring aesni IMO)

This revision is now accepted and ready to land.May 1 2023, 6:28 PM
In D39783#908284, @jhb wrote:

Adding in all the other variants of AES-GCM for x86 as a separate followup is fine with me. Hopefully it also makes it easier to do this for the armv7 review?

Yes, this will make the diff smaller, though it'll require a bit of work to update the existing diff to use a ossl_aes_gcm_ops table.

I'd also like to move the AES-GCM bits using OpenSSL assembly out of armv8crypto and into ossl instead (it would be nice to retire armv8crypto and instead just use ossl similar to retiring aesni IMO)

I'll take a quick look at this, I hadn't realized that armv8crypto was using OpenSSL routines.