lib/nvmf: mask pending AENs

Per section 5.2 of the NVMe 1.3 spec:

"When the controller posts a completion queue entry for an outstanding
Asynchronous Event Request command and thus reports an asynchronous
event, subsequent events of that event type are automatically masked by
the controller until the host clears that event. An event is cleared by
reading the log page associated with that event using the Get Log Page
command (see section 5.14)."

Signed-off-by: Jiewei Ke <jiewei@smartx.com>
Change-Id: I0773ce1b704c2124db354fd19e0a8a19da48da54
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5462
Community-CI: Broadcom CI
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>

lib/nvmf/ctrlr.c

@@ -1692,11 +1692,46 @@ nvmf_get_firmware_slot_log_page(void *buffer, uint64_t offset, uint32_t length)
 	}
 }
 
+/*
+ * Asynchronous Event Mask Bit
+ */
+enum spdk_nvme_async_event_mask_bit {
+	/* Mask Namespace Change Notificaton */
+	SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGE_MASK_BIT		= 0,
+	/* Mask Asymmetric Namespace Access Change Notification */
+	SPDK_NVME_ASYNC_EVENT_ANA_CHANGE_MASK_BIT		= 1,
+	/* Mask Discovery Log Change Notification */
+	SPDK_NVME_ASYNC_EVENT_DISCOVERY_LOG_CHANGE_MASK_BIT	= 2,
+	/* Mask Reservation Log Page Available Notification */
+	SPDK_NVME_ASYNC_EVENT_RESERVATION_LOG_AVAIL_MASK_BIT	= 3,
+
+	/* 4 - 63 Reserved */
+};
+
+static inline void
+nvmf_ctrlr_unmask_aen(struct spdk_nvmf_ctrlr *ctrlr,
+		      enum spdk_nvme_async_event_mask_bit mask)
+{
+	ctrlr->notice_aen_mask &= ~(1 << mask);
+}
+
+static inline bool
+nvmf_ctrlr_mask_aen(struct spdk_nvmf_ctrlr *ctrlr,
+		    enum spdk_nvme_async_event_mask_bit mask)
+{
+	if (ctrlr->notice_aen_mask & (1 << mask)) {
+		return false;
+	} else {
+		ctrlr->notice_aen_mask |= (1 << mask);
+		return true;
+	}
+}
+
 #define SPDK_NVMF_ANA_DESC_SIZE	(sizeof(struct spdk_nvme_ana_group_descriptor) +	\
 				 sizeof(uint32_t))
 static void
 nvmf_get_ana_log_page(struct spdk_nvmf_ctrlr *ctrlr, void *data,
-		      uint64_t offset, uint32_t length)
+		      uint64_t offset, uint32_t length, uint32_t rae)
 {
 	char *buf = data;
 	struct spdk_nvme_ana_page ana_hdr;
@@ -1760,9 +1795,14 @@ nvmf_get_ana_log_page(struct spdk_nvmf_ctrlr *ctrlr, void *data,
 		offset = 0;
 
 		if (length == 0) {
-			return;
+			goto done;
 		}
 	}
+
+done:
+	if (!rae) {
+		nvmf_ctrlr_unmask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_ANA_CHANGE_MASK_BIT);
+	}
 }
 
 void
@@ -1795,7 +1835,7 @@ nvmf_ctrlr_ns_changed(struct spdk_nvmf_ctrlr *ctrlr, uint32_t nsid)
 
 static void
 nvmf_get_changed_ns_list_log_page(struct spdk_nvmf_ctrlr *ctrlr,
-				  void *buffer, uint64_t offset, uint32_t length)
+				  void *buffer, uint64_t offset, uint32_t length, uint32_t rae)
 {
 	size_t copy_length;
 
@@ -1809,6 +1849,10 @@ nvmf_get_changed_ns_list_log_page(struct spdk_nvmf_ctrlr *ctrlr,
 	/* Clear log page each time it is read */
 	ctrlr->changed_ns_list_count = 0;
 	memset(&ctrlr->changed_ns_list, 0, sizeof(ctrlr->changed_ns_list));
+
+	if (!rae) {
+		nvmf_ctrlr_unmask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGE_MASK_BIT);
+	}
 }
 
 /* The structure can be modified if we provide support for other commands in future */
