nvme: configure AER for discovery controllers

Move the CONFIGURE_AER state before SET_KEEP_ALIVE to
make sure that we run the CONFIGURE_AER state for
discovery controllers.

Signed-off-by: Jim Harris <james.r.harris@intel.com>
Change-Id: Ia4e24f6507c43e3fece06b9161ff8e0b8fa0e97d
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10332
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Aleksey Marchuk <alexeymar@mellanox.com>

lib/nvme/nvme_ctrlr.c

@@ -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_CONFIGURE_AER:
+		return "configure AER";
+	case NVME_CTRLR_STATE_WAIT_FOR_CONFIGURE_AER:
+		return "wait for configure aer";
 	case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
 		return "set keep alive timeout";
 	case NVME_CTRLR_STATE_WAIT_FOR_KEEP_ALIVE_TIMEOUT:
@@ -1387,10 +1391,6 @@ nvme_ctrlr_state_string(enum nvme_ctrlr_state state)
 		return "identify ns iocs specific";
 	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_NS_IOCS_SPECIFIC:
 		return "wait for identify ns iocs specific";
-	case NVME_CTRLR_STATE_CONFIGURE_AER:
-		return "configure AER";
-	case NVME_CTRLR_STATE_WAIT_FOR_CONFIGURE_AER:
-		return "wait for configure aer";
 	case NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES:
 		return "set supported log pages";
 	case NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES:
@@ -1945,7 +1945,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_SET_KEEP_ALIVE_TIMEOUT,
+	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER,
 			     ctrlr->opts.admin_timeout_ms);
 }
 
@@ -2480,7 +2480,7 @@ nvme_ctrlr_identify_namespaces_iocs_specific_next(struct spdk_nvme_ctrlr *ctrlr,
 	ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);
 	if (ns == NULL) {
 		/* No first/next active NS, move on to the next state */
-		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER,
+		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES,
 				     ctrlr->opts.admin_timeout_ms);
 		return 0;
 	}
@@ -2491,7 +2491,7 @@ nvme_ctrlr_identify_namespaces_iocs_specific_next(struct spdk_nvme_ctrlr *ctrlr,
 		ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);
 		if (ns == NULL) {
 			/* no namespace with (supported) iocs specific data found */
-			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER,
+			nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES,
 					     ctrlr->opts.admin_timeout_ms);
 			return 0;
 		}
@@ -2562,7 +2562,7 @@ nvme_ctrlr_identify_namespaces_iocs_specific(struct spdk_nvme_ctrlr *ctrlr)
 {
 	if (!nvme_ctrlr_multi_iocs_enabled(ctrlr)) {
 		/* Multi IOCS not supported/enabled, move on to the next state */
-		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_CONFIGURE_AER,
+		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES,
 				     ctrlr->opts.admin_timeout_ms);
 		return 0;
 	}
@@ -3227,8 +3227,7 @@ nvme_ctrlr_configure_aer_done(void *arg, const struct spdk_nvme_cpl *cpl)
 			return;
 		}
 	}
-	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES,
+	nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT, ctrlr->opts.admin_timeout_ms);
-			     ctrlr->opts.admin_timeout_ms);
 }
 
 static int
@@ -3903,6 +3902,10 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
 		rc = nvme_ctrlr_identify(ctrlr);
 		break;
 
+	case NVME_CTRLR_STATE_CONFIGURE_AER:
+		rc = nvme_ctrlr_configure_aer(ctrlr);
+		break;
+
 	case NVME_CTRLR_STATE_SET_KEEP_ALIVE_TIMEOUT:
 		rc = nvme_ctrlr_set_keep_alive_timeout(ctrlr);
 		break;
@@ -3936,10 +3939,6 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
 		rc = nvme_ctrlr_identify_namespaces_iocs_specific(ctrlr);
 		break;
 
