Re: [Qemu-devel] Question about add AF_ALG backend for virtio-crypto

[Daniel P. Berrange]

On Wed, Feb 08, 2017 at 06:46:04PM +0800, Longpeng (Mike) wrote:
> Hi Daniel,
> 
> I was writing AF_ALG-backed for QEMU crypto these days, I think there're more
> than two ways to implements it.
> 
> The first one look likes below:
> [ cipher.c ]
> qcrypto_cipher_new(...)
> {
>       if (...) { /* use AF_ALG */
>               cipher = afalg_cipher_new(...)
>               if (cipher) {
>                       return cipher;
>               }
>       }
>       
>       /* disabled AF_ALG or AF_ALG failed, then back to
>        * using 'builtin'(gcrypt/nettle/...)
>        */
>       cipher = __qcrypto_cipher_new(...)
> }
> 
> [ cipher-afalg.c ]
> afalg_cipher_new(...) {....}
> afalg_cipher_encrypt(...) {...}
> ......
> 
> [ cipher-gcrypt.c ]
> __qcrypto_cipher_new(...) {...}
> __qcrypto_cipher_encrypt(...) {...}
> ......
> 
> [ cipher-nettle.c ]
> __qcrypto_cipher_new(...) {...}
> __qcrypto_cipher_encrypt(...) {...}
> ......
> 
> In this way, I think I need to rename most functions in
> cipher-gcrypt.c/cipher-nettle.c with a prefixion(such as '__')
> 
> 
> Alternative way is:
> [ cipher-afalg.c ]
> afalg_cipher_new(...) {....}
> afalg_cipher_encrypt(...) {...}
> ......
> 
> [ cipher-gcrypt.c ]
> qcrypto_cipher_new(...)
> {
>       if (...) { /* use AF_ALG */
>               cipher = afalg_cipher_new(...)
>               if (cipher) {
>                       return cipher;
>               }
>       }
>       
>       /* disabled AF_ALG or AF_ALG failed, then back to
>        * using 'builtin'
>        */
>       .......( the existing code )
> }
> ......
> 
> [ cipher-nettle.c ]
> qcrypto_cipher_new(...)
> {
>       if (...) { /* use AF_ALG */
>               cipher = afalg_cipher_new(...)
>               if (cipher) {
>                       return cipher;
>               }
>       }
>       
>       /* disabled AF_ALG or AF_ALG failed, then back to
>        * using 'builtin'
>        */
>       .......( the existing code )
> }
> ......
> 
> In this way, we should add AF_ALG-backed code in most functions in
> cipher-gcrypt.c/cipher-nettle.c, I'm afraid this would introduce lots of
> duplicate code because the same AF_ALG-backed code must in both gcrypt-backed
> impls and nettle-backed impls as above.
> 
> I'm confusing about which way you'd prefer, or do you have any better
> suggestion?

Yeah, both approaches have some reasonably significant downsides. Approach
1 is sort of like providing a virtual driver table, except it is hardcoded
to switch between 2 impls only.

A variant on approach 1 is to actually setup a proper driver-table dispatch
layer. eg define a struct that contains callbacks for each public api
operation. The qcrypto_cipher_new() method will then either setup callbacks
for AF_ALG, or for the library impl.

This is the design we took in crypto/{ivgen.c,ivgenpriv.h}


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|