Commit 70396390 authored by Jim Harris's avatar Jim Harris
Browse files

nvme: read full discovery page after reading header 



Some targets report they support log page offset,
but then fail GET_LOG_PAGE commands that specify
a non-zero offset, or report the wrong number
of discovery entries when reading more than the
discovery log page header but not the entire log
page.

So just revert to reading the entire discovery log
page, after we've read the header to know how big the
log page will be. This means that when we read the
log page initially (without the individual entries),
we need to save off the genctr, since it will get
overwritten when we read the log page again.  We can
just store this in the discovery context, and compare
it to the genctr that we read with the whole log page.

Signed-off-by: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: I34929253312fed9924db58904a051f3979283730
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11478


Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com>
Community-CI: Mellanox Build Bot
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@mellanox.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
parent 9ff419a9

lib/nvme/nvme_discovery.c

+16 −53
Original line number Diff line number Diff line
@@ -37,11 +37,10 @@ 
struct nvme_discovery_ctx {

	struct spdk_nvme_ctrlr			*ctrlr;

	struct spdk_nvmf_discovery_log_page	*log_page;

	uint64_t				genctr;

	uint64_t				start_genctr;

	uint64_t				end_genctr;

	spdk_nvme_discovery_cb			cb_fn;

	void					*cb_arg;

	struct spdk_nvme_cpl			cpl;

	uint32_t				outstanding_commands;

};



static void
@@ -58,7 +57,7 @@ get_log_page_completion_final(void *cb_arg, const struct spdk_nvme_cpl *cpl) 
	}



	/* Compare original genctr with latest genctr. If it changed, we need to restart. */

	if (ctx->log_page->genctr == ctx->genctr) {

	if (ctx->start_genctr == ctx->end_genctr) {

		ctx->cb_fn(ctx->cb_arg, 0, cpl, ctx->log_page);

	} else {

		free(ctx->log_page);
@@ -77,25 +76,14 @@ get_log_page_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl) 
	int rc;



	if (spdk_nvme_cpl_is_error(cpl)) {

		/* Only save the cpl for the first error that we encounter. */

		if (!spdk_nvme_cpl_is_error(&ctx->cpl)) {

			ctx->cpl = *cpl;

		}

	}

	ctx->outstanding_commands--;

	if (ctx->outstanding_commands > 0) {

		return;

	}



	if (spdk_nvme_cpl_is_error(&ctx->cpl)) {

		free(ctx->log_page);

		ctx->cb_fn(ctx->cb_arg, 0, &ctx->cpl, NULL);

		ctx->cb_fn(ctx->cb_arg, 0, cpl, NULL);

		free(ctx);

		return;

	}



	rc = spdk_nvme_ctrlr_cmd_get_log_page(ctx->ctrlr, SPDK_NVME_LOG_DISCOVERY, 0,

					      &ctx->genctr, sizeof(ctx->genctr), 0,

					      &ctx->end_genctr, sizeof(ctx->end_genctr), 0,

					      get_log_page_completion_final, ctx);

	if (rc != 0) {

		free(ctx->log_page);
@@ -112,8 +100,6 @@ discovery_log_header_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl) 
	size_t page_size;

	uint16_t recfmt;

	uint64_t numrec;

	uint64_t remaining;

	uint64_t offset;

	int rc;



	if (spdk_nvme_cpl_is_error(cpl)) {
@@ -138,14 +124,14 @@ discovery_log_header_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl) 


	if (numrec == 0) {

		/* No entries in the discovery log. So we can just return the header to the

		 * caller. Increment outstanding_commands and use the get_log_page_completion()

		 * function to avoid duplicating that code here.

		 * caller.

		 */

		ctx->outstanding_commands++;

		get_log_page_completion(ctx, cpl);

		return;

	}



	ctx->start_genctr = ctx->log_page->genctr;



	/*

	 * Now that we know how many entries should be in the log page, we can allocate

	 * the full log page buffer.
@@ -164,37 +150,14 @@ discovery_log_header_completion(void *cb_arg, const struct spdk_nvme_cpl *cpl) 


	ctx->log_page = new_page;



	/* Retrieve the rest of the discovery log page */

	offset = offsetof(struct spdk_nvmf_discovery_log_page, entries);

	remaining = page_size - offset;

	while (remaining) {

		uint32_t size;



		/* Retrieve up to 4 KB at a time */

		size = spdk_min(remaining, 4096);



		ctx->outstanding_commands++;

	/* Retrieve the entire discovery log page */

	rc = spdk_nvme_ctrlr_cmd_get_log_page(ctx->ctrlr, SPDK_NVME_LOG_DISCOVERY,

						      0, (char *)ctx->log_page + offset, size, offset,

					      0, (char *)ctx->log_page, page_size, 0,

					      get_log_page_completion, ctx);

	if (rc != 0) {

			/* We may have already successfully submitted some get_log_page commands,

			 * so we cannot just call the user's callback with error status and free

			 * the log page here.  Simulate a completion instead, so that we keep

			 * all of the cleanup code in the get_log_page_completion() function.

			 */

			struct spdk_nvme_cpl cpl = { 0 };



			SPDK_ERRLOG("spdk_nvme_ctrlr_cmd_get_log_page() failed\n");

			cpl.status.sct = SPDK_NVME_SCT_GENERIC;

			cpl.status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;

			cpl.status.dnr = 1;

			get_log_page_completion(ctx, &cpl);

			return;

		}



		offset += size;

		remaining -= size;

		free(ctx->log_page);

		ctx->cb_fn(ctx->cb_arg, rc, NULL, NULL);

		free(ctx);

	}

}