From de1b00657ce035c184220cebf285443b73abba82 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Tue, 13 Dec 2016 13:24:28 -0700 Subject: [PATCH] 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 --- lib/nvmf/request.c | 5 +- lib/nvmf/subsystem.c | 90 +++++++++++++++++----- lib/nvmf/subsystem.h | 3 +- test/lib/nvmf/request/request_ut.c | 2 +- test/lib/nvmf/subsystem/subsystem_ut.c | 102 ++++++++++++++++++++++++- 5 files changed, 175 insertions(+), 27 deletions(-) diff --git a/lib/nvmf/request.c b/lib/nvmf/request.c index 2566bbf11..a62f70768 100644 --- a/lib/nvmf/request.c +++ b/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; - log->numrec = 0; - spdk_format_discovery_log(log, req->length); + spdk_nvmf_get_discovery_log_page(req->data, log_page_offset, req->length); return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } else { SPDK_ERRLOG("Unsupported log page %u\n", cmd->cdw10 & 0xFF); diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index bfc0a76f5..b63131fc0 100644 --- a/lib/nvmf/subsystem.c +++ b/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 -spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_t length) +static void +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 */ - if (length > sizeof(struct spdk_nvmf_discovery_log_page)) { - 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); + size_t new_size = cur_size + sizeof(*entry); + void *new_log_page = realloc(disc_log, new_size); - transport = spdk_nvmf_transport_get(listen_addr->trname); - assert(transport != NULL); - - transport->listen_addr_discover(listen_addr, entry); + if (new_log_page == NULL) { + SPDK_ERRLOG("Discovery log page memory allocation error\n"); + break; } + + 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 diff --git a/lib/nvmf/subsystem.h b/lib/nvmf/subsystem.h index b4c05fbb3..9b2ec125d 100644 --- a/lib/nvmf/subsystem.h +++ b/lib/nvmf/subsystem.h @@ -39,8 +39,7 @@ #include "spdk/nvme.h" #include "spdk/nvmf.h" -void -spdk_format_discovery_log(struct spdk_nvmf_discovery_log_page *disc_log, uint32_t length); +void spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, 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; diff --git a/test/lib/nvmf/request/request_ut.c b/test/lib/nvmf/request/request_ut.c index de898ce9b..a17fcd4b3 100644 --- a/test/lib/nvmf/request/request_ut.c +++ b/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) { } diff --git a/test/lib/nvmf/subsystem/subsystem_ut.c b/test/lib/nvmf/subsystem/subsystem_ut.c index 1aebc5b02..26a667f25 100644 --- a/test/lib/nvmf/subsystem/subsystem_ut.c +++ b/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(); }