crypto: improve performance of XTS cipher mode

Add testing coverage for AES with XTS, ECB and CTR modes

Signed-off-by Daniel P. Berrangé <[email protected]>

---
 tests/benchmark-crypto-cipher.c | 149 +++++++++++++++++++++++++++-----

 1 file changed, 126 insertions(+), 23 deletions(-)

tests/benchmark-crypto-cipher.c +126 -24

index f5a0d0bc32..a8325a9510 100644
@@ -15,17 +15,27 @@
 #include "crypto/init.h"
 #include "crypto/cipher.h"
 
-static void test_cipher_speed(const void *opaque)
+static void test_cipher_speed(size_t chunk_size,
+                              QCryptoCipherMode mode,
+                              QCryptoCipherAlgorithm alg)
 {
     QCryptoCipher *cipher;
     Error *err = NULL;
     double total = 0.0;
-    size_t chunk_size = (size_t)opaque;
     uint8_t *key = NULL, *iv = NULL;
     uint8_t *plaintext = NULL, *ciphertext = NULL;
-    size_t nkey = qcrypto_cipher_get_key_len(QCRYPTO_CIPHER_ALG_AES_128);
-    size_t niv = qcrypto_cipher_get_iv_len(QCRYPTO_CIPHER_ALG_AES_128,
-                                           QCRYPTO_CIPHER_MODE_CBC);
+    size_t nkey;
+    size_t niv;
+
+    if (!qcrypto_cipher_supports(alg, mode)) {
+        return;
+    }
+
+    nkey = qcrypto_cipher_get_key_len(alg);
+    niv = qcrypto_cipher_get_iv_len(alg, mode);
+    if (mode == QCRYPTO_CIPHER_MODE_XTS) {
+        nkey *= 2;
+    }
 
     key = g_new0(uint8_t, nkey);
     memset(key, g_test_rand_int(), nkey);

@@ -38,14 +48,14 @@ static void test_cipher_speed(const void *opaque)
     plaintext = g_new0(uint8_t, chunk_size);
     memset(plaintext, g_test_rand_int(), chunk_size);
 
-    cipher = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_128,
-                                QCRYPTO_CIPHER_MODE_CBC,
+    cipher = qcrypto_cipher_new(alg, mode,
                                 key, nkey, &err);
     g_assert(cipher != NULL);
 
-    g_assert(qcrypto_cipher_setiv(cipher,
-                                  iv, niv,
-                                  &err) == 0);
+    if (mode != QCRYPTO_CIPHER_MODE_ECB)
+        g_assert(qcrypto_cipher_setiv(cipher,
+                                      iv, niv,
+                                      &err) == 0);
 
     g_test_timer_start();
     do {

@@ -55,13 +65,26 @@ static void test_cipher_speed(const void *opaque)
                                         chunk_size,
                                         &err) == 0);
         total += chunk_size;
-    } while (g_test_timer_elapsed() < 5.0);
+    } while (g_test_timer_elapsed() < 1.0);
 
     total /= MiB;
-    g_print("cbc(aes128): ");
-    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("Enc chunk %zu bytes ", chunk_size);
+    g_print("%.2f MB/sec ", total / g_test_timer_last());
+
+    total = 0.0;
+    g_test_timer_start();
+    do {
+        g_assert(qcrypto_cipher_decrypt(cipher,
+                                        plaintext,
+                                        ciphertext,
+                                        chunk_size,
+                                        &err) == 0);
+        total += chunk_size;
+    } while (g_test_timer_elapsed() < 1.0);
+
+    total /= MiB;
+    g_print("Dec chunk %zu bytes ", chunk_size);
+    g_print("%.2f MB/sec ", total / g_test_timer_last());
 
     qcrypto_cipher_free(cipher);
     g_free(plaintext);

@@ -70,19 +93,99 @@ static void test_cipher_speed(const void *opaque)
     g_free(key);
 }
 
