ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

crypto: add proper reset handling

Browse Source 
Previously there was no consideration for IO that were outstanding
to the crypto device when handling a reset.  This patch makes sure
that those IO are completed with FAIL status prior to completing
the reset that we pass down the stack.

It does so by sending down the reset first and in the completion
using spdk_for_each_channel and the poller to quiesce each channel
allowing the crypto side to complete all IOs before we finally
complete the reset IO after the last channel is quiesced.

Resets are tracked on a per bdev basis.

Addresses github issue #449.

Change-Id: Iadb07bada1fcaad33d9f224a60d983a7eb835236
Signed-off-by: paul luse <paul.e.luse@intel.com>
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-on: https://review.gerrithub.io/428552
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: Seth Howell <seth.howell5141@gmail.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
This commit is contained in:
paul luse 2018-10-09 13:38:29 -07:00 committed by Ben Walker
parent 40da9ab5e3
commit 7f6738e12d
2 changed files with 106 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

109
lib/bdev/crypto/vbdev_crypto.c
View File

@ -168,6 +168,8 @@ struct crypto_io_channel {
struct spdk_io_channel *base_ch; /* IO channel of base device */
struct spdk_poller *poller; /* completion poller */
struct device_qp *device_qp; /* unique device/qp combination for this channel */
TAILQ_HEAD(, spdk_bdev_io) pending_cry_ios; /* outstanding operations to the crypto device */
struct spdk_io_channel_iter *iter; /* used with for_each_channel in reset */
};
/* This is the crypto per IO context that the bdev layer allocates for us opaquely and attaches to
@ -388,41 +390,43 @@ _crypto_operation_complete(struct spdk_bdev_io *bdev_io)
struct spdk_bdev_io *free_me = io_ctx->read_io;
int rc = 0;
if (bdev_io->internal.status != SPDK_BDEV_IO_STATUS_FAILED) {
if (bdev_io->type == SPDK_BDEV_IO_TYPE_READ) {
TAILQ_REMOVE(&crypto_ch->pending_cry_ios, bdev_io, module_link);
/* Complete the original IO and then free the one that we created
* as a result of issuing an IO via submit_reqeust.
*/
if (bdev_io->type == SPDK_BDEV_IO_TYPE_READ) {
/* Complete the original IO and then free the one that we created
* as a result of issuing an IO via submit_reqeust.
*/
if (bdev_io->internal.status != SPDK_BDEV_IO_STATUS_FAILED) {
spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_SUCCESS);
spdk_bdev_free_io(free_me);
} else {
SPDK_ERRLOG("Issue with decryption on bdev_io %p\n", bdev_io);
rc = -EINVAL;
}
spdk_bdev_free_io(free_me);
} else if (bdev_io->type == SPDK_BDEV_IO_TYPE_WRITE) {
} else if (bdev_io->type == SPDK_BDEV_IO_TYPE_WRITE) {
if (bdev_io->internal.status != SPDK_BDEV_IO_STATUS_FAILED) {
/* Write the encrypted data. */
rc = spdk_bdev_writev_blocks(crypto_bdev->base_desc, crypto_ch->base_ch,
&io_ctx->cry_iov, 1, io_ctx->cry_offset_blocks,
io_ctx->cry_num_blocks, _complete_internal_write,
bdev_io);
} else {
/* Something really went haywire if this function got called with a type
* other than read or write.
*/
rc = -1;
SPDK_ERRLOG("Issue with encryption on bdev_io %p\n", bdev_io);
rc = -EINVAL;
}
} else {
/* If the poller found that one of the crypto ops had failed as part of this
* bdev_io it would have updated the internal status indicate failure.
*/
rc = -1;
SPDK_ERRLOG("Unknown bdev type %u on crypto operation completion\n",
bdev_io->type);
rc = -EINVAL;
}
if (rc != 0) {
SPDK_ERRLOG("ERROR on crypto operation completion!\n");
if (rc) {
spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_FAILED);
}
}
/* This is the poller for the crypto device. It uses a single API to dequeue whatever is ready at
@ -484,6 +488,13 @@ crypto_dev_poller(void *args)
/* done encrypting, complete the bdev_io */
if (--io_ctx->cryop_cnt_remaining == 0) {
/* If we're completing this with an outstanding reset we need
* to fail it.
*/
if (crypto_ch->iter) {
bdev_io->internal.status = SPDK_BDEV_IO_STATUS_FAILED;
}
/* Complete the IO */
_crypto_operation_complete(bdev_io);
@ -504,6 +515,16 @@ crypto_dev_poller(void *args)
num_mbufs);
}
/* If the channel iter is not NULL, we need to continue to poll
* until the pending list is empty, then we can move on to the
* next channel.
*/
if (crypto_ch->iter && TAILQ_EMPTY(&crypto_ch->pending_cry_ios)) {
SPDK_NOTICELOG("Channel %p has been quiesced.\n", crypto_ch);
spdk_for_each_channel_continue(crypto_ch->iter, 0);
crypto_ch->iter = NULL;
}
return num_dequeued_ops;
}
@ -726,6 +747,9 @@ _crypto_operation(struct spdk_bdev_io *bdev_io, enum rte_crypto_cipher_operation
}
} while (enqueued < cryop_cnt);
/* Add this bdev_io to our outstanding list. */
TAILQ_INSERT_TAIL(&crypto_ch->pending_cry_ios, bdev_io, module_link);
return rc;
/* Error cleanup paths. */
@ -752,6 +776,35 @@ error_get_dst:
return rc;
}
/* This function is called after all channels have been quiesced following
* a bdev reset.
*/
static void
_ch_quiesce_done(struct spdk_io_channel_iter *i, int status)
{
struct crypto_bdev_io *io_ctx = spdk_io_channel_iter_get_ctx(i);
assert(TAILQ_EMPTY(&io_ctx->crypto_ch->pending_cry_ios));
assert(io_ctx->orig_io != NULL);
spdk_bdev_io_complete(io_ctx->orig_io, SPDK_BDEV_IO_STATUS_SUCCESS);
}
/* This function is called per channel to quiesce IOs before completing a
* bdev reset that we received.
*/
static void
_ch_quiesce(struct spdk_io_channel_iter *i)
{
struct spdk_io_channel *ch = spdk_io_channel_iter_get_channel(i);
struct crypto_io_channel *crypto_ch = spdk_io_channel_get_ctx(ch);
crypto_ch->iter = i;
/* When the poller runs, it will see the non-NULL iter and handle
* the quiesce.
*/
}
/* Completion callback for IO that were issued from this bdev other than read/write.
* They have their own for readability.
*/
@ -761,6 +814,20 @@ _complete_internal_io(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg)
struct spdk_bdev_io *orig_io = cb_arg;
int status = success ? SPDK_BDEV_IO_STATUS_SUCCESS : SPDK_BDEV_IO_STATUS_FAILED;
if (bdev_io->type == SPDK_BDEV_IO_TYPE_RESET) {
struct crypto_bdev_io *orig_ctx = (struct crypto_bdev_io *)orig_io->driver_ctx;
assert(orig_io == orig_ctx->orig_io);
spdk_bdev_free_io(bdev_io);
spdk_for_each_channel(orig_ctx->crypto_bdev,
_ch_quiesce,
orig_ctx,
_ch_quiesce_done);
return;
}
spdk_bdev_io_complete(orig_io, status);
spdk_bdev_free_io(bdev_io);
}
@ -1015,6 +1082,10 @@ crypto_bdev_ch_create_cb(void *io_device, void *ctx_buf)
}
pthread_mutex_unlock(&g_device_qp_lock);
assert(crypto_ch->device_qp);
/* We use this queue to track outstanding IO in our lyaer. */
TAILQ_INIT(&crypto_ch->pending_cry_ios);
return 0;
}

