From 957076108f6e455fe2985bf7b74561b5ed7ace62 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Mon, 16 Jan 2023 14:35:55 +0100 Subject: [PATCH] accel: remove nbytes from spdk_accel_task All operations are using iovecs to describe their buffers and only encrypt/decrypt additionally used nbytes to store the total size of a src buffer. We don't really need this value in the generic accel code, so we can let modules calculate it, if necessary. That way, we won't waste cycles calculating it if a module doesn't use it and it makes the code a bit easier, as we won't have to deal with the fact that nbytes is only valid for certain operations. Signed-off-by: Konrad Sztyber Change-Id: I29252be34a9af9fd40f4c7fec9d0a0c1139c562d Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16306 Tested-by: SPDK CI Jenkins Reviewed-by: Aleksey Marchuk Reviewed-by: Ben Walker --- include/spdk_internal/accel_module.h | 1 - lib/accel/accel.c | 32 ------------------- lib/accel/accel_sw.c | 20 ++++++++++-- .../dpdk_cryptodev/accel_dpdk_cryptodev.c | 27 ++++++++++++---- .../accel_dpdk_cryptodev_ut.c | 14 ++------ 5 files changed, 40 insertions(+), 54 deletions(-) diff --git a/include/spdk_internal/accel_module.h b/include/spdk_internal/accel_module.h index 90c9a08d4..b7d532b7a 100644 --- a/include/spdk_internal/accel_module.h +++ b/include/spdk_internal/accel_module.h @@ -79,7 +79,6 @@ struct spdk_accel_task { uint32_t block_size; /* for crypto op */ }; enum accel_opcode op_code; - uint64_t nbytes; uint64_t iv; /* Initialization vector (tweak) for crypto op */ int flags; int status; diff --git a/lib/accel/accel.c b/lib/accel/accel.c index 92dbdf85a..0a3581149 100644 --- a/lib/accel/accel.c +++ b/lib/accel/accel.c @@ -567,26 +567,11 @@ spdk_accel_submit_encrypt(struct spdk_io_channel *ch, struct spdk_accel_crypto_k struct spdk_accel_task *accel_task; struct spdk_accel_module_if *module = g_modules_opc[ACCEL_OPC_ENCRYPT]; struct spdk_io_channel *module_ch = accel_ch->module_ch[ACCEL_OPC_ENCRYPT]; - size_t src_nbytes = 0, dst_nbytes = 0; - uint32_t i; if (spdk_unlikely(!dst_iovs || !dst_iovcnt || !src_iovs || !src_iovcnt || !key || !block_size)) { return -EINVAL; } - for (i = 0; i < src_iovcnt; i++) { - src_nbytes += src_iovs[i].iov_len; - } - for (i = 0; i < dst_iovcnt; i++) { - dst_nbytes += dst_iovs[i].iov_len; - } - if (spdk_unlikely(src_nbytes != dst_nbytes || !src_nbytes)) { - return -ERANGE; - } - if (spdk_unlikely(src_nbytes % block_size != 0)) { - return -EINVAL; - } - accel_task = _get_task(accel_ch, cb_fn, cb_arg); if (accel_task == NULL) { return -ENOMEM; @@ -597,7 +582,6 @@ spdk_accel_submit_encrypt(struct spdk_io_channel *ch, struct spdk_accel_crypto_k accel_task->s.iovcnt = src_iovcnt; accel_task->d.iovs = dst_iovs; accel_task->d.iovcnt = dst_iovcnt; - accel_task->nbytes = src_nbytes; accel_task->iv = iv; accel_task->block_size = block_size; accel_task->flags = flags; @@ -617,26 +601,11 @@ spdk_accel_submit_decrypt(struct spdk_io_channel *ch, struct spdk_accel_crypto_k struct spdk_accel_task *accel_task; struct spdk_accel_module_if *module = g_modules_opc[ACCEL_OPC_DECRYPT]; struct spdk_io_channel *module_ch = accel_ch->module_ch[ACCEL_OPC_DECRYPT]; - size_t src_nbytes = 0, dst_nbytes = 0; - uint32_t i; if (spdk_unlikely(!dst_iovs || !dst_iovcnt || !src_iovs || !src_iovcnt || !key || !block_size)) { return -EINVAL; } - for (i = 0; i < src_iovcnt; i++) { - src_nbytes += src_iovs[i].iov_len; - } - for (i = 0; i < dst_iovcnt; i++) { - dst_nbytes += dst_iovs[i].iov_len; - } - if (spdk_unlikely(src_nbytes != dst_nbytes || !src_nbytes)) { - return -ERANGE; - } - if (spdk_unlikely(src_nbytes % block_size != 0)) { - return -EINVAL; - } - accel_task = _get_task(accel_ch, cb_fn, cb_arg); if (accel_task == NULL) { return -ENOMEM; @@ -647,7 +616,6 @@ spdk_accel_submit_decrypt(struct spdk_io_channel *ch, struct spdk_accel_crypto_k accel_task->s.iovcnt = src_iovcnt; accel_task->d.iovs = dst_iovs; accel_task->d.iovcnt = dst_iovcnt; - accel_task->nbytes = src_nbytes; accel_task->iv = iv; accel_task->block_size = block_size; accel_task->flags = flags; diff --git a/lib/accel/accel_sw.c b/lib/accel/accel_sw.c index 0029ca8df..0f8df09e8 100644 --- a/lib/accel/accel_sw.c +++ b/lib/accel/accel_sw.c @@ -371,10 +371,10 @@ _sw_accel_crypto_operation(struct spdk_accel_task *accel_task, struct spdk_accel { #ifdef SPDK_CONFIG_ISAL_CRYPTO uint64_t iv[2]; - size_t remaining_len; + size_t remaining_len, dst_len; uint64_t src_offset = 0, dst_offset = 0; uint32_t src_iovpos = 0, dst_iovpos = 0, src_iovcnt, dst_iovcnt; - uint32_t block_size, crypto_len, crypto_accum_len = 0; + uint32_t i, block_size, crypto_len, crypto_accum_len = 0; struct iovec *src_iov, *dst_iov; uint8_t *src, *dst; @@ -399,7 +399,21 @@ _sw_accel_crypto_operation(struct spdk_accel_task *accel_task, struct spdk_accel return -EINVAL; } - remaining_len = accel_task->nbytes; + remaining_len = 0; + for (i = 0; i < src_iovcnt; i++) { + remaining_len += src_iov[i].iov_len; + } + dst_len = 0; + for (i = 0; i < dst_iovcnt; i++) { + dst_len += dst_iov[i].iov_len; + } + + if (spdk_unlikely(remaining_len != dst_len || !remaining_len)) { + return -ERANGE; + } + if (spdk_unlikely(remaining_len % accel_task->block_size != 0)) { + return -EINVAL; + } while (remaining_len) { crypto_len = spdk_min(block_size - crypto_accum_len, src_iov->iov_len - src_offset); diff --git a/module/accel/dpdk_cryptodev/accel_dpdk_cryptodev.c b/module/accel/dpdk_cryptodev/accel_dpdk_cryptodev.c index 9a7be552e..d9993c168 100644 --- a/module/accel/dpdk_cryptodev/accel_dpdk_cryptodev.c +++ b/module/accel/dpdk_cryptodev/accel_dpdk_cryptodev.c @@ -591,10 +591,10 @@ accel_dpdk_cryptodev_process_task(struct accel_dpdk_cryptodev_io_channel *crypto uint16_t num_enqueued_ops; uint32_t cryop_cnt; uint32_t crypto_len = task->base.block_size; - uint64_t total_length = task->base.nbytes; + uint64_t dst_length, total_length; uint64_t iv_start = task->base.iv; struct accel_dpdk_cryptodev_queued_op *op_to_queue; - uint32_t crypto_index; + uint32_t i, crypto_index; struct rte_crypto_op *crypto_ops[ACCEL_DPDK_CRYPTODEV_MAX_ENQUEUE_ARRAY_SIZE]; struct rte_mbuf *src_mbufs[ACCEL_DPDK_CRYPTODEV_MAX_ENQUEUE_ARRAY_SIZE]; struct rte_mbuf *dst_mbufs[ACCEL_DPDK_CRYPTODEV_MAX_ENQUEUE_ARRAY_SIZE]; @@ -611,18 +611,31 @@ accel_dpdk_cryptodev_process_task(struct accel_dpdk_cryptodev_io_channel *crypto task->base.crypto_key->module_if != &g_accel_dpdk_cryptodev_module)) { return -EINVAL; } - priv = task->base.crypto_key->priv; - assert(task->base.nbytes); - assert(task->base.block_size); - assert(task->base.nbytes % task->base.block_size == 0); + total_length = 0; + for (i = 0; i < task->base.s.iovcnt; i++) { + total_length += task->base.s.iovs[i].iov_len; + } + dst_length = 0; + for (i = 0; i < task->base.d.iovcnt; i++) { + dst_length += task->base.d.iovs[i].iov_len; + } + + if (spdk_unlikely(total_length != dst_length || !total_length)) { + return -ERANGE; + } + if (spdk_unlikely(total_length % task->base.block_size != 0)) { + return -EINVAL; + } + + priv = task->base.crypto_key->priv; assert(priv->driver < ACCEL_DPDK_CRYPTODEV_DRIVER_LAST); if (total_length > ACCEL_DPDK_CRYPTODEV_CRYPTO_MAX_IO) { return -E2BIG; } - cryop_cnt = task->base.nbytes / task->base.block_size; + cryop_cnt = total_length / task->base.block_size; qp = crypto_ch->device_qp[priv->driver]; assert(qp); dev = qp->device; diff --git a/test/unit/lib/accel/dpdk_cryptodev.c/accel_dpdk_cryptodev_ut.c b/test/unit/lib/accel/dpdk_cryptodev.c/accel_dpdk_cryptodev_ut.c index 9509cbc7f..c3cc8e77e 100644 --- a/test/unit/lib/accel/dpdk_cryptodev.c/accel_dpdk_cryptodev_ut.c +++ b/test/unit/lib/accel/dpdk_cryptodev.c/accel_dpdk_cryptodev_ut.c @@ -371,7 +371,6 @@ test_error_paths(void) task.base.s.iovs = &src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = 512; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -394,11 +393,11 @@ test_error_paths(void) CU_ASSERT(rc == -EINVAL); key.module_if = &g_accel_dpdk_cryptodev_module; - /* case 3 - nbytes too big */ - task.base.nbytes = ACCEL_DPDK_CRYPTODEV_CRYPTO_MAX_IO + 512; + /* case 3 - buffers are too big */ + dst_iov.iov_len = src_iov.iov_len = ACCEL_DPDK_CRYPTODEV_CRYPTO_MAX_IO + 512; rc = accel_dpdk_cryptodev_submit_tasks(g_io_ch, &task.base); CU_ASSERT(rc == -E2BIG); - task.base.nbytes = 512; + dst_iov.iov_len = src_iov.iov_len = 512; /* case 4 - no key handle in the channel */ rc = accel_dpdk_cryptodev_submit_tasks(g_io_ch, &task.base); @@ -438,7 +437,6 @@ test_simple_encrypt(void) task.base.s.iovs = src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = 512; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -525,7 +523,6 @@ test_simple_decrypt(void) task.base.s.iovs = src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = 512; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -614,7 +611,6 @@ test_large_enc_dec(void) task.base.s.iovs = &src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = ACCEL_DPDK_CRYPTODEV_CRYPTO_MAX_IO; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -722,7 +718,6 @@ test_dev_full(void) task.base.s.iovs = &src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = 1024; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -790,7 +785,6 @@ test_crazy_rw(void) task.base.d.iovcnt = 3; task.base.d.iovs = dst_iov; task.base.block_size = 512; - task.base.nbytes = num_blocks * task.base.block_size; task.base.crypto_key = &g_key; task.base.iv = 1; @@ -817,7 +811,6 @@ test_crazy_rw(void) num_blocks = 8; task.base.op_code = ACCEL_OPC_ENCRYPT; task.cryop_cnt_remaining = 0; - task.base.nbytes = task.base.block_size * num_blocks; task.base.s.iovcnt = 4; task.base.d.iovcnt = 4; task.base.s.iovs[0].iov_len = 2048; @@ -1054,7 +1047,6 @@ test_poller(void) task.base.s.iovs = &src_iov; task.base.d.iovcnt = 1; task.base.d.iovs = &dst_iov; - task.base.nbytes = 1024; task.base.block_size = 512; task.base.crypto_key = &g_key; task.base.iv = 1;