@@ -1867,7 +1911,7 @@ nvmf_get_cmds_and_effects_log_page(void *buffer,
 
 static void
 nvmf_get_reservation_notification_log_page(struct spdk_nvmf_ctrlr *ctrlr,
-		void *data, uint64_t offset, uint32_t length)
+		void *data, uint64_t offset, uint32_t length, uint32_t rae)
 {
 	uint32_t unit_log_len, avail_log_len, next_pos, copy_len;
 	struct spdk_nvmf_reservation_log *log, *log_tmp;
@@ -1904,6 +1948,10 @@ nvmf_get_reservation_notification_log_page(struct spdk_nvmf_ctrlr *ctrlr,
 			break;
 		}
 	}
+
+	if (!rae) {
+		nvmf_ctrlr_unmask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_RESERVATION_LOG_AVAIL_MASK_BIT);
+	}
 	return;
 }
 
@@ -1915,7 +1963,7 @@ nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)
 	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;
 	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;
 	uint64_t offset, len;
-	uint32_t numdl, numdu;
+	uint32_t rae, numdl, numdu;
 	uint8_t lid;
 
 	if (req->data == NULL) {
@@ -1933,6 +1981,7 @@ nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)
 		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 	}
 
+	rae = cmd->cdw10_bits.get_log_page.rae;
 	numdl = cmd->cdw10_bits.get_log_page.numdl;
 	numdu = cmd->cdw11_bits.get_log_page.numdu;
 	len = ((numdu << 16) + numdl + (uint64_t)1) * 4;
@@ -1945,14 +1994,17 @@ nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)
 	}
 
 	lid = cmd->cdw10_bits.get_log_page.lid;
-	SPDK_DEBUGLOG(nvmf, "Get log page: LID=0x%02X offset=0x%" PRIx64 " len=0x%" PRIx64 "\n",
+	SPDK_DEBUGLOG(nvmf, "Get log page: LID=0x%02X offset=0x%" PRIx64 " len=0x%" PRIx64 " rae=%u\n",
-		      lid, offset, len);
+		      lid, offset, len, rae);
 
 	if (subsystem->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) {
 		switch (lid) {
 		case SPDK_NVME_LOG_DISCOVERY:
 			nvmf_get_discovery_log_page(subsystem->tgt, ctrlr->hostnqn, req->iov, req->iovcnt, offset,
 						    len);
+			if (!rae) {
+				nvmf_ctrlr_unmask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_DISCOVERY_LOG_CHANGE_MASK_BIT);
+			}
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		default:
 			goto invalid_log_page;
@@ -1968,7 +2020,7 @@ nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		case SPDK_NVME_LOG_ASYMMETRIC_NAMESPACE_ACCESS:
 			if (subsystem->flags.ana_reporting) {
-				nvmf_get_ana_log_page(ctrlr, req->data, offset, len);
+				nvmf_get_ana_log_page(ctrlr, req->data, offset, len, rae);
 				return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 			} else {
 				goto invalid_log_page;
@@ -1977,10 +2029,10 @@ nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)
 			nvmf_get_cmds_and_effects_log_page(req->data, offset, len);
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		case SPDK_NVME_LOG_CHANGED_NS_LIST:
-			nvmf_get_changed_ns_list_log_page(ctrlr, req->data, offset, len);
+			nvmf_get_changed_ns_list_log_page(ctrlr, req->data, offset, len, rae);
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		case SPDK_NVME_LOG_RESERVATION_NOTIFICATION:
-			nvmf_get_reservation_notification_log_page(ctrlr, req->data, offset, len);
+			nvmf_get_reservation_notification_log_page(ctrlr, req->data, offset, len, rae);
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		default:
 			goto invalid_log_page;
@@ -2857,6 +2909,10 @@ nvmf_ctrlr_async_event_ns_notice(struct spdk_nvmf_ctrlr *ctrlr)
 		return 0;
 	}
 
+	if (!nvmf_ctrlr_mask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGE_MASK_BIT)) {
+		return 0;
+	}
+
 	event.bits.async_event_type = SPDK_NVME_ASYNC_EVENT_TYPE_NOTICE;
 	event.bits.async_event_info = SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGED;
 	event.bits.log_page_identifier = SPDK_NVME_LOG_CHANGED_NS_LIST;
@@ -2883,6 +2939,10 @@ nvmf_ctrlr_async_event_ana_change_notice(struct spdk_nvmf_ctrlr *ctrlr)
 		return 0;
 	}
 
+	if (!nvmf_ctrlr_mask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_ANA_CHANGE_MASK_BIT)) {
+		return 0;
+	}
+
 	event.bits.async_event_type = SPDK_NVME_ASYNC_EVENT_TYPE_NOTICE;
 	event.bits.async_event_info = SPDK_NVME_ASYNC_EVENT_ANA_CHANGE;
 	event.bits.log_page_identifier = SPDK_NVME_LOG_ASYMMETRIC_NAMESPACE_ACCESS;
