* [PATCH 0/4] crypto: improve performance of ciphers in XTS mode
@ 2019-10-17 14:56 Daniel P. Berrangé
2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Daniel P. Berrangé @ 2019-10-17 14:56 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé
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 | 36 ++++++++++++++++++++++++++++-
crypto/cipher-nettle.c | 18 +++++++++++++++
tests/Makefile.include | 2 +-
tests/benchmark-crypto-cipher.c | 39 +++++++++++++++++++++-----------
tests/benchmark-crypto-hash.c | 17 +++++++-------
7 files changed, 130 insertions(+), 24 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests 2019-10-17 14:56 [PATCH 0/4] crypto: improve performance of ciphers in XTS mode Daniel P. Berrangé @ 2019-10-17 14:56 ` Daniel P. Berrangé 2019-10-25 13:24 ` Philippe Mathieu-Daudé 2019-10-25 13:45 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé ` (2 subsequent siblings) 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2019-10-17 14:56 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé 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 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.21.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests 2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé @ 2019-10-25 13:24 ` Philippe Mathieu-Daudé 2019-10-25 13:45 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Philippe Mathieu-Daudé @ 2019-10-25 13:24 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel On 10/17/19 4:56 PM, Daniel P. Berrangé wrote: > 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 > > 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); \ > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests 2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé 2019-10-25 13:24 ` Philippe Mathieu-Daudé @ 2019-10-25 13:45 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Stefano Garzarella @ 2019-10-25 13:45 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel On Thu, Oct 17, 2019 at 03:56:51PM +0100, Daniel P. Berrangé wrote: > 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 > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > tests/benchmark-crypto-cipher.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > > 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.21.0 > > -- ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period 2019-10-17 14:56 [PATCH 0/4] crypto: improve performance of ciphers in XTS mode Daniel P. Berrangé 2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé @ 2019-10-17 14:56 ` Daniel P. Berrangé 2019-10-25 13:36 ` Philippe Mathieu-Daudé 2019-10-25 13:46 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé 2019-10-17 14:56 ` [PATCH 4/4] crypto: add support for nettle's " Daniel P. Berrangé 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2019-10-17 14:56 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé 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. 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..d8db5504d4 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; + 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..67b7067223 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; + 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.21.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period 2019-10-17 14:56 ` [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé @ 2019-10-25 13:36 ` Philippe Mathieu-Daudé 2019-10-25 13:46 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Philippe Mathieu-Daudé @ 2019-10-25 13:36 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel On 10/17/19 4:56 PM, Daniel P. Berrangé wrote: > 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. > > 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..d8db5504d4 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; > + size_t total = 2 * GiB; Can you use 'const size_t total'?. > + 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..67b7067223 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; > + size_t total = 2 * GiB; Ditto: 'const size_t total'. > + 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); > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period 2019-10-17 14:56 ` [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé 2019-10-25 13:36 ` Philippe Mathieu-Daudé @ 2019-10-25 13:46 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Stefano Garzarella @ 2019-10-25 13:46 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel On Thu, Oct 17, 2019 at 03:56:52PM +0100, Daniel P. Berrangé wrote: > 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. > > 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..d8db5504d4 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; > + 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; Are we sure that total is a multiple of chunk_size? Maybe I would have increased 'done' and checked (done <= total), but maybe it doesn't matter since I think that chunk_size will always be a power of two. > + } > + 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..67b7067223 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; > + 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); Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] crypto: add support for gcrypt's native XTS impl 2019-10-17 14:56 [PATCH 0/4] crypto: improve performance of ciphers in XTS mode Daniel P. Berrangé 2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé 2019-10-17 14:56 ` [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé @ 2019-10-17 14:56 ` Daniel P. Berrangé 2019-10-25 13:31 ` Philippe Mathieu-Daudé 2019-10-25 14:03 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 4/4] crypto: add support for nettle's " Daniel P. Berrangé 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2019-10-17 14:56 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé 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. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- configure | 22 ++++++++++++++++++++++ crypto/Makefile.objs | 2 +- crypto/cipher-gcrypt.c | 36 +++++++++++++++++++++++++++++++++++- tests/Makefile.include | 2 +- 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 08ca4bcb46..98edb0ff44 100755 --- a/configure +++ b/configure @@ -473,6 +473,8 @@ gnutls="" nettle="" gcrypt="" gcrypt_hmac="no" +gcrypt_xts="no" +qemu_private_xts="yes" auth_pam="" vte="" virglrenderer="" @@ -2902,6 +2904,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 @@ -6317,6 +6331,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" @@ -6794,6 +6813,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..ace719526a 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,12 +311,14 @@ 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 { +#endif err = gcry_cipher_encrypt(ctx->handle, out, len, in, len); @@ -303,7 +327,9 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher, gcry_strerror(err)); return -1; } +#ifdef CONFIG_QEMU_PRIVATE_XTS } +#endif return 0; } @@ -325,12 +351,14 @@ 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 { +#endif err = gcry_cipher_decrypt(ctx->handle, out, len, in, len); @@ -339,7 +367,9 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher, gcry_strerror(err)); return -1; } +#ifdef CONFIG_QEMU_PRIVATE_XTS } +#endif return 0; } @@ -358,9 +388,11 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, return -1; } +#ifdef CONFIG_QEMU_PRIVATE_XTS if (ctx->iv) { memcpy(ctx->iv, iv, niv); } else { +#endif if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) { err = gcry_cipher_setctr(ctx->handle, iv, niv); if (err != 0) { @@ -377,7 +409,9 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, return -1; } } +#ifdef CONFIG_QEMU_PRIVATE_XTS } +#endif return 0; } diff --git a/tests/Makefile.include b/tests/Makefile.include index 3543451ed3..2e5b0d3604 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -133,7 +133,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.21.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] crypto: add support for gcrypt's native XTS impl 2019-10-17 14:56 ` [PATCH 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé @ 2019-10-25 13:31 ` Philippe Mathieu-Daudé 2019-10-25 14:03 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Philippe Mathieu-Daudé @ 2019-10-25 13:31 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel On 10/17/19 4:56 PM, Daniel P. Berrangé wrote: > 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. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > configure | 22 ++++++++++++++++++++++ > crypto/Makefile.objs | 2 +- > crypto/cipher-gcrypt.c | 36 +++++++++++++++++++++++++++++++++++- > tests/Makefile.include | 2 +- > 4 files changed, 59 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index 08ca4bcb46..98edb0ff44 100755 > --- a/configure > +++ b/configure > @@ -473,6 +473,8 @@ gnutls="" > nettle="" > gcrypt="" > gcrypt_hmac="no" > +gcrypt_xts="no" > +qemu_private_xts="yes" > auth_pam="" > vte="" > virglrenderer="" > @@ -2902,6 +2904,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 > @@ -6317,6 +6331,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" > @@ -6794,6 +6813,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 Why not check $gcrypt_xts = "no", so we can avoid using $qemu_private_xts? > + 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..ace719526a 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,12 +311,14 @@ 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 { > +#endif > err = gcry_cipher_encrypt(ctx->handle, > out, len, > in, len); > @@ -303,7 +327,9 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher, > gcry_strerror(err)); > return -1; > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif > > return 0; > } > @@ -325,12 +351,14 @@ 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 { > +#endif > err = gcry_cipher_decrypt(ctx->handle, > out, len, > in, len); > @@ -339,7 +367,9 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher, > gcry_strerror(err)); > return -1; > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif > > return 0; > } > @@ -358,9 +388,11 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, > return -1; > } > > +#ifdef CONFIG_QEMU_PRIVATE_XTS > if (ctx->iv) { > memcpy(ctx->iv, iv, niv); > } else { > +#endif > if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) { > err = gcry_cipher_setctr(ctx->handle, iv, niv); > if (err != 0) { > @@ -377,7 +409,9 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, > return -1; > } > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif This is a lot of ifdef'ry... > > return 0; > } > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 3543451ed3..2e5b0d3604 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -133,7 +133,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) > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] crypto: add support for gcrypt's native XTS impl 2019-10-17 14:56 ` [PATCH 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé 2019-10-25 13:31 ` Philippe Mathieu-Daudé @ 2019-10-25 14:03 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Stefano Garzarella @ 2019-10-25 14:03 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel On Thu, Oct 17, 2019 at 03:56:53PM +0100, Daniel P. Berrangé wrote: > 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. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > configure | 22 ++++++++++++++++++++++ > crypto/Makefile.objs | 2 +- > crypto/cipher-gcrypt.c | 36 +++++++++++++++++++++++++++++++++++- > tests/Makefile.include | 2 +- > 4 files changed, 59 insertions(+), 3 deletions(-) > > diff --git a/configure b/configure > index 08ca4bcb46..98edb0ff44 100755 > --- a/configure > +++ b/configure > @@ -473,6 +473,8 @@ gnutls="" > nettle="" > gcrypt="" > gcrypt_hmac="no" > +gcrypt_xts="no" > +qemu_private_xts="yes" > auth_pam="" > vte="" > virglrenderer="" > @@ -2902,6 +2904,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 > @@ -6317,6 +6331,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" > @@ -6794,6 +6813,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..ace719526a 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,12 +311,14 @@ 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); What about adding 'return 0' here and avoiding the next ifdef? > } else { > +#endif > err = gcry_cipher_encrypt(ctx->handle, > out, len, > in, len); > @@ -303,7 +327,9 @@ qcrypto_gcrypt_cipher_encrypt(QCryptoCipher *cipher, > gcry_strerror(err)); > return -1; > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif > > return 0; > } > @@ -325,12 +351,14 @@ 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); Also here... > } else { > +#endif > err = gcry_cipher_decrypt(ctx->handle, > out, len, > in, len); > @@ -339,7 +367,9 @@ qcrypto_gcrypt_cipher_decrypt(QCryptoCipher *cipher, > gcry_strerror(err)); > return -1; > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif > > return 0; > } > @@ -358,9 +388,11 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, > return -1; > } > > +#ifdef CONFIG_QEMU_PRIVATE_XTS > if (ctx->iv) { > memcpy(ctx->iv, iv, niv); And maybe here... > } else { > +#endif > if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) { > err = gcry_cipher_setctr(ctx->handle, iv, niv); > if (err != 0) { > @@ -377,7 +409,9 @@ qcrypto_gcrypt_cipher_setiv(QCryptoCipher *cipher, > return -1; > } > } > +#ifdef CONFIG_QEMU_PRIVATE_XTS > } > +#endif > > return 0; > } > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 3543451ed3..2e5b0d3604 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -133,7 +133,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) Anyway the patch LGTM, but I don't know this code very well, then: Acked-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] crypto: add support for nettle's native XTS impl 2019-10-17 14:56 [PATCH 0/4] crypto: improve performance of ciphers in XTS mode Daniel P. Berrangé ` (2 preceding siblings ...) 2019-10-17 14:56 ` [PATCH 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé @ 2019-10-17 14:56 ` Daniel P. Berrangé 2019-10-25 13:33 ` Philippe Mathieu-Daudé 2019-10-25 14:13 ` Stefano Garzarella 3 siblings, 2 replies; 13+ messages in thread From: Daniel P. Berrangé @ 2019-10-17 14:56 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel P. Berrangé 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. 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 98edb0ff44..6650c72348 100755 --- a/configure +++ b/configure @@ -471,6 +471,7 @@ gtk_gl="no" tls_priority="NORMAL" gnutls="" nettle="" +nettle_xts="no" gcrypt="" gcrypt_hmac="no" gcrypt_xts="no" @@ -2862,6 +2863,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 @@ -6337,6 +6351,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..08794a9b10 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_encrypt_native, + ctx->alg_decrypt_native, + ctx->iv, len, out, in); +#endif break; case QCRYPTO_CIPHER_MODE_CTR: ctr_crypt(ctx->ctx, ctx->alg_encrypt_native, -- 2.21.0 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] crypto: add support for nettle's native XTS impl 2019-10-17 14:56 ` [PATCH 4/4] crypto: add support for nettle's " Daniel P. Berrangé @ 2019-10-25 13:33 ` Philippe Mathieu-Daudé 2019-10-25 14:13 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Philippe Mathieu-Daudé @ 2019-10-25 13:33 UTC (permalink / raw) To: Daniel P. Berrangé, qemu-devel On 10/17/19 4:56 PM, Daniel P. Berrangé wrote: > 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. > > 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 98edb0ff44..6650c72348 100755 > --- a/configure > +++ b/configure > @@ -471,6 +471,7 @@ gtk_gl="no" > tls_priority="NORMAL" > gnutls="" > nettle="" > +nettle_xts="no" > gcrypt="" > gcrypt_hmac="no" > gcrypt_xts="no" > @@ -2862,6 +2863,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 Ah, now this variable makes sense. Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + fi > + fi > if test "$pass" = "no" && test "$nettle" = "yes"; then > feature_not_found "nettle" "Install nettle devel >= 2.7.1" > else > @@ -6337,6 +6351,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..08794a9b10 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_encrypt_native, > + ctx->alg_decrypt_native, > + ctx->iv, len, out, in); > +#endif > break; > case QCRYPTO_CIPHER_MODE_CTR: > ctr_crypt(ctx->ctx, ctx->alg_encrypt_native, > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] crypto: add support for nettle's native XTS impl 2019-10-17 14:56 ` [PATCH 4/4] crypto: add support for nettle's " Daniel P. Berrangé 2019-10-25 13:33 ` Philippe Mathieu-Daudé @ 2019-10-25 14:13 ` Stefano Garzarella 1 sibling, 0 replies; 13+ messages in thread From: Stefano Garzarella @ 2019-10-25 14:13 UTC (permalink / raw) To: Daniel P. Berrangé; +Cc: qemu-devel On Thu, Oct 17, 2019 at 03:56:54PM +0100, Daniel P. Berrangé wrote: > 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. > > 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 98edb0ff44..6650c72348 100755 > --- a/configure > +++ b/configure > @@ -471,6 +471,7 @@ gtk_gl="no" > tls_priority="NORMAL" > gnutls="" > nettle="" > +nettle_xts="no" > gcrypt="" > gcrypt_hmac="no" > gcrypt_xts="no" > @@ -2862,6 +2863,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 > @@ -6337,6 +6351,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..08794a9b10 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_encrypt_native, > + ctx->alg_decrypt_native, > + ctx->iv, len, out, in); > +#endif > break; > case QCRYPTO_CIPHER_MODE_CTR: > ctr_crypt(ctx->ctx, ctx->alg_encrypt_native, It seems clear to me: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-10-25 14:18 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-17 14:56 [PATCH 0/4] crypto: improve performance of ciphers in XTS mode Daniel P. Berrangé 2019-10-17 14:56 ` [PATCH 1/4] tests: allow filtering crypto cipher benchmark tests Daniel P. Berrangé 2019-10-25 13:24 ` Philippe Mathieu-Daudé 2019-10-25 13:45 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 2/4] tests: benchmark crypto with fixed data size, not time period Daniel P. Berrangé 2019-10-25 13:36 ` Philippe Mathieu-Daudé 2019-10-25 13:46 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 3/4] crypto: add support for gcrypt's native XTS impl Daniel P. Berrangé 2019-10-25 13:31 ` Philippe Mathieu-Daudé 2019-10-25 14:03 ` Stefano Garzarella 2019-10-17 14:56 ` [PATCH 4/4] crypto: add support for nettle's " Daniel P. Berrangé 2019-10-25 13:33 ` Philippe Mathieu-Daudé 2019-10-25 14:13 ` Stefano Garzarella
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).