Commit 81e92f6b authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

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: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Change-Id: Ieb10b9c793c4f48ffd88d517c0e9a55184b7d935
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13653


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent f97f8032

module/bdev/nvme/bdev_nvme.c

+42 −4
Original line number Diff line number Diff line
@@ -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);

}

module/bdev/nvme/bdev_nvme.h

+1 −1
Original line number Diff line number Diff line
@@ -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;