All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/3] crypto: Implement a generic crypto statistics
@ 2017-12-20 20:09 Corentin Labbe
  2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Corentin Labbe @ 2017-12-20 20:09 UTC (permalink / raw)
  To: davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel, Corentin Labbe

Hello

This patch is a try to implement a generic crypto driver statistics.
The goal is to have an "ifconfig" for crypto device.

Some driver tried to implement this via a debugfs interface.

My proposed way is to embed this in the crypto framework by registering
a /sys/kernel/crypto tree and create a directory for each cra_driver_name.
Note that I assume than cra_driver_name will only exists once, which seems false for arm64 ctr-aes-ce.
But for me, it's a bug.
Each crypto algorithm "cra_name" can have multiple implementation called
"cra_driver_name".
If two different implementation have the same cra_driver_name, nothing
can easily differentiate them.
Furthermore the mechanism for getting a crypto algorithm with its
implementation name (crypto_alg_match() in crypto/crypto_user.c) will
get only the first one found.

Then an userspace tool will collect information in this tree.

Example of output:
./cryptostat
aes-arm (cipher) aes
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
aes-generic (cipher) aes
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
arc4-generic (cipher) arc4
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
cbc(aes-arm) (cipher) cbc(aes)
	Encrypt: 94	bytes 6096
	Decrypt: 94	bytes 6096
cbc-aes-sun8i-ce (cipher) cbc(aes)
	Encrypt: 24	bytes 3712
	Decrypt: 24	bytes 3712
cbc(des3_ede-generic) (cipher) cbc(des3_ede)
	Encrypt: 14	bytes 5104
	Decrypt: 14	bytes 5104
cbc-des3-sun8i-ce (cipher) cbc(des3_ede)
	Encrypt: 10	bytes 3488
	Decrypt: 10	bytes 3488
cipher_null-generic (cipher) cipher_null
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
compress_null-generic (compress) compress_null
	Compress: 0	bytes 0
	Decompress: 0	bytes 0
crc32c-generic (hash) crc32c
	Hash: 88	bytes 15826
crct10dif-generic (hash) crct10dif
	Hash: 20	bytes 2246
ctr(aes-arm) (cipher) ctr(aes)
	Encrypt: 31	bytes 10225
	Decrypt: 31	bytes 10225
ctr-aes-sun8i-ce (cipher) ctr(aes)
	Encrypt: 24	bytes 6738
	Decrypt: 24	bytes 6738
cts(cbc(aes-arm)) (cipher) cts(cbc(aes))
	Encrypt: 33	bytes 1241
	Decrypt: 33	bytes 1241
cts(cbc-aes-sun8i-ce) (cipher) cts(cbc(aes))
	Encrypt: 24	bytes 956
	Decrypt: 24	bytes 956
deflate-generic (compress) deflate
	Compress: 0	bytes 0
	Decompress: 0	bytes 0
deflate-scomp (compress) deflate
	Compress: 2	bytes 261
	Decompress: 4	bytes 320
des3_ede-generic (cipher) des3_ede
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
des-generic (cipher) des
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
dh-generic (kpp) dh
	Set_secret: 2
	Generate_public_key: 2
	Compute_shared_secret: 2
digest_null-generic (hash) digest_null
	Hash: 0	bytes 0
drbg_nopr_hmac_sha1 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
drbg_nopr_hmac_sha256 (rng) stdrng
	Generate: 8	bytes 1024
	Seed: 4
drbg_nopr_hmac_sha384 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
drbg_nopr_hmac_sha512 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
drbg_pr_hmac_sha1 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
drbg_pr_hmac_sha256 (rng) stdrng
	Generate: 8	bytes 1024
	Seed: 4
drbg_pr_hmac_sha384 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
drbg_pr_hmac_sha512 (rng) stdrng
	Generate: 0	bytes 0
	Seed: 0
ecb(aes-arm) (cipher) ecb(aes)
	Encrypt: 20	bytes 4160
	Decrypt: 20	bytes 4160
ecb-aes-sun8i-ce (cipher) ecb(aes)
	Encrypt: 18	bytes 3168
	Decrypt: 18	bytes 3168
ecb(arc4)-generic (cipher) ecb(arc4)
	Encrypt: 21	bytes 270
	Decrypt: 21	bytes 270
ecb-cipher_null (cipher) ecb(cipher_null)
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
ecb(des3_ede-generic) (cipher) ecb(des3_ede)
	Encrypt: 24	bytes 4584
	Decrypt: 24	bytes 4584
ecb-des3-sun8i-ce (cipher) ecb(des3_ede)
	Encrypt: 18	bytes 3072
	Decrypt: 18	bytes 3072
hmac(sha256-neon) (hash) hmac(sha256)
	Hash: 40	bytes 1788
jitterentropy_rng (rng) jitterentropy_rng
	Generate: 0	bytes 0
	Seed: 0
lzo-generic (compress) lzo
	Compress: 0	bytes 0
	Decompress: 0	bytes 0
lzo-scomp (compress) lzo
	Compress: 2	bytes 229
	Decompress: 4	bytes 367
md5-generic (hash) md5
	Hash: 28	bytes 718
pkcs1pad(rsa-sun8i-ce,sha1) (asymetric) pkcs1pad(rsa,sha1)
	Encrypt: 0	bytes 0
	Decrypt: 0	bytes 0
	Verify: 2
	Sign: 0
rsa-generic (asymetric) rsa
	Encrypt: 14	bytes 0
	Decrypt: 9	bytes 0
	Verify: 5
	Sign: 0
rsa-sun8i-ce (asymetric) rsa
	Encrypt: 7	bytes 0
	Decrypt: 6	bytes 0
	Verify: 2
	Sign: 0
sha1-asm (hash) sha1
	Hash: 24	bytes 4864
sha1-generic (hash) sha1
	Hash: 24	bytes 4864
sha1-neon (hash) sha1
	Hash: 24	bytes 4864
sha224-asm (hash) sha224
	Hash: 20	bytes 4528
sha224-generic (hash) sha224
	Hash: 20	bytes 4528
sha224-neon (hash) sha224
	Hash: 20	bytes 4528
sha256-asm (hash) sha256
	Hash: 20	bytes 4528
sha256-generic (hash) sha256
	Hash: 20	bytes 4528
sha256-neon (hash) sha256
	Hash: 20	bytes 4528
sha384-generic (hash) sha384
	Hash: 24	bytes 5036