@@ -2907,6 +2967,11 @@ nvmf_ctrlr_async_event_reservation_notification(struct spdk_nvmf_ctrlr *ctrlr)
 	if (!ctrlr->num_avail_log_pages) {
 		return;
 	}
+
+	if (!nvmf_ctrlr_mask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_RESERVATION_LOG_AVAIL_MASK_BIT)) {
+		return;
+	}
+
 	event.bits.async_event_type = SPDK_NVME_ASYNC_EVENT_TYPE_IO;
 	event.bits.async_event_info = SPDK_NVME_ASYNC_EVENT_RESERVATION_LOG_AVAIL;
 	event.bits.log_page_identifier = SPDK_NVME_LOG_RESERVATION_NOTIFICATION;
@@ -2936,6 +3001,10 @@ nvmf_ctrlr_async_event_discovery_log_change_notice(struct spdk_nvmf_ctrlr *ctrlr
 		return 0;
 	}
 
+	if (!nvmf_ctrlr_mask_aen(ctrlr, SPDK_NVME_ASYNC_EVENT_DISCOVERY_LOG_CHANGE_MASK_BIT)) {
+		return 0;
+	}
+
 	event.bits.async_event_type = SPDK_NVME_ASYNC_EVENT_TYPE_NOTICE;
 	event.bits.async_event_info = SPDK_NVME_ASYNC_EVENT_DISCOVERY_LOG_CHANGE;
 	event.bits.log_page_identifier = SPDK_NVME_LOG_DISCOVERY;

lib/nvmf/nvmf_internal.h

@@ -232,6 +232,7 @@ struct spdk_nvmf_ctrlr {
 
 	struct spdk_nvmf_request *aer_req[NVMF_MAX_ASYNC_EVENTS];
 	STAILQ_HEAD(, spdk_nvmf_async_event_completion) async_events;
+	uint64_t notice_aen_mask;
 	uint8_t nr_aer_reqs;
 	struct spdk_uuid  hostid;
 

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

@@ -1334,6 +1334,18 @@ cleanup_pending_async_events(struct spdk_nvmf_ctrlr *ctrlr)
 	}
 }
 
+static int
+num_pending_async_events(struct spdk_nvmf_ctrlr *ctrlr)
+{
+	int num = 0;
+	struct spdk_nvmf_async_event_completion *event;
+
+	STAILQ_FOREACH(event, &ctrlr->async_events, link) {
+		num++;
+	}
+	return num;
+}
+
 static void
 test_reservation_notification_log_page(void)
 {
@@ -1394,7 +1406,7 @@ test_reservation_notification_log_page(void)
 	SPDK_CU_ASSERT_FATAL(ctrlr.num_avail_log_pages == 3);
 
 	/* Test Case: Get Log Page to clear the log pages */
-	nvmf_get_reservation_notification_log_page(&ctrlr, (void *)logs, 0, sizeof(logs));
+	nvmf_get_reservation_notification_log_page(&ctrlr, (void *)logs, 0, sizeof(logs), 0);
 	SPDK_CU_ASSERT_FATAL(ctrlr.num_avail_log_pages == 0);
 
 	cleanup_pending_async_events(&ctrlr);
@@ -1794,7 +1806,7 @@ test_get_ana_log_page(void)
 	offset = 0;
 	while (offset < UT_ANA_LOG_PAGE_SIZE) {
 		length = spdk_min(16, UT_ANA_LOG_PAGE_SIZE - offset);
-		nvmf_get_ana_log_page(&ctrlr, &actual_page[offset], offset, length);
+		nvmf_get_ana_log_page(&ctrlr, &actual_page[offset], offset, length, 0);
 		offset += length;
 	}
 
@@ -1875,6 +1887,98 @@ test_multi_async_events(void)
 	cleanup_pending_async_events(&ctrlr);
 }
 
