1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/5] crypto threads
  4. View patch

[Qemu-devel] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions

Vladimir Sementsov-Ogievskiy posted 5 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
Posted by Vladimir Sementsov-Ogievskiy 7 years, 2 months ago 
qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
almost identical, let's reduce code duplication and simplify further
improvements.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/block.c | 81 +++++++++++++++++++-------------------------------
 1 file changed, 31 insertions(+), 50 deletions(-)

diff --git a/crypto/block.c b/crypto/block.c
index e59d1140fe..f4101f0841 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -190,14 +190,21 @@ void qcrypto_block_free(QCryptoBlock *block)
 }
 
 
-int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
-                                 size_t niv,
-                                 QCryptoIVGen *ivgen,
-                                 int sectorsize,
-                                 uint64_t offset,
-                                 uint8_t *buf,
-                                 size_t len,
-                                 Error **errp)
+typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
+                                        const void *in,
+                                        void *out,
+                                        size_t len,
+                                        Error **errp);
+
+static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
+                                    size_t niv,
+                                    QCryptoIVGen *ivgen,
+                                    int sectorsize,
+                                    uint64_t offset,
+                                    uint8_t *buf,
+                                    size_t len,
+                                    QCryptoCipherEncryptFunc func,
+                                    Error **errp)
 {
     uint8_t *iv;
     int ret = -1;
@@ -226,8 +233,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
         }
 
         nbytes = len > sectorsize ? sectorsize : len;
-        if (qcrypto_cipher_decrypt(cipher, buf, buf,
-                                   nbytes, errp) < 0) {
+        if (func(cipher, buf, buf, nbytes, errp) < 0) {
             goto cleanup;
         }
 
@@ -243,7 +249,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
 }
 
 
-int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
+int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
                                  size_t niv,
                                  QCryptoIVGen *ivgen,
                                  int sectorsize,
@@ -252,45 +258,20 @@ int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
                                  size_t len,
                                  Error **errp)
 {
-    uint8_t *iv;
-    int ret = -1;
-    uint64_t startsector = offset / sectorsize;
-
-    assert(QEMU_IS_ALIGNED(offset, sectorsize));
-    assert(QEMU_IS_ALIGNED(len, sectorsize));
-
-    iv = niv ? g_new0(uint8_t, niv) : NULL;
-
-    while (len > 0) {
-        size_t nbytes;
-        if (niv) {
-            if (qcrypto_ivgen_calculate(ivgen,
-                                        startsector,
-                                        iv, niv,
-                                        errp) < 0) {
-                goto cleanup;
-            }
-
-            if (qcrypto_cipher_setiv(cipher,
-                                     iv, niv,
-                                     errp) < 0) {
-                goto cleanup;
-            }
-        }
-
-        nbytes = len > sectorsize ? sectorsize : len;
-        if (qcrypto_cipher_encrypt(cipher, buf, buf,
-                                   nbytes, errp) < 0) {
-            goto cleanup;
-        }
+    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                    buf, len, qcrypto_cipher_decrypt, errp);
+}
 
-        startsector++;
-        buf += nbytes;
-        len -= nbytes;
-    }
 
-    ret = 0;
- cleanup:
-    g_free(iv);
-    return ret;
+int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
+                                 size_t niv,
+                                 QCryptoIVGen *ivgen,
+                                 int sectorsize,
+                                 uint64_t offset,
+                                 uint8_t *buf,
+                                 size_t len,
+                                 Error **errp)
+{
+    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
+                                    buf, len, qcrypto_cipher_encrypt, errp);
 }
