diff --git a/CHANGELOG.md b/CHANGELOG.md index efe628159..0d1da49ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ Added `oncs` to `struct spdk_nvmf_ctrlr_data` so that the transport layer can decide support RESERVATION feature or not. An `opts_size` element was added in the `spdk_nvmf_ns_opts` structure to solve the -ABI compatibility issue between different SPDK version. +ABI compatibility issue between different SPDK version. An new option `anagrpid` was +added in the `spdk_nvmf_ns_opts` structure. ### bdev diff --git a/include/spdk/nvmf.h b/include/spdk/nvmf.h index 0e3d0f276..a9e5031b4 100644 --- a/include/spdk/nvmf.h +++ b/include/spdk/nvmf.h @@ -729,6 +729,13 @@ struct spdk_nvmf_ns_opts { * New added fields should be put at the end of the struct. */ size_t opts_size; + + /** + * ANA group ID + * + * Set to be equal with the NSID if not specified. + */ + uint32_t anagrpid; }; /** diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index a73fc6e81..150cdac9a 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -1935,37 +1935,35 @@ nvmf_ctrlr_mask_aen(struct spdk_nvmf_ctrlr *ctrlr, } } -#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, struct iovec *iovs, int iovcnt, uint64_t offset, uint32_t length, uint32_t rae) { struct spdk_nvme_ana_page ana_hdr; - char _ana_desc[SPDK_NVMF_ANA_DESC_SIZE]; - struct spdk_nvme_ana_group_descriptor *ana_desc; + struct spdk_nvme_ana_group_descriptor ana_desc; size_t copy_len, copied_len; - uint32_t num_ns = 0; + uint32_t num_anagrp = 0, anagrpid; struct spdk_nvmf_ns *ns; struct copy_iovs_ctx copy_ctx; _init_copy_iovs_ctx(©_ctx, iovs, iovcnt); if (length == 0) { - return; + goto done; } if (offset >= sizeof(ana_hdr)) { offset -= sizeof(ana_hdr); } else { - for (ns = spdk_nvmf_subsystem_get_first_ns(ctrlr->subsys); ns != NULL; - ns = spdk_nvmf_subsystem_get_next_ns(ctrlr->subsys, ns)) { - num_ns++; + for (anagrpid = 1; anagrpid <= ctrlr->subsys->max_nsid; anagrpid++) { + if (ctrlr->subsys->ana_group[anagrpid - 1] > 0) { + num_anagrp++; + } } memset(&ana_hdr, 0, sizeof(ana_hdr)); - ana_hdr.num_ana_group_desc = num_ns; + ana_hdr.num_ana_group_desc = num_anagrp; /* TODO: Support Change Count. */ ana_hdr.change_count = 0; @@ -1977,35 +1975,59 @@ nvmf_get_ana_log_page(struct spdk_nvmf_ctrlr *ctrlr, struct iovec *iovs, int iov } if (length == 0) { - return; + goto done; } - ana_desc = (void *)_ana_desc; - - for (ns = spdk_nvmf_subsystem_get_first_ns(ctrlr->subsys); ns != NULL; - ns = spdk_nvmf_subsystem_get_next_ns(ctrlr->subsys, ns)) { - if (offset >= SPDK_NVMF_ANA_DESC_SIZE) { - offset -= SPDK_NVMF_ANA_DESC_SIZE; + for (anagrpid = 1; anagrpid <= ctrlr->subsys->max_nsid; anagrpid++) { + if (ctrlr->subsys->ana_group[anagrpid - 1] == 0) { continue; } - memset(ana_desc, 0, SPDK_NVMF_ANA_DESC_SIZE); + if (offset >= sizeof(ana_desc)) { + offset -= sizeof(ana_desc); + } else { + memset(&ana_desc, 0, sizeof(ana_desc)); - ana_desc->ana_group_id = ns->nsid; - ana_desc->num_of_nsid = 1; - ana_desc->ana_state = ctrlr->listener->ana_state; - ana_desc->nsid[0] = ns->nsid; - /* TODO: Support Change Count. */ - ana_desc->change_count = 0; + ana_desc.ana_group_id = anagrpid; + ana_desc.num_of_nsid = ctrlr->subsys->ana_group[anagrpid - 1]; + ana_desc.ana_state = ctrlr->listener->ana_state; - copy_len = spdk_min(SPDK_NVMF_ANA_DESC_SIZE - offset, length); - copied_len = _copy_buf_to_iovs(©_ctx, (const char *)ana_desc + offset, copy_len); - assert(copied_len == copy_len); - length -= copied_len; - offset = 0; + copy_len = spdk_min(sizeof(ana_desc) - offset, length); + copied_len = _copy_buf_to_iovs(©_ctx, (const char *)&ana_desc + offset, + copy_len); + assert(copied_len == copy_len); + length -= copied_len; + offset = 0; - if (length == 0) { - goto done; + if (length == 0) { + goto done; + } + } + + /* TODO: Revisit here about O(n^2) cost if we have subsystem with + * many namespaces in the future. + */ + for (ns = spdk_nvmf_subsystem_get_first_ns(ctrlr->subsys); ns != NULL; + ns = spdk_nvmf_subsystem_get_next_ns(ctrlr->subsys, ns)) { + if (ns->anagrpid != anagrpid) { + continue; + } + + if (offset >= sizeof(uint32_t)) { + offset -= sizeof(uint32_t); + continue; + } + + copy_len = spdk_min(sizeof(uint32_t) - offset, length); + copied_len = _copy_buf_to_iovs(©_ctx, (const char *)&ns->nsid + offset, + copy_len); + assert(copied_len == copy_len); + length -= copied_len; + offset = 0; + + if (length == 0) { + goto done; + } } } @@ -2296,8 +2318,7 @@ spdk_nvmf_ctrlr_identify_ns(struct spdk_nvmf_ctrlr *ctrlr, } if (subsystem->flags.ana_reporting) { - /* ANA group ID matches NSID. */ - nsdata->anagrpid = ns->nsid; + nsdata->anagrpid = ns->anagrpid; if (ctrlr->listener->ana_state == SPDK_NVME_ANA_INACCESSIBLE_STATE || ctrlr->listener->ana_state == SPDK_NVME_ANA_PERSISTENT_LOSS_STATE) { diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 7cb99b160..a55c41dd2 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -170,6 +170,7 @@ struct spdk_nvmf_registrant { struct spdk_nvmf_ns { uint32_t nsid; + uint32_t anagrpid; struct spdk_nvmf_subsystem *subsystem; struct spdk_bdev *bdev; struct spdk_bdev_desc *desc; @@ -316,6 +317,11 @@ struct spdk_nvmf_subsystem { char sn[SPDK_NVME_CTRLR_SN_LEN + 1]; char mn[SPDK_NVME_CTRLR_MN_LEN + 1]; char subnqn[SPDK_NVMF_NQN_MAX_LEN + 1]; + + /* Array of namespace count per ANA group of size max_nsid indexed anagrpid - 1 + * It will be enough for ANA group to use the same size as namespaces. + */ + uint32_t *ana_group; }; int nvmf_poll_group_add_transport(struct spdk_nvmf_poll_group *group, diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c index 9cf4f8fce..b5cce966d 100644 --- a/lib/nvmf/subsystem.c +++ b/lib/nvmf/subsystem.c @@ -303,6 +303,14 @@ spdk_nvmf_subsystem_create(struct spdk_nvmf_tgt *tgt, free(subsystem); return NULL; } + subsystem->ana_group = calloc(num_ns, sizeof(uint32_t)); + if (subsystem->ana_group == NULL) { + SPDK_ERRLOG("ANA group memory allocation failed\n"); + pthread_mutex_destroy(&subsystem->mutex); + free(subsystem->ns); + free(subsystem); + return NULL; + } } memset(subsystem->sn, '0', sizeof(subsystem->sn) - 1); @@ -381,6 +389,7 @@ spdk_nvmf_subsystem_destroy(struct spdk_nvmf_subsystem *subsystem) } free(subsystem->ns); + free(subsystem->ana_group); subsystem->tgt->subsystems[subsystem->id] = NULL; nvmf_update_discovery_log(subsystem->tgt, NULL); @@ -1199,6 +1208,11 @@ spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t ns subsystem->ns[nsid - 1] = NULL; + assert(ns->anagrpid - 1 < subsystem->max_nsid); + assert(subsystem->ana_group[ns->anagrpid - 1] > 0); + + subsystem->ana_group[ns->anagrpid - 1]--; + free(ns->ptpl_file); nvmf_ns_reservation_clear_all_registrants(ns); spdk_bdev_module_release_bdev(ns->bdev); @@ -1395,6 +1409,7 @@ spdk_nvmf_ns_opts_get_defaults(struct spdk_nvmf_ns_opts *opts, size_t opts_size) if (FIELD_OK(uuid)) { memset(&opts->uuid, 0, sizeof(opts->uuid)); } + SET_FIELD(anagrpid, 0); #undef FIELD_OK #undef SET_FIELD @@ -1423,13 +1438,14 @@ nvmf_ns_opts_copy(struct spdk_nvmf_ns_opts *opts, if (FIELD_OK(uuid)) { memcpy(&opts->uuid, &user_opts->uuid, sizeof(opts->uuid)); } + SET_FIELD(anagrpid); opts->opts_size = user_opts->opts_size; /* We should not remove this statement, but need to update the assert statement * if we add a new field, and also add a corresponding SET_FIELD statement. */ - SPDK_STATIC_ASSERT(sizeof(struct spdk_nvmf_ns_opts) == 56, "Incorrect size"); + SPDK_STATIC_ASSERT(sizeof(struct spdk_nvmf_ns_opts) == 64, "Incorrect size"); #undef FIELD_OK #undef SET_FIELD @@ -1495,6 +1511,15 @@ spdk_nvmf_subsystem_add_ns_ext(struct spdk_nvmf_subsystem *subsystem, const char return 0; } + if (opts.anagrpid == 0) { + opts.anagrpid = opts.nsid; + } + + if (opts.anagrpid > subsystem->max_nsid) { + SPDK_ERRLOG("ANAGRPID greater than maximum NSID not allowed\n"); + return 0; + } + ns = calloc(1, sizeof(*ns)); if (ns == NULL) { SPDK_ERRLOG("Namespace allocation failed\n"); @@ -1542,6 +1567,8 @@ spdk_nvmf_subsystem_add_ns_ext(struct spdk_nvmf_subsystem *subsystem, const char ns->subsystem = subsystem; subsystem->ns[opts.nsid - 1] = ns; ns->nsid = opts.nsid; + ns->anagrpid = opts.anagrpid; + subsystem->ana_group[ns->anagrpid - 1]++; TAILQ_INIT(&ns->registrants); if (ptpl_file) { diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index c09fdcdaf..ec38b68cd 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -1063,6 +1063,8 @@ test_set_get_features(void) struct spdk_nvmf_request req; int rc; + ns[0].anagrpid = 1; + ns[2].anagrpid = 3; subsystem.ns = ns_arr; subsystem.max_nsid = SPDK_COUNTOF(ns_arr); listener.ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; @@ -1673,6 +1675,7 @@ test_fused_compare_and_write(void) struct spdk_io_channel io_ch = {}; ns.bdev = &bdev; + ns.anagrpid = 1; subsystem.id = 0; subsystem.max_nsid = 1; @@ -1827,12 +1830,13 @@ test_multi_async_event_reqs(void) TAILQ_REMOVE(&qpair.outstanding, &req[1], link); } +static void +test_get_ana_log_page_one_ns_per_anagrp(void) +{ #define UT_ANA_DESC_SIZE (sizeof(struct spdk_nvme_ana_group_descriptor) + sizeof(uint32_t)) #define UT_ANA_LOG_PAGE_SIZE (sizeof(struct spdk_nvme_ana_page) + 3 * UT_ANA_DESC_SIZE) -static void -test_get_ana_log_page(void) -{ - struct spdk_nvmf_subsystem subsystem = {}; + uint32_t ana_group[3]; + struct spdk_nvmf_subsystem subsystem = { .ana_group = ana_group }; struct spdk_nvmf_ctrlr ctrlr = {}; struct spdk_nvmf_subsystem_listener listener = {}; struct spdk_nvmf_ns ns[3]; @@ -1849,12 +1853,16 @@ test_get_ana_log_page(void) subsystem.ns = ns_arr; subsystem.max_nsid = 3; + for (i = 0; i < 3; i++) { + subsystem.ana_group[i] = 1; + } ctrlr.subsys = &subsystem; ctrlr.listener = &listener; listener.ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; for (i = 0; i < 3; i++) { ns_arr[i]->nsid = i + 1; + ns_arr[i]->anagrpid = i + 1; } /* create expected page */ @@ -1900,8 +1908,103 @@ test_get_ana_log_page(void) nvmf_get_ana_log_page(&ctrlr, &iovs[0], 2, 0, UT_ANA_LOG_PAGE_SIZE, 0); CU_ASSERT(memcmp(expected_page, actual_page, UT_ANA_LOG_PAGE_SIZE) == 0); + +#undef UT_ANA_DESC_SIZE +#undef UT_ANA_LOG_PAGE_SIZE } +static void +test_get_ana_log_page_multi_ns_per_anagrp(void) +{ +#define UT_ANA_LOG_PAGE_SIZE (sizeof(struct spdk_nvme_ana_page) + \ + sizeof(struct spdk_nvme_ana_group_descriptor) * 2 + \ + sizeof(uint32_t) * 5) + struct spdk_nvmf_ns ns[5]; + struct spdk_nvmf_ns *ns_arr[5] = {&ns[0], &ns[1], &ns[2], &ns[3], &ns[4]}; + uint32_t ana_group[5] = {0}; + struct spdk_nvmf_subsystem subsystem = { .ns = ns_arr, .ana_group = ana_group, }; + struct spdk_nvmf_subsystem_listener listener = {}; + struct spdk_nvmf_ctrlr ctrlr = { .subsys = &subsystem, .listener = &listener, }; + char expected_page[UT_ANA_LOG_PAGE_SIZE] = {0}; + char actual_page[UT_ANA_LOG_PAGE_SIZE] = {0}; + struct iovec iov, iovs[2]; + struct spdk_nvme_ana_page *ana_hdr; + char _ana_desc[UT_ANA_LOG_PAGE_SIZE]; + struct spdk_nvme_ana_group_descriptor *ana_desc; + uint64_t offset; + uint32_t length; + int i; + + subsystem.max_nsid = 5; + subsystem.ana_group[1] = 3; + subsystem.ana_group[2] = 2; + listener.ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; + + for (i = 0; i < 5; i++) { + ns_arr[i]->nsid = i + 1; + } + ns_arr[0]->anagrpid = 2; + ns_arr[1]->anagrpid = 3; + ns_arr[2]->anagrpid = 2; + ns_arr[3]->anagrpid = 3; + ns_arr[4]->anagrpid = 2; + + /* create expected page */ + ana_hdr = (void *)&expected_page[0]; + ana_hdr->num_ana_group_desc = 2; + ana_hdr->change_count = 0; + + /* descriptor may be unaligned. So create data and then copy it to the location. */ + ana_desc = (void *)_ana_desc; + offset = sizeof(struct spdk_nvme_ana_page); + + memset(_ana_desc, 0, sizeof(_ana_desc)); + ana_desc->ana_group_id = 2; + ana_desc->num_of_nsid = 3; + ana_desc->change_count = 0; + ana_desc->ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; + ana_desc->nsid[0] = 1; + ana_desc->nsid[1] = 3; + ana_desc->nsid[2] = 5; + memcpy(&expected_page[offset], ana_desc, sizeof(struct spdk_nvme_ana_group_descriptor) + + sizeof(uint32_t) * 3); + offset += sizeof(struct spdk_nvme_ana_group_descriptor) + sizeof(uint32_t) * 3; + + memset(_ana_desc, 0, sizeof(_ana_desc)); + ana_desc->ana_group_id = 3; + ana_desc->num_of_nsid = 2; + ana_desc->change_count = 0; + ana_desc->ana_state = SPDK_NVME_ANA_OPTIMIZED_STATE; + ana_desc->nsid[0] = 2; + ana_desc->nsid[1] = 4; + memcpy(&expected_page[offset], ana_desc, sizeof(struct spdk_nvme_ana_group_descriptor) + + sizeof(uint32_t) * 2); + + /* read entire actual log page, and compare expected page and actual page. */ + offset = 0; + while (offset < UT_ANA_LOG_PAGE_SIZE) { + length = spdk_min(16, UT_ANA_LOG_PAGE_SIZE - offset); + iov.iov_base = &actual_page[offset]; + iov.iov_len = length; + nvmf_get_ana_log_page(&ctrlr, &iov, 1, offset, length, 0); + offset += length; + } + + CU_ASSERT(memcmp(expected_page, actual_page, UT_ANA_LOG_PAGE_SIZE) == 0); + + memset(&actual_page[0], 0, UT_ANA_LOG_PAGE_SIZE); + offset = 0; + iovs[0].iov_base = &actual_page[offset]; + iovs[0].iov_len = UT_ANA_LOG_PAGE_SIZE - sizeof(uint32_t) * 5; + offset += UT_ANA_LOG_PAGE_SIZE - sizeof(uint32_t) * 5; + iovs[1].iov_base = &actual_page[offset]; + iovs[1].iov_len = sizeof(uint32_t) * 5; + nvmf_get_ana_log_page(&ctrlr, &iovs[0], 2, 0, UT_ANA_LOG_PAGE_SIZE, 0); + + CU_ASSERT(memcmp(expected_page, actual_page, UT_ANA_LOG_PAGE_SIZE) == 0); + +#undef UT_ANA_LOG_PAGE_SIZE +} static void test_multi_async_events(void) { @@ -2272,6 +2375,7 @@ test_spdk_nvmf_request_zcopy_start(void) ns.bdev = &bdev; ns.zcopy = true; + ns.anagrpid = 1; subsystem.id = 0; subsystem.max_nsid = 1; @@ -2395,6 +2499,7 @@ test_zcopy_read(void) ns.bdev = &bdev; ns.zcopy = true; + ns.anagrpid = 1; subsystem.id = 0; subsystem.max_nsid = 1; @@ -2480,6 +2585,7 @@ test_zcopy_write(void) ns.bdev = &bdev; ns.zcopy = true; + ns.anagrpid = 1; subsystem.id = 0; subsystem.max_nsid = 1; @@ -2630,7 +2736,8 @@ int main(int argc, char **argv) CU_ADD_TEST(suite, test_custom_admin_cmd); CU_ADD_TEST(suite, test_fused_compare_and_write); CU_ADD_TEST(suite, test_multi_async_event_reqs); - CU_ADD_TEST(suite, test_get_ana_log_page); + CU_ADD_TEST(suite, test_get_ana_log_page_one_ns_per_anagrp); + CU_ADD_TEST(suite, test_get_ana_log_page_multi_ns_per_anagrp); CU_ADD_TEST(suite, test_multi_async_events); CU_ADD_TEST(suite, test_rae); CU_ADD_TEST(suite, test_nvmf_ctrlr_create_destruct); diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index af90b0f0e..9147de0f9 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c @@ -300,7 +300,7 @@ test_spdk_nvmf_subsystem_add_ns(void) struct spdk_nvmf_subsystem subsystem = { .max_nsid = 1024, .ns = NULL, - .tgt = &tgt + .tgt = &tgt, }; struct spdk_nvmf_ns_opts ns_opts; uint32_t nsid; @@ -308,6 +308,8 @@ test_spdk_nvmf_subsystem_add_ns(void) subsystem.ns = calloc(subsystem.max_nsid, sizeof(struct spdk_nvmf_subsystem_ns *)); SPDK_CU_ASSERT_FATAL(subsystem.ns != NULL); + subsystem.ana_group = calloc(subsystem.max_nsid, sizeof(uint32_t)); + SPDK_CU_ASSERT_FATAL(subsystem.ana_group != NULL); tgt.max_subsystems = 1024; tgt.subsystems = calloc(tgt.max_subsystems, sizeof(struct spdk_nvmf_subsystem *)); @@ -340,6 +342,7 @@ test_spdk_nvmf_subsystem_add_ns(void) CU_ASSERT(rc == 0); free(subsystem.ns); + free(subsystem.ana_group); free(tgt.subsystems); } @@ -1341,7 +1344,7 @@ test_spdk_nvmf_ns_event(void) struct spdk_nvmf_subsystem subsystem = { .max_nsid = 1024, .ns = NULL, - .tgt = &tgt + .tgt = &tgt, }; struct spdk_nvmf_ctrlr ctrlr = { .subsys = &subsystem @@ -1352,6 +1355,8 @@ test_spdk_nvmf_ns_event(void) subsystem.ns = calloc(subsystem.max_nsid, sizeof(struct spdk_nvmf_subsystem_ns *)); SPDK_CU_ASSERT_FATAL(subsystem.ns != NULL); + subsystem.ana_group = calloc(subsystem.max_nsid, sizeof(uint32_t)); + SPDK_CU_ASSERT_FATAL(subsystem.ana_group != NULL); tgt.max_subsystems = 1024; tgt.subsystems = calloc(tgt.max_subsystems, sizeof(struct spdk_nvmf_subsystem *)); @@ -1408,6 +1413,7 @@ test_spdk_nvmf_ns_event(void) poll_threads(); free(subsystem.ns); + free(subsystem.ana_group); free(tgt.subsystems); }