2017-12-20 20:09:49

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH RFC 0/3] crypto: Implement a generic crypto statistics

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


2017-12-20 20:09:54

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name

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 <[email protected]>
---
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

2017-12-20 20:10:11

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace

Add an example tool for getting easily crypto statistics.

Signed-off-by: Corentin Labbe <[email protected]>
---
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

2017-12-20 20:10:25

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics

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

Signed-off-by: Corentin Labbe <[email protected]>
---
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

2017-12-20 23:37:42

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH RFC 3/3] crypto: tools: Add cryptostat userspace

On 12/20/2017 12:09 PM, Corentin Labbe wrote:
> Add an example tool for getting easily crypto statistics.
>
> Signed-off-by: Corentin Labbe <[email protected]>
> ---
> 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

2017-12-20 23:37:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics

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 <[email protected]>
> ---
> 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

2017-12-21 06:28:09

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name

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

2017-12-21 06:36:00

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name

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 <[email protected]>

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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2017-12-21 06:38:43

by Stephan Müller

[permalink] [raw]
Subject: Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics

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 <[email protected]>
> ---
> 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

2017-12-21 12:35:40

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name

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 <[email protected]>
>
> 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

2017-12-21 20:03:49

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics

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 <[email protected]>
> > ---
> > 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

2017-12-21 20:05:27

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH RFC 1/3] crypto: Prevent to register duplicate cra_driver_name

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 <[email protected]>
> >
> > 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

2017-12-22 06:39:11

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH RFC 2/3] crypto: Implement a generic crypto statistics

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 <[email protected]>

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

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