nvme: set keep alive for discovery controllers

Discovery services using the SPDK nvme driver may
use long-lasting connections that detect AER completions
to determine when there are changes in the discovery
log. This means that we still need to send keep alives
on discovery controller admin queues. So move the
SET_KEEP_ALIVE_TIMEOUT state immediately after
IDENTIFY, and run the SET_KEEP_ALIVE_TIMEOUT state
even for discovery controllers.

Note, we need the IDENTIFY's KAS value to properly
set the keep alive timeout, so we have to keep the
IDENTIFY state before SET_KEEP_ALIVE_TIMEOUT.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: I5c6403c28fb72d42629c5f9009a89c4bfd44d162
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10329
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>
This commit is contained in:
Jim Harris 2021-11-19 09:35:56 +00:00 committed by Tomasz Zawadzki
parent 5c3da57040
commit 1c083e6200
3 changed files with 55 additions and 38 deletions

View File

@ -1355,6 +1355,10 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state)
return "identify controller";
case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY:
return "wait for identify controller";
case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
return "set keep alive timeout";
case NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT:
return "wait for set keep alive timeout";
case NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC:
return "identify controller iocs specific";
case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_IOCS_SPECIFIC:
@ -1395,10 +1399,6 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state)
return "set doorbell buffer config";
case NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG:
return "wait for doorbell buffer config";
case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
return "set keep alive timeout";
case NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT:
return "wait for set keep alive timeout";
case NVME_CTRLR_STATE_SET_HOST_ID:
return "set host ID";
case NVME_CTRLR_STATE_WAIT_FOR_HOST_ID:
@ -1490,7 +1490,7 @@ nvme_ctrlr_set_doorbell_buffer_config_done(void *arg, const struct spdk_nvme_cpl
} else {
NVME_CTRLR_INFOLOG(ctrlr, "Doorbell buffer config enabled\n");
}
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
ctrlr->opts.admin_timeout_ms);
}
@ -1501,13 +1501,13 @@ nvme_ctrlr_set_doorbell_buffer_config(struct spdk_nvme_ctrlr *ctrlr)
uint64_t prp1, prp2, len;
if (!ctrlr->cdata.oacs.doorbell_buffer_config) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
ctrlr->opts.admin_timeout_ms);
return 0;
}
if (ctrlr->trid.trtype != SPDK_NVME_TRANSPORT_PCIE) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
ctrlr->opts.admin_timeout_ms);
return 0;
}
@ -1925,7 +1925,7 @@ nvme_ctrlr_identify_done(void *arg, const struct spdk_nvme_cpl *cpl)
ctrlr->flags |= SPDK_NVME_CTRLR_COMPARE_AND_WRITE_SUPPORTED;
}
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC,
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
ctrlr->opts.admin_timeout_ms);
}
@ -2768,8 +2768,12 @@ nvme_ctrlr_set_keep_alive_timeout_done(void *arg, const struct spdk_nvme_cpl *cp
ctrlr->next_keep_alive_tick = spdk_get_ticks();
}
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
ctrlr->opts.admin_timeout_ms);
if (spdk_nvme_ctrlr_is_discovery(ctrlr)) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
} else {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC,
ctrlr->opts.admin_timeout_ms);
}
}
static int
@ -2778,15 +2782,20 @@ nvme_ctrlr_set_keep_alive_timeout(struct spdk_nvme_ctrlr *ctrlr)
int rc;
if (ctrlr->opts.keep_alive_timeout_ms == 0) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
ctrlr->opts.admin_timeout_ms);
if (spdk_nvme_ctrlr_is_discovery(ctrlr)) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
} else {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC,
ctrlr->opts.admin_timeout_ms);
}
return 0;
}
if (ctrlr->cdata.kas == 0) {
/* Note: Discovery controller identify data does not populate KAS according to spec. */
if (!spdk_nvme_ctrlr_is_discovery(ctrlr) && ctrlr->cdata.kas == 0) {
NVME_CTRLR_DEBUGLOG(ctrlr, "Controller KAS is 0 - not enabling Keep Alive\n");
ctrlr->opts.keep_alive_timeout_ms = 0;
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_HOST_ID,
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC,
ctrlr->opts.admin_timeout_ms);
return 0;
}
@ -3864,17 +3873,17 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
case NVME_CTRLR_STATE_RESET_ADMIN_QUEUE:
nvme_transport_qpair_reset(ctrlr->adminq);
if (spdk_nvme_ctrlr_is_discovery(ctrlr)) {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_READY, NVME_TIMEOUT_INFINITE);
} else {
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY, ctrlr->opts.admin_timeout_ms);
}
nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_IDENTIFY, NVME_TIMEOUT_INFINITE);
break;
case NVME_CTRLR_STATE_IDENTIFY:
rc = nvme_ctrlr_identify(ctrlr);
break;
case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
rc = nvme_ctrlr_set_keep_alive_timeout(ctrlr);
break;
case NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC:
rc = nvme_ctrlr_identify_iocs_specific(ctrlr);
break;
@ -3924,10 +3933,6 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
rc = nvme_ctrlr_set_doorbell_buffer_config(ctrlr);
break;
case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
rc = nvme_ctrlr_set_keep_alive_timeout(ctrlr);
break;
case NVME_CTRLR_STATE_SET_HOST_ID:
rc = nvme_ctrlr_set_host_id(ctrlr);
break;
@ -3949,6 +3954,7 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
case NVME_CTRLR_STATE_ENABLE_WAIT_FOR_CC:
case NVME_CTRLR_STATE_ENABLE_WAIT_FOR_READY_1_WAIT_FOR_CSTS:
case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY:
case NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT:
case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_IOCS_SPECIFIC:
case NVME_CTRLR_STATE_WAIT_FOR_GET_ZNS_CMD_EFFECTS_LOG:
case NVME_CTRLR_STATE_WAIT_FOR_SET_NUM_QUEUES:
@ -3958,7 +3964,6 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_NS_IOCS_SPECIFIC:
case NVME_CTRLR_STATE_WAIT_FOR_CONFIGURE_AER:
case NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG:
case NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT:
case NVME_CTRLR_STATE_WAIT_FOR_HOST_ID:
spdk_nvme_qpair_process_completions(ctrlr->adminq, 0);
break;

