Commit 07bfc3cb authored by Shuhei Matsumoto's avatar Shuhei Matsumoto Committed by Tomasz Zawadzki
Browse files

nvmf: Add anagrpid to ns and count number of ns per ANA group in subsystem 



This is the first patch in the patch series to control ANA states not only
as a unit of subsystem listener but also as a unit of ANA group and create
user preferred mapping between namespaces and ANA groups within a single
subsystem.

This patch adds anagrpid to both spdk_nvmf_ns and spdk_nvmf_ns_opts, and adds
ana_group array to spdk_nvmf_subsystem to count number of namespaces per ANA
group within a single subsystem. The size of the ana_group array is equal
with the size of the namespaces.

For each subsystem, allocate ana_group array regardless of the value of
ana_reporting of the subsystem.

For each namespace, at its creation, initialize anagrpid explicitly to be equal
with nsid by default and increments the corresponding entry of the ana_group
array of the subsystem regardless of teh value of the ana_reporting of thee
subsystem.

Hence the contents of the created ANA log page is not changed even if the
algorithm to crete ANA log page is changed.

Additionally this patch adds a unit test case that one ANA group
has multiple namespaces.

Signed-off-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Change-Id: I78539db4e7248c2953c6927ff8128cb5a7e34b96
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9102


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 avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent d2b9b2af

CHANGELOG.md

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

include/spdk/nvmf.h

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

};



/**

lib/nvmf/ctrlr.c

+54 −33
Original line number Diff line number Diff line
@@ -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(&copy_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,29 +1975,52 @@ 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 (anagrpid = 1; anagrpid <= ctrlr->subsys->max_nsid; anagrpid++) {

		if (ctrlr->subsys->ana_group[anagrpid - 1] == 0) {

			continue;

		}



		if (offset >= sizeof(ana_desc)) {

			offset -= sizeof(ana_desc);

		} else {

			memset(&ana_desc, 0, sizeof(ana_desc));



			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(sizeof(ana_desc) - offset, length);

			copied_len = _copy_buf_to_iovs(&copy_ctx, (const char *)&ana_desc + offset,

						       copy_len);

			assert(copied_len == copy_len);

			length -= copied_len;

			offset = 0;



			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 (offset >= SPDK_NVMF_ANA_DESC_SIZE) {

			offset -= SPDK_NVMF_ANA_DESC_SIZE;

			if (ns->anagrpid != anagrpid) {

				continue;

			}



		memset(ana_desc, 0, SPDK_NVMF_ANA_DESC_SIZE);



		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;

			if (offset >= sizeof(uint32_t)) {

				offset -= sizeof(uint32_t);

				continue;

			}



		copy_len = spdk_min(SPDK_NVMF_ANA_DESC_SIZE - offset, length);

		copied_len = _copy_buf_to_iovs(&copy_ctx, (const char *)ana_desc + offset, copy_len);

			copy_len = spdk_min(sizeof(uint32_t) - offset, length);

			copied_len = _copy_buf_to_iovs(&copy_ctx, (const char *)&ns->nsid + offset,

						       copy_len);

			assert(copied_len == copy_len);

			length -= copied_len;

			offset = 0;
@@ -2008,6 +2029,7 @@ nvmf_get_ana_log_page(struct spdk_nvmf_ctrlr *ctrlr, struct iovec *iovs, int iov 
				goto done;

			}

		}

	}



done:

	if (!rae) {
@@ -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) {

lib/nvmf/nvmf_internal.h

+6 −0
Original line number Diff line number Diff line
@@ -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,

lib/nvmf/subsystem.c

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