Commit 4b4b3cca authored by Evgeniy Kochetov's avatar Evgeniy Kochetov Committed by Tomasz Zawadzki
Browse files

nvme/ctrlr: Allow targets not supporting Keep Alive Timer feature ID 



NVMe spec defines "Keep Alive Timer" feature ID as optional and there
are targets that do not support this. SPDK fails to connect to such
targets.

This patch allows Get Feature "Keep Alive" target to fail with
INVALID_FIELD status. In this case we just continue with keep alive
timer value stored in controller opts structure. This value is already
communicated to target in CONNECT command.

Fixes #1328

Signed-off-by: default avatarEvgeniy Kochetov <evgeniik@mellanox.com>
Change-Id: I52e7ea3cb66073ce6cc168a169989bd179041618
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/1625


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 7066e3c9

lib/nvme/nvme_ctrlr.c

+16 −11
Original line number Diff line number Diff line
@@ -1789,19 +1789,24 @@ nvme_ctrlr_set_keep_alive_timeout_done(void *arg, const struct spdk_nvme_cpl *cp 
	struct spdk_nvme_ctrlr *ctrlr = (struct spdk_nvme_ctrlr *)arg;



	if (spdk_nvme_cpl_is_error(cpl)) {

		if ((cpl->status.sct == SPDK_NVME_SCT_GENERIC) &&

		    (cpl->status.sc == SPDK_NVME_SC_INVALID_FIELD)) {

			SPDK_DEBUGLOG(SPDK_LOG_NVME, "Keep alive timeout Get Feature is not supported\n");

		} else {

			SPDK_ERRLOG("Keep alive timeout Get Feature failed: SC %x SCT %x\n",

				    cpl->status.sc, cpl->status.sct);

			ctrlr->opts.keep_alive_timeout_ms = 0;

			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_ERROR, NVME_TIMEOUT_INFINITE);

			return;

		}



	} else {

		if (ctrlr->opts.keep_alive_timeout_ms != cpl->cdw0) {

			SPDK_DEBUGLOG(SPDK_LOG_NVME, "Controller adjusted keep alive timeout to %u ms\n",

				      cpl->cdw0);

		}



		ctrlr->opts.keep_alive_timeout_ms = cpl->cdw0;

	}



	keep_alive_interval_ms = ctrlr->opts.keep_alive_timeout_ms / 2;

	if (keep_alive_interval_ms == 0) {

test/unit/lib/nvme/nvme_ctrlr.c/nvme_ctrlr_ut.c

+39 −2
Original line number Diff line number Diff line
@@ -234,8 +234,8 @@ spdk_nvme_ctrlr_cmd_get_feature(struct spdk_nvme_ctrlr *ctrlr, uint8_t feature, 
				uint32_t cdw11, void *payload, uint32_t payload_size,

				spdk_nvme_cmd_cb cb_fn, void *cb_arg)

{

	CU_ASSERT(0);

	return -1;

	fake_cpl_sc(cb_fn, cb_arg);

	return 0;

}



int
@@ -2065,6 +2065,42 @@ test_nvme_ctrlr_init_set_num_queues(void) 
	nvme_ctrlr_destruct(&ctrlr);

}



static void

test_nvme_ctrlr_init_set_keep_alive_timeout(void)

{

	DECLARE_AND_CONSTRUCT_CTRLR();



	ctrlr.opts.keep_alive_timeout_ms = 60000;

	ctrlr.cdata.kas = 1;

	ctrlr.state = NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT;

	fake_cpl.cdw0 = 120000;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_HOST_ID */

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_HOST_ID);

	CU_ASSERT(ctrlr.opts.keep_alive_timeout_ms == 120000);

	fake_cpl.cdw0 = 0;



	/* Target does not support Get Feature "Keep Alive Timer" */

	ctrlr.opts.keep_alive_timeout_ms = 60000;

	ctrlr.cdata.kas = 1;

	ctrlr.state = NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT;

	set_status_code = SPDK_NVME_SC_INVALID_FIELD;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_HOST_ID */

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_HOST_ID);

	CU_ASSERT(ctrlr.opts.keep_alive_timeout_ms == 60000);

	set_status_code = SPDK_NVME_SC_SUCCESS;



	/* Target fails Get Feature "Keep Alive Timer" for another reason */

	ctrlr.opts.keep_alive_timeout_ms = 60000;

	ctrlr.cdata.kas = 1;

	ctrlr.state = NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT;

	set_status_code = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;

	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> ERROR */

	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_ERROR);

	set_status_code = SPDK_NVME_SC_SUCCESS;



	nvme_ctrlr_destruct(&ctrlr);

}



int main(int argc, char **argv)

{

	CU_pSuite	suite = NULL;
@@ -2102,6 +2138,7 @@ int main(int argc, char **argv) 
	CU_ADD_TEST(suite, test_spdk_nvme_ctrlr_set_trid);

	CU_ADD_TEST(suite, test_nvme_ctrlr_init_set_nvmf_ioccsz);

	CU_ADD_TEST(suite, test_nvme_ctrlr_init_set_num_queues);

	CU_ADD_TEST(suite, test_nvme_ctrlr_init_set_keep_alive_timeout);



	CU_basic_set_mode(CU_BRM_VERBOSE);

	CU_basic_run_tests();