-- 
2.18.0
Re: [Qemu-devel] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Wed, Dec 05, 2018 at 05:46:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
> almost identical, let's reduce code duplication and simplify further
> improvements.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  crypto/block.c | 81 +++++++++++++++++++-------------------------------
>  1 file changed, 31 insertions(+), 50 deletions(-)
> 
> diff --git a/crypto/block.c b/crypto/block.c
> index e59d1140fe..f4101f0841 100644
> --- a/crypto/block.c
> +++ b/crypto/block.c
> @@ -190,14 +190,21 @@ void qcrypto_block_free(QCryptoBlock *block)
>  }
>  
>  
> -int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
> -                                 size_t niv,
> -                                 QCryptoIVGen *ivgen,
> -                                 int sectorsize,
> -                                 uint64_t offset,
> -                                 uint8_t *buf,
> -                                 size_t len,
> -                                 Error **errp)
> +typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
> +                                        const void *in,
> +                                        void *out,
> +                                        size_t len,
> +                                        Error **errp);
> +
> +static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,

Can we call this functuon 'encdec', since it is misleading to call
it just 'encrypt' when its used for decrypt too.

> +                                    size_t niv,
> +                                    QCryptoIVGen *ivgen,
> +                                    int sectorsize,
> +                                    uint64_t offset,
> +                                    uint8_t *buf,
> +                                    size_t len,
> +                                    QCryptoCipherEncryptFunc func,

And call this 'EncDecFunc' too

> +                                    Error **errp)
>  {
>      uint8_t *iv;
>      int ret = -1;
> @@ -226,8 +233,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
>          }
>  
>          nbytes = len > sectorsize ? sectorsize : len;
> -        if (qcrypto_cipher_decrypt(cipher, buf, buf,
> -                                   nbytes, errp) < 0) {
> +        if (func(cipher, buf, buf, nbytes, errp) < 0) {
>              goto cleanup;
>          }
>  
> @@ -243,7 +249,7 @@ int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
>  }
>  
>  
> -int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
> +int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
>                                   size_t niv,
>                                   QCryptoIVGen *ivgen,
>                                   int sectorsize,
> @@ -252,45 +258,20 @@ int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
>                                   size_t len,
>                                   Error **errp)
>  {
> -    uint8_t *iv;
> -    int ret = -1;
> -    uint64_t startsector = offset / sectorsize;
> -
> -    assert(QEMU_IS_ALIGNED(offset, sectorsize));
> -    assert(QEMU_IS_ALIGNED(len, sectorsize));
> -
> -    iv = niv ? g_new0(uint8_t, niv) : NULL;
> -
> -    while (len > 0) {
> -        size_t nbytes;
> -        if (niv) {
> -            if (qcrypto_ivgen_calculate(ivgen,
> -                                        startsector,
> -                                        iv, niv,
> -                                        errp) < 0) {
> -                goto cleanup;
> -            }
> -
> -            if (qcrypto_cipher_setiv(cipher,
> -                                     iv, niv,
> -                                     errp) < 0) {
> -                goto cleanup;
> -            }
> -        }
> -
> -        nbytes = len > sectorsize ? sectorsize : len;
> -        if (qcrypto_cipher_encrypt(cipher, buf, buf,
> -                                   nbytes, errp) < 0) {
> -            goto cleanup;
> -        }
> +    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
> +                                    buf, len, qcrypto_cipher_decrypt, errp);
> +}
>  
> -        startsector++;
> -        buf += nbytes;
> -        len -= nbytes;
> -    }
>  
> -    ret = 0;
> - cleanup:
> -    g_free(iv);
> -    return ret;
> +int qcrypto_block_encrypt_helper(QCryptoCipher *cipher,
> +                                 size_t niv,
> +                                 QCryptoIVGen *ivgen,
> +                                 int sectorsize,
> +                                 uint64_t offset,
> +                                 uint8_t *buf,
> +                                 size_t len,
> +                                 Error **errp)
> +{
> +    return do_qcrypto_block_encrypt(cipher, niv, ivgen, sectorsize, offset,
> +                                    buf, len, qcrypto_cipher_encrypt, errp);
>  }
> -- 
> 2.18.0
> 

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 :|
Re: [Qemu-devel] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
Posted by Vladimir Sementsov-Ogievskiy 7 years, 2 months ago 
06.12.2018 13:36, Daniel P. Berrangé wrote:
> On Wed, Dec 05, 2018 at 05:46:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
>> almost identical, let's reduce code duplication and simplify further
>> improvements.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   crypto/block.c | 81 +++++++++++++++++++-------------------------------
>>   1 file changed, 31 insertions(+), 50 deletions(-)
>>
>> diff --git a/crypto/block.c b/crypto/block.c
>> index e59d1140fe..f4101f0841 100644
>> --- a/crypto/block.c
>> +++ b/crypto/block.c
>> @@ -190,14 +190,21 @@ void qcrypto_block_free(QCryptoBlock *block)
>>   }
>>   
>>   
>> -int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
>> -                                 size_t niv,
>> -                                 QCryptoIVGen *ivgen,
>> -                                 int sectorsize,
>> -                                 uint64_t offset,
>> -                                 uint8_t *buf,
>> -                                 size_t len,
>> -                                 Error **errp)
>> +typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
>> +                                        const void *in,
>> +                                        void *out,
>> +                                        size_t len,
>> +                                        Error **errp);
>> +
>> +static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
> 
> Can we call this functuon 'encdec', since it is misleading to call
> it just 'encrypt' when its used for decrypt too.

