qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL 0/4] Crypto luks patches
@ 2019-10-28 15:49 Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-10-28 15:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé

The following changes since commit 187f35512106501fe9a11057f4d8705431e0026d:

  Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-251019-3' into staging (2019-10-26 10:13:48 +0100)

are available in the Git repository at:

  https://github.com/berrange/qemu tags/crypto-luks-pull-request

for you to fetch changes up to dc2207af2de162005f7e9e534850d07232290cee:

  crypto: add support for nettle's native XTS impl (2019-10-28 16:45:07 +0100)

----------------------------------------------------------------
crypto: improve performance of ciphers in XTS mode

Currently QEMU uses its own XTS cipher mode, however, this has
relatively poor performance.

Gcrypt now includes its own XTS cipher which is at least x2 faster than
what we get with QEMU's on Fedora/RHEL hosts. With gcrypt git master, a
further x5-6 speed up is seen.

This is essential for QEMU's LUKS performance to be viable.

----------------------------------------------------------------

Daniel P. Berrangé (4):
  tests: allow filtering crypto cipher benchmark tests
  tests: benchmark crypto with fixed data size, not time period
  crypto: add support for gcrypt's native XTS impl
  crypto: add support for nettle's native XTS impl

 configure                       | 40 ++++++++++++++
 crypto/Makefile.objs            |  2 +-
 crypto/cipher-gcrypt.c          | 97 ++++++++++++++++++++++-----------
 crypto/cipher-nettle.c          | 18 ++++++
 tests/Makefile.include          |  2 +-
 tests/benchmark-crypto-cipher.c | 39 ++++++++-----
 tests/benchmark-crypto-hash.c   | 17 +++---
 7 files changed, 159 insertions(+), 56 deletions(-)

-- 
2.23.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PULL 1/4] tests: allow filtering crypto cipher benchmark tests
  2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
@ 2019-10-28 15:49 ` Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-10-28 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé, Stefano Garzarella

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é <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 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 67fdf8c31d..3ca31a2779 100644
--- a/tests/benchmark-crypto-cipher.c
+++ b/tests/benchmark-crypto-cipher.c
@@ -161,15 +161,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.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 2/4] tests: benchmark crypto with fixed data size, not time period
  2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé
@ 2019-10-28 15:49 ` Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-10-28 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé, Stefano Garzarella

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é <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 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 3ca31a2779..53032334ec 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.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 3/4] crypto: add support for gcrypt's native XTS impl
  2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé
@ 2019-10-28 15:49 ` Daniel P. Berrangé
  2019-10-28 15:49 ` [PULL 4/4] crypto: add support for nettle's " Daniel P. Berrangé
  2019-10-29 16:26 ` [PULL 0/4] Crypto luks patches Peter Maydell
  4 siblings, 0 replies; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-10-28 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé, Stefano Garzarella

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é <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 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 145fcabbb3..d1e9e457ce 100755
--- a/configure
+++ b/configure
@@ -474,6 +474,8 @@ gnutls=""
 nettle=""
 gcrypt=""
 gcrypt_hmac="no"
+gcrypt_xts="no"
+qemu_private_xts="yes"
 auth_pam=""
 vte=""
 virglrenderer=""
@@ -2911,6 +2913,18 @@ EOF
         if compile_prog "$gcrypt_cflags" "$gcrypt_libs" ; then
             gcrypt_hmac=yes
         fi
+        cat > $TMPC << EOF
+#include <gcrypt.h>
+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
@@ -6326,6 +6340,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)"
 echo "libtasn1          $tasn1"
 echo "PAM               $auth_pam"
@@ -6804,6 +6823,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 <gcrypt.h>
@@ -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 09e5b410dc..4efdd0cd6e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -140,7 +140,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.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PULL 4/4] crypto: add support for nettle's native XTS impl
  2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
                   ` (2 preceding siblings ...)
  2019-10-28 15:49 ` [PULL 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé
@ 2019-10-28 15:49 ` Daniel P. Berrangé
  2019-10-30 14:21   ` Alex Bennée
  2019-10-29 16:26 ` [PULL 0/4] Crypto luks patches Peter Maydell
  4 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-10-28 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé, Daniel P. Berrangé, Stefano Garzarella

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é <philmd@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 configure              | 18 ++++++++++++++++++
 crypto/cipher-nettle.c | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/configure b/configure
index d1e9e457ce..452c2dfe4e 100755
--- a/configure
+++ b/configure
@@ -472,6 +472,7 @@ gtk_gl="no"
 tls_priority="NORMAL"
 gnutls=""
 nettle=""
+nettle_xts="no"
 gcrypt=""
 gcrypt_hmac="no"
 gcrypt_xts="no"
