From: Eric Biggers <ebiggers@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-crypto@vger.kernel.org, herbert@gondor.apana.org.au,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH 0/7] crypto: switch to static calls for CRC-T10DIF
Date: Mon, 11 Jan 2021 13:05:03 -0800 [thread overview]
Message-ID: <X/y9f4vbJwqfKZh5@sol.localdomain> (raw)
In-Reply-To: <20210111165237.18178-1-ardb@kernel.org>
On Mon, Jan 11, 2021 at 05:52:30PM +0100, Ard Biesheuvel wrote:
> CRC-T10DIF is a very poor match for the crypto API:
> - every user in the kernel calls it via a library wrapper around the
> shash API, so all callers share a single instance of the transform
> - each architecture provides at most a single optimized implementation,
> based on SIMD instructions for carryless multiplication
>
> In other words, none of the flexibility it provides is put to good use,
> and so it is better to get rid of this complexity, and expose the optimized
> implementations via static calls instead. This removes a substantial chunk
> of code, and also gets rid of any indirect calls on architectures that
> obsess about those (x86)
>
> If this approach is deemed suitable, there are other places where we might
> consider adopting it: CRC32 and CRC32(C).
>
> Patch #1 does some preparatory refactoring and removes the library wrapper
> around the shash transform.
>
> Patch #2 introduces the actual static calls, along with the registration
> routines to update the crc-t10dif implementation at runtime.
>
> Patch #3 updates the generic CRC-T10DIF shash driver so it distinguishes
> between the optimized library code and the generic library code.
>
> Patches #4 to #7 update the various arch implementations to switch over to
> the new method.
>
> Special request to Peter to take a look at patch #2, and in particular,
> whether synchronize_rcu_tasks() is sufficient to ensure that a module
> providing the target of a static call can be unloaded safely.
>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: Eric Biggers <ebiggers@google.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
>
> Ard Biesheuvel (7):
> crypto: crc-t10dif - turn library wrapper for shash into generic
> library
> crypto: lib/crc-t10dif - add static call support for optimized
> versions
> crypto: generic/crc-t10dif - expose both arch and generic shashes
> crypto: x86/crc-t10dif - convert to static call library API
> crypto: arm/crc-t10dif - convert to static call library API
> crypto: arm64/crc-t10dif - convert to static call API
> crypto: powerpc/crc-t10dif - convert to static call API
>
> arch/arm/crypto/Kconfig | 2 +-
> arch/arm/crypto/crct10dif-ce-glue.c | 58 ++------
> arch/arm64/crypto/Kconfig | 3 +-
> arch/arm64/crypto/crct10dif-ce-glue.c | 85 ++---------
> arch/powerpc/crypto/crct10dif-vpmsum_glue.c | 51 +------
> arch/x86/crypto/crct10dif-pclmul_glue.c | 90 ++----------
> crypto/Kconfig | 7 +-
> crypto/Makefile | 2 +-
> crypto/crct10dif_common.c | 82 -----------
> crypto/crct10dif_generic.c | 100 +++++++++----
> include/linux/crc-t10dif.h | 21 ++-
> lib/Kconfig | 2 -
> lib/crc-t10dif.c | 152 +++++++++-----------
> 13 files changed, 204 insertions(+), 451 deletions(-)
> delete mode 100644 crypto/crct10dif_common.c
There is already a library API for two other hash functions, BLAKE2s and
Poly1305, which takes advantage of architecture-specific implementations without
using static calls. Also, those algorithms are likewise also exposed through
the shash API, but in a different way from what this patchset proposes.
Is there a reason not to do things in the same way? What are the advantages of
the new approach that you're proposing?
- Eric
next prev parent reply other threads:[~2021-01-11 21:05 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 16:52 [PATCH 0/7] crypto: switch to static calls for CRC-T10DIF Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 1/7] crypto: crc-t10dif - turn library wrapper for shash into generic library Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 2/7] crypto: lib/crc-t10dif - add static call support for optimized versions Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 3/7] crypto: generic/crc-t10dif - expose both arch and generic shashes Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 4/7] crypto: x86/crc-t10dif - convert to static call library API Ard Biesheuvel
2021-01-12 0:01 ` kernel test robot
2021-01-12 0:01 ` kernel test robot
2021-01-11 16:52 ` [PATCH 5/7] crypto: arm/crc-t10dif " Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 6/7] crypto: arm64/crc-t10dif - convert to static call API Ard Biesheuvel
2021-01-11 16:52 ` [PATCH 7/7] crypto: powerpc/crc-t10dif " Ard Biesheuvel
2021-01-11 17:27 ` [PATCH 0/7] crypto: switch to static calls for CRC-T10DIF Ard Biesheuvel
2021-01-11 18:36 ` Ard Biesheuvel
2021-01-11 20:56 ` Peter Zijlstra
2021-01-11 21:01 ` Ard Biesheuvel
2021-01-11 21:05 ` Eric Biggers [this message]
2021-01-11 21:31 ` Ard Biesheuvel
2021-01-28 8:19 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=X/y9f4vbJwqfKZh5@sol.localdomain \
--to=ebiggers@kernel.org \
--cc=ardb@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=linux-crypto@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.