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 0000000000000000000000000000000000000000..d204f017b830d15bb9609570e39fb11e34676203 --- /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 + 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 0000000000000000000000000000000000000000..5aed7d626edf019c94c64939f35357274e39136a --- /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 + diff --git a/qemu.spec b/qemu.spec index 2d8081e75abe6458efd7a32349c6a27ebe172eb9..709632d7765154adfe528f13fa32869d77d538c8 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 @@ -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 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 0000000000000000000000000000000000000000..51f6b70461b0ade8ff80b2f8ac0302546593228f --- /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 + 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 0000000000000000000000000000000000000000..8841294a8a43877948bd2c74228794aafdaf0114 --- /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 +