nvmf_tgt: implement log page offset for discovery

Generate the full discovery log page in a memory buffer, then copy just
the requested part of it for each Get Log Page call.

Change-Id: I12730c59c0395cdac57aaab96337e938952e3011
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>

lib/nvmf/request.c

@@ -73,7 +73,6 @@ nvmf_process_discovery_cmd(struct spdk_nvmf_request *req)
 	struct spdk_nvmf_session *session = req->conn->sess;
 	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;
 	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;
-	struct spdk_nvmf_discovery_log_page *log;
 	uint64_t log_page_offset;
 
 	/* pre-set response details for this command */
@@ -107,9 +106,7 @@ nvmf_process_discovery_cmd(struct spdk_nvmf_request *req)
 		}
 
 		if ((cmd->cdw10 & 0xFF) == SPDK_NVME_LOG_DISCOVERY) {
-			log = (struct spdk_nvmf_discovery_log_page *)req->data;
+			spdk_nvmf_get_discovery_log_page(req->data, log_page_offset, req->length);
-			log->numrec = 0;
-			spdk_format_discovery_log(log, req->length);
 			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;
 		} else {
 			SPDK_ERRLOG("Unsupported log page %u\n", cmd->cdw10 & 0xFF);

lib/nvmf/subsystem.c

@@ -47,6 +47,8 @@
 
 static TAILQ_HEAD(, spdk_nvmf_subsystem) g_subsystems = TAILQ_HEAD_INITIALIZER(g_subsystems);
 static uint64_t g_discovery_genctr = 0;
+static struct spdk_nvmf_discovery_log_page *g_discovery_log_page = NULL;
+static size_t g_discovery_log_page_size = 0;
 
 bool
 spdk_nvmf_subsystem_exists(const char *subnqn)
@@ -322,14 +324,26 @@ nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem,
 	return 0;
 }
 
-void
+static void
-spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_t length)
+nvmf_update_discovery_log(void)
 {
-	int numrec = 0;
+	uint64_t numrec = 0;
 	struct spdk_nvmf_subsystem *subsystem;
 	struct spdk_nvmf_listen_addr *listen_addr;
 	struct spdk_nvmf_discovery_log_page_entry *entry;
 	const struct spdk_nvmf_transport *transport;
+	struct spdk_nvmf_discovery_log_page *disc_log;
+	size_t cur_size;
+
+	SPDK_TRACELOG(SPDK_TRACE_NVMF, "Generating log page for genctr %" PRIu64 "\n",
+		      g_discovery_genctr);
+
+	cur_size = sizeof(struct spdk_nvmf_discovery_log_page);
+	disc_log = calloc(1, cur_size);
+	if (disc_log == NULL) {
+		SPDK_ERRLOG("Discovery log page memory allocation error\n");
+		return;
+	}
 
 	TAILQ_FOREACH(subsystem, &g_subsystems, entries) {
 		if (subsystem->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) {
@@ -337,30 +351,68 @@ spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_
 		}
 
 		TAILQ_FOREACH(listen_addr, &subsystem->listen_addrs, link) {
-			/* include the discovery log entry */
+			size_t new_size = cur_size + sizeof(*entry);
-			if (length > sizeof(struct spdk_nvmf_discovery_log_page)) {
+			void *new_log_page = realloc(disc_log, new_size);
-				if (sizeof(struct spdk_nvmf_discovery_log_page) + (numrec + 1) * sizeof(
-					    struct spdk_nvmf_discovery_log_page_entry) > length) {
-					break;
-				}
-				entry = &disc_log->entries[numrec];
-				entry->portid = numrec;
-				entry->cntlid = 0xffff;
-				entry->asqsz = g_nvmf_tgt.max_queue_depth;
-				entry->subtype = subsystem->subtype;
-				snprintf(entry->subnqn, sizeof(entry->subnqn), "%s", subsystem->subnqn);
 
-				transport = spdk_nvmf_transport_get(listen_addr->trname);
+			if (new_log_page == NULL) {
-				assert(transport != NULL);
+				SPDK_ERRLOG("Discovery log page memory allocation error\n");
-
+				break;
-				transport->listen_addr_discover(listen_addr, entry);
 			}
+
+			disc_log = new_log_page;
+			cur_size = new_size;
+
+			entry = &disc_log->entries[numrec];
+			memset(entry, 0, sizeof(*entry));
+			entry->portid = numrec;
+			entry->cntlid = 0xffff;
+			entry->asqsz = g_nvmf_tgt.max_queue_depth;
+			entry->subtype = subsystem->subtype;
+			snprintf(entry->subnqn, sizeof(entry->subnqn), "%s", subsystem->subnqn);
+
+			transport = spdk_nvmf_transport_get(listen_addr->trname);
+			assert(transport != NULL);
+
+			transport->listen_addr_discover(listen_addr, entry);
+
 			numrec++;
 		}
 	}
 
 	disc_log->numrec = numrec;
 	disc_log->genctr = g_discovery_genctr;
+
+	free(g_discovery_log_page);
+
+	g_discovery_log_page = disc_log;
+	g_discovery_log_page_size = cur_size;
+}
+
+void
+spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length)
+{
+	size_t copy_len = 0;
+	size_t zero_len = length;
+
+	if (g_discovery_log_page == NULL ||
+	    g_discovery_log_page->genctr != g_discovery_genctr) {
+		nvmf_update_discovery_log();
+	}
+
+	/* Copy the valid part of the discovery log page, if any */
+	if (g_discovery_log_page && offset < g_discovery_log_page_size) {
+		copy_len = nvmf_min(g_discovery_log_page_size - offset, length);
+		zero_len -= copy_len;
+		memcpy(buffer, (char *)g_discovery_log_page + offset, copy_len);
+	}
+
+	/* Zero out the rest of the buffer */
+	if (zero_len) {
+		memset((char *)buffer + copy_len, 0, zero_len);
+	}
+
+	/* We should have copied or zeroed every byte of the output buffer. */
+	assert(copy_len + zero_len == length);
 }
 
 int

