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

["Daniel P. Berrange" <berrange@redhat.com>]

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/ :|