sha512-generic (hash) sha512
	Hash: 24	bytes 5036
zlib-deflate-scomp (compress) zlib-deflate
	Compress: 2	bytes 261
	Decompress: 4	bytes 345

Note that this patch could be a starting base for a /proc/crypto replacement.

Please let me know your opinions about it

Regards

Corentin Labbe (3):
  crypto: Prevent to register duplicate cra_driver_name
  crypto: Implement a generic crypto statistics
  crypto: tools: Add cryptostat userspace

 crypto/Kconfig             |  11 +++
 crypto/ahash.c             |  18 +++++
 crypto/algapi.c            | 191 +++++++++++++++++++++++++++++++++++++++++++++
 crypto/rng.c               |   3 +
 include/crypto/acompress.h |  10 +++
 include/crypto/akcipher.h  |  12 +++
 include/crypto/kpp.h       |   9 +++
 include/crypto/rng.h       |   5 ++
 include/crypto/skcipher.h  |   8 ++
 include/linux/crypto.h     |  22 ++++++
 tools/crypto/cryptostat    |  40 ++++++++++
 11 files changed, 329 insertions(+)
 create mode 100755 tools/crypto/cryptostat

-- 
2.13.6

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name
  2017-12-20 20:09 [PATCH RFC 0/3] crypto: Implement a generic crypto statistics Corentin Labbe
@ 2017-12-20 20:09 ` Corentin Labbe
  2017-12-21  6:27   ` Stephan Mueller
  2017-12-21  6:35   ` Herbert Xu
  2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
  2017-12-20 20:09 ` [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace Corentin Labbe
  2 siblings, 2 replies; 13+ messages in thread
From: Corentin Labbe @ 2017-12-20 20:09 UTC (permalink / raw)
  To: davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel, Corentin Labbe

Each crypto algorithm "cra_name" can have multiple implementation called
"cra_driver_name".
If two different implementation have the same cra_driver_name, nothing
can easily differentiate them.
Furthermore the mechanism for getting a crypto algorithm with its
implementation name (crypto_alg_match() in crypto/crypto_user.c) will
get only the first one found.

So this patch prevent the registration of two implementation with the
same cra_driver_name.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 crypto/algapi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/crypto/algapi.c b/crypto/algapi.c
index 60d7366ed343..b8f6122f37e9 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -208,6 +208,11 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
 				goto err;
 			continue;
 		}
+		if (!strcmp(q->cra_driver_name, alg->cra_driver_name)) {
+			pr_err("Cannot register since cra_driver_name %s is already used\n",
+			       alg->cra_driver_name);
+			goto err;
+		}
 
 		if (!strcmp(q->cra_driver_name, alg->cra_name) ||
 		    !strcmp(q->cra_name, alg->cra_driver_name))
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH RFC 2/3] crypto: Implement a generic crypto statistics
  2017-12-20 20:09 [PATCH RFC 0/3] crypto: Implement a generic crypto statistics Corentin Labbe
  2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
@ 2017-12-20 20:09 ` Corentin Labbe
  2017-12-20 23:37   ` Randy Dunlap
                     ` (2 more replies)
  2017-12-20 20:09 ` [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace Corentin Labbe
  2 siblings, 3 replies; 13+ messages in thread
From: Corentin Labbe @ 2017-12-20 20:09 UTC (permalink / raw)
  To: davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel, Corentin Labbe

This patch implement a generic way to get statistics about all crypto
usages.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 crypto/Kconfig             |  11 +++
 crypto/ahash.c             |  18 +++++
 crypto/algapi.c            | 186 +++++++++++++++++++++++++++++++++++++++++++++
 crypto/rng.c               |   3 +
 include/crypto/acompress.h |  10 +++
 include/crypto/akcipher.h  |  12 +++
 include/crypto/kpp.h       |   9 +++
 include/crypto/rng.h       |   5 ++
 include/crypto/skcipher.h  |   8 ++
 include/linux/crypto.h     |  22 ++++++
 10 files changed, 284 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index d6e9b60fc063..69f1822a026b 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -1781,6 +1781,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/ahash.c b/crypto/ahash.c
index 3a35d67de7d9..93b56892f1b8 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -356,18 +356,36 @@ static int crypto_ahash_op(struct ahash_request *req,
 
 int crypto_ahash_final(struct ahash_request *req)
 {
+#ifdef CONFIG_CRYPTO_STATS
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += req->nbytes;
+#endif
 	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
 }
 EXPORT_SYMBOL_GPL(crypto_ahash_final);
 
 int crypto_ahash_finup(struct ahash_request *req)
 {
+#ifdef CONFIG_CRYPTO_STATS
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += req->nbytes;
+#endif
 	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->finup);
 }
 EXPORT_SYMBOL_GPL(crypto_ahash_finup);
 
 int crypto_ahash_digest(struct ahash_request *req)
 {
+#ifdef CONFIG_CRYPTO_STATS
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += req->nbytes;
+#endif
 	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->digest);
 }
 EXPORT_SYMBOL_GPL(crypto_ahash_digest);
diff --git a/crypto/algapi.c b/crypto/algapi.c
index b8f6122f37e9..4fca4576af78 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -20,11 +20,158 @@
 #include <linux/rtnetlink.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/kobject.h>
 
 #include "internal.h"
 
 static LIST_HEAD(crypto_template_list);
 