@@ -2871,6 +2872,19 @@ if test "$nettle" != "no"; then
             pass="yes"
         fi
     fi
+    if test "$pass" = "yes"
+    then
+        cat > $TMPC << EOF
+#include <nettle/xts.h>
+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
@@ -6346,6 +6360,10 @@ then
    echo "  XTS             $gcrypt_xts"
 fi
 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 <nettle/nettle-types.h>
@@ -30,6 +32,9 @@
 #include <nettle/serpent.h>
 #include <nettle/twofish.h>
 #include <nettle/ctr.h>
+#ifndef CONFIG_QEMU_PRIVATE_XTS
+#include <nettle/xts.h>
+#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.23.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PULL 0/4] Crypto luks patches
  2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
                   ` (3 preceding siblings ...)
  2019-10-28 15:49 ` [PULL 4/4] crypto: add support for nettle's " Daniel P. Berrangé
@ 2019-10-29 16:26 ` Peter Maydell
  4 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2019-10-29 16:26 UTC (permalink / raw)
  To: Daniel P. Berrangé; +Cc: QEMU Developers

On Mon, 28 Oct 2019 at 15:58, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> The following changes since commit 187f35512106501fe9a11057f4d8705431e0026d:
>
>   Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-next-251019-3' into staging (2019-10-26 10:13:48 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/berrange/qemu tags/crypto-luks-pull-request
>
> for you to fetch changes up to dc2207af2de162005f7e9e534850d07232290cee:
>
>   crypto: add support for nettle's native XTS impl (2019-10-28 16:45:07 +0100)
>
> ----------------------------------------------------------------
> crypto: improve performance of ciphers in XTS mode
>
> Currently QEMU uses its own XTS cipher mode, however, this has
> relatively poor performance.
>
> Gcrypt now includes its own XTS cipher which is at least x2 faster than
> what we get with QEMU's on Fedora/RHEL hosts. With gcrypt git master, a
> further x5-6 speed up is seen.
>
> This is essential for QEMU's LUKS performance to be viable.
>
> ----------------------------------------------------------------

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PULL 4/4] crypto: add support for nettle's native XTS impl
  2019-10-28 15:49 ` [PULL 4/4] crypto: add support for nettle's " Daniel P. Berrangé
@ 2019-10-30 14:21   ` Alex Bennée
  0 siblings, 0 replies; 7+ messages in thread
From: Alex Bennée @ 2019-10-30 14:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé, Philippe Mathieu-Daudé, Stefano Garzarella


Daniel P. Berrangé <berrange@redhat.com> writes:

> 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.

Unfortunately this has broken some of the Travis tests, specifically:

  # QEMU configure log Wed 30 Oct 14:16:57 GMT 2019
  # Configured with: '../../configure' '--disable-tools' '--disable-docs' '--static' '--disable-system'

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  configure              | 18 ++++++++++++++++++
>  crypto/cipher-nettle.c | 18 ++++++++++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/configure b/configure
> index d1e9e457ce..452c2dfe4e 100755
> --- a/configure
> +++ b/configure
> @@ -472,6 +472,7 @@ gtk_gl="no"
>  tls_priority="NORMAL"
>  gnutls=""
>  nettle=""
> +nettle_xts="no"
>  gcrypt=""
>  gcrypt_hmac="no"
>  gcrypt_xts="no"
> @@ -2871,6 +2872,19 @@ if test "$nettle" != "no"; then
>              pass="yes"
>          fi
>      fi
> +    if test "$pass" = "yes"
> +    then
> +        cat > $TMPC << EOF
> +#include <nettle/xts.h>
> +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
> @@ -6346,6 +6360,10 @@ then
>     echo "  XTS             $gcrypt_xts"
>  fi
>  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 <nettle/nettle-types.h>
> @@ -30,6 +32,9 @@
>  #include <nettle/serpent.h>
>  #include <nettle/twofish.h>
>  #include <nettle/ctr.h>
> +#ifndef CONFIG_QEMU_PRIVATE_XTS
> +#include <nettle/xts.h>
> +#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,


--
Alex Bennée


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-30 14:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 15:49 [PULL 0/4] Crypto luks patches Daniel P. Berrangé
2019-10-28 15:49 ` [PULL 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé
2019-10-28 15:49 ` [PULL 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé
2019-10-28 15:49 ` [PULL 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé
2019-10-28 15:49 ` [PULL 4/4] crypto: add support for nettle's " Daniel P. Berrangé
2019-10-30 14:21   ` Alex Bennée
2019-10-29 16:26 ` [PULL 0/4] Crypto luks patches Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).