Commit 3824f6e3 authored by Konrad Sztyber's avatar Konrad Sztyber
Browse files

bdev/crypto: complete IOs on ENOMEM from accel 



spdk_bdev_queue_io_wait() can only be used when one of bdev submission
functions returns ENOMEM (i.e. there are no more spdk_bdev_ios on that
IO channel).  Using it in any other case, e.g. on spdk_accel_append_*()
returning ENOMEM, will most likely result in failure.  Therefore, to
avoid that, the IOs are completed with NOMEM status relying on the bdev
layer to retry them.

Signed-off-by: default avatarKonrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Ie0f03496e5d3180c481815b3f1b021e74ae2f46d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17319


Community-CI: Mellanox Build Bot
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
parent 31bfcb45

module/bdev/crypto/vbdev_crypto.c

+3 −13
Original line number Diff line number Diff line
@@ -44,7 +44,6 @@ struct crypto_io_channel { 
};



enum crypto_io_resubmit_state {

	CRYPTO_IO_NEW,		/* Resubmit IO from the scratch */

	CRYPTO_IO_DECRYPT_DONE,	/* Appended decrypt, need to read */

	CRYPTO_IO_ENCRYPT_DONE,	/* Need to write */

};
@@ -164,7 +163,7 @@ crypto_encrypt(struct crypto_io_channel *crypto_ch, struct spdk_bdev_io *bdev_io 
				   crypto_io->aux_domain, crypto_io->aux_domain_ctx);

		if (rc == -ENOMEM) {

			SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n");

			vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW);

			spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM);

		} else {

			SPDK_ERRLOG("Failed to submit bdev_io!\n");

			crypto_io_fail(crypto_io);
@@ -193,14 +192,8 @@ vbdev_crypto_resubmit_io(void *arg) 
{

	struct spdk_bdev_io *bdev_io = (struct spdk_bdev_io *)arg;

	struct crypto_bdev_io *crypto_io = (struct crypto_bdev_io *)bdev_io->driver_ctx;

	struct spdk_io_channel *ch;



	switch (crypto_io->resubmit_state) {

	case CRYPTO_IO_NEW:

		assert(crypto_io->crypto_ch);

		ch = spdk_io_channel_from_ctx(crypto_io->crypto_ch);

		vbdev_crypto_submit_request(ch, bdev_io);

		break;

	case CRYPTO_IO_ENCRYPT_DONE:

		crypto_write(crypto_io->crypto_ch, bdev_io);

		break;
@@ -223,9 +216,6 @@ vbdev_crypto_queue_io(struct spdk_bdev_io *bdev_io, enum crypto_io_resubmit_stat 
	crypto_io->bdev_io_wait.cb_arg = bdev_io;

	crypto_io->resubmit_state = state;



	/* TODO: We shouldn't use spdk_bdev_queue_io_wait() for queueing IOs due to receiving ENOMEM

	 * from anything other than one of the bdev functions (e.g. accel).  We should have a

	 * different mechanism for handling such requests. */

	rc = spdk_bdev_queue_io_wait(bdev_io->bdev, crypto_io->crypto_ch->base_ch,

				     &crypto_io->bdev_io_wait);

	if (rc != 0) {
@@ -294,7 +284,7 @@ crypto_read_get_buf_cb(struct spdk_io_channel *ch, struct spdk_bdev_io *bdev_io, 
	if (rc != 0) {

		if (rc == -ENOMEM) {

			SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n");

			vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW);

			spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM);

		} else {

			SPDK_ERRLOG("Failed to submit bdev_io!\n");

			crypto_io_fail(crypto_io);
@@ -371,7 +361,7 @@ vbdev_crypto_submit_request(struct spdk_io_channel *ch, struct spdk_bdev_io *bde 
	if (rc != 0) {

		if (rc == -ENOMEM) {

			SPDK_DEBUGLOG(vbdev_crypto, "No memory, queue the IO.\n");

			vbdev_crypto_queue_io(bdev_io, CRYPTO_IO_NEW);

			spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_NOMEM);

		} else {

			SPDK_ERRLOG("Failed to submit bdev_io!\n");

			crypto_io_fail(crypto_io);

test/bdev/chaining.sh

+30 −3
Original line number Diff line number Diff line
@@ -49,17 +49,21 @@ update_stats() { 
	stats[copy_executed]=$(get_stat executed copy)

}



cleanup() {

tgtcleanup() {

	[[ -v input ]] && rm -f "$input" || :

	[[ -v output ]] && rm -f "$output" || :

	nvmftestfini

}



bperfcleanup() {

	[[ -v bperfpid ]] && killprocess $bperfpid

}



nvmftestinit

nvmfappstart -m 0x2



input=$(mktemp) output=$(mktemp)

trap 'cleanup; exit 1' SIGINT SIGTERM EXIT

trap 'tgtcleanup; exit 1' SIGINT SIGTERM EXIT



rpc_cmd <<- CONFIG

	bdev_malloc_create 32 4096 -b malloc0
@@ -115,4 +119,27 @@ spdk_dd --of "$output" --ib Nvme0n1 --bs 4096 --count 16 
cmp "$input" "$output"



trap - SIGINT SIGTERM EXIT

cleanup
 
tgtcleanup



# Verify bdev crypto ENOMEM handling by setting low accel task count and sending IO with high qd

trap 'bperfcleanup; exit 1' SIGINT SIGTERM EXIT



"$rootdir/build/examples/bdevperf" -t 5 -w verify -o 4096 -q 256 --wait-for-rpc -z &

bperfpid=$!



waitforlisten $bperfpid

rpc_cmd <<- CONFIG

	accel_set_options --task-count 16

	framework_start_init

	bdev_malloc_create 32 4096 -b malloc0

	accel_crypto_key_create -c AES_XTS -k "${key0[0]}" -e "${key0[1]}" -n key0

	accel_crypto_key_create -c AES_XTS -k "${key1[0]}" -e "${key1[1]}" -n key1

	bdev_crypto_create malloc0 crypto0 -n key0

	bdev_crypto_create crypto0 crypto1 -n key1

CONFIG



"$rootdir/examples/bdev/bdevperf/bdevperf.py" perform_tests



trap - SIGINT SIGTERM EXIT

killprocess $bperfpid

wait $bperfpid

test/unit/lib/bdev/crypto.c/crypto_ut.c

+7 −13
Original line number Diff line number Diff line
@@ -254,13 +254,10 @@ test_error_paths(void) 


	/* test error returned by accel fw */

	MOCK_SET(spdk_accel_append_encrypt, -ENOMEM);

	g_completion_called = false;

	vbdev_crypto_submit_request(g_io_ch, g_bdev_io);

	CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS);

	CU_ASSERT(g_io_ctx->bdev_io_wait.bdev == &g_crypto_bdev.crypto_bdev);

	CU_ASSERT(g_io_ctx->bdev_io_wait.cb_fn == vbdev_crypto_resubmit_io);

	CU_ASSERT(g_io_ctx->bdev_io_wait.cb_arg == g_bdev_io);

	CU_ASSERT(g_io_ctx->resubmit_state == CRYPTO_IO_NEW);

	memset(&g_io_ctx->bdev_io_wait, 0, sizeof(g_io_ctx->bdev_io_wait));

	CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_NOMEM);

	CU_ASSERT(g_completion_called);



	MOCK_SET(spdk_accel_append_encrypt, -EINVAL);

	vbdev_crypto_submit_request(g_io_ch, g_bdev_io);
@@ -328,15 +325,12 @@ test_error_paths(void) 
	/* test error returned by accel fw */

	g_bdev_io->internal.status = SPDK_BDEV_IO_STATUS_SUCCESS;

	MOCK_SET(spdk_accel_append_decrypt, -ENOMEM);

	g_completion_called = false;

	vbdev_crypto_submit_request(g_io_ch, g_bdev_io);

	poll_threads();

	CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_SUCCESS);

	CU_ASSERT(g_io_ctx->bdev_io_wait.bdev == &g_crypto_bdev.crypto_bdev);

	CU_ASSERT(g_io_ctx->bdev_io_wait.cb_fn == vbdev_crypto_resubmit_io);

	CU_ASSERT(g_io_ctx->bdev_io_wait.cb_arg == g_bdev_io);

	CU_ASSERT(g_io_ctx->resubmit_state == CRYPTO_IO_NEW);

	memset(&g_io_ctx->bdev_io_wait, 0, sizeof(g_io_ctx->bdev_io_wait));

	CU_ASSERT(g_bdev_io->internal.status == SPDK_BDEV_IO_STATUS_NOMEM);

	CU_ASSERT(g_completion_called);

	MOCK_SET(spdk_accel_append_decrypt, 0);

	g_completion_called = false;

}



static void