+#ifdef CONFIG_CRYPTO_STATS
+static struct kobject *crypto_root_kobj;
+
+static struct kobj_type dynamic_kobj_ktype = {
+	.sysfs_ops      = &kobj_sysfs_ops,
+};
+
+static ssize_t fcrypto_priority(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%d\n", alg->cra_priority);
+}
+
+static ssize_t fcrypto_stat_enc_cnt(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->enc_cnt);
+}
+
+static ssize_t fcrypto_stat_enc_tlen(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->enc_tlen);
+}
+
+static ssize_t fcrypto_stat_dec_cnt(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->dec_cnt);
+}
+
+static ssize_t fcrypto_stat_dec_tlen(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->dec_tlen);
+}
+
+static ssize_t fcrypto_stat_verify_cnt(struct kobject *kobj,
+				       struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->verify_cnt);
+}
+
+static ssize_t fcrypto_stat_sign_cnt(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 9, "%lu\n", alg->sign_cnt);
+}
+
+static ssize_t fcrypto_stat_type(struct kobject *kobj,
+				 struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+	u32 type;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	type = (alg->cra_flags & CRYPTO_ALG_TYPE_MASK);
+	if (type == CRYPTO_ALG_TYPE_ABLKCIPHER ||
+	    type == CRYPTO_ALG_TYPE_SKCIPHER ||
+	    type == CRYPTO_ALG_TYPE_CIPHER ||
+	    type == CRYPTO_ALG_TYPE_BLKCIPHER
+		)
+		return snprintf(buf, 9, "cipher\n");
+	if (type == CRYPTO_ALG_TYPE_AHASH ||
+	    type == CRYPTO_ALG_TYPE_HASH
+		)
+		return snprintf(buf, 9, "hash\n");
+	if (type == CRYPTO_ALG_TYPE_COMPRESS ||
+	    type == CRYPTO_ALG_TYPE_SCOMPRESS)
+		return snprintf(buf, 11, "compress\n");
+	if (type == CRYPTO_ALG_TYPE_RNG)
+		return snprintf(buf, 9, "rng\n");
+	if (type == CRYPTO_ALG_TYPE_AKCIPHER)
+		return snprintf(buf, 11, "asymetric\n");
+	if (type == CRYPTO_ALG_TYPE_KPP)
+		return snprintf(buf, 4, "kpp\n");
+	return snprintf(buf, 16, "unknown %x\n", type);
+}
+
+static ssize_t fcrypto_stat_algname(struct kobject *kobj,
+				    struct kobj_attribute *attr, char *buf)
+{
+	struct crypto_alg *alg;
+
+	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
+	return snprintf(buf, 32, "%s\n", alg->cra_name);
+}
+
+static struct kobj_attribute crypto_stat_attribute_priority =
+	__ATTR(priority, 0444, fcrypto_priority, NULL);
+static struct kobj_attribute crypto_stat_attribute_enc_cnt =
+	__ATTR(enc_cnt, 0444, fcrypto_stat_enc_cnt, NULL);
+static struct kobj_attribute crypto_stat_attribute_enc_tlen =
+	__ATTR(enc_tlen, 0444, fcrypto_stat_enc_tlen, NULL);
+static struct kobj_attribute crypto_stat_attribute_dec_cnt =
+	__ATTR(dec_cnt, 0444, fcrypto_stat_dec_cnt, NULL);
+static struct kobj_attribute crypto_stat_attribute_dec_tlen =
+	__ATTR(dec_tlen, 0444, fcrypto_stat_dec_tlen, NULL);
+static struct kobj_attribute crypto_stat_attribute_verify_cnt =
+	__ATTR(verify_cnt, 0444, fcrypto_stat_verify_cnt, NULL);
+static struct kobj_attribute crypto_stat_attribute_sign_cnt =
+	__ATTR(sign_cnt, 0444, fcrypto_stat_sign_cnt, NULL);
+static struct kobj_attribute crypto_stat_attribute_algtype =
+	__ATTR(algtype, 0444, fcrypto_stat_type, NULL);
+static struct kobj_attribute crypto_stat_attribute_algname =
+	__ATTR(algname, 0444, fcrypto_stat_algname, NULL);
+
+static struct attribute *attrs[] = {
+	&crypto_stat_attribute_priority.attr,
+	&crypto_stat_attribute_enc_cnt.attr,
+	&crypto_stat_attribute_enc_tlen.attr,
+	&crypto_stat_attribute_dec_cnt.attr,
+	&crypto_stat_attribute_dec_tlen.attr,
+	&crypto_stat_attribute_verify_cnt.attr,
+	&crypto_stat_attribute_sign_cnt.attr,
+	&crypto_stat_attribute_algtype.attr,
+	&crypto_stat_attribute_algname.attr,
+	NULL,
+};
+
+static struct attribute_group attr_group = {
+	.attrs = attrs,
+};
+#endif
+
 static inline int crypto_set_driver_name(struct crypto_alg *alg)
 {
 	static const char suffix[] = "-generic";
@@ -73,6 +220,9 @@ static void crypto_free_instance(struct crypto_instance *inst)
 		inst->tmpl->free(inst);
 		return;
 	}
+#ifdef CONFIG_CRYPTO_STATS
+	kobject_put(&inst->alg.cra_stat_obj);
+#endif
 
 	inst->alg.cra_type->free(inst);
 }
@@ -237,6 +387,38 @@ static struct crypto_larval *__crypto_register_alg(struct crypto_alg *alg)
 	list_add(&alg->cra_list, &crypto_alg_list);
 	list_add(&larval->alg.cra_list, &crypto_alg_list);
 
+#ifdef CONFIG_CRYPTO_STATS
+	if (!crypto_root_kobj) {
+		pr_info("crypto: register crypto sysfs\n");
+		crypto_root_kobj = kobject_create_and_add("crypto",
+							  kernel_kobj);
+		if (!crypto_root_kobj) {
+			pr_err("crypto: register crypto class failed\n");
+			ret = -ENOMEM;
+			goto err;
+		}
+		/* TODO class_destroy */
+	}
+
+	pr_debug("Register %s %s %p\n", alg->cra_name, alg->cra_driver_name,
+		 alg->cra_type);
+
+	ret = kobject_init_and_add(&alg->cra_stat_obj, &dynamic_kobj_ktype,
+				   crypto_root_kobj, "%s", alg->cra_driver_name);
+	if (ret == 0) {
+		alg->enc_cnt = 0;
+		alg->dec_cnt = 0;
+		alg->enc_tlen = 0;
+		alg->dec_tlen = 0;
+		ret = sysfs_create_group(&alg->cra_stat_obj, &attr_group);
+		if (ret) {
+			pr_err("crypto: Failed to add stats for %s\n",
+			       alg->cra_driver_name);
+			kobject_put(&alg->cra_stat_obj);
+		}
+	}
+#endif
+
 out:
 	return larval;
 
@@ -401,6 +583,10 @@ int crypto_unregister_alg(struct crypto_alg *alg)
 	ret = crypto_remove_alg(alg, &list);
 	up_write(&crypto_alg_sem);
 
+#ifdef CONFIG_CRYPTO_STATS
+	kobject_put(&alg->cra_stat_obj);
+#endif
+
 	if (ret)
 		return ret;
 
diff --git a/crypto/rng.c b/crypto/rng.c
index b4a618668161..1e9d45c8e9b2 100644
--- a/crypto/rng.c
+++ b/crypto/rng.c
@@ -49,6 +49,9 @@ int crypto_rng_reset(struct crypto_rng *tfm, const u8 *seed, unsigned int slen)
 		seed = buf;
 	}
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->dec_cnt++;
+#endif
 	err = crypto_rng_alg(tfm)->seed(tfm, seed, slen);
 out:
 	kzfree(buf);