lib/nvmf/subsystem.h

@@ -39,8 +39,7 @@
 #include "spdk/nvme.h"
 #include "spdk/nvmf.h"
 
-void
+void spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length);
-spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_t length);
 
 extern const struct spdk_nvmf_ctrlr_ops spdk_nvmf_direct_ctrlr_ops;
 extern const struct spdk_nvmf_ctrlr_ops spdk_nvmf_virtual_ctrlr_ops;

test/lib/nvmf/request/request_ut.c

@@ -119,7 +119,7 @@ spdk_nvmf_property_set(struct spdk_nvmf_session *session,
 }
 
 void
-spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_t length)
+spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length)
 {
 }
 

test/lib/nvmf/subsystem/subsystem_ut.c

@@ -48,9 +48,31 @@ SPDK_LOG_REGISTER_TRACE_FLAG("nvmf", SPDK_TRACE_NVMF)
 
 struct spdk_nvmf_globals g_nvmf_tgt;
 
+static int
+test_transport1_listen_addr_add(struct spdk_nvmf_listen_addr *listen_addr)
+{
+	return 0;
+}
+
+static void
+test_transport1_listen_addr_discover(struct spdk_nvmf_listen_addr *listen_addr,
+				     struct spdk_nvmf_discovery_log_page_entry *entry)
+{
+	entry->trtype = 42;
+}
+
+static const struct spdk_nvmf_transport test_transport1 = {
+	.listen_addr_add = test_transport1_listen_addr_add,
+	.listen_addr_discover = test_transport1_listen_addr_discover,
+};
+
 const struct spdk_nvmf_transport *
 spdk_nvmf_transport_get(const char *trname)
 {
+	if (!strcasecmp(trname, "test_transport1")) {
+		return &test_transport1;
+	}
+
 	return NULL;
 }
 
@@ -130,6 +152,83 @@ nvmf_test_find_subsystem(void)
 	CU_ASSERT_PTR_NULL(nvmf_find_subsystem("fake"));
 }
 
