[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 04/13] cryptodev: introduce a new cryptodev b
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v4 04/13] cryptodev: introduce a new cryptodev backend |
Date: |
Mon, 3 Oct 2016 17:31:40 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Wed, Sep 28, 2016 at 04:25:43PM +0800, Gonglei wrote:
> +/* Max number of symetrical sessions */
s/symetrical/symmetric/
But why does the comment say "symetrical" when the constant name
MAX_NUM_SESSIONS seems to be a global limit for *all* sessions (not just
symmetric)?
> +#define MAX_NUM_SESSIONS 256
The guest can only have 256 sessions open?
What are the limits of real crypto libraries and accelerators?
> +
> +
> +struct QCryptoCryptoDevBackendBuiltin {
> + QCryptoCryptoDevBackend parent_obj;
> +
> + QCryptoCryptoDevBackendBuiltinSession *sessions[MAX_NUM_SESSIONS];
> +};
> +
> +static void qcrypto_cryptodev_backend_builtin_init(
> + QCryptoCryptoDevBackend *backend, Error **errp)
> +{
> + /* Only support one queue */
> + int queues = MAX(backend->conf.peers.queues, 1);
> + size_t i;
> + QCryptoCryptoDevBackendClientState *cc;
> +
> + for (i = 0; i < queues; i++) {
> + cc = qcrypto_cryptodev_backend_new_client(
> + "cryptodev-builtin", NULL);
> + snprintf(cc->info_str, sizeof(cc->info_str),
> + "cryptodev-builtin%lu", 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_builtin_get_unused_session_index(
> + QCryptoCryptoDevBackendBuiltin *builtin)
> +{
> + size_t i;
> +
> + for (i = 0; i < MAX_NUM_SESSIONS; i++) {
> + if (builtin->sessions[i] == NULL) {
> + return i;
> + }
> + }
> +
> + return -1;
> +}
> +
> +static int
> +qcrypto_cryptodev_backend_builtin_get_algo(uint32_t key_len,
> + Error **errp)
> +{
> + int algo;
> +
> + if (key_len == 128 / 8) {
> + algo = QCRYPTO_CIPHER_ALG_AES_128;
> + } else if (key_len == 192 / 8) {
> + algo = QCRYPTO_CIPHER_ALG_AES_192;
> + } else if (key_len == 256 / 8) {
> + algo = QCRYPTO_CIPHER_ALG_AES_256;
> + } else {
> + error_setg(errp, "unsupported key length :%u", key_len);
> + return -1;
> + }
> +
> + return algo;
> +}
> +
> +static int qcrypto_cryptodev_backend_builtin_create_cipher_session(
> + QCryptoCryptoDevBackendBuiltin *builtin,
> + QCryptoCryptoDevBackendSymSessionInfo *sess_info,
> + Error **errp)
> +{
> + int algo;
> + int mode;
> + QCryptoCipher *cipher;
> + int index;
> + QCryptoCryptoDevBackendBuiltinSession *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_builtin_get_unused_session_index(builtin);
Feel free to omit the function name prefix for static functions. These
names are very long.
> + if (index < 0) {
> + error_setg(errp, "the total number of created session exceed %u",
"Total number of sessions created exceeds %u"
> + MAX_NUM_SESSIONS);
> + return -1;
> + }
> +
> + switch (sess_info->cipher_alg) {
> + case VIRTIO_CRYPTO_CIPHER_AES_ECB:
> + algo = qcrypto_cryptodev_backend_builtin_get_algo(sess_info->key_len,
> + errp);
> + if (algo < 0) {
> + return -1;
> + }
> + mode = QCRYPTO_CIPHER_MODE_ECB;
> + break;
> + case VIRTIO_CRYPTO_CIPHER_AES_CBC:
> + algo = qcrypto_cryptodev_backend_builtin_get_algo(sess_info->key_len,
> + errp);
> + if (algo < 0) {
> + return -1;
> + }
> + mode = QCRYPTO_CIPHER_MODE_CBC;
> + break;
> + case VIRTIO_CRYPTO_CIPHER_AES_CTR:
> + algo = qcrypto_cryptodev_backend_builtin_get_algo(sess_info->key_len,
> + errp);
> + if (algo < 0) {
> + return -1;
> + }
> + mode = QCRYPTO_CIPHER_MODE_CTR;
> + break;
> + default:
> + error_setg(errp, "unsupported cipher alg :%u",
> + sess_info->cipher_alg);
> + return -1;
> + }
Code duplication can be eliminated:
switch (sess_info->cipher_alg) {
case VIRTIO_CRYPTO_CIPHER_AES_ECB:
mode = QCRYPTO_CIPHER_MODE_ECB;
break;
...
}
algo = qcrypto_cryptodev_backend_builtin_get_algo(sess_info->key_len,
errp);
if (algo < 0) {
return -1;
}
> +static void qcrypto_cryptodev_backend_builtin_cleanup(
> + QCryptoCryptoDevBackend *backend,
> + Error **errp)
> +{
> + QCryptoCryptoDevBackendBuiltin *builtin =
> + QCRYPTO_CRYPTODEV_BACKEND_BUILTIN(backend);
> + size_t i;
> + int queues = backend->conf.peers.queues;
> + QCryptoCryptoDevBackendClientState *cc;
> +
> + for (i = 0; i < MAX_NUM_SESSIONS; i++) {
> + if (builtin->sessions[i] != NULL) {
> + qcrypto_cryptodev_backend_builtin_sym_close_session(
> + backend, i, 0, errp);
> + }
> + }
> +
> + for (i = 0; i < queues; i++) {
This device doesn't seem to support queues because queue_index is
ignored by all functions that take it.
Perhaps there should be an error if the device is realized with more
than 1 queue?
signature.asc
Description: PGP signature
- Re: [Qemu-devel] [PATCH v4 04/13] cryptodev: introduce a new cryptodev backend,
Stefan Hajnoczi <=