svn commit: r359374 - in head: . share/man/man4 share/man/man7 share/man/man9 sys/crypto/aesni sys/crypto/armv8 sys/crypto/blake2 sys/crypto/ccp sys/crypto/via sys/dev/cesa sys/dev/cxgbe sys/dev/cx...

[John Baldwin]

On 4/13/20 12:31 PM, Warner Losh wrote:
> On Mon, Apr 13, 2020 at 1:22 PM John-Mark Gurney <jmg at funkthat.com> wrote:
> 
>> John Baldwin wrote this message on Mon, Apr 13, 2020 at 09:56 -0700:
>>> On 4/12/20 1:08 PM, Xin Li wrote:
>>>>
>>>>
>>>> On 3/27/20 11:25 AM, John Baldwin wrote:
>>>> [...]>   - Drivers no longer register a list of supported algorithms.
>> This
>>>>>     doesn't quite work when you factor in modes (e.g. a driver might
>>>>>     support both AES-CBC and SHA2-256-HMAC separately but not combined
>>>>>     for ETA).  Instead, a new 'crypto_probesession' method has been
>>>>>     added to the kobj interface for symmteric crypto drivers.  This
>>>>>     method returns a negative value on success (similar to how
>>>>>     device_probe works) and the crypto framework uses this value to
>> pick
>>>>>     the "best" driver.  There are three constants for hardware
>>>>>     (e.g. ccr), accelerated software (e.g. aesni), and plain software
>>>>>     (cryptosoft) that give preference in that order.  One effect of
>> this
>>>>>     is that if you request only hardware when creating a new session,
>>>>>     you will no longer get a session using accelerated software.
>>>>>     Another effect is that the default setting to disallow software
>>>>>     crypto via /dev/crypto now disables accelerated software.
>>>>
>>>> For user-visible interface, it seems like we are essentially treating
>>>> "accelerated software" like AES-NI the same way of plain software.  For
>>>> example, geom_eli would now say:
>>>>
>>>> GEOM_ELI: Encryption: AES-XTS 128
>>>> GEOM_ELI:     Crypto: software
>>>>
>>>> Instead of:
>>>>
>>>> GEOM_ELI: Encryption: AES-XTS 128
>>>> GEOM_ELI:     Crypto: hardware
>>>>
>>>> When AES-NI is used (which is because we only have two bits to
>> represent
>>>> hardware and software, and have gave neither bits clear with its own
>>>> meaning (use specific driver)).
>>>>
>>>> If we are not going to add a new bit to represent accelerated software,
>>>> why are they categorized as software providers?  Technically, all these
>>>> still requires hardware that implements the cryptographic primitives to
>>>> work, and it's much easier for system administrators if we expose the
>>>> fact that they are using some kind of acceleration than asking them to
>>>> run DTrace etc. to find out.  Personally, I think it's probably better
>>>> to change the notion to either "accelerated" (by either hardware or
>>>> software) and "software"...
>>>
>>> The only case where this is visible is in fact GELI (there is no printf
>>> for IPsec or KTLS).  For /dev/crypto using aesni.ko is a bug, not a
>>> feature, as any such software would be much better off using AES-NI in
>>> userland instead of round-tripping through the kernel.  We could add a
>>> bit to appease the GELI printf, or we could just kill the GELI
>>> printf.  I think a more useful approach would probably be to kill the
>>> GELI printf and instead add something less GELI-specific such as
>>> counters of active sessions for the various cryptographic drivers that
>>> would show which drivers are in use for any in-kernel crypto.  This
>>> approach also lends itself to supporting a more flexible API where a
>>> single crypto session might be backed by multiple drivers where a
>>> binary hardware / software setting might not even make sense as you
>>> might have a mix.  (I know of other out-of-tree crypto use cases that
>>> experimented with splitting in-kernel crypto workloads between an
>>> async co-processor and AES-NI.)
>>
>> As long as there remains some what to warn the user that when they've
>> mounted a geli volume that they're using slow crypto, I'm fine...
>>
>> We have done a bad job on doing the right thing, and I'm afraid that
>> removing this print w/o doing something to address it will go from
>> FreeBSD already not doing the right thing, but to not allowing the user
>> to know that FreeBSD isn't doing the right thing.
>>
>> Even a simple print in the crypto driver when the processor supports
>> AES-NI, but the aesni module isn't loaded would be useful..
>>
>> Without the geli print, it's likely articles, like the recent Anandtech,
>> will think FreeBSD's encrypted volumes are slow when it's just a failure
>> for us to do the correct thing.
>>
>> I haven't looeked at the new code, but the resaon that aesni.ko was
>> considered a hardware vs software device was that the existing crypto
>> framework didn't have a well to tell the two apart, and it wasn't
>> possible to select the aesni.ko routines over the software routines
>> in a reliable manner.
>>
> 
> Seems to me this isn't a binary, but a triary: software, CPU assist,
> hardware offload. but that maybe over-complicating things...

The new code does the trinary under the hood for the interface between
sessions and drivers.  It's even documented that way now in
crypto_driver.9:

     When a new session is created by crypto_newsession(),
     CRYPTODEV_PROBESESSION() is invoked by the cryptographic framework on
     each active driver to determine the best driver to use for the session.
     This method should inspect the session parameters in csp.  If a driver
     does not support requests described by csp, this method should return an
     error value.  If the driver does support requests described by csp, it
     should return a negative value.  The framework prefers drivers with the
     largest negative value, similar to DEVICE_PROBE(9).  The following values
     are defined for non-error return values from this method:

     CRYPTODEV_PROBE_HARDWARE        The driver processes requests via a co-
                                     processor.

     CRYPTODEV_PROBE_ACCEL_SOFTWARE  The driver processes requests on the host
                                     CPU using optimized instructions such as
                                     AES-NI.

     CRYPTODEV_PROBE_SOFTWARE        The driver processes requests on the host
                                     CPU.

     This method should not sleep.

This ensures that aesni.ko is always preferred to plain C, but not necessarily
to async co-processors (but see my caveat that we probably want the ability for
more nuance here).

However, the other end of the API that governs how in-kernel consumers
request sessions is still binary, and GELI is using that part of the API
that I didn't change for its printf.  However, in-kernel consumers mostly
don't care in that they always want some crypto even if it's the slow
fallback.  The only cases that care are /dev/crypto (for which the right
answer is to not user any kind of software by default), and the GELI printf.

At the moment I'm working on further performance improvements and cleanups in
OCF motivated by KTLS, though they should also help at least some cases in
GELI by reducing the number of copies needed.  I can circle back to adding a
flag for GELI's printf at some point after I've made more progress on the
current work.

-- 
John Baldwin