From 0a5c002bb0cdf4e8c406ab8f2bc80b09cb381c65 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 4 Dec 2019 17:32:38 -0500 Subject: [PATCH] lib/nvmf: Accept KATO for discovery controller Some NVMe applications require SPDK NVMe-oF target to support KATO for discovery controller. Hence change discovery controller to accept KATO. Update unit tests accordingly. Fixes the issue #1089. Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476810 (master) Community-CI: Broadcom SPDK FC-NVMe CI Community-CI: SPDK CI Jenkins (cherry picked from commit e37fc5a32a94ec675e512eacf2ec1f7b5e827954) Change-Id: Ib56e3b0b0faaf58276f9e692704763c1e5e5b042 Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478361 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto --- lib/nvmf/ctrlr.c | 16 ++++-------- test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 35 +++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 16 deletions(-) diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index fdef435887..90242c7e38 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -292,17 +292,9 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, ctrlr->feat.volatile_write_cache.bits.wce = 1; if (ctrlr->subsys->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) { - /* Don't accept keep-alive timeout for discovery controllers */ - if (ctrlr->feat.keep_alive_timer.bits.kato != 0) { - SPDK_ERRLOG("Discovery controller don't accept keep-alive timeout\n"); - spdk_bit_array_free(&ctrlr->qpair_mask); - free(ctrlr); - return NULL; - } - /* - * Discovery controllers use some arbitrary high value in order - * to cleanup stale discovery sessions + * If keep-alive timeout is not set, discovery controllers use some + * arbitrary high value in order to cleanup stale discovery sessions * * From the 1.0a nvme-of spec: * "The Keep Alive command is reserved for @@ -314,7 +306,9 @@ spdk_nvmf_ctrlr_create(struct spdk_nvmf_subsystem *subsystem, * actions for Keep Alive Timer expiration". * kato is in millisecond. */ - ctrlr->feat.keep_alive_timer.bits.kato = NVMF_DISC_KATO_IN_MS; + if (ctrlr->feat.keep_alive_timer.bits.kato == 0) { + ctrlr->feat.keep_alive_timer.bits.kato = NVMF_DISC_KATO_IN_MS; + } } /* Subtract 1 for admin queue, 1 for 0's based */ diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index 742506696d..ae90827880 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -583,19 +583,44 @@ test_connect(void) CU_ASSERT(rsp.connect_rsp.status_code_specific.invalid.ipo == 42); CU_ASSERT(qpair.ctrlr == NULL); - /* I/O connect to discovery controller keep-alive-timeout should be 0 */ + /* I/O connect to discovery controller with keep-alive-timeout != 0 */ cmd.connect_cmd.qid = 0; + cmd.connect_cmd.kato = 120000; memset(&rsp, 0, sizeof(rsp)); subsystem.subtype = SPDK_NVMF_SUBTYPE_DISCOVERY; subsystem.state = SPDK_NVMF_SUBSYSTEM_ACTIVE; TAILQ_INSERT_TAIL(&qpair.outstanding, &req, link); rc = spdk_nvmf_ctrlr_connect(&req); poll_threads(); - CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); - CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC); - CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_INTERNAL_DEVICE_ERROR); - CU_ASSERT(qpair.ctrlr == NULL); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS); + CU_ASSERT(nvme_status_success(&rsp.nvme_cpl.status)); + CU_ASSERT(qpair.ctrlr != NULL); + CU_ASSERT(qpair.ctrlr->keep_alive_poller != NULL); + spdk_nvmf_ctrlr_stop_keep_alive_timer(qpair.ctrlr); + spdk_bit_array_free(&qpair.ctrlr->qpair_mask); + free(qpair.ctrlr); + qpair.ctrlr = NULL; + + /* I/O connect to discovery controller with keep-alive-timeout == 0. + * Then, a fixed timeout value is set to keep-alive-timeout. + */ + cmd.connect_cmd.kato = 0; + memset(&rsp, 0, sizeof(rsp)); + subsystem.subtype = SPDK_NVMF_SUBTYPE_DISCOVERY; + subsystem.state = SPDK_NVMF_SUBSYSTEM_ACTIVE; + TAILQ_INSERT_TAIL(&qpair.outstanding, &req, link); + rc = spdk_nvmf_ctrlr_connect(&req); + poll_threads(); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS); + CU_ASSERT(nvme_status_success(&rsp.nvme_cpl.status)); + CU_ASSERT(qpair.ctrlr != NULL); + CU_ASSERT(qpair.ctrlr->keep_alive_poller != NULL); + spdk_nvmf_ctrlr_stop_keep_alive_timer(qpair.ctrlr); + spdk_bit_array_free(&qpair.ctrlr->qpair_mask); + free(qpair.ctrlr); + qpair.ctrlr = NULL; cmd.connect_cmd.qid = 1; + cmd.connect_cmd.kato = 120000; subsystem.subtype = SPDK_NVMF_SUBTYPE_NVME; /* I/O connect to disabled controller */