diff --git a/include/crypto/acompress.h b/include/crypto/acompress.h
index e328b52425a8..3a091fb914cf 100644
--- a/include/crypto/acompress.h
+++ b/include/crypto/acompress.h
@@ -247,6 +247,11 @@ static inline int crypto_acomp_compress(struct acomp_req *req)
 {
 	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += req->slen;
+#endif
+
 	return tfm->compress(req);
 }
 
@@ -263,6 +268,11 @@ static inline int crypto_acomp_decompress(struct acomp_req *req)
 {
 	struct crypto_acomp *tfm = crypto_acomp_reqtfm(req);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->dec_cnt++;
+	tfm->base.__crt_alg->dec_tlen += req->slen;
+#endif
+
 	return tfm->decompress(req);
 }
 
diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index b5e11de4d497..6a625f319fd0 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -286,6 +286,9 @@ static inline int crypto_akcipher_encrypt(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->enc_cnt++;
+#endif
 	return alg->encrypt(req);
 }
 
@@ -304,6 +307,9 @@ static inline int crypto_akcipher_decrypt(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->dec_cnt++;
+#endif
 	return alg->decrypt(req);
 }
 
@@ -322,6 +328,9 @@ static inline int crypto_akcipher_sign(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->sign_cnt++;
+#endif
 	return alg->sign(req);
 }
 
@@ -340,6 +349,9 @@ static inline int crypto_akcipher_verify(struct akcipher_request *req)
 	struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
 	struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->verify_cnt++;
+#endif
 	return alg->verify(req);
 }
 
diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index 1bde0a6514fa..d17a84e43b3c 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -288,6 +288,9 @@ static inline int crypto_kpp_set_secret(struct crypto_kpp *tfm,
 {
 	struct kpp_alg *alg = crypto_kpp_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->dec_cnt++;
+#endif
 	return alg->set_secret(tfm, buffer, len);
 }
 
@@ -309,6 +312,9 @@ static inline int crypto_kpp_generate_public_key(struct kpp_request *req)
 	struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
 	struct kpp_alg *alg = crypto_kpp_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->enc_cnt++;
+#endif
 	return alg->generate_public_key(req);
 }
 
@@ -327,6 +333,9 @@ static inline int crypto_kpp_compute_shared_secret(struct kpp_request *req)
 	struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
 	struct kpp_alg *alg = crypto_kpp_alg(tfm);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->verify_cnt++;
+#endif
 	return alg->compute_shared_secret(req);
 }
 
diff --git a/include/crypto/rng.h b/include/crypto/rng.h
index b95ede354a66..7893217b2fb5 100644
--- a/include/crypto/rng.h
+++ b/include/crypto/rng.h
@@ -140,6 +140,11 @@ static inline int crypto_rng_generate(struct crypto_rng *tfm,
 				      const u8 *src, unsigned int slen,
 				      u8 *dst, unsigned int dlen)
 {
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += dlen;
+#endif
+
 	return crypto_rng_alg(tfm)->generate(tfm, src, slen, dst, dlen);
 }
 
diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
index 562001cb412b..3b730384e27f 100644
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -442,6 +442,10 @@ static inline int crypto_skcipher_encrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->enc_cnt++;
+	tfm->base.__crt_alg->enc_tlen += req->cryptlen;
+#endif
 	return tfm->encrypt(req);
 }
 
@@ -460,6 +464,10 @@ static inline int crypto_skcipher_decrypt(struct skcipher_request *req)
 {
 	struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
 
+#ifdef CONFIG_CRYPTO_STATS
+	tfm->base.__crt_alg->dec_cnt++;
+	tfm->base.__crt_alg->dec_tlen += req->cryptlen;
+#endif
 	return tfm->decrypt(req);
 }
 
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 78508ca4b108..18831a386c36 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -19,6 +19,7 @@
 
 #include <linux/atomic.h>
 #include <linux/kernel.h>
+#include <linux/kobject.h>
 #include <linux/list.h>
 #include <linux/bug.h>
 #include <linux/slab.h>