28
test/unit/lib/bdev/crypto.c/crypto_ut.c
View File

@ -52,7 +52,6 @@ DEFINE_STUB(spdk_conf_section_get_nval, char *,
(struct spdk_conf_section *sp, const char *key, int idx), NULL);
DEFINE_STUB(spdk_conf_section_get_nmval, char *,
(struct spdk_conf_section *sp, const char *key, int idx1, int idx2), NULL);
DEFINE_STUB_V(spdk_bdev_module_list_add, (struct spdk_bdev_module *bdev_module));
DEFINE_STUB_V(spdk_bdev_free_io, (struct spdk_bdev_io *g_bdev_io));
DEFINE_STUB(spdk_bdev_io_type_supported, bool, (struct spdk_bdev *bdev,
@ -130,9 +129,9 @@ uint16_t g_dequeue_mock;
uint16_t g_enqueue_mock;
unsigned ut_rte_crypto_op_bulk_alloc;
int ut_rte_crypto_op_attach_sym_session = 0;
int ut_rte_cryptodev_info_get = 0;
bool ut_rte_cryptodev_info_get_mocked = false;
void
rte_cryptodev_info_get(uint8_t dev_id, struct rte_cryptodev_info *dev_info)
{
@ -320,6 +319,7 @@ test_setup(void)
g_crypto_ch->device_qp = &g_dev_qp;
g_test_config = calloc(1, sizeof(struct rte_config));
g_test_config->lcore_count = 1;
TAILQ_INIT(&g_crypto_ch->pending_cry_ios);
/* Allocate a real mbuf pool so we can test error paths */
g_mbuf_mp = spdk_mempool_create("mbuf_mp", NUM_MBUFS, sizeof(struct rte_mbuf),
@ -379,6 +379,7 @@ test_error_paths(void)
/* same thing but switch to reads to test error path in _crypto_complete_io() */
g_bdev_io->type = SPDK_BDEV_IO_TYPE_READ;
g_bdev_io->internal.status = SPDK_BDEV_IO_STATUS_SUCCESS;
TAILQ_INSERT_TAIL(&g_crypto_ch->pending_cry_ios, g_bdev_io, module_link);
vbdev_crypto_submit_request(g_io_ch, g_bdev_io);
CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED);
/* Now with the read_blocks failing */
@ -695,21 +696,22 @@ test_passthru(void)
CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED);
MOCK_CLEAR(spdk_bdev_flush_blocks);
g_bdev_io->type = SPDK_BDEV_IO_TYPE_RESET;
MOCK_SET(spdk_bdev_reset, 0);
vbdev_crypto_submit_request(g_io_ch, g_bdev_io);
CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS);
MOCK_SET(spdk_bdev_reset, -1);
vbdev_crypto_submit_request(g_io_ch, g_bdev_io);
CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED);
MOCK_CLEAR(spdk_bdev_reset);
/* We should never get a WZ command, we report that we don't support it. */
g_bdev_io->type = SPDK_BDEV_IO_TYPE_WRITE_ZEROES;
vbdev_crypto_submit_request(g_io_ch, g_bdev_io);
CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_FAILED);
}
static void
test_reset(void)
{
/* TODO: There are a few different ways to do this given that
* the code uses spdk_for_each_channel() to implement reset
* handling. SUbmitting w/o UT for this function for now and
* will follow up with something shortly.
*/
}
static void
test_initdrivers(void)
{
@ -894,7 +896,9 @@ main(int argc, char **argv)
CU_add_test(suite, "test_crypto_op_complete",
test_crypto_op_complete) == NULL ||
CU_add_test(suite, "test_supported_io",
test_supported_io) == NULL
test_supported_io) == NULL ||
CU_add_test(suite, "test_reset",
test_reset) == NULL
) {
CU_cleanup_registry();
return CU_get_error();