On Thu, Oct 17, 2019 at 09:08:59PM +0200, Ard Biesheuvel wrote:
> +static inline void chacha_crypt(u32 *state, u8 *dst, const u8 *src,
> +                             unsigned int bytes, int nrounds)
> +{
> +     if (IS_ENABLED(CONFIG_CRYPTO_ARCH_HAVE_LIB_CHACHA))
> +             chacha_crypt_arch(state, dst, src, bytes, nrounds);
> +     else
> +             chacha_crypt_generic(state, dst, src, bytes, nrounds);
> +}

How about also providing chacha20_crypt() which calls chacha_crypt(..., 20)?
The 'nrounds' parameter is really for implementations, rather than users of the
library API.  Users don't really have any business specifying the number of
rounds as an int.

> +static inline int chacha_setkey(struct crypto_skcipher *tfm, const u8 *key,
> +                             unsigned int keysize, int nrounds)
> +{
> +     struct chacha_ctx *ctx = crypto_skcipher_ctx(tfm);
> +     int i;
> +
> +     if (keysize != CHACHA_KEY_SIZE)
> +             return -EINVAL;
> +
> +     for (i = 0; i < ARRAY_SIZE(ctx->key); i++)
> +             ctx->key[i] = get_unaligned_le32(key + i * sizeof(u32));
> +
> +     ctx->nrounds = nrounds;
> +     return 0;
> +}

At the end of this patch series there are 5 drivers which wrap chacha_setkey()
with chacha20_setkey() and chacha12_setkey() -- all 5 pairs identical.  How
about providing those as inline functions here?

> +config CRYPTO_LIB_CHACHA
> +     tristate "ChaCha library interface"
> +     depends on CRYPTO_ARCH_HAVE_LIB_CHACHA || !CRYPTO_ARCH_HAVE_LIB_CHACHA
> +     select CRYPTO_LIB_CHACHA_GENERIC if CRYPTO_ARCH_HAVE_LIB_CHACHA=n
> +     help
> +       Enable the ChaCha library interface. This interface may be fulfilled
> +       by either the generic implementation or an arch-specific one, if one
> +       is available and enabled.

Since this is a library for use within the kernel, and not a user-visible
feature, I don't think it should be explicitly selectable.  I.e. it should just
be "tristate", without the prompt string.

- Eric

Reply via email to