OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: [PATCH v3 04/10] cryptodev: introduce gcrypt lib as a new cryptodev backend


> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange@redhat.com]
> Sent: Monday, September 19, 2016 4:56 PM
> Subject: Re: [PATCH v3 04/10] cryptodev: introduce gcrypt lib as a new
> cryptodev backend
> 
> On Mon, Sep 19, 2016 at 04:16:16PM +0800, Gonglei wrote:
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  crypto/Makefile.objs      |   1 +
> >  crypto/cryptodev-gcrypt.c | 329
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  qemu-options.hx           |  18 +++
> >  3 files changed, 348 insertions(+)
> >  create mode 100644 crypto/cryptodev-gcrypt.c
> >
> > diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
> > index f7f3c4f..bd8aea7 100644
> > --- a/crypto/Makefile.objs
> > +++ b/crypto/Makefile.objs
> > @@ -27,6 +27,7 @@ crypto-obj-y += block.o
> >  crypto-obj-y += block-qcow.o
> >  crypto-obj-y += block-luks.o
> >  crypto-obj-y += cryptodev.o
> > +crypto-obj-$(CONFIG_GCRYPT) += cryptodev-gcrypt.o
> 
> This can be just crypto-obj-y +=
> 
Yes.

> >  # Let the userspace emulators avoid linking gnutls/etc
> >  crypto-aes-obj-y = aes.o
> > diff --git a/crypto/cryptodev-gcrypt.c b/crypto/cryptodev-gcrypt.c
> > new file mode 100644
> > index 0000000..66a0e5e
> > --- /dev/null
> > +++ b/crypto/cryptodev-gcrypt.c
> > +/**
> > + * @TYPE_QCRYPTO_CRYPTODEV_BACKEND_GCRYPT:
> > + * name of backend that uses gcrypt library
> > + */
> > +#define TYPE_QCRYPTO_CRYPTODEV_BACKEND_GCRYPT
> "cryptodev-backend-gcrypt"
> 
> I'd suggest we just call this backend "builtin", so do a
> replace of "gcrypt" with "builtin" throughout.
> 
OK, good name ;)

> > +static void qcrypto_cryptodev_backend_gcrypt_init(
> > +             QCryptoCryptoDevBackend *backend, Error **errp)
> > +{
> > +    /* Only support one queue */
> > +    int queues = MAX(backend->conf.peers.queues, 1);
> > +    int i;
> 
> Nitpick, I prefer to see 'size_t' for list iterators
> that are always positive. Similar comment in other
> places in this series using int i
> 
OK

> > +    QCryptoCryptoDevBackendClientState *cc;
> > +
> > +    for (i = 0; i < queues; i++) {
> > +        cc = qcrypto_cryptodev_backend_new_client(
> > +                  "cryptodev-gcrypt", NULL);
> > +        snprintf(cc->info_str, sizeof(cc->info_str),
> > +                 "cryptodev-gcrypt%d", i);
> > +        cc->queue_index = i;
> > +
> > +        backend->conf.peers.ccs[i] = cc;
> > +    }
> > +
> > +    backend->conf.crypto_services =
> > +                         1u << VIRTIO_CRYPTO_SERVICE_CIPHER |
> > +                         1u << VIRTIO_CRYPTO_SERVICE_HASH |
> > +                         1u << VIRTIO_CRYPTO_SERVICE_MAC;
> > +    backend->conf.cipher_algo_l = 1u <<
> VIRTIO_CRYPTO_CIPHER_AES_CBC;
> > +    backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
> > +}
> > +
> > +static int
> > +qcrypto_cryptodev_backend_gcrypt_get_unused_session_index(
> > +      QCryptoCryptoDevBackendGcrypt *gcrypt)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < MAX_NUM_SESSIONS; i++) {
> > +        if (gcrypt->sessions[i] == NULL) {
> > +            return i;
> > +        }
> > +    }
> > +
> > +    return -1;
> > +}
> > +
> > +static int qcrypto_cryptodev_backend_gcrypt_create_cipher_session(
> > +                    QCryptoCryptoDevBackendGcrypt *gcrypt,
> > +                    QCryptoCryptoDevBackendSymSessionInfo
> *sess_info,
> > +                    Error **errp)
> > +{
> > +    int algo;
> > +    int mode;
> > +    QCryptoCipher *cipher;
> > +    int index;
> > +    QCryptoCryptoDevBackendGcryptSession *sess;
> > +
> > +    if (sess_info->op_type != VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> > +        error_setg(errp, "unsupported optype :%u", sess_info->op_type);
> > +        return -1;
> > +    }
> > +
> > +    index =
> qcrypto_cryptodev_backend_gcrypt_get_unused_session_index(gcrypt);
> > +    if (index < 0) {
> > +        error_setg(errp, "the total number of created session
> exceed %u",
> > +                  MAX_NUM_SESSIONS);
> > +        return -1;
> > +    }
> > +
> > +    switch (sess_info->cipher_alg) {
> > +    case VIRTIO_CRYPTO_CIPHER_AES_ECB:
> > +        if (sess_info->key_len == 128 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_128;
> > +        } else if (sess_info->key_len == 192 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_192;
> > +        } else if (sess_info->key_len == 256 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_256;
> > +        } else {
> > +            error_setg(errp, "unsupported key length :%u",
> > +                       sess_info->key_len);
> > +            return -1;
> > +        }
> > +        mode = QCRYPTO_CIPHER_MODE_ECB;
> > +        break;
> > +    case VIRTIO_CRYPTO_CIPHER_AES_CBC:
> > +        if (sess_info->key_len == 128 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_128;
> > +        } else if (sess_info->key_len == 192 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_192;
> > +        } else if (sess_info->key_len == 256 / 8) {
> > +            algo = QCRYPTO_CIPHER_ALG_AES_256;
> > +        } else {
> > +            error_setg(errp, "unsupported key length :%u",
> > +                       sess_info->key_len);
> > +            return -1;
> > +        }
> > +        mode = QCRYPTO_CIPHER_MODE_CBC;
> > +        break;
> > +    case VIRTIO_CRYPTO_CIPHER_AES_CTR:
> 
> Although the QEMU cipher.h API does not export CTR mode currently
> it should be trivial to add it. So feel free to add a patch at
> the start of the series implementing CTR mode in the cipher API.
> Both gcrypt and nettle have support for it which is all we need.
> 
OK, will do.

Thanks,
-Gonglei


[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]