mi: track controller lifetime under endpoint and provide endpoint scan

This change introduces a list of controllers under each MI endpoint, and
ties the controller's lifetime to the endpoints - when an endpoint is
free()ed, so are its controllers.

We also provide a new function to populate the controllers list:

   nvme_mi_scan_ep()

- which scans the endpoint for its owned controllers. For retrieving the
scan results, we now have a new macro:

   nvme_mi_for_each_ctrl(ep, ctrl) {
     ...
   }

Add a couple of new tests: one to check the lifetime semantics, and
another to test the scan using a fake Read MI Data (Controller List)
response.

Signed-off-by: Jeremy Kerr <[email protected]>

Author: jk-ozlabs

src/libnvme-mi.map

@@ -19,8 +19,11 @@ LIBNVME_MI_1_1 {
 		nvme_mi_root_close;
 		nvme_mi_first_endpoint;
 		nvme_mi_next_endpoint;
+		nvme_mi_first_ctrl;
+		nvme_mi_next_ctrl;
 		nvme_mi_open_mctp;
 		nvme_mi_scan_mctp;
+		nvme_mi_scan_ep;
 	local:
 		*;
 };

src/nvme/mi.c

@@ -54,6 +54,8 @@ struct nvme_mi_ep *nvme_mi_init_ep(nvme_root_t root)
 	ep = calloc(1, sizeof(*ep));
 	list_node_init(&ep->root_entry);
 	ep->root = root;
+	ep->controllers_scanned = false;
+	list_head_init(&ep->controllers);
 
 	list_add(&root->endpoints, &ep->root_entry);
 
@@ -71,9 +73,52 @@ struct nvme_mi_ctrl *nvme_mi_init_ctrl(nvme_mi_ep_t ep, __u16 ctrl_id)
 	ctrl->ep = ep;
 	ctrl->id = ctrl_id;
 
+	list_add_tail(&ep->controllers, &ctrl->ep_entry);
+
 	return ctrl;
 }
 