-int main(int argc, char **argv)
+
+static void test_cipher_speed_ecb_aes_128(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_ECB,
+                      QCRYPTO_CIPHER_ALG_AES_128);
+}
+
+static void test_cipher_speed_ecb_aes_256(const void *opaque)
 {
-    size_t i;
-    char name[64];
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_ECB,
+                      QCRYPTO_CIPHER_ALG_AES_256);
+}
+
+static void test_cipher_speed_cbc_aes_128(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_CBC,
+                      QCRYPTO_CIPHER_ALG_AES_128);
+}
 
+static void test_cipher_speed_cbc_aes_256(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_CBC,
+                      QCRYPTO_CIPHER_ALG_AES_256);
+}
+
+static void test_cipher_speed_ctr_aes_128(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_CTR,
+                      QCRYPTO_CIPHER_ALG_AES_128);
+}
+
+static void test_cipher_speed_ctr_aes_256(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_CTR,
+                      QCRYPTO_CIPHER_ALG_AES_256);
+}
+
+static void test_cipher_speed_xts_aes_128(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_XTS,
+                      QCRYPTO_CIPHER_ALG_AES_128);
+}
+
+static void test_cipher_speed_xts_aes_256(const void *opaque)
+{
+    size_t chunk_size = (size_t)opaque;
+    test_cipher_speed(chunk_size,
+                      QCRYPTO_CIPHER_MODE_XTS,
+                      QCRYPTO_CIPHER_ALG_AES_256);
+}
+
+
+int main(int argc, char **argv)
+{
     g_test_init(&argc, &argv, NULL);
     g_assert(qcrypto_init(NULL) == 0);
 
-    for (i = 512; i <= 64 * KiB; i *= 2) {
-        memset(name, 0 , sizeof(name));
-        snprintf(name, sizeof(name), "/crypto/cipher/speed-%zu", i);
-        g_test_add_data_func(name, (void *)i, test_cipher_speed);
-    }
+#define ADD_TEST(mode, cipher, keysize, chunk)                          \
+    g_test_add_data_func(                                               \
+        "/crypto/cipher/" #mode "-" #cipher "-" #keysize "/chunk-" #chunk, \
+        (void *)chunk,                                                  \
+        test_cipher_speed_ ## mode ## _ ## cipher ## _ ## keysize)
+
+#define ADD_TESTS(chunk)                        \
+    do {                                        \
+        ADD_TEST(ecb, aes, 128, chunk);         \
+        ADD_TEST(ecb, aes, 256, chunk);         \
+        ADD_TEST(cbc, aes, 128, chunk);         \
+        ADD_TEST(cbc, aes, 256, chunk);         \
+        ADD_TEST(ctr, aes, 128, chunk);         \
+        ADD_TEST(ctr, aes, 256, chunk);         \
+        ADD_TEST(xts, aes, 128, chunk);         \
+        ADD_TEST(xts, aes, 256, chunk); \
+    } while (0)
+
+    ADD_TESTS(512);
+    ADD_TESTS(4096);
+    ADD_TESTS(16384);
+    ADD_TESTS(65536);
 
     return g_test_run();
 }
--

2.17.1

Add testing coverage for AES with XTS, ECB and CTR modes

Signed-off-by Daniel P. Berrangé <[email protected]>

The tweak encrypt/decrypt functions are identical except for the comments, so can be merged. Profiling data shows that the compiler is in fact already merging the two merges in the object files.

Signed-off-by Daniel P. Berrangé <[email protected]>

The tweak encrypt/decrypt functions are identical except for the comments, so can be merged. Profiling data shows that the compiler is in fact already merging the two merges in the object files.

Signed-off-by Daniel P. Berrangé <[email protected]>

---
 crypto/xts.c | 64 ++++++++++++----------------------------------------
 1 file changed, 15 insertions(+), 49 deletions(-)

crypto/xts.c +15 -50

index 95212341f6..3c1a92f01d 100644
@@ -43,20 +43,20 @@ static void xts_mult_x(uint8_t *I)
 
 
 /**
- * xts_tweak_uncrypt:
+ * xts_tweak_encdec:
  * @param ctxt: the cipher context
  * @param func: the cipher function
- * @src: buffer providing the cipher text of XTS_BLOCK_SIZE bytes
- * @dst: buffer to output the plain text of XTS_BLOCK_SIZE bytes
+ * @src: buffer providing the input text of XTS_BLOCK_SIZE bytes
+ * @dst: buffer to output the output text of XTS_BLOCK_SIZE bytes
  * @iv: the initialization vector tweak of XTS_BLOCK_SIZE bytes
  *
- * Decrypt data with a tweak
+ * Encrypt/decrypt data with a tweak
  */
-static void xts_tweak_decrypt(const void *ctx,
-                              xts_cipher_func *func,
-                              const uint8_t *src,
-                              uint8_t *dst,
-                              uint8_t *iv)
+static void xts_tweak_encdec(const void *ctx,
+                             xts_cipher_func *func,
+                             const uint8_t *src,
+                             uint8_t *dst,
+                             uint8_t *iv)
 {
     unsigned long x;
 

@@ -105,7 +105,7 @@ void xts_decrypt(const void *datactx,
     encfunc(tweakctx, XTS_BLOCK_SIZE, T, iv);
 
     for (i = 0; i < lim; i++) {
-        xts_tweak_decrypt(datactx, decfunc, src, dst, T);
+        xts_tweak_encdec(datactx, decfunc, src, dst, T);
 
         src += XTS_BLOCK_SIZE;
         dst += XTS_BLOCK_SIZE;

@@ -117,7 +117,7 @@ void xts_decrypt(const void *datactx,
         xts_mult_x(CC);
 
         /* PP = tweak decrypt block m-1 */
-        xts_tweak_decrypt(datactx, decfunc, src, PP, CC);
+        xts_tweak_encdec(datactx, decfunc, src, PP, CC);
 
         /* Pm = first length % XTS_BLOCK_SIZE bytes of PP */
         for (i = 0; i < mo; i++) {

@@ -129,7 +129,7 @@ void xts_decrypt(const void *datactx,
         }
 
         /* Pm-1 = Tweak uncrypt CC */
-        xts_tweak_decrypt(datactx, decfunc, CC, dst, T);
+        xts_tweak_encdec(datactx, decfunc, CC, dst, T);
     }
 
     /* Decrypt the iv back */

@@ -137,40 +137,6 @@ void xts_decrypt(const void *datactx,
 }
 
 
-/**
- * xts_tweak_crypt:
- * @param ctxt: the cipher context
- * @param func: the cipher function
- * @src: buffer providing the plain text of XTS_BLOCK_SIZE bytes
- * @dst: buffer to output the cipher text of XTS_BLOCK_SIZE bytes
- * @iv: the initialization vector tweak of XTS_BLOCK_SIZE bytes
- *
- * Encrypt data with a tweak
- */
-static void xts_tweak_encrypt(const void *ctx,
-                              xts_cipher_func *func,
-                              const uint8_t *src,
-                              uint8_t *dst,
-                              uint8_t *iv)
-{
-    unsigned long x;
-
-    /* tweak encrypt block i */
-    for (x = 0; x < XTS_BLOCK_SIZE; x++) {
-        dst[x] = src[x] ^ iv[x];
-    }
-
-    func(ctx, XTS_BLOCK_SIZE, dst, dst);
-
-    for (x = 0; x < XTS_BLOCK_SIZE; x++) {
-        dst[x] = dst[x] ^ iv[x];
-    }
-
-    /* LFSR the tweak */
-    xts_mult_x(iv);
-}
-
-
 void xts_encrypt(const void *datactx,
                  const void *tweakctx,
                  xts_cipher_func *encfunc,

@@ -200,7 +166,7 @@ void xts_encrypt(const void *datactx,
     encfunc(tweakctx, XTS_BLOCK_SIZE, T, iv);
 
     for (i = 0; i < lim; i++) {
-        xts_tweak_encrypt(datactx, encfunc, src, dst, T);
+        xts_tweak_encdec(datactx, encfunc, src, dst, T);
 
         dst += XTS_BLOCK_SIZE;
         src += XTS_BLOCK_SIZE;

@@ -209,7 +175,7 @@ void xts_encrypt(const void *datactx,
     /* if length is not a multiple of XTS_BLOCK_SIZE then */
     if (mo > 0) {
         /* CC = tweak encrypt block m-1 */
-        xts_tweak_encrypt(datactx, encfunc, src, CC, T);
+        xts_tweak_encdec(datactx, encfunc, src, CC, T);
 
         /* Cm = first length % XTS_BLOCK_SIZE bytes of CC */
         for (i = 0; i < mo; i++) {

@@ -222,7 +188,7 @@ void xts_encrypt(const void *datactx,
         }
 
         /* Cm-1 = Tweak encrypt PP */
-        xts_tweak_encrypt(datactx, encfunc, PP, dst, T);
+        xts_tweak_encdec(datactx, encfunc, PP, dst, T);
     }
 
     /* Decrypt the iv back */
--

2.17.1

The new type is designed to allow use of 64-bit arithmetic instead of operating 1-byte at a time. The following patches will use this to improve performance.

Signed-off-by Daniel P. Berrangé <[email protected]>

crypto/xts.c +1 -1

@@ -85,7 +90,7 @@ void xts_decrypt(const void *datactx,
                  uint8_t *dst,
                  const uint8_t *src)
 {
-    uint8_t PP[XTS_BLOCK_SIZE], CC[XTS_BLOCK_SIZE], T[XTS_BLOCK_SIZE];
+    xts_uint128 PP, CC, T;
     unsigned long i, m, mo, lim;

crypto/xts.c void xts_decrypt(const void *datactx, · line 94

Quoted 7 lines · Daniel P. Berrangé

crypto/xts.c +1 -1

@@ -85,7 +90,7 @@ void xts_decrypt(const void *datactx,
                  uint8_t *dst,
                  const uint8_t *src)
 {
-    uint8_t PP[XTS_BLOCK_SIZE], CC[XTS_BLOCK_SIZE], T[XTS_BLOCK_SIZE];
+    xts_uint128 PP, CC, T;
     unsigned long i, m, mo, lim;

   [...]

crypto/xts.c void xts_decrypt(const void *datactx, · line 131

Quoted 7 lines

crypto/xts.c +2 -2

         /* Pm = first length % XTS_BLOCK_SIZE bytes of PP */
         for (i = 0; i < mo; i++) {
-            CC[i] = src[XTS_BLOCK_SIZE + i];
-            dst[XTS_BLOCK_SIZE + i] = PP[i];
+            ((uint8_t *)&CC)[i] = src[XTS_BLOCK_SIZE + i];
+            dst[XTS_BLOCK_SIZE + i] = ((uint8_t *)&PP)[i];
         }

On second thoughts, these casts are a bit cumbersome. I wonder if it isn't better to keep the array a uint8_t[] and only treat it as xts_uint128 in the places where you actually do 64-bit operations (xts_uint128_xor, xts_mult_x).

Quoted 18 lines

crypto/xts.c void xts_decrypt(const void *datactx, · line 94

Quoted 7 lines

crypto/xts.c +1 -1

@@ -85,7 +90,7 @@ void xts_decrypt(const void *datactx,
                  uint8_t *dst,
                  const uint8_t *src)
 {
-    uint8_t PP[XTS_BLOCK_SIZE], CC[XTS_BLOCK_SIZE], T[XTS_BLOCK_SIZE];
+    xts_uint128 PP, CC, T;
     unsigned long i, m, mo, lim;

   [...]

Quoted 7 lines

crypto/xts.c +2 -2

         /* Pm = first length % XTS_BLOCK_SIZE bytes of PP */
         for (i = 0; i < mo; i++) {
-            CC[i] = src[XTS_BLOCK_SIZE + i];
-            dst[XTS_BLOCK_SIZE + i] = PP[i];
+            ((uint8_t *)&CC)[i] = src[XTS_BLOCK_SIZE + i];
+            dst[XTS_BLOCK_SIZE + i] = ((uint8_t *)&PP)[i];
         }

Quoted 4 lines

On second thoughts, these casts are a bit cumbersome. I wonder if it isn't better to keep the array a uint8_t[] and only treat it as xts_uint128 in the places where you actually do 64-bit operations (xts_uint128_xor, xts_mult_x).

I had done that originally, but it just shifts ugly casts from one place to another place in the code.

Using 64-bit arithmetic increases the performance for xts-aes-128 when built with gcrypt:

Encrypt: 235 MB/s -> 320 MB/s
Decrypt: 245 MB/s -> 325 MB/s

Signed-off-by Daniel P. Berrangé <[email protected]>

---
 crypto/xts.c | 52 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

crypto/xts.c +9 -3

index ded4365191..f109c8a3ee 100644
@@ -31,6 +31,12 @@ typedef struct {
     uint64_t b;
 } xts_uint128;
 
+#define xts_uint128_xor(D, S1, S2)              \
+    do {                                        \
+        (D)->a = (S1)->a ^ (S2)->a;             \
+        (D)->b = (S1)->b ^ (S2)->b;             \
+    } while (0)
+
 static void xts_mult_x(uint8_t *I)
 {
     int x;

@@ -59,25 +65,19 @@ static void xts_mult_x(uint8_t *I)
  */
 static void xts_tweak_encdec(const void *ctx,
                              xts_cipher_func *func,
-                             const uint8_t *src,
-                             uint8_t *dst,
-                             uint8_t *iv)
+                             const xts_uint128 *src,
+                             xts_uint128 *dst,
+                             xts_uint128 *iv)
 {

Quoted 39 lines · Daniel P. Berrangé

Using 64-bit arithmetic increases the performance for xts-aes-128 when built with gcrypt:

Encrypt: 235 MB/s -> 320 MB/s
Decrypt: 245 MB/s -> 325 MB/s

Signed-off-by Daniel P. Berrangé <[email protected]>

---
 crypto/xts.c | 52 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

crypto/xts.c +9 -3

index ded4365191..f109c8a3ee 100644
@@ -31,6 +31,12 @@ typedef struct {
     uint64_t b;
 } xts_uint128;
 
+#define xts_uint128_xor(D, S1, S2)              \
+    do {                                        \
+        (D)->a = (S1)->a ^ (S2)->a;             \
+        (D)->b = (S1)->b ^ (S2)->b;             \
+    } while (0)
+
 static void xts_mult_x(uint8_t *I)
 {
     int x;

@@ -59,25 +65,19 @@ static void xts_mult_x(uint8_t *I)
  */
 static void xts_tweak_encdec(const void *ctx,
                              xts_cipher_func *func,
-                             const uint8_t *src,
-                             uint8_t *dst,
-                             uint8_t *iv)
+                             const xts_uint128 *src,
+                             xts_uint128 *dst,
+                             xts_uint128 *iv)
 {

crypto/xts.c static void xts_mult_x(uint8_t *I) · line 75

Quoted 10 lines

crypto/xts.c +2 -6

-    unsigned long x;
-
     /* tweak encrypt block i */
-    for (x = 0; x < XTS_BLOCK_SIZE; x++) {
-        dst[x] = src[x] ^ iv[x];
-    }
+    xts_uint128_xor(dst, src, iv);
 
-    func(ctx, XTS_BLOCK_SIZE, dst, dst);
+    func(ctx, XTS_BLOCK_SIZE, (uint8_t *)dst, (uint8_t *)dst);

In the line of what I said earlier, perhaps it's clearer if you leave everything as uint8_t * and simply make xts_uint128_xor() treat the array as xts_uint128 internally.

crypto/xts.c void xts_decrypt(const void *datactx, · line 117

Quoted 7 lines

crypto/xts.c +5 -1

     for (i = 0; i < lim; i++) {
-        xts_tweak_encdec(datactx, decfunc, src, dst, (uint8_t *)&T);
+        xts_uint128 S, D;
+
+        memcpy(&S, src, XTS_BLOCK_SIZE);
+        xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
+        memcpy(dst, &D, XTS_BLOCK_SIZE);

Why do you need S and D?

Quoted 8 lines

Quoted 7 lines

     for (i = 0; i < lim; i++) {

crypto/xts.c +5 -1

-        xts_tweak_encdec(datactx, decfunc, src, dst, (uint8_t *)&T);
+        xts_uint128 S, D;
+
+        memcpy(&S, src, XTS_BLOCK_SIZE);
+        xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
+        memcpy(dst, &D, XTS_BLOCK_SIZE);

Quoted 1 line

Why do you need S and D?

I think src & dst pointers can't be guaranteed to be aligned sufficiently for int64 operations, if we just cast from uint8t*.

Quoted 10 lines

Quoted 9 lines

crypto/xts.c void xts_decrypt(const void *datactx, · line 117

Quoted 7 lines

     for (i = 0; i < lim; i++) {

crypto/xts.c +5 -1

-        xts_tweak_encdec(datactx, decfunc, src, dst, (uint8_t *)&T);
+        xts_uint128 S, D;
+
+        memcpy(&S, src, XTS_BLOCK_SIZE);
+        xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
+        memcpy(dst, &D, XTS_BLOCK_SIZE);

Why do you need S and D?

Quoted 2 lines

I think src & dst pointers can't be guaranteed to be aligned sufficiently for int64 operations, if we just cast from uint8t*.

I see. I did a quick test without the memcpy() calls and it doesn't seem to have a visible effect on performance, but if it turns out that it does then maybe this is worth investigating further. I suspect all buffers received by this code are allocated with qemu_try_blockalign() anyway, so it should be safe.

Quoted 10 lines · Alberto Garcia

Quoted 10 lines

Quoted 9 lines

crypto/xts.c void xts_decrypt(const void *datactx, · line 117

Quoted 7 lines

     for (i = 0; i < lim; i++) {

crypto/xts.c +5 -1

-        xts_tweak_encdec(datactx, decfunc, src, dst, (uint8_t *)&T);
+        xts_uint128 S, D;
+
+        memcpy(&S, src, XTS_BLOCK_SIZE);
+        xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
+        memcpy(dst, &D, XTS_BLOCK_SIZE);

Why do you need S and D?

Quoted 8 lines

Quoted 2 lines

I think src & dst pointers can't be guaranteed to be aligned sufficiently for int64 operations, if we just cast from uint8t*.

I see. I did a quick test without the memcpy() calls and it doesn't seem to have a visible effect on performance, but if it turns out that it does then maybe this is worth investigating further. I suspect all buffers received by this code are allocated with qemu_try_blockalign() anyway, so it should be safe.

The extra memcpy() calls certainly had a perf impact when I added them, so if we can determine that we can safely do without, that would be desirable.

crypto/xts.c +2 -8

-static void xts_mult_x(uint8_t *I)
+static void xts_mult_x(xts_uint128 *I)
 {
-    int x;
-    uint8_t t, tt;
+    uint64_t tt;
 
-    for (x = t = 0; x < 16; x++) {
-        tt = I[x] >> 7;
-        I[x] = ((I[x] << 1) | t) & 0xFF;
-        t = tt;
-    }

Quoted 16 lines · Daniel P. Berrangé

crypto/xts.c +4 -10

-static void xts_mult_x(uint8_t *I)
+static void xts_mult_x(xts_uint128 *I)
 {
-    int x;
-    uint8_t t, tt;
+    uint64_t tt;
 
-    for (x = t = 0; x < 16; x++) {
-        tt = I[x] >> 7;
-        I[x] = ((I[x] << 1) | t) & 0xFF;
-        t = tt;
-    }
-    if (tt) {
-        I[0] ^= 0x87;
+    tt = I->a >> 63;
+    I->a = I->a << 1;

crypto/xts.c typedef struct { · line 51

Quoted 6 lines

crypto/xts.c +4 -0

+
+    if (I->b >> 63) {
+        I->a ^= 0x87;
     }
+    I->b = (I->b << 1) | tt;
 }

Does this work fine in big-endian CPUs?

Quoted 17 lines · Alberto Garcia

Quoted 16 lines · Daniel P. Berrangé

crypto/xts.c +4 -10

-static void xts_mult_x(uint8_t *I)
+static void xts_mult_x(xts_uint128 *I)
 {
-    int x;
-    uint8_t t, tt;
+    uint64_t tt;
 
-    for (x = t = 0; x < 16; x++) {
-        tt = I[x] >> 7;
-        I[x] = ((I[x] << 1) | t) & 0xFF;
-        t = tt;
-    }
-    if (tt) {
-        I[0] ^= 0x87;
+    tt = I->a >> 63;
+    I->a = I->a << 1;

Quoted 8 lines

crypto/xts.c typedef struct { · line 51

Quoted 6 lines

crypto/xts.c +4 -0

+
+    if (I->b >> 63) {
+        I->a ^= 0x87;
     }
+    I->b = (I->b << 1) | tt;
 }

Does this work fine in big-endian CPUs?

Hmm, that's a good question. I'd expect tests/test-crypto-xts to crash and burn if it doesn't, so guess I'll need to find somewhere to validate that.

Encouraging the compiler to inline xts_tweak_encdec increases the performance for xts-aes-128 when built with gcrypt:

Encrypt: 460 MB/s -> 485 MB/s
Decrypt: 485 MB/s -> 505 MB/s

Signed-off-by Daniel P. Berrangé <[email protected]>

The XTS cipher mode is significantly slower than CBC mode. This series approximately doubles the XTS performance which will improve the I/O rate for LUKS disks.

Daniel P. Berrangé (6):
crypto: expand algorithm coverage for cipher benchmark
crypto: remove code duplication in tweak encrypt/decrypt
crypto: introduce a xts_uint128 data type
crypto: convert xts_tweak_encdec to use xts_uint128 type
crypto: convert xts_mult_x to use xts_uint128 type
crypto: annotate xts_tweak_encdec as inlineable

 crypto/xts.c                    | 147 ++++++++++++++-----------------
 tests/benchmark-crypto-cipher.c | 149 +++++++++++++++++++++++++++-----
 2 files changed, 191 insertions(+), 105 deletions(-)

Hi

Quoted 13 lines · Daniel P. Berrangé

The XTS cipher mode is significantly slower than CBC mode. This series approximately doubles the XTS performance which will improve the I/O rate for LUKS disks.

Daniel P. Berrangé (6):
crypto: expand algorithm coverage for cipher benchmark
crypto: remove code duplication in tweak encrypt/decrypt
crypto: introduce a xts_uint128 data type
crypto: convert xts_tweak_encdec to use xts_uint128 type
crypto: convert xts_mult_x to use xts_uint128 type
crypto: annotate xts_tweak_encdec as inlineable

Quoted 3 lines

 crypto/xts.c                    | 147 ++++++++++++++-----------------
 tests/benchmark-crypto-cipher.c | 149 +++++++++++++++++++++++++++-----
 2 files changed, 191 insertions(+), 105 deletions(-)

By using a constant amount of data to process, it's easier to measure perfomance with perf stat:

Quoted 16 lines · Marc-André Lureau

Hi

Quoted 13 lines · Daniel P. Berrangé

The XTS cipher mode is significantly slower than CBC mode. This series approximately doubles the XTS performance which will improve the I/O rate for LUKS disks.

Daniel P. Berrangé (6):
crypto: expand algorithm coverage for cipher benchmark
crypto: remove code duplication in tweak encrypt/decrypt
crypto: introduce a xts_uint128 data type
crypto: convert xts_tweak_encdec to use xts_uint128 type
crypto: convert xts_mult_x to use xts_uint128 type
crypto: annotate xts_tweak_encdec as inlineable

Quoted 6 lines

Quoted 3 lines

 crypto/xts.c                    | 147 ++++++++++++++-----------------
 tests/benchmark-crypto-cipher.c | 149 +++++++++++++++++++++++++++-----
 2 files changed, 191 insertions(+), 105 deletions(-)

By using a constant amount of data to process, it's easier to measure perfomance with perf stat:

The problem is that the different encryption modes have wildly different performance. eg while XTS gets 400 MB/s, ECB gets 3000 MB/s. I want the test to run long enough to minimize the noise, and picking a data size large enough for best ECB perf while not being excessively large for XTS is hard. THus I prefer to have a fixed execution time for each test.

Regards,
Daniel
--

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|