From ed784c457a9b70395cabba3ee341a1f64205c0d9 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Mon, 14 Oct 2019 17:28:27 +0100 Subject: [PATCH 1/6] crypto: add support for nettle's native XTS impl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nettle 3.5.0 will add support for the XTS mode. Use this because long term we wish to delete QEMU's XTS impl to avoid carrying private crypto algorithm impls. Unfortunately this degrades nettle performance from 612 MB/s to 568 MB/s as nettle's XTS impl isn't so well optimized yet. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Signed-off-by: Daniel P. Berrangé --- ...support-for-nettle-s-native-XTS-impl.patch | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 crypto-add-support-for-nettle-s-native-XTS-impl.patch diff --git a/crypto-add-support-for-nettle-s-native-XTS-impl.patch b/crypto-add-support-for-nettle-s-native-XTS-impl.patch new file mode 100644 index 0000000..5aed7d6 --- /dev/null +++ b/crypto-add-support-for-nettle-s-native-XTS-impl.patch @@ -0,0 +1,126 @@ +From c4db6fcb2c45b800cd46e088f8265ccc0631b6fc Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Mon, 14 Oct 2019 17:28:27 +0100 +Subject: [PATCH] crypto: add support for nettle's native XTS impl +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Nettle 3.5.0 will add support for the XTS mode. Use this because long +term we wish to delete QEMU's XTS impl to avoid carrying private crypto +algorithm impls. + +Unfortunately this degrades nettle performance from 612 MB/s to 568 MB/s +as nettle's XTS impl isn't so well optimized yet. + +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Stefano Garzarella +Signed-off-by: Daniel P. Berrangé +--- + configure | 18 ++++++++++++++++++ + crypto/cipher-nettle.c | 18 ++++++++++++++++++ + 2 files changed, 36 insertions(+) + +diff --git a/configure b/configure +index 577533e9ed..5dcaac3b95 100755 +--- a/configure ++++ b/configure +@@ -473,6 +473,7 @@ gtk_gl="no" + tls_priority="NORMAL" + gnutls="" + nettle="" ++nettle_xts="no" + gcrypt="" + gcrypt_hmac="no" + auth_pam="" +@@ -2918,6 +2919,19 @@ if test "$nettle" != "no"; then + pass="yes" + fi + fi ++ if test "$pass" = "yes" ++ then ++ cat > $TMPC << EOF ++#include ++int main(void) { ++ return 0; ++} ++EOF ++ if compile_prog "$nettle_cflags" "$nettle_libs" ; then ++ nettle_xts=yes ++ qemu_private_xts=no ++ fi ++ fi + if test "$pass" = "no" && test "$nettle" = "yes"; then + feature_not_found "nettle" "Install nettle devel >= 2.7.1" + else +@@ -6391,6 +6405,10 @@ echo "TLS priority $tls_priority" + echo "GNUTLS support $gnutls" + echo "libgcrypt $gcrypt" + echo "nettle $nettle $(echo_version $nettle $nettle_version)" ++if test "$nettle" = "yes" ++then ++ echo " XTS $nettle_xts" ++fi + echo "libtasn1 $tasn1" + echo "PAM $auth_pam" + echo "iconv support $iconv" +diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c +index d7411bb8ff..7e9a4cc199 100644 +--- a/crypto/cipher-nettle.c ++++ b/crypto/cipher-nettle.c +@@ -19,7 +19,9 @@ + */ + + #include "qemu/osdep.h" ++#ifdef CONFIG_QEMU_PRIVATE_XTS + #include "crypto/xts.h" ++#endif + #include "cipherpriv.h" + + #include +@@ -30,6 +32,9 @@ + #include + #include + #include ++#ifndef CONFIG_QEMU_PRIVATE_XTS ++#include ++#endif + + typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx, + size_t length, +@@ -626,9 +631,15 @@ qcrypto_nettle_cipher_encrypt(QCryptoCipher *cipher, + break; + + case QCRYPTO_CIPHER_MODE_XTS: ++#ifdef CONFIG_QEMU_PRIVATE_XTS + xts_encrypt(ctx->ctx, ctx->ctx_tweak, + ctx->alg_encrypt_wrapper, ctx->alg_encrypt_wrapper, + ctx->iv, len, out, in); ++#else ++ xts_encrypt_message(ctx->ctx, ctx->ctx_tweak, ++ ctx->alg_encrypt_native, ++ ctx->iv, len, out, in); ++#endif + break; + + case QCRYPTO_CIPHER_MODE_CTR: +@@ -673,9 +684,16 @@ qcrypto_nettle_cipher_decrypt(QCryptoCipher *cipher, + break; + + case QCRYPTO_CIPHER_MODE_XTS: ++#ifdef CONFIG_QEMU_PRIVATE_XTS + xts_decrypt(ctx->ctx, ctx->ctx_tweak, + ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper, + ctx->iv, len, out, in); ++#else ++ xts_decrypt_message(ctx->ctx, ctx->ctx_tweak, ++ ctx->alg_decrypt_native, ++ ctx->alg_encrypt_native, ++ ctx->iv, len, out, in); ++#endif + break; + case QCRYPTO_CIPHER_MODE_CTR: + ctr_crypt(ctx->ctx, ctx->alg_encrypt_native, +-- +2.27.0 + -- Gitee From 460316109fc3edc72a165e4f6905598b865bec24 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Mon, 14 Oct 2019 17:28:27 +0100 Subject: [PATCH 2/6] crypto: add support for gcrypt's native XTS impl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Libgcrypt 1.8.0 added support for the XTS mode. Use this because long term we wish to delete QEMU's XTS impl to avoid carrying private crypto algorithm impls. As an added benefit, using this improves performance from 531 MB/sec to 670 MB/sec, since we are avoiding several layers of function call indirection. This is even more noticable with the gcrypt builds in Fedora or RHEL-8 which have a non-upstream patch for FIPS mode which does mutex locking. This is catastrophic for encryption performance with small block sizes, meaning this patch improves encryption from 240 MB/sec to 670 MB/sec. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Signed-off-by: Daniel P. Berrangé --- ...support-for-gcrypt-s-native-XTS-impl.patch | 346 ++++++++++++++++++ 1 file changed, 346 insertions(+) create mode 100644 crypto-add-support-for-gcrypt-s-native-XTS-impl.patch diff --git a/crypto-add-support-for-gcrypt-s-native-XTS-impl.patch b/crypto-add-support-for-gcrypt-s-native-XTS-impl.patch new file mode 100644 index 0000000..d204f01 --- /dev/null +++ b/crypto-add-support-for-gcrypt-s-native-XTS-impl.patch @@ -0,0 +1,346 @@ +From 84352558eec97cfb0e4517fbb53d75d9f15cbcf9 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Mon, 14 Oct 2019 17:28:27 +0100 +Subject: [PATCH] crypto: add support for gcrypt's native XTS impl +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Libgcrypt 1.8.0 added support for the XTS mode. Use this because long +term we wish to delete QEMU's XTS impl to avoid carrying private crypto +algorithm impls. + +As an added benefit, using this improves performance from 531 MB/sec to +670 MB/sec, since we are avoiding several layers of function call +indirection. + +This is even more noticable with the gcrypt builds in Fedora or RHEL-8 +which have a non-upstream patch for FIPS mode which does mutex locking. +This is catastrophic for encryption performance with small block sizes, +meaning this patch improves encryption from 240 MB/sec to 670 MB/sec. + +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Stefano Garzarella +Signed-off-by: Daniel P. Berrangé +--- + configure | 22 ++++++++++ + crypto/Makefile.objs | 2 +- + crypto/cipher-gcrypt.c | 97 ++++++++++++++++++++++++++++-------------- + tests/Makefile.include | 2 +- + 4 files changed, 88 insertions(+), 35 deletions(-) + +diff --git a/configure b/configure +index 5dcaac3b95..a88cdd5109 100755 +--- a/configure ++++ b/configure +@@ -476,6 +476,8 @@ nettle="" + nettle_xts="no" + gcrypt="" + gcrypt_hmac="no" ++gcrypt_xts="no" ++qemu_private_xts="yes" + auth_pam="" + vte="" + virglrenderer="" +@@ -2974,6 +2976,18 @@ EOF + if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then + gcrypt_hmac=yes + fi ++ cat > $TMPC << EOF ++#include ++int main(void) { ++ gcry_cipher_hd_t handle; ++ gcry_cipher_open(&handle, GCRY_CIPHER_AES, GCRY_CIPHER_MODE_XTS, 0); ++ return 0; ++} ++EOF ++ if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then ++ gcrypt_xts=yes ++ qemu_private_xts=no ++ fi + elif test "$gcrypt" = "yes"; then + feature_not_found "gcrypt" "Install gcrypt devel >= 1.5.0" + else +@@ -6404,6 +6418,11 @@ echo "VTE support $vte $(echo_version $vte $vteversion)" + echo "TLS priority $tls_priority" + echo "GNUTLS support $gnutls" + echo "libgcrypt $gcrypt" ++if test "$gcrypt" = "yes" ++then ++ echo " hmac $gcrypt_hmac" ++ echo " XTS $gcrypt_xts" ++fi + echo "nettle $nettle $(echo_version $nettle $nettle_version)" + if test "$nettle" = "yes" + then +@@ -6889,6 +6908,9 @@ if test "$nettle" = "yes" ; then + echo "CONFIG_NETTLE=y" >> $config_host_mak + echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak + fi ++if test "$qemu_private_xts" = "yes" ; then ++ echo "CONFIG_QEMU_PRIVATE_XTS=y" >> $config_host_mak ++fi + if test "$tasn1" = "yes" ; then + echo "CONFIG_TASN1=y" >> $config_host_mak + fi +diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs +index 7fe2fa9da2..cdb01f9de9 100644 +--- a/crypto/Makefile.objs ++++ b/crypto/Makefile.objs +@@ -31,7 +31,7 @@ crypto-obj-y += ivgen-essiv.o + crypto-obj-y += ivgen-plain.o + crypto-obj-y += ivgen-plain64.o + crypto-obj-y += afsplit.o +-crypto-obj-y += xts.o ++crypto-obj-$(CONFIG_QEMU_PRIVATE_XTS) += xts.o + crypto-obj-y += block.o + crypto-obj-y += block-qcow.o + crypto-obj-y += block-luks.o +diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c +index 5cece9b244..2864099527 100644 +--- a/crypto/cipher-gcrypt.c ++++ b/crypto/cipher-gcrypt.c +@@ -19,7 +19,9 @@ + */ + + #include "qemu/osdep.h" ++#ifdef CONFIG_QEMU_PRIVATE_XTS + #include "crypto/xts.h" ++#endif + #include "cipherpriv.h" + + #include +@@ -59,10 +61,12 @@ bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg, + typedef struct QCryptoCipherGcrypt QCryptoCipherGcrypt; + struct QCryptoCipherGcrypt { + gcry_cipher_hd_t handle; +- gcry_cipher_hd_t tweakhandle; + size_t blocksize; ++#ifdef CONFIG_QEMU_PRIVATE_XTS ++ gcry_cipher_hd_t tweakhandle; + /* Initialization vector or Counter */ + uint8_t *iv; ++#endif + }; + + static void +@@ -74,10 +78,12 @@ qcrypto_gcrypt_cipher_free_ctx(QCryptoCipherGcrypt *ctx, + } + + gcry_cipher_close(ctx->handle); ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (mode == QCRYPTO_CIPHER_MODE_XTS) { + gcry_cipher_close(ctx->tweakhandle); + } + g_free(ctx->iv); ++#endif + g_free(ctx); + } + +@@ -94,8 +100,14 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + + switch (mode) { + case QCRYPTO_CIPHER_MODE_ECB: ++ gcrymode = GCRY_CIPHER_MODE_ECB; ++ break; + case QCRYPTO_CIPHER_MODE_XTS: ++#ifdef CONFIG_QEMU_PRIVATE_XTS + gcrymode = GCRY_CIPHER_MODE_ECB; ++#else ++ gcrymode = GCRY_CIPHER_MODE_XTS; ++#endif + break; + case QCRYPTO_CIPHER_MODE_CBC: + gcrymode = GCRY_CIPHER_MODE_CBC; +@@ -172,6 +184,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + gcry_strerror(err)); + goto error; + } ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (mode == QCRYPTO_CIPHER_MODE_XTS) { + err = gcry_cipher_open(&ctx->tweakhandle, gcryalg, gcrymode, 0); + if (err != 0) { +@@ -180,6 +193,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + goto error; + } + } ++#endif + + if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) { + /* We're using standard DES cipher from gcrypt, so we need +@@ -191,6 +205,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + g_free(rfbkey); + ctx->blocksize = 8; + } else { ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (mode == QCRYPTO_CIPHER_MODE_XTS) { + nkey /= 2; + err = gcry_cipher_setkey(ctx->handle, key, nkey); +@@ -201,8 +216,11 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + } + err = gcry_cipher_setkey(ctx->tweakhandle, key + nkey, nkey); + } else { ++#endif + err = gcry_cipher_setkey(ctx->handle, key, nkey); ++#ifdef CONFIG_QEMU_PRIVATE_XTS + } ++#endif + if (err != 0) { + error_setg(errp, "Cannot set key: %s", + gcry_strerror(err)); +@@ -228,6 +246,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + } + } + ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (mode == QCRYPTO_CIPHER_MODE_XTS) { + if (ctx->blocksize != XTS_BLOCK_SIZE) { + error_setg(errp, +@@ -237,6 +256,7 @@ static QCryptoCipherGcrypt *qcrypto_cipher_ctx_new(QCryptoCipherAlgorithm alg, + } + ctx->iv = g_new0(uint8_t, ctx->blocksize); + } ++#endif + + return ctx; + +@@ -253,6 +273,7 @@ qcrypto_gcrypt_cipher_ctx_free(QCryptoCipher *cipher) + } + + ++#ifdef CONFIG_QEMU_PRIVATE_XTS + static void qcrypto_gcrypt_xts_encrypt(const void *ctx, + size_t length, + uint8_t *dst, +@@ -272,6 +293,7 @@ static void qcrypto_gcrypt_xts_decrypt(const void *ctx, + err = gcry_cipher_decrypt((gcry_cipher_hd_t)ctx, dst, length, src, length); + g_assert(err == 0); + } ++#endif + + static int + qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher, +@@ -289,20 +311,23 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher, + return -1; + } + ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) { + xts_encrypt(ctx->handle, ctx->tweakhandle, + qcrypto_gcrypt_xts_encrypt, + qcrypto_gcrypt_xts_decrypt, + ctx->iv, len, out, in); +- } else { +- err = gcry_cipher_encrypt(ctx->handle, +- out, len, +- in, len); +- if (err != 0) { +- error_setg(errp, "Cannot encrypt data: %s", +- gcry_strerror(err)); +- return -1; +- } ++ return 0; ++ } ++#endif ++ ++ err = gcry_cipher_encrypt(ctx->handle, ++ out, len, ++ in, len); ++ if (err != 0) { ++ error_setg(errp, "Cannot encrypt data: %s", ++ gcry_strerror(err)); ++ return -1; + } + + return 0; +@@ -325,20 +350,23 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher, + return -1; + } + ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (cipher->mode == QCRYPTO_CIPHER_MODE_XTS) { + xts_decrypt(ctx->handle, ctx->tweakhandle, + qcrypto_gcrypt_xts_encrypt, + qcrypto_gcrypt_xts_decrypt, + ctx->iv, len, out, in); +- } else { +- err = gcry_cipher_decrypt(ctx->handle, +- out, len, +- in, len); +- if (err != 0) { +- error_setg(errp, "Cannot decrypt data: %s", +- gcry_strerror(err)); +- return -1; +- } ++ return 0; ++ } ++#endif ++ ++ err = gcry_cipher_decrypt(ctx->handle, ++ out, len, ++ in, len); ++ if (err != 0) { ++ error_setg(errp, "Cannot decrypt data: %s", ++ gcry_strerror(err)); ++ return -1; + } + + return 0; +@@ -358,24 +386,27 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, + return -1; + } + ++#ifdef CONFIG_QEMU_PRIVATE_XTS + if (ctx->iv) { + memcpy(ctx->iv, iv, niv); +- } else { +- if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) { +- err = gcry_cipher_setctr(ctx->handle, iv, niv); +- if (err != 0) { +- error_setg(errp, "Cannot set Counter: %s", ++ return 0; ++ } ++#endif ++ ++ if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) { ++ err = gcry_cipher_setctr(ctx->handle, iv, niv); ++ if (err != 0) { ++ error_setg(errp, "Cannot set Counter: %s", + gcry_strerror(err)); +- return -1; +- } +- } else { +- gcry_cipher_reset(ctx->handle); +- err = gcry_cipher_setiv(ctx->handle, iv, niv); +- if (err != 0) { +- error_setg(errp, "Cannot set IV: %s", ++ return -1; ++ } ++ } else { ++ gcry_cipher_reset(ctx->handle); ++ err = gcry_cipher_setiv(ctx->handle, iv, niv); ++ if (err != 0) { ++ error_setg(errp, "Cannot set IV: %s", + gcry_strerror(err)); +- return -1; +- } ++ return -1; + } + } + +diff --git a/tests/Makefile.include b/tests/Makefile.include +index d6de4e1042..3be60ab999 100644 +--- a/tests/Makefile.include ++++ b/tests/Makefile.include +@@ -132,7 +132,7 @@ check-unit-y += tests/test-base64$(EXESUF) + check-unit-$(call land,$(CONFIG_BLOCK),$(if $(CONFIG_NETTLE),y,$(CONFIG_GCRYPT))) += tests/test-crypto-pbkdf$(EXESUF) + check-unit-$(CONFIG_BLOCK) += tests/test-crypto-ivgen$(EXESUF) + check-unit-$(CONFIG_BLOCK) += tests/test-crypto-afsplit$(EXESUF) +-check-unit-$(CONFIG_BLOCK) += tests/test-crypto-xts$(EXESUF) ++check-unit-$(if $(CONFIG_BLOCK),$(CONFIG_QEMU_PRIVATE_XTS)) += tests/test-crypto-xts$(EXESUF) + check-unit-$(CONFIG_BLOCK) += tests/test-crypto-block$(EXESUF) + check-unit-y += tests/test-logging$(EXESUF) + check-unit-$(call land,$(CONFIG_BLOCK),$(CONFIG_REPLICATION)) += tests/test-replication$(EXESUF) +-- +2.27.0 + -- Gitee From f55d31032c2259f83967ae45e6e01706f733295f Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Thu, 17 Oct 2019 14:22:19 +0100 Subject: [PATCH 3/6] tests: benchmark crypto with fixed data size, not time period MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently the crypto benchmarks are processing data in varying chunk sizes, over a fixed time period. This turns out to be a terrible idea because with small chunk sizes the overhead of checking the elapsed time on each loop iteration masks the true performance. Benchmarking over a fixed data size avoids the loop running any system calls which can interfere with the performance measurements. Before this change Enc chunk 512 bytes 2283.47 MB/sec Dec chunk 512 bytes 2236.23 MB/sec OK Enc chunk 4096 bytes 2744.97 MB/sec Dec chunk 4096 bytes 2614.71 MB/sec OK Enc chunk 16384 bytes 2777.53 MB/sec Dec chunk 16384 bytes 2678.44 MB/sec OK Enc chunk 65536 bytes 2809.34 MB/sec Dec chunk 65536 bytes 2699.47 MB/sec OK After this change Enc chunk 512 bytes 2058.22 MB/sec Dec chunk 512 bytes 2030.11 MB/sec OK Enc chunk 4096 bytes 2699.27 MB/sec Dec chunk 4096 bytes 2573.78 MB/sec OK Enc chunk 16384 bytes 2748.52 MB/sec Dec chunk 16384 bytes 2653.76 MB/sec OK Enc chunk 65536 bytes 2814.08 MB/sec Dec chunk 65536 bytes 2712.74 MB/sec OK The actual crypto performance hasn't changed, which shows how significant the mis-measurement has been for small data sizes. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Signed-off-by: Daniel P. Berrangé --- ...crypto-with-fixed-data-size-not-time.patch | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 tests-benchmark-crypto-with-fixed-data-size-not-time.patch diff --git a/tests-benchmark-crypto-with-fixed-data-size-not-time.patch b/tests-benchmark-crypto-with-fixed-data-size-not-time.patch new file mode 100644 index 0000000..8841294 --- /dev/null +++ b/tests-benchmark-crypto-with-fixed-data-size-not-time.patch @@ -0,0 +1,150 @@ +From c151519a7f5c08dde9a32534bc485588a5793967 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Thu, 17 Oct 2019 14:22:19 +0100 +Subject: [PATCH] tests: benchmark crypto with fixed data size, not time period +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Currently the crypto benchmarks are processing data in varying chunk +sizes, over a fixed time period. This turns out to be a terrible idea +because with small chunk sizes the overhead of checking the elapsed +time on each loop iteration masks the true performance. + +Benchmarking over a fixed data size avoids the loop running any system +calls which can interfere with the performance measurements. + +Before this change + +Enc chunk 512 bytes 2283.47 MB/sec Dec chunk 512 bytes 2236.23 MB/sec OK +Enc chunk 4096 bytes 2744.97 MB/sec Dec chunk 4096 bytes 2614.71 MB/sec OK +Enc chunk 16384 bytes 2777.53 MB/sec Dec chunk 16384 bytes 2678.44 MB/sec OK +Enc chunk 65536 bytes 2809.34 MB/sec Dec chunk 65536 bytes 2699.47 MB/sec OK + +After this change + +Enc chunk 512 bytes 2058.22 MB/sec Dec chunk 512 bytes 2030.11 MB/sec OK +Enc chunk 4096 bytes 2699.27 MB/sec Dec chunk 4096 bytes 2573.78 MB/sec OK +Enc chunk 16384 bytes 2748.52 MB/sec Dec chunk 16384 bytes 2653.76 MB/sec OK +Enc chunk 65536 bytes 2814.08 MB/sec Dec chunk 65536 bytes 2712.74 MB/sec OK + +The actual crypto performance hasn't changed, which shows how +significant the mis-measurement has been for small data sizes. + +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Stefano Garzarella +Signed-off-by: Daniel P. Berrangé +--- + tests/benchmark-crypto-cipher.c | 26 ++++++++++++++------------ + tests/benchmark-crypto-hash.c | 17 +++++++++-------- + 2 files changed, 23 insertions(+), 20 deletions(-) + +diff --git a/tests/benchmark-crypto-cipher.c b/tests/benchmark-crypto-cipher.c +index 67fdf8c31d..cb6b7200a5 100644 +--- a/tests/benchmark-crypto-cipher.c ++++ b/tests/benchmark-crypto-cipher.c +@@ -21,11 +21,12 @@ static void test_cipher_speed(size_t chunk_size, + { + QCryptoCipher *cipher; + Error *err = NULL; +- double total = 0.0; + uint8_t *key = NULL, *iv = NULL; + uint8_t *plaintext = NULL, *ciphertext = NULL; + size_t nkey; + size_t niv; ++ const size_t total = 2 * GiB; ++ size_t remain; + + if (!qcrypto_cipher_supports(alg, mode)) { + return; +@@ -58,33 +59,34 @@ static void test_cipher_speed(size_t chunk_size, + &err) == 0); + + g_test_timer_start(); +- do { ++ remain = total; ++ while (remain) { + g_assert(qcrypto_cipher_encrypt(cipher, + plaintext, + ciphertext, + chunk_size, + &err) == 0); +- total += chunk_size; +- } while (g_test_timer_elapsed() < 1.0); ++ remain -= chunk_size; ++ } ++ g_test_timer_elapsed(); + +- total /= MiB; + g_print("Enc chunk %zu bytes ", chunk_size); +- g_print("%.2f MB/sec ", total / g_test_timer_last()); ++ g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last()); + +- total = 0.0; + g_test_timer_start(); +- do { ++ remain = total; ++ while (remain) { + g_assert(qcrypto_cipher_decrypt(cipher, + plaintext, + ciphertext, + chunk_size, + &err) == 0); +- total += chunk_size; +- } while (g_test_timer_elapsed() < 1.0); ++ remain -= chunk_size; ++ } ++ g_test_timer_elapsed(); + +- total /= MiB; + g_print("Dec chunk %zu bytes ", chunk_size); +- g_print("%.2f MB/sec ", total / g_test_timer_last()); ++ g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last()); + + qcrypto_cipher_free(cipher); + g_free(plaintext); +diff --git a/tests/benchmark-crypto-hash.c b/tests/benchmark-crypto-hash.c +index 9b6f7a9155..7f659f7323 100644 +--- a/tests/benchmark-crypto-hash.c ++++ b/tests/benchmark-crypto-hash.c +@@ -20,7 +20,8 @@ static void test_hash_speed(const void *opaque) + size_t chunk_size = (size_t)opaque; + uint8_t *in = NULL, *out = NULL; + size_t out_len = 0; +- double total = 0.0; ++ const size_t total = 2 * GiB; ++ size_t remain; + struct iovec iov; + int ret; + +@@ -31,20 +32,20 @@ static void test_hash_speed(const void *opaque) + iov.iov_len = chunk_size; + + g_test_timer_start(); +- do { ++ remain = total; ++ while (remain) { + ret = qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256, + &iov, 1, &out, &out_len, + NULL); + g_assert(ret == 0); + +- total += chunk_size; +- } while (g_test_timer_elapsed() < 5.0); ++ remain -= chunk_size; ++ } ++ g_test_timer_elapsed(); + +- total /= MiB; + g_print("sha256: "); +- g_print("Testing chunk_size %zu bytes ", chunk_size); +- g_print("done: %.2f MB in %.2f secs: ", total, g_test_timer_last()); +- g_print("%.2f MB/sec\n", total / g_test_timer_last()); ++ g_print("Hash %zu GB chunk size %zu bytes ", total / GiB, chunk_size); ++ g_print("%.2f MB/sec ", (double)total / MiB / g_test_timer_last()); + + g_free(out); + g_free(in); +-- +2.27.0 + -- Gitee From 00355207209fd1e7a9fe286be40213520404bfa0 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 15 Oct 2019 11:19:29 +0100 Subject: [PATCH 4/6] tests: allow filtering crypto cipher benchmark tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for specifying a cipher mode and chunk size as argv to filter which combinations are benchmarked. For example to only benchmark XTS mode with 512 byte chunks: ./tests/benchmark-crypto-cipher xts 512 Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefano Garzarella Signed-off-by: Daniel P. Berrangé --- ...tering-crypto-cipher-benchmark-tests.patch | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests-allow-filtering-crypto-cipher-benchmark-tests.patch diff --git a/tests-allow-filtering-crypto-cipher-benchmark-tests.patch b/tests-allow-filtering-crypto-cipher-benchmark-tests.patch new file mode 100644 index 0000000..51f6b70 --- /dev/null +++ b/tests-allow-filtering-crypto-cipher-benchmark-tests.patch @@ -0,0 +1,56 @@ +From c2a6b4b3204aef2efc39f1b59bc110b54ca24587 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Tue, 15 Oct 2019 11:19:29 +0100 +Subject: [PATCH] tests: allow filtering crypto cipher benchmark tests +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Add support for specifying a cipher mode and chunk size as argv to +filter which combinations are benchmarked. For example to only +benchmark XTS mode with 512 byte chunks: + + ./tests/benchmark-crypto-cipher xts 512 + +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Stefano Garzarella +Signed-off-by: Daniel P. Berrangé +--- + tests/benchmark-crypto-cipher.c | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/tests/benchmark-crypto-cipher.c b/tests/benchmark-crypto-cipher.c +index cb6b7200a5..53032334ec 100644 +--- a/tests/benchmark-crypto-cipher.c ++++ b/tests/benchmark-crypto-cipher.c +@@ -163,15 +163,26 @@ static void test_cipher_speed_xts_aes_256(const void *opaque) + + int main(int argc, char **argv) + { ++ char *alg = NULL; ++ char *size = NULL; + g_test_init(&argc, &argv, NULL); + g_assert(qcrypto_init(NULL) == 0); + + #define ADD_TEST(mode, cipher, keysize, chunk) \ +- g_test_add_data_func( \ ++ if ((!alg || g_str_equal(alg, #mode)) && \ ++ (!size || g_str_equal(size, #chunk))) \ ++ g_test_add_data_func( \ + "/crypto/cipher/" #mode "-" #cipher "-" #keysize "/chunk-" #chunk, \ + (void *)chunk, \ + test_cipher_speed_ ## mode ## _ ## cipher ## _ ## keysize) + ++ if (argc >= 2) { ++ alg = argv[1]; ++ } ++ if (argc >= 3) { ++ size = argv[2]; ++ } ++ + #define ADD_TESTS(chunk) \ + do { \ + ADD_TEST(ecb, aes, 128, chunk); \ +-- +2.27.0 + -- Gitee From c2924fc4eecc24524f41f641bee78384dbad0281 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 20 Jul 2021 21:27:28 +0800 Subject: [PATCH 5/6] spec: Update patch and changelog with !162 block/crypto: improved performance for AES-XTS encryption for LUKS disk encryption !162 crypto: add support for nettle's native XTS impl crypto: add support for gcrypt's native XTS impl tests: benchmark crypto with fixed data size, not time period tests: allow filtering crypto cipher benchmark tests Signed-off-by: Chen Qun --- qemu.spec | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/qemu.spec b/qemu.spec index 2d8081e..57173f6 100644 --- a/qemu.spec +++ b/qemu.spec @@ -349,6 +349,10 @@ Patch0336: target-i386-Export-TAA_NO-bit-to-guests.patch Patch0337: target-i386-Introduce-Denverton-CPU-model.patch Patch0338: target-i386-Add-Snowridge-v2-no-MPX-CPU-model.patch Patch0339: i386-Add-CPUID-bit-for-CLZERO-and-XSAVEERPTR.patch +Patch0340: crypto-add-support-for-nettle-s-native-XTS-impl.patch +Patch0341: crypto-add-support-for-gcrypt-s-native-XTS-impl.patch +Patch0342: tests-benchmark-crypto-with-fixed-data-size-not-time.patch +Patch0343: tests-allow-filtering-crypto-cipher-benchmark-tests.patch BuildRequires: flex BuildRequires: gcc @@ -743,6 +747,12 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Tue Jul 20 2021 Chen Qun +- crypto: add support for nettle's native XTS impl +- crypto: add support for gcrypt's native XTS impl +- tests: benchmark crypto with fixed data size, not time period +- tests: allow filtering crypto cipher benchmark tests + * Tue Jul 20 2021 Chen Qun - target/i386: Introduce Denverton CPU model - target/i386: Add Snowridge-v2 (no MPX) CPU model -- Gitee From 8dd195e99a51d315b73974b6a0a5aa2eac683bad Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 20 Jul 2021 21:27:28 +0800 Subject: [PATCH 6/6] spec: Update release version with !162 increase release verison by one Signed-off-by: Chen Qun --- qemu.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu.spec b/qemu.spec index 57173f6..709632d 100644 --- a/qemu.spec +++ b/qemu.spec @@ -1,6 +1,6 @@ Name: qemu Version: 4.1.0 -Release: 63 +Release: 64 Epoch: 2 Summary: QEMU is a generic and open source machine emulator and virtualizer License: GPLv2 and BSD and MIT and CC-BY-SA-4.0 -- Gitee