+static bool
+all_zero(const void *buf, size_t size)
+{
+	const uint8_t *b = buf;
+
+	while (size--) {
+		if (*b != 0) {
+			return false;
+		}
+		b++;
+	}
+
+	return true;
+}
+
+static void
+test_discovery_log(void)
+{
+	struct spdk_nvmf_subsystem *subsystem;
+	uint8_t buffer[8192];
+	struct spdk_nvmf_discovery_log_page *disc_log;
+	struct spdk_nvmf_discovery_log_page_entry *entry;
+
+	/* Reset discovery-related globals */
+	g_discovery_genctr = 0;
+	free(g_discovery_log_page);
+	g_discovery_log_page = NULL;
+	g_discovery_log_page_size = 0;
+
+	/* Add one subsystem and verify that the discovery log contains it */
+	subsystem = spdk_nvmf_create_subsystem("nqn.2016-06.io.spdk:subsystem1", SPDK_NVMF_SUBTYPE_NVME,
+					       NVMF_SUBSYSTEM_MODE_DIRECT, NULL, NULL, NULL);
+	SPDK_CU_ASSERT_FATAL(subsystem != NULL);
+
+	SPDK_CU_ASSERT_FATAL(spdk_nvmf_subsystem_add_listener(subsystem, "test_transport1",
+			     "1234", "5678") == 0);
+
+	/* Get only genctr (first field in the header) */
+	memset(buffer, 0xCC, sizeof(buffer));
+	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;
+	spdk_nvmf_get_discovery_log_page(buffer, 0, sizeof(disc_log->genctr));
+	CU_ASSERT(disc_log->genctr == 2); /* one added subsystem + one added listen address */
+
+	/* Get only the header, no entries */
+	memset(buffer, 0xCC, sizeof(buffer));
+	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;
+	spdk_nvmf_get_discovery_log_page(buffer, 0, sizeof(*disc_log));
+	CU_ASSERT(disc_log->genctr == 2);
+	CU_ASSERT(disc_log->numrec == 1);
+
+	/* Offset 0, exact size match */
+	memset(buffer, 0xCC, sizeof(buffer));
+	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;
+	spdk_nvmf_get_discovery_log_page(buffer, 0, sizeof(*disc_log) + sizeof(disc_log->entries[0]));
+	CU_ASSERT(disc_log->genctr != 0);
+	CU_ASSERT(disc_log->numrec == 1);
+	CU_ASSERT(disc_log->entries[0].trtype == 42);
+
+	/* Offset 0, oversize buffer */
+	memset(buffer, 0xCC, sizeof(buffer));
+	disc_log = (struct spdk_nvmf_discovery_log_page *)buffer;
+	spdk_nvmf_get_discovery_log_page(buffer, 0, sizeof(buffer));
+	CU_ASSERT(disc_log->genctr != 0);
+	CU_ASSERT(disc_log->numrec == 1);
+	CU_ASSERT(disc_log->entries[0].trtype == 42);
+	CU_ASSERT(all_zero(buffer + sizeof(*disc_log) + sizeof(disc_log->entries[0]),
+			   sizeof(buffer) - (sizeof(*disc_log) + sizeof(disc_log->entries[0]))));
+
+	/* Get just the first entry, no header */
+	memset(buffer, 0xCC, sizeof(buffer));
+	entry = (struct spdk_nvmf_discovery_log_page_entry *)buffer;
+	spdk_nvmf_get_discovery_log_page(buffer,
+					 offsetof(struct spdk_nvmf_discovery_log_page, entries[0]),
+					 sizeof(*entry));
+	CU_ASSERT(entry->trtype == 42);
+}
+
 int main(int argc, char **argv)
 {
 	CU_pSuite	suite = NULL;
@@ -147,7 +246,8 @@ int main(int argc, char **argv)
 
 	if (
 		CU_add_test(suite, "create_subsystem", nvmf_test_create_subsystem) == NULL ||
-		CU_add_test(suite, "find_subsystem", nvmf_test_find_subsystem) == NULL) {
+		CU_add_test(suite, "find_subsystem", nvmf_test_find_subsystem) == NULL ||
+		CU_add_test(suite, "discovery_log", test_discovery_log) == NULL) {
 		CU_cleanup_registry();
 		return CU_get_error();
 	}