Commit 037d5165 authored by Ankit Kumar's avatar Ankit Kumar Committed by Tomasz Zawadzki
Browse files

nvmf: fdp capability to the subsystem 



Set FDP support for controller data, based on the subsystem capability.

Enable checks to only allow either fdp or non fdp capable namespaces to
the subsystem. The first nvmf namespace added to the subsystem will set a
precedent. Add a unit test to verify the same.

Change-Id: Id9f317b81022698eee08985c3943364e3fa0c386
Signed-off-by: default avatarAnkit Kumar <ankit.kumar@samsung.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/22790


Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Reviewed-by: default avatarJim Harris <jim.harris@samsung.com>
Community-CI: Mellanox Build Bot
parent 9e6aa50f

lib/nvmf/ctrlr.c

+1 −0
Original line number Diff line number Diff line
@@ -2805,6 +2805,7 @@ spdk_nvmf_ctrlr_identify_ctrlr(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvme_c 
		cdata->cmic.multi_ctrlr = 1;

		cdata->oaes.ns_attribute_notices = 1;

		cdata->ctratt.bits.host_id_exhid_supported = 1;

		cdata->ctratt.bits.fdps = ctrlr->subsys->fdp_supported;

		/* We do not have any actual limitation to the number of abort commands.

		 * We follow the recommendation by the NVMe specification.

		 */

lib/nvmf/nvmf_internal.h

+3 −0
Original line number Diff line number Diff line
@@ -277,6 +277,9 @@ struct spdk_nvmf_subsystem { 
	bool						destroying;

	bool						async_destroy;



	/* FDP related fields */

	bool						fdp_supported;



	/* Zoned storage related fields */

	bool						zone_append_supported;

	uint64_t					max_zone_append_size_kib;

lib/nvmf/subsystem.c

+23 −1
Original line number Diff line number Diff line
@@ -1595,6 +1595,12 @@ spdk_nvmf_subsystem_remove_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t ns 
	spdk_bdev_close(ns->desc);

	free(ns);



	if (subsystem->fdp_supported && !spdk_nvmf_subsystem_get_first_ns(subsystem)) {

		subsystem->fdp_supported = false;

		SPDK_DEBUGLOG(nvmf, "Subsystem with id: %u doesn't have FDP capability.\n",

			      subsystem->id);

	}



	for (transport = spdk_nvmf_transport_get_first(subsystem->tgt); transport;

	     transport = spdk_nvmf_transport_get_next(transport)) {

		if (transport->ops->subsystem_remove_ns) {
@@ -1860,7 +1866,7 @@ spdk_nvmf_subsystem_add_ns_ext(struct spdk_nvmf_subsystem *subsystem, const char 
{

	struct spdk_nvmf_transport *transport;

	struct spdk_nvmf_ns_opts opts;

	struct spdk_nvmf_ns *ns;

	struct spdk_nvmf_ns *ns, *first_ns;

	struct spdk_nvmf_ctrlr *ctrlr;

	struct spdk_nvmf_reservation_info info = {0};

	int rc;
@@ -1996,6 +2002,22 @@ spdk_nvmf_subsystem_add_ns_ext(struct spdk_nvmf_subsystem *subsystem, const char 
		subsystem->max_zone_append_size_kib = max_zone_append_size_kib;

	}



	first_ns = spdk_nvmf_subsystem_get_first_ns(subsystem);

	if (!first_ns) {

		if (spdk_bdev_get_nvme_ctratt(ns->bdev).bits.fdps) {

			SPDK_DEBUGLOG(nvmf, "Subsystem with id: %u has FDP capability.\n",

				      subsystem->id);

			subsystem->fdp_supported = true;

		}

	} else {

		if (spdk_bdev_get_nvme_ctratt(first_ns->bdev).bits.fdps !=

		    spdk_bdev_get_nvme_ctratt(ns->bdev).bits.fdps) {

			SPDK_ERRLOG("Subsystem with id: %u can%s FDP namespace.\n", subsystem->id,

				    spdk_bdev_get_nvme_ctratt(first_ns->bdev).bits.fdps ? " only add" : "not add");

			goto err;

		}

	}



	ns->opts = opts;

	ns->subsystem = subsystem;

	subsystem->ns[opts.nsid - 1] = ns;

test/unit/lib/nvmf/ctrlr_discovery.c/ctrlr_discovery_ut.c

+3 −0
Original line number Diff line number Diff line
@@ -84,6 +84,9 @@ DEFINE_STUB(spdk_bdev_is_zoned, bool, (const struct spdk_bdev *bdev), false); 
DEFINE_STUB(spdk_bdev_get_max_zone_append_size, uint32_t,

	    (const struct spdk_bdev *bdev), 0);



DEFINE_STUB(spdk_bdev_get_nvme_ctratt, union spdk_bdev_nvme_ctratt,

	    (struct spdk_bdev *bdev), {});



const char *

spdk_bdev_get_name(const struct spdk_bdev *bdev)

{

test/unit/lib/nvmf/subsystem.c/subsystem_ut.c

+57 −0
Original line number Diff line number Diff line
@@ -196,6 +196,7 @@ nvmf_ctrlr_async_event_ns_notice(struct spdk_nvmf_ctrlr *ctrlr) 
static struct spdk_bdev g_bdevs[] = {

	{ .name = "bdev1" },

	{ .name = "bdev2" },

	{ .name = "bdev3", .ctratt.raw = 0x80000 },

};



struct spdk_bdev_desc {
@@ -248,6 +249,11 @@ spdk_bdev_get_uuid(const struct spdk_bdev *bdev) 
	return &bdev->uuid;

}



union spdk_bdev_nvme_ctratt spdk_bdev_get_nvme_ctratt(struct spdk_bdev *bdev)

{

	return bdev->ctratt;

}



static void

test_spdk_nvmf_subsystem_add_ns(void)

{
@@ -299,6 +305,56 @@ test_spdk_nvmf_subsystem_add_ns(void) 
	free(subsystem.ana_group);

}



static void

test_spdk_nvmf_subsystem_add_fdp_ns(void)

{

	struct spdk_nvmf_tgt tgt = {};

	struct spdk_nvmf_subsystem subsystem = {

		.max_nsid = 1024,

		.ns = NULL,

		.tgt = &tgt,

	};

	struct spdk_nvmf_ns_opts ns_opts;

	uint32_t nsid;

	int rc;



	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;

	RB_INIT(&tgt.subsystems);



	CU_ASSERT(subsystem.fdp_supported == false);



	/* Add a FDP supported namespace to the subsystem */

	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));

	ns_opts.nsid = 3;

	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev3", &ns_opts, sizeof(ns_opts), NULL);

	CU_ASSERT(nsid == 3);

	CU_ASSERT(subsystem.max_nsid == 1024);

	SPDK_CU_ASSERT_FATAL(subsystem.ns[nsid - 1] != NULL);

	CU_ASSERT(subsystem.ns[nsid - 1]->bdev == &g_bdevs[2]);

	CU_ASSERT(subsystem.fdp_supported == true);



	/* Try to add a non FDP supported namespace to the subsystem */

	spdk_nvmf_ns_opts_get_defaults(&ns_opts, sizeof(ns_opts));

	ns_opts.nsid = 5;

	nsid = spdk_nvmf_subsystem_add_ns_ext(&subsystem, "bdev2", &ns_opts, sizeof(ns_opts), NULL);

	CU_ASSERT(nsid == 0);

	CU_ASSERT(subsystem.max_nsid == 1024);

	CU_ASSERT(subsystem.fdp_supported == true);



	/* Remove last FDP namespace from the subsystem */

	rc = spdk_nvmf_subsystem_remove_ns(&subsystem, 3);

	CU_ASSERT(rc == 0);

	CU_ASSERT(subsystem.fdp_supported == false);



	free(subsystem.ns);

	free(subsystem.ana_group);

}



static void

nvmf_test_create_subsystem(void)

{
@@ -2151,6 +2207,7 @@ main(int argc, char **argv) 


	CU_ADD_TEST(suite, nvmf_test_create_subsystem);

	CU_ADD_TEST(suite, test_spdk_nvmf_subsystem_add_ns);

	CU_ADD_TEST(suite, test_spdk_nvmf_subsystem_add_fdp_ns);

	CU_ADD_TEST(suite, test_spdk_nvmf_subsystem_set_sn);

	CU_ADD_TEST(suite, test_spdk_nvmf_ns_visible);

	CU_ADD_TEST(suite, test_reservation_register);