@@ -466,6 +467,17 @@ struct crypto_alg {
 	void (*cra_destroy)(struct crypto_alg *alg);
 	
 	struct module *cra_module;
+
+#ifdef CONFIG_CRYPTO_STATS
+	struct kobject cra_stat_obj;
+	/* enc is for encrypt / compress/ hash / generate,
+	 * dec is decrypt / decompress / seed
+	 */
+	unsigned long enc_cnt, dec_cnt;
+	unsigned long enc_tlen, dec_tlen;
+	unsigned long verify_cnt;
+	unsigned long sign_cnt;
+#endif
 } CRYPTO_MINALIGN_ATTR;
 
 /*
@@ -901,6 +913,11 @@ static inline int crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
 {
 	struct ablkcipher_tfm *crt =
 		crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
+
+#ifdef CONFIG_CRYPTO_STATS
+	crt->base->base.__crt_alg->enc_cnt++;
+	crt->base->base.__crt_alg->enc_tlen += req->nbytes;
+#endif
 	return crt->encrypt(req);
 }
 
@@ -919,6 +936,11 @@ static inline int crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
 {
 	struct ablkcipher_tfm *crt =
 		crypto_ablkcipher_crt(crypto_ablkcipher_reqtfm(req));
+
+#ifdef CONFIG_CRYPTO_STATS
+	crt->base->base.__crt_alg->dec_cnt++;
+	crt->base->base.__crt_alg->dec_tlen += req->nbytes;
+#endif
 	return crt->decrypt(req);
 }
 
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace
  2017-12-20 20:09 [PATCH RFC 0/3] crypto: Implement a generic crypto statistics Corentin Labbe
  2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
  2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
@ 2017-12-20 20:09 ` Corentin Labbe
  2017-12-20 23:37   ` Randy Dunlap
  2 siblings, 1 reply; 13+ messages in thread
From: Corentin Labbe @ 2017-12-20 20:09 UTC (permalink / raw)
  To: davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel, Corentin Labbe

Add an example tool for getting easily crypto statistics.

Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 tools/crypto/cryptostat | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100755 tools/crypto/cryptostat

diff --git a/tools/crypto/cryptostat b/tools/crypto/cryptostat
new file mode 100755
index 000000000000..314e108eb608
--- /dev/null
+++ b/tools/crypto/cryptostat
@@ -0,0 +1,40 @@
+#!/bin/bash
+
+for dn in `ls /sys/kernel/crypto/`
+do
+	dnpath=/sys/kernel/crypto/$dn/
+	algtype=$(cat $dnpath/algtype)
+	algname=$(cat $dnpath/algname)
+	echo "$dn ($algtype) $algname"
+	case $algtype in
+	hash)
+	echo -e "\tHash: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	;;
+	cipher)
+	echo -e "\tEncrypt: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	echo -e "\tDecrypt: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
+	;;
+	compress)
+	echo -e "\tCompress: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	echo -e "\tDecompress: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
+	;;
+	asymetric)
+	echo -e "\tEncrypt: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	echo -e "\tDecrypt: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
+	echo -e "\tVerify: $(cat $dnpath/verify_cnt)"
+	echo -e "\tSign: $(cat $dnpath/sign_cnt)"
+	;;
+	rng)
+	echo -e "\tGenerate: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	echo -e "\tSeed: $(cat $dnpath/dec_cnt)"
+	;;
+	kpp)
+	echo -e "\tSet_secret: $(cat $dnpath/dec_cnt)"
+	echo -e "\tGenerate_public_key: $(cat $dnpath/enc_cnt)"
+	echo -e "\tCompute_shared_secret: $(cat $dnpath/verify_cnt)"
+	;;
+	*)
+	echo -e "\t$(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
+	;;
+	esac
+done
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics
  2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
@ 2017-12-20 23:37   ` Randy Dunlap
  2017-12-21  6:38   ` Stephan Mueller
  2017-12-22  6:38   ` Herbert Xu
  2 siblings, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2017-12-20 23:37 UTC (permalink / raw)
  To: Corentin Labbe, davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel

On 12/20/2017 12:09 PM, Corentin Labbe wrote:
> This patch implement a generic way to get statistics about all crypto
> usages.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  crypto/Kconfig             |  11 +++
>  crypto/ahash.c             |  18 +++++
>  crypto/algapi.c            | 186 +++++++++++++++++++++++++++++++++++++++++++++
>  crypto/rng.c               |   3 +
>  include/crypto/acompress.h |  10 +++
>  include/crypto/akcipher.h  |  12 +++
>  include/crypto/kpp.h       |   9 +++
>  include/crypto/rng.h       |   5 ++
>  include/crypto/skcipher.h  |   8 ++
>  include/linux/crypto.h     |  22 ++++++
>  10 files changed, 284 insertions(+)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index d6e9b60fc063..69f1822a026b 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1781,6 +1781,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

	                                        symmetric

> +	  - 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/algapi.c b/crypto/algapi.c
> index b8f6122f37e9..4fca4576af78 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -20,11 +20,158 @@
>  #include <linux/rtnetlink.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
> +#include <linux/kobject.h>
>  
>  #include "internal.h"
>  

> +static ssize_t fcrypto_stat_type(struct kobject *kobj,
> +				 struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +	u32 type;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	type = (alg->cra_flags & CRYPTO_ALG_TYPE_MASK);
> +	if (type == CRYPTO_ALG_TYPE_ABLKCIPHER ||
> +	    type == CRYPTO_ALG_TYPE_SKCIPHER ||
> +	    type == CRYPTO_ALG_TYPE_CIPHER ||
> +	    type == CRYPTO_ALG_TYPE_BLKCIPHER
> +		)
> +		return snprintf(buf, 9, "cipher\n");
> +	if (type == CRYPTO_ALG_TYPE_AHASH ||
> +	    type == CRYPTO_ALG_TYPE_HASH
> +		)
> +		return snprintf(buf, 9, "hash\n");
> +	if (type == CRYPTO_ALG_TYPE_COMPRESS ||
> +	    type == CRYPTO_ALG_TYPE_SCOMPRESS)
> +		return snprintf(buf, 11, "compress\n");
> +	if (type == CRYPTO_ALG_TYPE_RNG)
> +		return snprintf(buf, 9, "rng\n");
> +	if (type == CRYPTO_ALG_TYPE_AKCIPHER)
> +		return snprintf(buf, 11, "asymetric\n");

		                          asymmetric

> +	if (type == CRYPTO_ALG_TYPE_KPP)
> +		return snprintf(buf, 4, "kpp\n");
> +	return snprintf(buf, 16, "unknown %x\n", type);
> +}


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace
  2017-12-20 20:09 ` [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace Corentin Labbe
@ 2017-12-20 23:37   ` Randy Dunlap
  0 siblings, 0 replies; 13+ messages in thread
From: Randy Dunlap @ 2017-12-20 23:37 UTC (permalink / raw)
  To: Corentin Labbe, davem, herbert, nhorman; +Cc: linux-crypto, linux-kernel

On 12/20/2017 12:09 PM, Corentin Labbe wrote:
> Add an example tool for getting easily crypto statistics.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  tools/crypto/cryptostat | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100755 tools/crypto/cryptostat
> 
> diff --git a/tools/crypto/cryptostat b/tools/crypto/cryptostat
> new file mode 100755
> index 000000000000..314e108eb608
> --- /dev/null
> +++ b/tools/crypto/cryptostat
> @@ -0,0 +1,40 @@
> +#!/bin/bash
> +
> +for dn in `ls /sys/kernel/crypto/`
> +do
> +	dnpath=/sys/kernel/crypto/$dn/
> +	algtype=$(cat $dnpath/algtype)
> +	algname=$(cat $dnpath/algname)
> +	echo "$dn ($algtype) $algname"
> +	case $algtype in
> +	hash)
> +	echo -e "\tHash: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	;;
> +	cipher)
> +	echo -e "\tEncrypt: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	echo -e "\tDecrypt: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
> +	;;
> +	compress)
> +	echo -e "\tCompress: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	echo -e "\tDecompress: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
> +	;;
> +	asymetric)

spello:
	asymmetric)

> +	echo -e "\tEncrypt: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	echo -e "\tDecrypt: $(cat $dnpath/dec_cnt)\tbytes $(cat $dnpath/dec_tlen)"
> +	echo -e "\tVerify: $(cat $dnpath/verify_cnt)"
> +	echo -e "\tSign: $(cat $dnpath/sign_cnt)"
> +	;;
> +	rng)
> +	echo -e "\tGenerate: $(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	echo -e "\tSeed: $(cat $dnpath/dec_cnt)"
> +	;;
> +	kpp)
> +	echo -e "\tSet_secret: $(cat $dnpath/dec_cnt)"
> +	echo -e "\tGenerate_public_key: $(cat $dnpath/enc_cnt)"
> +	echo -e "\tCompute_shared_secret: $(cat $dnpath/verify_cnt)"
> +	;;
> +	*)
> +	echo -e "\t$(cat $dnpath/enc_cnt)\tbytes $(cat $dnpath/enc_tlen)"
> +	;;
> +	esac
> +done
> 


-- 
~Randy

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name
  2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
@ 2017-12-21  6:27   ` Stephan Mueller
  2017-12-21  6:35   ` Herbert Xu
  1 sibling, 0 replies; 13+ messages in thread
From: Stephan Mueller @ 2017-12-21  6:27 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: davem, herbert, nhorman, linux-crypto, linux-kernel

Am Mittwoch, 20. Dezember 2017, 21:09:25 CET schrieb Corentin Labbe:

Hi Corentin,

> Each crypto algorithm "cra_name" can have multiple implementation called
> "cra_driver_name".
> If two different implementation have the same cra_driver_name, nothing
> can easily differentiate them.
> Furthermore the mechanism for getting a crypto algorithm with its
> implementation name (crypto_alg_match() in crypto/crypto_user.c) will
> get only the first one found.
> 
> So this patch prevent the registration of two implementation with the
> same cra_driver_name.

Have you seen that happen? The driver_name should be an unambiguous name that 
is unique throughout all crypto implementations.

If you have seen a duplication, then this duplication should be fixed.

Ciao
Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name
  2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
  2017-12-21  6:27   ` Stephan Mueller
@ 2017-12-21  6:35   ` Herbert Xu
  2017-12-21 12:35     ` LABBE Corentin
  1 sibling, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2017-12-21  6:35 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: davem, nhorman, linux-crypto, linux-kernel

On Wed, Dec 20, 2017 at 08:09:25PM +0000, Corentin Labbe wrote:
> Each crypto algorithm "cra_name" can have multiple implementation called
> "cra_driver_name".
> If two different implementation have the same cra_driver_name, nothing
> can easily differentiate them.
> Furthermore the mechanism for getting a crypto algorithm with its
> implementation name (crypto_alg_match() in crypto/crypto_user.c) will
> get only the first one found.
> 
> So this patch prevent the registration of two implementation with the
> same cra_driver_name.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

No this is intentional.  The idea is that you can hot-replace
an implementation by registering a new version of it while the
old one is still in use.  The new one will be used for all new
allocations.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics
  2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
  2017-12-20 23:37   ` Randy Dunlap
@ 2017-12-21  6:38   ` Stephan Mueller
  2017-12-21 20:03     ` LABBE Corentin
  2017-12-22  6:38   ` Herbert Xu
  2 siblings, 1 reply; 13+ messages in thread
From: Stephan Mueller @ 2017-12-21  6:38 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: davem, herbert, nhorman, linux-crypto, linux-kernel

Am Mittwoch, 20. Dezember 2017, 21:09:26 CET schrieb Corentin Labbe:

Hi Corentin,

> This patch implement a generic way to get statistics about all crypto
> usages.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  crypto/Kconfig             |  11 +++
>  crypto/ahash.c             |  18 +++++
>  crypto/algapi.c            | 186
> +++++++++++++++++++++++++++++++++++++++++++++ crypto/rng.c               | 
>  3 +
>  include/crypto/acompress.h |  10 +++
>  include/crypto/akcipher.h  |  12 +++
>  include/crypto/kpp.h       |   9 +++
>  include/crypto/rng.h       |   5 ++
>  include/crypto/skcipher.h  |   8 ++
>  include/linux/crypto.h     |  22 ++++++
>  10 files changed, 284 insertions(+)
> 
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index d6e9b60fc063..69f1822a026b 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1781,6 +1781,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/ahash.c b/crypto/ahash.c
> index 3a35d67de7d9..93b56892f1b8 100644
> --- a/crypto/ahash.c
> +++ b/crypto/ahash.c
> @@ -356,18 +356,36 @@ static int crypto_ahash_op(struct ahash_request *req,
> 
>  int crypto_ahash_final(struct ahash_request *req)
>  {
> +#ifdef CONFIG_CRYPTO_STATS
> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> +
> +	tfm->base.__crt_alg->enc_cnt++;
> +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> +#endif
>  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
>  }
>  EXPORT_SYMBOL_GPL(crypto_ahash_final);
> 
>  int crypto_ahash_finup(struct ahash_request *req)
>  {
> +#ifdef CONFIG_CRYPTO_STATS
> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> +
> +	tfm->base.__crt_alg->enc_cnt++;
> +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> +#endif
>  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->finup);
>  }
>  EXPORT_SYMBOL_GPL(crypto_ahash_finup);
> 
>  int crypto_ahash_digest(struct ahash_request *req)
>  {
> +#ifdef CONFIG_CRYPTO_STATS
> +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> +
> +	tfm->base.__crt_alg->enc_cnt++;
> +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> +#endif

This is ugly. May I ask that one inlne function is made that has these ifdefs 
instead of adding them throughout multiple functions? This comment would apply 
to the other instrumentation code below as well. The issue is that these 
ifdefs should not be spread around through the code.

Besides, I would think that the use of normal integers is not thread-safe. 
What about using atomic_t?

>  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->digest);
>  }
>  EXPORT_SYMBOL_GPL(crypto_ahash_digest);
> diff --git a/crypto/algapi.c b/crypto/algapi.c
> index b8f6122f37e9..4fca4576af78 100644
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
> @@ -20,11 +20,158 @@
>  #include <linux/rtnetlink.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
> +#include <linux/kobject.h>
> 
>  #include "internal.h"
> 
>  static LIST_HEAD(crypto_template_list);
> 
> +#ifdef CONFIG_CRYPTO_STATS

Instead of ifdefing in the code, may I suggest adding a new file that is 
compiled / not compiled via the Makefile?

> +static struct kobject *crypto_root_kobj;
> +
> +static struct kobj_type dynamic_kobj_ktype = {
> +	.sysfs_ops      = &kobj_sysfs_ops,
> +};
> +
> +static ssize_t fcrypto_priority(struct kobject *kobj,
> +				struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%d\n", alg->cra_priority);
> +}
> +
> +static ssize_t fcrypto_stat_enc_cnt(struct kobject *kobj,
> +				    struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->enc_cnt);
> +}
> +
> +static ssize_t fcrypto_stat_enc_tlen(struct kobject *kobj,
> +				     struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->enc_tlen);
> +}
> +
> +static ssize_t fcrypto_stat_dec_cnt(struct kobject *kobj,
> +				    struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->dec_cnt);
> +}
> +
> +static ssize_t fcrypto_stat_dec_tlen(struct kobject *kobj,
> +				     struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->dec_tlen);
> +}
> +
> +static ssize_t fcrypto_stat_verify_cnt(struct kobject *kobj,
> +				       struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->verify_cnt);
> +}
> +
> +static ssize_t fcrypto_stat_sign_cnt(struct kobject *kobj,
> +				     struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	return snprintf(buf, 9, "%lu\n", alg->sign_cnt);
> +}
> +
> +static ssize_t fcrypto_stat_type(struct kobject *kobj,
> +				 struct kobj_attribute *attr, char *buf)
> +{
> +	struct crypto_alg *alg;
> +	u32 type;
> +
> +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> +	type = (alg->cra_flags & CRYPTO_ALG_TYPE_MASK);
> +	if (type == CRYPTO_ALG_TYPE_ABLKCIPHER ||
> +	    type == CRYPTO_ALG_TYPE_SKCIPHER ||
> +	    type == CRYPTO_ALG_TYPE_CIPHER ||
> +	    type == CRYPTO_ALG_TYPE_BLKCIPHER
> +		)
> +		return snprintf(buf, 9, "cipher\n");

"skcipher" ?

Ciao
Stephan

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name
  2017-12-21  6:35   ` Herbert Xu
@ 2017-12-21 12:35     ` LABBE Corentin
  2017-12-21 20:05       ` LABBE Corentin
  0 siblings, 1 reply; 13+ messages in thread
From: LABBE Corentin @ 2017-12-21 12:35 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, nhorman, linux-crypto, linux-kernel

On Thu, Dec 21, 2017 at 05:35:22PM +1100, Herbert Xu wrote:
> On Wed, Dec 20, 2017 at 08:09:25PM +0000, Corentin Labbe wrote:
> > Each crypto algorithm "cra_name" can have multiple implementation called
> > "cra_driver_name".
> > If two different implementation have the same cra_driver_name, nothing
> > can easily differentiate them.
> > Furthermore the mechanism for getting a crypto algorithm with its
> > implementation name (crypto_alg_match() in crypto/crypto_user.c) will
> > get only the first one found.
> > 
> > So this patch prevent the registration of two implementation with the
> > same cra_driver_name.
> > 
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> 
> No this is intentional.  The idea is that you can hot-replace
> an implementation by registering a new version of it while the
> old one is still in use.  The new one will be used for all new
> allocations.
> 

But the new implementation is different from the first so should have a new name.
The only case I found is ctr-aes-ce, and both are different (use/dontuse simd) so qualifying for different name.

Anyway, any advice on how to populate properly /sys/crypto with unique name ?
I have two idea:
- A number which increment after each register
- cra_driver_name-priority

Or does I use /sys/crypto/cra_driver_name/priority ? (which need to use some usage count on cra_driver_name node)

Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics
  2017-12-21  6:38   ` Stephan Mueller
@ 2017-12-21 20:03     ` LABBE Corentin
  0 siblings, 0 replies; 13+ messages in thread
From: LABBE Corentin @ 2017-12-21 20:03 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: davem, herbert, nhorman, linux-crypto, linux-kernel

On Thu, Dec 21, 2017 at 07:38:35AM +0100, Stephan Mueller wrote:
> Am Mittwoch, 20. Dezember 2017, 21:09:26 CET schrieb Corentin Labbe:
> 
> Hi Corentin,
> 
> > This patch implement a generic way to get statistics about all crypto
> > usages.
> > 
> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > ---
> >  crypto/Kconfig             |  11 +++
> >  crypto/ahash.c             |  18 +++++
> >  crypto/algapi.c            | 186
> > +++++++++++++++++++++++++++++++++++++++++++++ crypto/rng.c               | 
> >  3 +
> >  include/crypto/acompress.h |  10 +++
> >  include/crypto/akcipher.h  |  12 +++
> >  include/crypto/kpp.h       |   9 +++
> >  include/crypto/rng.h       |   5 ++
> >  include/crypto/skcipher.h  |   8 ++
> >  include/linux/crypto.h     |  22 ++++++
> >  10 files changed, 284 insertions(+)
> > 
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index d6e9b60fc063..69f1822a026b 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -1781,6 +1781,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/ahash.c b/crypto/ahash.c
> > index 3a35d67de7d9..93b56892f1b8 100644
> > --- a/crypto/ahash.c
> > +++ b/crypto/ahash.c
> > @@ -356,18 +356,36 @@ static int crypto_ahash_op(struct ahash_request *req,
> > 
> >  int crypto_ahash_final(struct ahash_request *req)
> >  {
> > +#ifdef CONFIG_CRYPTO_STATS
> > +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> > +
> > +	tfm->base.__crt_alg->enc_cnt++;
> > +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> > +#endif
> >  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->final);
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_ahash_final);
> > 
> >  int crypto_ahash_finup(struct ahash_request *req)
> >  {
> > +#ifdef CONFIG_CRYPTO_STATS
> > +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> > +
> > +	tfm->base.__crt_alg->enc_cnt++;
> > +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> > +#endif
> >  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->finup);
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_ahash_finup);
> > 
> >  int crypto_ahash_digest(struct ahash_request *req)
> >  {
> > +#ifdef CONFIG_CRYPTO_STATS
> > +	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
> > +
> > +	tfm->base.__crt_alg->enc_cnt++;
> > +	tfm->base.__crt_alg->enc_tlen += req->nbytes;
> > +#endif
> 
> This is ugly. May I ask that one inlne function is made that has these ifdefs 
> instead of adding them throughout multiple functions? This comment would apply 
> to the other instrumentation code below as well. The issue is that these 
> ifdefs should not be spread around through the code.
> 
> Besides, I would think that the use of normal integers is not thread-safe. 
> What about using atomic_t?

I will do all your suggestions.

> 
> >  	return crypto_ahash_op(req, crypto_ahash_reqtfm(req)->digest);
> >  }
> >  EXPORT_SYMBOL_GPL(crypto_ahash_digest);
> > diff --git a/crypto/algapi.c b/crypto/algapi.c
> > index b8f6122f37e9..4fca4576af78 100644
> > --- a/crypto/algapi.c
> > +++ b/crypto/algapi.c
> > @@ -20,11 +20,158 @@
> >  #include <linux/rtnetlink.h>
> >  #include <linux/slab.h>
> >  #include <linux/string.h>
> > +#include <linux/kobject.h>
> > 
> >  #include "internal.h"
> > 
> >  static LIST_HEAD(crypto_template_list);
> > 
> > +#ifdef CONFIG_CRYPTO_STATS
> 
> Instead of ifdefing in the code, may I suggest adding a new file that is 
> compiled / not compiled via the Makefile?

I agree, it will be better.

> 
> > +static struct kobject *crypto_root_kobj;
> > +
> > +static struct kobj_type dynamic_kobj_ktype = {
> > +	.sysfs_ops      = &kobj_sysfs_ops,
> > +};
> > +
> > +static ssize_t fcrypto_priority(struct kobject *kobj,
> > +				struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%d\n", alg->cra_priority);
> > +}
> > +
> > +static ssize_t fcrypto_stat_enc_cnt(struct kobject *kobj,
> > +				    struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->enc_cnt);
> > +}
> > +
> > +static ssize_t fcrypto_stat_enc_tlen(struct kobject *kobj,
> > +				     struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->enc_tlen);
> > +}
> > +
> > +static ssize_t fcrypto_stat_dec_cnt(struct kobject *kobj,
> > +				    struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->dec_cnt);
> > +}
> > +
> > +static ssize_t fcrypto_stat_dec_tlen(struct kobject *kobj,
> > +				     struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->dec_tlen);
> > +}
> > +
> > +static ssize_t fcrypto_stat_verify_cnt(struct kobject *kobj,
> > +				       struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->verify_cnt);
> > +}
> > +
> > +static ssize_t fcrypto_stat_sign_cnt(struct kobject *kobj,
> > +				     struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	return snprintf(buf, 9, "%lu\n", alg->sign_cnt);
> > +}
> > +
> > +static ssize_t fcrypto_stat_type(struct kobject *kobj,
> > +				 struct kobj_attribute *attr, char *buf)
> > +{
> > +	struct crypto_alg *alg;
> > +	u32 type;
> > +
> > +	alg = container_of(kobj, struct crypto_alg, cra_stat_obj);
> > +	type = (alg->cra_flags & CRYPTO_ALG_TYPE_MASK);
> > +	if (type == CRYPTO_ALG_TYPE_ABLKCIPHER ||
> > +	    type == CRYPTO_ALG_TYPE_SKCIPHER ||
> > +	    type == CRYPTO_ALG_TYPE_CIPHER ||
> > +	    type == CRYPTO_ALG_TYPE_BLKCIPHER
> > +		)
> > +		return snprintf(buf, 9, "cipher\n");
> 
> "skcipher" ?

Fixed!

Thanks for your review.
Regards
Corentin Labbe

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name
  2017-12-21 12:35     ` LABBE Corentin
@ 2017-12-21 20:05       ` LABBE Corentin
  0 siblings, 0 replies; 13+ messages in thread
From: LABBE Corentin @ 2017-12-21 20:05 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, nhorman, linux-crypto, linux-kernel

On Thu, Dec 21, 2017 at 01:35:27PM +0100, LABBE Corentin wrote:
> On Thu, Dec 21, 2017 at 05:35:22PM +1100, Herbert Xu wrote:
> > On Wed, Dec 20, 2017 at 08:09:25PM +0000, Corentin Labbe wrote:
> > > Each crypto algorithm "cra_name" can have multiple implementation called
> > > "cra_driver_name".
> > > If two different implementation have the same cra_driver_name, nothing
> > > can easily differentiate them.
> > > Furthermore the mechanism for getting a crypto algorithm with its
> > > implementation name (crypto_alg_match() in crypto/crypto_user.c) will
> > > get only the first one found.
> > > 
> > > So this patch prevent the registration of two implementation with the
> > > same cra_driver_name.
> > > 
> > > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> > 
> > No this is intentional.  The idea is that you can hot-replace
> > an implementation by registering a new version of it while the
> > old one is still in use.  The new one will be used for all new
> > allocations.
> > 
> 
> But the new implementation is different from the first so should have a new name.
> The only case I found is ctr-aes-ce, and both are different (use/dontuse simd) so qualifying for different name.
> 
> Anyway, any advice on how to populate properly /sys/crypto with unique name ?
> I have two idea:
> - A number which increment after each register
> - cra_driver_name-priority
> 
> Or does I use /sys/crypto/cra_driver_name/priority ? (which need to use some usage count on cra_driver_name node)

I just see that kobject already have reference counting so this solution is the better.

Regards

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics
  2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
  2017-12-20 23:37   ` Randy Dunlap
  2017-12-21  6:38   ` Stephan Mueller
@ 2017-12-22  6:38   ` Herbert Xu
  2 siblings, 0 replies; 13+ messages in thread
From: Herbert Xu @ 2017-12-22  6:38 UTC (permalink / raw)
  To: Corentin Labbe; +Cc: davem, nhorman, linux-crypto, linux-kernel

On Wed, Dec 20, 2017 at 08:09:26PM +0000, Corentin Labbe wrote:
> This patch implement a generic way to get statistics about all crypto
> usages.
> 
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

Please don't use sysfs.  We already have crypto_user and this
should be exposed through that.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-12-22  6:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-20 20:09 [PATCH RFC 0/3] crypto: Implement a generic crypto statistics Corentin Labbe
2017-12-20 20:09 ` [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name Corentin Labbe
2017-12-21  6:27   ` Stephan Mueller
2017-12-21  6:35   ` Herbert Xu
2017-12-21 12:35     ` LABBE Corentin
2017-12-21 20:05       ` LABBE Corentin
2017-12-20 20:09 ` [PATCH RFC 2/3] crypto: Implement a generic crypto statistics Corentin Labbe
2017-12-20 23:37   ` Randy Dunlap
2017-12-21  6:38   ` Stephan Mueller
2017-12-21 20:03     ` LABBE Corentin
2017-12-22  6:38   ` Herbert Xu
2017-12-20 20:09 ` [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace Corentin Labbe
2017-12-20 23:37   ` Randy Dunlap

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.