+int nvme_mi_scan_ep(nvme_mi_ep_t ep, bool force_rescan)
+{
+	struct nvme_ctrl_list list;
+	unsigned int i, n_ctrl;
+	int rc;
+
+	if (ep->controllers_scanned) {
+		if (force_rescan) {
+			struct nvme_mi_ctrl *ctrl, *tmp;
+			nvme_mi_for_each_ctrl_safe(ep, ctrl, tmp)
+				nvme_mi_close_ctrl(ctrl);
+		} else {
+			return 0;
+		}
+	}
+
+	rc = nvme_mi_mi_read_mi_data_ctrl_list(ep, 0, &list);
+	if (rc)
+		return -1;
+
+	n_ctrl = le16_to_cpu(list.num);
+	if (n_ctrl > NVME_ID_CTRL_LIST_MAX)
+		return -1;
+
+	for (i = 0; i < n_ctrl; i++) {
+		struct nvme_mi_ctrl *ctrl;
+		__u16 id;
+
+		id = le32_to_cpu(list.identifier[i]);
+		if (!id)
+			continue;
+
+		ctrl = nvme_mi_init_ctrl(ep, id);
+		if (!ctrl)
+			break;
+	}
+
+	ep->controllers_scanned = true;
+	return 0;
+}
+
 __u32 nvme_mi_crc32_update(__u32 crc, void *data, size_t len)
 {
 	int i;
@@ -618,6 +663,14 @@ int nvme_mi_mi_subsystem_health_status_poll(nvme_mi_ep_t ep, bool clear,
 
 void nvme_mi_close(nvme_mi_ep_t ep)
 {
+	struct nvme_mi_ctrl *ctrl, *tmp;
+
+	/* don't look for controllers during destruction */
+	ep->controllers_scanned = true;
+
+	nvme_mi_for_each_ctrl_safe(ep, ctrl, tmp)
+		nvme_mi_close_ctrl(ctrl);
+
 	if (ep->transport->close)
 		ep->transport->close(ep);
 	list_del(&ep->root_entry);
@@ -626,6 +679,7 @@ void nvme_mi_close(nvme_mi_ep_t ep)
 
 void nvme_mi_close_ctrl(nvme_mi_ctrl_t ctrl)
 {
+	list_del(&ctrl->ep_entry);
 	free(ctrl);
 }
 
@@ -668,3 +722,13 @@ nvme_mi_ep_t nvme_mi_next_endpoint(nvme_root_t m, nvme_mi_ep_t ep)
 {
 	return ep ? list_next(&m->endpoints, ep, root_entry) : NULL;
 }
+
+nvme_mi_ctrl_t nvme_mi_first_ctrl(nvme_mi_ep_t ep)
+{
+	return list_top(&ep->controllers, struct nvme_mi_ctrl, ep_entry);
+}
+
+nvme_mi_ctrl_t nvme_mi_next_ctrl(nvme_mi_ep_t ep, nvme_mi_ctrl_t c)
+{
+	return c ? list_next(&ep->controllers, c, ep_entry) : NULL;
+}

src/nvme/mi.h

@@ -338,6 +338,63 @@ struct nvme_mi_ctrl;
  */
 typedef struct nvme_mi_ctrl * nvme_mi_ctrl_t;
 
+/**
+ * nvme_mi_first_ctrl - Start controller iterator
+ * @ep: &nvme_mi_ep_t object
+ *
+ * Return: first MI controller object under this root, or NULL if no controllers
+ *         are present.
+ *
+ * See: &nvme_mi_next_ctrl, &nvme_mi_for_each_ctrl
+ */
+nvme_mi_ctrl_t nvme_mi_first_ctrl(nvme_mi_ep_t ep);
+
+/**
+ * nvme_mi_next_ctrl - Continue ctrl iterator
+ * @ep: &nvme_mi_ep_t object
+ * @c: &nvme_mi_ctrl_t current position of iterator
+ *
+ * Return: next MI controller object after @c under this endpoint, or NULL
+ *         if no further controllers are present.
+ *
+ * See: &nvme_mi_first_ctrl, &nvme_mi_for_each_ctrl
+ */
+nvme_mi_ctrl_t nvme_mi_next_ctrl(nvme_mi_ep_t ep, nvme_mi_ctrl_t c);
+
+/**
+ * nvme_mi_for_each_ctrl - Iterator for NVMe-MI controllers.
+ * @ep: &nvme_mi_ep_t containing endpoints
+ * @c: &nvme_mi_ctrl_t object, set on each iteration
+ *
+ * Allows iteration of the list of controllers behind an endpoint. Unless the
+ * controllers have already been created explicitly, you'll probably want to
+ * call &nvme_mi_scan_ep() to scan for the controllers first.
+ *
+ * See: &nvme_mi_scan_ep()
+ */
+#define nvme_mi_for_each_ctrl(ep, c)			\
+	for (c = nvme_mi_first_ctrl(ep); c != NULL;	\
+	     c = nvme_mi_next_ctrl(ep, c))
+
+/**
+ * nvme_mi_for_each_ctrl_safe - Iterator for NVMe-MI controllers, allowing
+ * deletion during traversal
+ * @ep: &nvme_mi_ep_t containing controllers
+ * @c: &nvme_mi_ctrl_t object, set on each iteration
+ * @_c: &nvme_mi_ctrl_t object used as temporary storage
+ *
+ * Allows iteration of the list of controllers behind an endpoint, safe against
+ * deletion during iteration. Unless the controllers have already been created
+ * explicitly (or you're just iterating to destroy controllers) you'll probably
+ * want to call &nvme_mi_scan_ep() to scan for the controllers first.
+ *
+ * See: &nvme_mi_scan_ep()
+ */
+#define nvme_mi_for_each_ctrl_safe(ep, c, _c)			      \
+	for (c = nvme_mi_first_ctrl(ep), _c = nvme_mi_next_ctrl(ep, c);	      \
+	     c != NULL;							      \
+	     c = _c, _c = nvme_mi_next_ctrl(ep, c))
+
 /**
  * nvme_mi_open_mctp() - Create an endpoint using a MCTP connection.
  * @root: root object to create under
@@ -354,7 +411,8 @@ typedef struct nvme_mi_ctrl * nvme_mi_ctrl_t;
 nvme_mi_ep_t nvme_mi_open_mctp(nvme_root_t root, unsigned int netid, uint8_t eid);
 
 /**
- * nvme_mi_close() - Close an endpoint connection and release resources
+ * nvme_mi_close() - Close an endpoint connection and release resources,
+ * including controller objects.
  *
  * @ep: Endpoint object to close
  */
@@ -374,6 +432,26 @@ void nvme_mi_close(nvme_mi_ep_t ep);
  */
 nvme_root_t nvme_mi_scan_mctp(void);
 
+/**
+ * nvme_mi_scan_ep - query an endpoint for its NVMe controllers.
+ * @ep: Endpoint to scan
+ * @force_rescan: close existing controllers and rescan
+ *
+ * This function queries an MI endpoint for the controllers available, by
+ * performing an MI Read MI Data Structure command (requesting the
+ * controller list). The controllers are stored in the endpoint's internal
+ * list, and can be iterated with nvme_mi_for_each_ctrl.
+ *
+ * This will only scan the endpoint once, unless @force_rescan is set. If
+ * so, all existing controller objects will be freed - the caller must not
+ * hold a reference to those across this call.
+ *
+ * Return: 0 on success, non-zero on failure
+ *
+ * See: &nvme_mi_for_each_ctrl
+ */
+int nvme_mi_scan_ep(nvme_mi_ep_t ep, bool force_rescan);
+
 /**
  * nvme_mi_init_ctrl() - initialise a NVMe controller.
  * @ep: Endpoint to create under

src/nvme/private.h

@@ -189,11 +189,14 @@ struct nvme_mi_ep {
 	const struct nvme_mi_transport *transport;
 	void *transport_data;
 	struct list_node root_entry;
+	struct list_head controllers;
+	bool controllers_scanned;
 };
 
 struct nvme_mi_ctrl {
 	struct nvme_mi_ep	*ep;
 	__u16			id;
+	struct list_node	ep_entry;
 };
 
 struct nvme_mi_ep *nvme_mi_init_ep(struct nvme_root *root);

test/mi.c

@@ -152,6 +152,47 @@ static void test_endpoint_lifetime(nvme_mi_ep_t ep)
 	assert(count == 1);
 }
 
+unsigned int count_ep_controllers(nvme_mi_ep_t ep)
+{
+	unsigned int i = 0;
+	nvme_mi_ctrl_t ctrl;
+
+	nvme_mi_for_each_ctrl(ep, ctrl)
+		i++;
+
+	return i;
+}
+
+/* test that the ep->controllers list is updated on controller
+ * creation/destruction */
+static void test_ctrl_lifetime(nvme_mi_ep_t ep)
+{
+	nvme_mi_ctrl_t c1, c2;
+	int count;
+
+	ep->controllers_scanned = true;
+
+	count = count_ep_controllers(ep);
+	assert(count == 0);
+
+	c1 = nvme_mi_init_ctrl(ep, 1);
+	count = count_ep_controllers(ep);
+	assert(count == 1);
+
+	c2 = nvme_mi_init_ctrl(ep, 2);
+	count = count_ep_controllers(ep);
+	assert(count == 2);
+
+	nvme_mi_close_ctrl(c1);
+	count = count_ep_controllers(ep);
+	assert(count == 1);
+
+	nvme_mi_close_ctrl(c2);
+	count = count_ep_controllers(ep);
+	assert(count == 0);
+}
+
+
 /* test: basic read MI datastructure command */
 static int test_read_mi_data_cb(struct nvme_mi_ep *ep,
 				 struct nvme_mi_req *req,
@@ -262,6 +303,75 @@ static void test_invalid_crc(nvme_mi_ep_t ep)
 	assert(rc != 0);
 }
 
+/* test: test that the controller list populates the endpoint's list of
+ * controllers */
+static int test_scan_ctrl_list_cb(struct nvme_mi_ep *ep,
+				  struct nvme_mi_req *req,
+				  struct nvme_mi_resp *resp,
+				  void *data)
+{
+	__u8 ror, mt, *hdr, *buf;
+
+	assert(req->hdr->type == NVME_MI_MSGTYPE_NVME);
+
+	ror = req->hdr->nmp >> 7;
+	mt = req->hdr->nmp >> 3 & 0x7;
+	assert(ror == NVME_MI_ROR_REQ);
+	assert(mt == NVME_MI_MT_MI);
+
+	/* do we have enough for a mi header? */
+	assert(req->hdr_len == sizeof(struct nvme_mi_mi_req_hdr));
+
+	/* inspect response as raw bytes */
+	hdr = (__u8 *)req->hdr;
+	assert(hdr[4] == nvme_mi_mi_opcode_mi_data_read);
+	assert(hdr[11] == nvme_mi_dtyp_ctrl_list);
+
+	/* create basic response */
+	assert(resp->hdr_len >= sizeof(struct nvme_mi_mi_resp_hdr));
+	assert(resp->data_len >= 4);
+
+	hdr = (__u8 *)resp->hdr;
+	hdr[4] = 0; /* status */
+
+	buf = (__u8 *)resp->data;
+	memset(buf, 0, resp->data_len);
+	buf[0] = 3; buf[1] = 0; /* num controllers */
+	buf[2] = 1; buf[3] = 0; /* id 1 */
+	buf[4] = 4; buf[5] = 0; /* id 4 */
+	buf[6] = 5; buf[7] = 0; /* id 5 */
+
+	test_transport_resp_calc_mic(resp);
+
+	return 0;
+}
+
+static void test_scan_ctrl_list(nvme_mi_ep_t ep)
+{
+	struct nvme_mi_ctrl *ctrl;
+
+	ep->controllers_scanned = false;
+
+	test_set_transport_callback(ep, test_scan_ctrl_list_cb, NULL);
+
+	nvme_mi_scan_ep(ep, false);
+
+	ctrl = nvme_mi_first_ctrl(ep);
+	assert(ctrl);
+	assert(ctrl->id == 1);
+
+	ctrl = nvme_mi_next_ctrl(ep, ctrl);
+	assert(ctrl);
+	assert(ctrl->id == 4);
+
+	ctrl = nvme_mi_next_ctrl(ep, ctrl);
+	assert(ctrl);
+	assert(ctrl->id == 5);
+
+	ctrl = nvme_mi_next_ctrl(ep, ctrl);
+	assert(ctrl == NULL);
+}
+
 /* test: simple NVMe admin request/response */
 static int test_admin_id_cb(struct nvme_mi_ep *ep,
 				  struct nvme_mi_req *req,
@@ -389,9 +499,11 @@ struct test {
 	void (*fn)(nvme_mi_ep_t);
 } tests[] = {
 	DEFINE_TEST(endpoint_lifetime),
+	DEFINE_TEST(ctrl_lifetime),
 	DEFINE_TEST(read_mi_data),
 	DEFINE_TEST(transport_fail),
 	DEFINE_TEST(transport_describe),
+	DEFINE_TEST(scan_ctrl_list),
 	DEFINE_TEST(invalid_crc),
 	DEFINE_TEST(admin_id),
 	DEFINE_TEST(admin_err_resp),