View File

@ -653,6 +653,16 @@ enum nvme_ctrlr_state {
*/
NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY,
/**
* Set Keep Alive Timeout of the controller.
*/
NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
/**
* Waiting for Set Keep Alive Timeout to be completed.
*/
NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT,
/**
* Get Identify I/O Command Set Specific Controller data structure.
*/
@ -754,16 +764,6 @@ enum nvme_ctrlr_state {
*/
NVME_CTRLR_STATE_WAIT_FOR_DB_BUF_CFG,
/**
* Set Keep Alive Timeout of the controller.
*/
NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT,
/**
* Waiting for Set Keep Alive Timeout to be completed.
*/
NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT,
/**
* Set Host ID of the controller.
*/

View File

@ -2284,6 +2284,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void)
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
@ -2303,6 +2305,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void)
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
@ -2324,6 +2328,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void)
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
@ -2345,6 +2351,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void)
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
@ -2366,6 +2374,8 @@ test_nvme_ctrlr_init_set_nvmf_ioccsz(void)
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
@ -2390,6 +2400,8 @@ test_nvme_ctrlr_init_set_num_queues(void)
SPDK_CU_ASSERT_FATAL(nvme_ctrlr_construct(&ctrlr) == 0);
ctrlr.state = NVME_CTRLR_STATE_IDENTIFY;
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_KEEP_ALIVE_TIMEOUT */
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_IDENTIFY_IOCS_SPECIFIC */
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> SET_NUM_QUEUES */
@ -2417,8 +2429,8 @@ test_nvme_ctrlr_init_set_keep_alive_timeout(void)
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(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_IOCS_SPECIFIC */
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(ctrlr.opts.keep_alive_timeout_ms == 120000);
fake_cpl.cdw0 = 0;
@ -2427,8 +2439,8 @@ test_nvme_ctrlr_init_set_keep_alive_timeout(void)
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(nvme_ctrlr_process_init(&ctrlr) == 0); /* -> IDENTIFY_IOCS_SPECIFIC */
CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_IDENTIFY_IOCS_SPECIFIC);
CU_ASSERT(ctrlr.opts.keep_alive_timeout_ms == 60000);
set_status_code = SPDK_NVME_SC_SUCCESS;