Re: [PATCH v3 1/2] crypto: Implement a generic crypto statistics

From: Eric Biggers
Date: Sat Nov 03 2018 - 18:19:49 EST


Hi Corentin,

On Wed, Sep 19, 2018 at 10:10:54AM +0000, Corentin Labbe wrote:
> This patch implement a generic way to get statistics about all crypto
> usages.
>
> Signed-off-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
> ---
> crypto/Kconfig | 11 +
> crypto/Makefile | 1 +
> crypto/ahash.c | 21 +-
> crypto/algapi.c | 8 +
> crypto/{crypto_user.c => crypto_user_base.c} | 9 +-
> crypto/crypto_user_stat.c | 463 +++++++++++++++++++++++++++
> crypto/rng.c | 1 +
> include/crypto/acompress.h | 38 ++-
> include/crypto/aead.h | 51 ++-
> include/crypto/akcipher.h | 76 ++++-
> include/crypto/hash.h | 32 +-
> include/crypto/internal/cryptouser.h | 8 +
> include/crypto/kpp.h | 51 ++-
> include/crypto/rng.h | 29 +-
> include/crypto/skcipher.h | 44 ++-
> include/linux/crypto.h | 110 ++++++-
> include/uapi/linux/cryptouser.h | 52 +++
> 17 files changed, 970 insertions(+), 35 deletions(-)
> rename crypto/{crypto_user.c => crypto_user_base.c} (97%)
> create mode 100644 crypto/crypto_user_stat.c
> create mode 100644 include/crypto/internal/cryptouser.h
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 90f2811fac5f..4ef95b0b25a3 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1799,6 +1799,17 @@ config CRYPTO_USER_API_AEAD
> This option enables the user-spaces interface for AEAD
> cipher algorithms.
>
> +config CRYPTO_STATS
> + bool "Crypto usage statistics for User-space"
> + help
> + This option enables the gathering of crypto stats.
> + This will collect:
> + - encrypt/decrypt size and numbers of symmeric operations
> + - compress/decompress size and numbers of compress operations
> + - size and numbers of hash operations
> + - encrypt/decrypt/sign/verify numbers for asymmetric operations
> + - generate/seed numbers for rng operations
> +
> config CRYPTO_HASH_INFO
> bool
>
> diff --git a/crypto/Makefile b/crypto/Makefile
> index d719843f8b6e..ff5c2bbda04a 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -54,6 +54,7 @@ cryptomgr-y := algboss.o testmgr.o
>
> obj-$(CONFIG_CRYPTO_MANAGER2) += cryptomgr.o
> obj-$(CONFIG_CRYPTO_USER) += crypto_user.o
> +crypto_user-y := crypto_user_base.o crypto_user_stat.o
> obj-$(CONFIG_CRYPTO_CMAC) += cmac.o
> obj-$(CONFIG_CRYPTO_HMAC) += hmac.o
> obj-$(CONFIG_CRYPTO_VMAC) += vmac.o

Why is crypto_user_stat.c always being compiled when CONFIG_CRYPTO_USER is
enabled, even when CONFIG_CRYPTO_STATS is not?

When CONFIG_CRYPTO_STATS is disabled, all the crypto stat stuff should be
stubbed out so that crypto_user_stat.c doesn't need to be compiled.

[...]
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index e8839d3a7559..3634ad6fe202 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -454,6 +454,33 @@ struct compress_alg {
> * @cra_refcnt: internally used
> * @cra_destroy: internally used
> *
> + * All following statistics are for this crypto_alg
> + * @encrypt_cnt: number of encrypt requests
> + * @decrypt_cnt: number of decrypt requests
> + * @compress_cnt: number of compress requests
> + * @decompress_cnt: number of decompress requests
> + * @generate_cnt: number of RNG generate requests
> + * @seed_cnt: number of times the rng was seeded
> + * @hash_cnt: number of hash requests
> + * @sign_cnt: number of sign requests
> + * @setsecret_cnt: number of setsecrey operation
> + * @generate_public_key_cnt: number of generate_public_key operation
> + * @verify_cnt: number of verify operation
> + * @compute_shared_secret_cnt: number of compute_shared_secret operation
> + * @encrypt_tlen: total data size handled by encrypt requests
> + * @decrypt_tlen: total data size handled by decrypt requests
> + * @compress_tlen: total data size handled by compress requests
> + * @decompress_tlen: total data size handled by decompress requests
> + * @generate_tlen: total data size of generated data by the RNG
> + * @hash_tlen: total data size hashed
> + * @akcipher_err_cnt: number of error for akcipher requests
> + * @cipher_err_cnt: number of error for akcipher requests
> + * @compress_err_cnt: number of error for akcipher requests
> + * @aead_err_cnt: number of error for akcipher requests
> + * @hash_err_cnt: number of error for akcipher requests
> + * @rng_err_cnt: number of error for akcipher requests
> + * @kpp_err_cnt: number of error for akcipher requests
> + *
> * The struct crypto_alg describes a generic Crypto API algorithm and is common
> * for all of the transformations. Any variable not documented here shall not
> * be used by a cipher implementation as it is internal to the Crypto API.
> @@ -487,6 +514,45 @@ struct crypto_alg {
> void (*cra_destroy)(struct crypto_alg *alg);
>
> struct module *cra_module;
> +
> + union {
> + atomic_t encrypt_cnt;
> + atomic_t compress_cnt;
> + atomic_t generate_cnt;
> + atomic_t hash_cnt;
> + atomic_t setsecret_cnt;
> + };
> + union {
> + atomic64_t encrypt_tlen;
> + atomic64_t compress_tlen;
> + atomic64_t generate_tlen;
> + atomic64_t hash_tlen;
> + };
> + union {
> + atomic_t akcipher_err_cnt;
> + atomic_t cipher_err_cnt;
> + atomic_t compress_err_cnt;
> + atomic_t aead_err_cnt;
> + atomic_t hash_err_cnt;
> + atomic_t rng_err_cnt;
> + atomic_t kpp_err_cnt;
> + };
> + union {
> + atomic_t decrypt_cnt;
> + atomic_t decompress_cnt;
> + atomic_t seed_cnt;
> + atomic_t generate_public_key_cnt;
> + };
> + union {
> + atomic64_t decrypt_tlen;
> + atomic64_t decompress_tlen;
> + };
> + union {
> + atomic_t verify_cnt;
> + atomic_t compute_shared_secret_cnt;
> + };
> + atomic_t sign_cnt;
> +
> } CRYPTO_MINALIGN_ATTR;

The new fields here should all be behind CONFIG_CRYPTO_STATS.

Ideally there would be zero overhead when CONFIG_CRYPTO_STATS is disabled.
That's not quite the case currently.

- Eric