-	case NVME_CTRLR_STATE_CONFIGURE_AER:
-		rc = nvme_ctrlr_configure_aer(ctrlr);
-		break;
-
 	case NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES:
 		rc = nvme_ctrlr_set_supported_log_pages(ctrlr);
 		nvme_ctrlr_set_state(ctrlr, NVME_CTRLR_STATE_SET_SUPPORTED_FEATURES,
@@ -3977,6 +3976,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_CONFIGURE_AER:
 	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:
@@ -3985,7 +3985,6 @@ nvme_ctrlr_process_init(struct spdk_nvme_ctrlr *ctrlr)
 	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_NS:
 	case NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_ID_DESCS:
 	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_HOST_ID:
 		spdk_nvme_qpair_process_completions(ctrlr->adminq, 0);

lib/nvme/nvme_internal.h

@@ -653,6 +653,16 @@ enum nvme_ctrlr_state {
 	 */
 	NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY,
 
+	/**
+	 * Configure AER of the controller.
+	 */
+	NVME_CTRLR_STATE_CONFIGURE_AER,
+
+	/**
+	 * Waiting for the Configure AER to be completed.
+	 */
+	NVME_CTRLR_STATE_WAIT_FOR_CONFIGURE_AER,
+
 	/**
 	 * Set Keep Alive Timeout of the controller.
 	 */
@@ -734,16 +744,6 @@ enum nvme_ctrlr_state {
 	 */
 	NVME_CTRLR_STATE_WAIT_FOR_IDENTIFY_ID_DESCS,
 
-	/**
-	 * Configure AER of the controller.
-	 */
-	NVME_CTRLR_STATE_CONFIGURE_AER,
-
-	/**
-	 * Waiting for the Configure AER to be completed.
-	 */
-	NVME_CTRLR_STATE_WAIT_FOR_CONFIGURE_AER,
-
 	/**
 	 * Set supported log pages of the controller.
 	 */

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

@@ -391,6 +391,7 @@ int
 nvme_qpair_submit_request(struct spdk_nvme_qpair *qpair, struct nvme_request *req)
 {
 	CU_ASSERT(req->cmd.opc == SPDK_NVME_OPC_ASYNC_EVENT_REQUEST);
+	STAILQ_INSERT_HEAD(&qpair->free_req, req, stailq);
 
 	/*
 	 * For the purposes of this unit test, we don't need to bother emulating request submission.
@@ -2284,6 +2285,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_CONFIGURE_AER);
+	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);
@@ -2305,6 +2308,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_CONFIGURE_AER);
+	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);
@@ -2328,6 +2333,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_CONFIGURE_AER);
+	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);
@@ -2351,6 +2358,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_CONFIGURE_AER);
+	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);
@@ -2374,6 +2383,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_CONFIGURE_AER);
+	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);
@@ -2400,11 +2411,13 @@ 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(nvme_ctrlr_process_init(&ctrlr) == 0);
+	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONFIGURE_AER);
+	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); /* -> SET_IDENTIFY_IOCS_SPECIFIC */
+	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); /* -> SET_NUM_QUEUES */
+	CU_ASSERT(nvme_ctrlr_process_init(&ctrlr) == 0);
 	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_NUM_QUEUES);
 
 	ctrlr.opts.num_io_queues = 64;
@@ -2788,7 +2801,7 @@ test_nvme_ctrlr_aer_callback(void)
 	ctrlr.vs.bits.ter = 0;
 	ctrlr.cdata.nn = 4096;
 
-	ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS;
+	ctrlr.state = NVME_CTRLR_STATE_CONFIGURE_AER;
 	g_active_ns_list = active_ns_list;
 	g_active_ns_list_length = SPDK_COUNTOF(active_ns_list);
 	while (ctrlr.state != NVME_CTRLR_STATE_READY) {
@@ -2835,9 +2848,10 @@ test_nvme_ctrlr_ns_attr_changed(void)
 	ctrlr.cap.bits.css |= SPDK_NVME_CAP_CSS_IOCS;
 	ctrlr.cdata.nn = 4096;
 
-	ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS;
+	ctrlr.state = NVME_CTRLR_STATE_CONFIGURE_AER;
 	g_active_ns_list = active_ns_list;
 	g_active_ns_list_length = SPDK_COUNTOF(active_ns_list);
+
 	while (ctrlr.state != NVME_CTRLR_STATE_READY) {
 		SPDK_CU_ASSERT_FATAL(nvme_ctrlr_process_init(&ctrlr) == 0);
 	}
@@ -2890,7 +2904,7 @@ test_nvme_ctrlr_identify_namespaces_iocs_specific_next(void)
 	ctrlr.opts.admin_timeout_ms = NVME_TIMEOUT_INFINITE;
 	rc = nvme_ctrlr_identify_namespaces_iocs_specific_next(&ctrlr, prev_nsid);
 	CU_ASSERT(rc == 0);
-	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONFIGURE_AER);
+	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES);
 	CU_ASSERT(ctrlr.state_timeout_tsc == NVME_TIMEOUT_INFINITE);
 
 	/* case 2: move on to the next active NS, and no namespace with (supported) iocs specific data found , expect: pass */
@@ -2902,7 +2916,7 @@ test_nvme_ctrlr_identify_namespaces_iocs_specific_next(void)
 	ns[1].id = 2;
 	rc = nvme_ctrlr_identify_namespaces_iocs_specific_next(&ctrlr, prev_nsid);
 	CU_ASSERT(rc == 0);
-	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_CONFIGURE_AER);
+	CU_ASSERT(ctrlr.state == NVME_CTRLR_STATE_SET_SUPPORTED_LOG_PAGES);
 	CU_ASSERT(ctrlr.state_timeout_tsc == NVME_TIMEOUT_INFINITE);
 
 	/* case 3: ns.csi is SPDK_NVME_CSI_ZNS, do not loop, expect: pass */
@@ -3108,7 +3122,7 @@ test_nvme_ctrlr_ana_resize(void)
 	ctrlr.cdata.cmic.ana_reporting = true;
 	ctrlr.cdata.nanagrpid = 1;
 
-	ctrlr.state = NVME_CTRLR_STATE_IDENTIFY_ACTIVE_NS;
+	ctrlr.state = NVME_CTRLR_STATE_CONFIGURE_AER;
 	/* Start with 2 active namespaces */
 	g_active_ns_list = active_ns_list;
 	g_active_ns_list_length = 2;