+static void
+test_rae(void)
+{
+	struct spdk_nvmf_subsystem subsystem = {};
+	struct spdk_nvmf_qpair qpair = {};
+	struct spdk_nvmf_ctrlr ctrlr = {};
+	struct spdk_nvmf_request req[3] = {};
+	struct spdk_nvmf_ns *ns_ptrs[1] = {};
+	struct spdk_nvmf_ns ns = {};
+	union nvmf_h2c_msg cmd[3] = {};
+	union nvmf_c2h_msg rsp[3] = {};
+	union spdk_nvme_async_event_completion event = {};
+	struct spdk_nvmf_poll_group group = {};
+	struct spdk_nvmf_subsystem_poll_group sgroups = {};
+	int i;
+	char data[4096];
+
+	ns_ptrs[0] = &ns;
+	subsystem.ns = ns_ptrs;
+	subsystem.max_nsid = 1;
+	subsystem.subtype = SPDK_NVMF_SUBTYPE_NVME;
+
+	ns.opts.nsid = 1;
+	group.sgroups = &sgroups;
+
+	qpair.ctrlr = &ctrlr;
+	qpair.group = &group;
+	TAILQ_INIT(&qpair.outstanding);
+
+	ctrlr.subsys = &subsystem;
+	ctrlr.vcprop.cc.bits.en = 1;
+	ctrlr.feat.async_event_configuration.bits.ns_attr_notice = 1;
+	init_pending_async_events(&ctrlr);
+
+	/* Target queue pending events when there is no outstanding AER request */
+	nvmf_ctrlr_async_event_ns_notice(&ctrlr);
+	nvmf_ctrlr_async_event_ns_notice(&ctrlr);
+	nvmf_ctrlr_async_event_ns_notice(&ctrlr);
+	/* only one event will be queued before RAE is clear */
+	CU_ASSERT(num_pending_async_events(&ctrlr) == 1);
+
+	req[0].qpair = &qpair;
+	req[0].cmd = &cmd[0];
+	req[0].rsp = &rsp[0];
+	cmd[0].nvme_cmd.opc = SPDK_NVME_OPC_ASYNC_EVENT_REQUEST;
+	cmd[0].nvme_cmd.nsid = 1;
+	cmd[0].nvme_cmd.cid = 0;
+
+	for (i = 1; i < 3; i++) {
+		req[i].qpair = &qpair;
+		req[i].cmd = &cmd[i];
+		req[i].rsp = &rsp[i];
+		req[i].data = &data;
+		req[i].length = sizeof(data);
+
+		cmd[i].nvme_cmd.opc = SPDK_NVME_OPC_GET_LOG_PAGE;
+		cmd[i].nvme_cmd.cdw10_bits.get_log_page.lid =
+			SPDK_NVME_LOG_CHANGED_NS_LIST;
+		cmd[i].nvme_cmd.cdw10_bits.get_log_page.numdl =
+			(req[i].length / 4 - 1);
+		cmd[i].nvme_cmd.cid = i;
+	}
+	cmd[1].nvme_cmd.cdw10_bits.get_log_page.rae = 1;
+	cmd[2].nvme_cmd.cdw10_bits.get_log_page.rae = 0;
+
+	/* consume the pending event */
+	TAILQ_INSERT_TAIL(&qpair.outstanding, &req[0], link);
+	CU_ASSERT(nvmf_ctrlr_process_admin_cmd(&req[0]) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);
+	event.raw = rsp[0].nvme_cpl.cdw0;
+	CU_ASSERT(event.bits.async_event_info == SPDK_NVME_ASYNC_EVENT_NS_ATTR_CHANGED);
+	CU_ASSERT(num_pending_async_events(&ctrlr) == 0);
+
+	/* get log with RAE set */
+	CU_ASSERT(nvmf_ctrlr_get_log_page(&req[1]) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);
+	CU_ASSERT(rsp[1].nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);
+	CU_ASSERT(rsp[1].nvme_cpl.status.sc == SPDK_NVME_SC_SUCCESS);
+
+	/* will not generate new event until RAE is clear */
+	nvmf_ctrlr_async_event_ns_notice(&ctrlr);
+	CU_ASSERT(num_pending_async_events(&ctrlr) == 0);
+
+	/* get log with RAE clear */
+	CU_ASSERT(nvmf_ctrlr_get_log_page(&req[2]) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);
+	CU_ASSERT(rsp[2].nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);
+	CU_ASSERT(rsp[2].nvme_cpl.status.sc == SPDK_NVME_SC_SUCCESS);
+
+	nvmf_ctrlr_async_event_ns_notice(&ctrlr);
+	CU_ASSERT(num_pending_async_events(&ctrlr) == 1);
+
+	cleanup_pending_async_events(&ctrlr);
+}
+
 int main(int argc, char **argv)
 {
 	CU_pSuite	suite = NULL;
@@ -1902,6 +2006,7 @@ int main(int argc, char **argv)
 	CU_ADD_TEST(suite, test_multi_async_event_reqs);
 	CU_ADD_TEST(suite, test_get_ana_log_page);
 	CU_ADD_TEST(suite, test_multi_async_events);
+	CU_ADD_TEST(suite, test_rae);
 
 	allocate_threads(1);
 	set_thread(0);