From 81e92f6bca5527dc1e33763f504e870ed0b87e63 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Wed, 13 Jul 2022 13:57:25 +0900 Subject: [PATCH] bdev/nvme: Calculate and use active ns count to read ANA log page nvme_ctrlr_init_ana_log_page() had used cdata->mnan as active ns count to allocate a buffer and read a ANA log page into the buffer. However, cdata->mnan was larger than the real active ns count and caused an issue when the corresponding NVMe-oF target set the bit 18 of the SGL support field in the Identify Controller Data structure. We still need to use cdata->mnan to allocate the buffer because number of namespaces may be increased dynamically after initialization. Hence, rename nvme_ctrlr::ana_log_page_size to nvme_ctrlr::max_ana_log_page_size and calculate and use the active ns count to read the ANA log page. Check if the current ana_log_page_size is not larger than nvme_ctrlr->max_ana_log_page_size. Fixes issue #2584 Signed-off-by: Shuhei Matsumoto Change-Id: Ieb10b9c793c4f48ffd88d517c0e9a55184b7d935 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13653 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Changpeng Liu Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins --- module/bdev/nvme/bdev_nvme.c | 46 ++++++++++++++++++++++++++++++++---- module/bdev/nvme/bdev_nvme.h | 2 +- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index a71ac0ed0..9a85652c7 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -2756,7 +2756,7 @@ bdev_nvme_parse_ana_log_page(struct nvme_ctrlr *nvme_ctrlr, copied_desc = nvme_ctrlr->copied_ana_desc; orig_desc = (uint8_t *)nvme_ctrlr->ana_log_page + sizeof(struct spdk_nvme_ana_page); - copy_len = nvme_ctrlr->ana_log_page_size - sizeof(struct spdk_nvme_ana_page); + copy_len = nvme_ctrlr->max_ana_log_page_size - sizeof(struct spdk_nvme_ana_page); for (i = 0; i < nvme_ctrlr->ana_log_page->num_ana_group_desc; i++) { memcpy(copied_desc, orig_desc, copy_len); @@ -3456,6 +3456,25 @@ nvme_ctrlr_depopulate_namespaces(struct nvme_ctrlr *nvme_ctrlr) } } +static uint32_t +nvme_ctrlr_get_ana_log_page_size(struct nvme_ctrlr *nvme_ctrlr) +{ + struct spdk_nvme_ctrlr *ctrlr = nvme_ctrlr->ctrlr; + const struct spdk_nvme_ctrlr_data *cdata; + uint32_t nsid, ns_count = 0; + + cdata = spdk_nvme_ctrlr_get_data(ctrlr); + + for (nsid = spdk_nvme_ctrlr_get_first_active_ns(ctrlr); + nsid != 0; nsid = spdk_nvme_ctrlr_get_next_active_ns(ctrlr, nsid)) { + ns_count++; + } + + return sizeof(struct spdk_nvme_ana_page) + cdata->nanagrpid * + sizeof(struct spdk_nvme_ana_group_descriptor) + ns_count * + sizeof(uint32_t); +} + static int nvme_ctrlr_set_ana_states(const struct spdk_nvme_ana_group_descriptor *desc, void *cb_arg) @@ -3538,12 +3557,21 @@ nvme_ctrlr_read_ana_log_page_done(void *ctx, const struct spdk_nvme_cpl *cpl) static int nvme_ctrlr_read_ana_log_page(struct nvme_ctrlr *nvme_ctrlr) { + uint32_t ana_log_page_size; int rc; if (nvme_ctrlr->ana_log_page == NULL) { return -EINVAL; } + ana_log_page_size = nvme_ctrlr_get_ana_log_page_size(nvme_ctrlr); + + if (ana_log_page_size > nvme_ctrlr->max_ana_log_page_size) { + SPDK_ERRLOG("ANA log page size %" PRIu32 " is larger than allowed %" PRIu32 "\n", + ana_log_page_size, nvme_ctrlr->max_ana_log_page_size); + return -EINVAL; + } + pthread_mutex_lock(&nvme_ctrlr->mutex); if (!nvme_ctrlr_is_available(nvme_ctrlr) || nvme_ctrlr->ana_log_page_updating) { @@ -3558,7 +3586,7 @@ nvme_ctrlr_read_ana_log_page(struct nvme_ctrlr *nvme_ctrlr) SPDK_NVME_LOG_ASYMMETRIC_NAMESPACE_ACCESS, SPDK_NVME_GLOBAL_NS_TAG, nvme_ctrlr->ana_log_page, - nvme_ctrlr->ana_log_page_size, 0, + ana_log_page_size, 0, nvme_ctrlr_read_ana_log_page_done, nvme_ctrlr); if (rc != 0) { @@ -3897,6 +3925,7 @@ nvme_ctrlr_init_ana_log_page(struct nvme_ctrlr *nvme_ctrlr, cdata = spdk_nvme_ctrlr_get_data(ctrlr); + /* Set buffer size enough to include maximum number of allowed namespaces. */ ana_log_page_size = sizeof(struct spdk_nvme_ana_page) + cdata->nanagrpid * sizeof(struct spdk_nvme_ana_group_descriptor) + cdata->mnan * sizeof(uint32_t); @@ -3920,15 +3949,24 @@ nvme_ctrlr_init_ana_log_page(struct nvme_ctrlr *nvme_ctrlr, return -ENOMEM; } - nvme_ctrlr->ana_log_page_size = ana_log_page_size; + nvme_ctrlr->max_ana_log_page_size = ana_log_page_size; nvme_ctrlr->probe_ctx = ctx; + /* Then, set the read size only to include the current active namespaces. */ + ana_log_page_size = nvme_ctrlr_get_ana_log_page_size(nvme_ctrlr); + + if (ana_log_page_size > nvme_ctrlr->max_ana_log_page_size) { + SPDK_ERRLOG("ANA log page size %" PRIu32 " is larger than allowed %" PRIu32 "\n", + ana_log_page_size, nvme_ctrlr->max_ana_log_page_size); + return -EINVAL; + } + return spdk_nvme_ctrlr_cmd_get_log_page(ctrlr, SPDK_NVME_LOG_ASYMMETRIC_NAMESPACE_ACCESS, SPDK_NVME_GLOBAL_NS_TAG, nvme_ctrlr->ana_log_page, - nvme_ctrlr->ana_log_page_size, 0, + ana_log_page_size, 0, nvme_ctrlr_init_ana_log_page_done, nvme_ctrlr); } diff --git a/module/bdev/nvme/bdev_nvme.h b/module/bdev/nvme/bdev_nvme.h index 8f481e121..9ff09c64b 100644 --- a/module/bdev/nvme/bdev_nvme.h +++ b/module/bdev/nvme/bdev_nvme.h @@ -126,7 +126,7 @@ struct nvme_ctrlr { TAILQ_HEAD(nvme_paths, nvme_path_id) trids; - uint32_t ana_log_page_size; + uint32_t max_ana_log_page_size; struct spdk_nvme_ana_page *ana_log_page; struct spdk_nvme_ana_group_descriptor *copied_ana_desc;