Maybe just _crypt ?)


-- 
Best regards,
Vladimir
Re: [Qemu-devel] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
Posted by Daniel P. Berrangé 7 years, 2 months ago 
On Thu, Dec 06, 2018 at 05:36:29PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> 06.12.2018 13:36, Daniel P. Berrangé wrote:
> > On Wed, Dec 05, 2018 at 05:46:57PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> >> qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
> >> almost identical, let's reduce code duplication and simplify further
> >> improvements.
> >>
> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> >> ---
> >>   crypto/block.c | 81 +++++++++++++++++++-------------------------------
> >>   1 file changed, 31 insertions(+), 50 deletions(-)
> >>
> >> diff --git a/crypto/block.c b/crypto/block.c
> >> index e59d1140fe..f4101f0841 100644
> >> --- a/crypto/block.c
> >> +++ b/crypto/block.c
> >> @@ -190,14 +190,21 @@ void qcrypto_block_free(QCryptoBlock *block)
> >>   }
> >>   
> >>   
> >> -int qcrypto_block_decrypt_helper(QCryptoCipher *cipher,
> >> -                                 size_t niv,
> >> -                                 QCryptoIVGen *ivgen,
> >> -                                 int sectorsize,
> >> -                                 uint64_t offset,
> >> -                                 uint8_t *buf,
> >> -                                 size_t len,
> >> -                                 Error **errp)
> >> +typedef int (*QCryptoCipherEncryptFunc)(QCryptoCipher *cipher,
> >> +                                        const void *in,
> >> +                                        void *out,
> >> +                                        size_t len,
> >> +                                        Error **errp);
> >> +
> >> +static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,
> > 
> > Can we call this functuon 'encdec', since it is misleading to call
> > it just 'encrypt' when its used for decrypt too.
> 
> Maybe just _crypt ?)

Sure thats fine

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 :|
Re: [Qemu-devel] [Qemu-block] [PATCH v2 2/5] crypto/block: refactor qcrypto_block_*crypt_helper functions
Posted by Alberto Garcia 7 years, 2 months ago 
On Wed 05 Dec 2018 03:46:57 PM CET, Vladimir Sementsov-Ogievskiy wrote:
> qcrypto_block_encrypt_helper and qcrypto_block_decrypt_helper are
> almost identical, let's reduce code duplication and simplify further
> improvements.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


> +static int do_qcrypto_block_encrypt(QCryptoCipher *cipher,

I also prefer encdec (which is the naming already used in crypto/xts.c),
but I'm also fine with crypt.

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.