Commit a9df326a authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvmf: only allow one Controller per Subsystem



Multiple NVMe controllers within a subsystem does not work correctly,
since we would need to virtualize the controller data, namespace IDs,
and so on.  For now, only allow pass-through mapping of a single NVMe
controller per subsystem.

Change-Id: Ib2d3576d2856c46a086f38eb6bec56f3e7a73575
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
parent 0e93df5c
Loading
Loading
Loading
Loading
+3 −4
Original line number Diff line number Diff line
@@ -94,7 +94,7 @@
#   session within the NVMf subsystem.  Any such session is allowed access
#   to all NVMe namespaces within the subsystem.
#
# SubsystemName, Mapping, Controller0 are minimum required
# SubsystemName, Mapping, Controller are minimum required
# The SubsystemName is concatenated with NodeBase above to form the NVMf
#   subsystem NQN that will be used by remote initiator to identify the
#   target subsystem for connection.
@@ -106,12 +106,11 @@
[Subsystem1]
  SubsystemName cnode1
  Mapping Port1 Host1
  # Using NVME 0 namespace 1
  Controller0 Nvme0
  Controller Nvme0

[Subsystem2]
  SubsystemName cnode2
  Mapping Port2 Host2
  # Using NVME 1 namespace 1
  Controller0 Nvme1
  Controller Nvme1
+14 −67
Original line number Diff line number Diff line
@@ -156,8 +156,6 @@ nvmf_process_admin_cmd(struct spdk_nvmf_request *req)
	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;
	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;
	struct spdk_nvmf_subsystem *subsystem = session->subsys;
	struct spdk_nvme_ctrlr *ctrlr = NULL;
	uint32_t nsid = 0;
	int rc = 0;
	uint8_t feature;

@@ -165,29 +163,6 @@ nvmf_process_admin_cmd(struct spdk_nvmf_request *req)
	response->status.sc = SPDK_NVME_SC_SUCCESS;
	response->cid = cmd->cid;

	if (cmd->nsid == 0) {
		/* may be valid for the requested command. but need
		   to at least map to a known valid controller.
		   Note:  Issue when in multi-controller subsystem
		   mode, commands that do not provide ns_id can not
		   be mapped to valid HW ctrlr!  This is where
		   definition of a virtual controller is required */
		ctrlr = subsystem->ns_list_map[0].ctrlr;
		nsid = 0;
	} else {
		/* verify namespace id */
		if (cmd->nsid > MAX_PER_SUBSYSTEM_NAMESPACES) {
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Invalid NS_ID %u\n",
				      cmd->nsid);
			response->status.sc = SPDK_NVME_SC_INVALID_NAMESPACE_OR_FORMAT;
			return true;
		}

		ctrlr = subsystem->ns_list_map[cmd->nsid - 1].ctrlr;
		nsid = subsystem->ns_list_map[cmd->nsid - 1].nvme_ns_id;
	}
	SPDK_TRACELOG(SPDK_TRACE_NVMF, "ctrlr %p nvme ns_id %u\n", ctrlr, nsid);

	switch (cmd->opc) {
	case SPDK_NVME_OPC_IDENTIFY:
		if (req->data == NULL) {
@@ -201,14 +176,9 @@ nvmf_process_admin_cmd(struct spdk_nvmf_request *req)
			const struct spdk_nvme_ns_data *nsdata;

			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Identify Namespace\n");
			if (nsid == 0) {
				SPDK_TRACELOG(SPDK_TRACE_NVMF, "Invalid NS_ID = 0\n");
				response->status.sc = SPDK_NVME_SC_INVALID_NAMESPACE_OR_FORMAT;
				return true;
			}
			ns = spdk_nvme_ctrlr_get_ns(ctrlr, nsid);
			ns = spdk_nvme_ctrlr_get_ns(subsystem->ctrlr, cmd->nsid);
			if (ns == NULL) {
				SPDK_TRACELOG(SPDK_TRACE_NVMF, "Unsuccessful query for Namespace reference\n");
				SPDK_TRACELOG(SPDK_TRACE_NVMF, "Unsuccessful query for nsid %u\n", cmd->nsid);
				response->status.sc = SPDK_NVME_SC_INVALID_FIELD;
				return true;
			}
@@ -234,10 +204,6 @@ nvmf_process_admin_cmd(struct spdk_nvmf_request *req)
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Get Features - Number of Queues\n");
			response->cdw0 = ((session->max_io_queues - 1) << 16) | (session->max_io_queues - 1);
			return true;
		case SPDK_NVME_FEAT_LBA_RANGE_TYPE:
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Get Features - LBA Range Type\n");
			cmd->nsid = nsid;
			goto passthrough;
		default:
			goto passthrough;
		}
@@ -300,8 +266,7 @@ nvmf_process_admin_cmd(struct spdk_nvmf_request *req)
	default:
passthrough:
		SPDK_TRACELOG(SPDK_TRACE_NVMF, "admin_cmd passthrough: opc 0x%02x\n", cmd->opc);
		cmd->nsid = nsid;
		rc = spdk_nvme_ctrlr_cmd_admin_raw(ctrlr,
		rc = spdk_nvme_ctrlr_cmd_admin_raw(subsystem->ctrlr,
						   cmd,
						   req->data, req->length,
						   nvmf_complete_cmd,
@@ -322,11 +287,7 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;
	struct spdk_nvme_cpl *response;
	struct spdk_nvmf_subsystem *subsystem = session->subsys;
	struct spdk_nvmf_namespace *nvmf_ns;
	struct spdk_nvme_ctrlr *ctrlr = NULL;
	struct spdk_nvme_ns *ns = NULL;
	struct spdk_nvme_qpair *qpair;
	uint32_t nsid = 0;
	struct spdk_nvme_ns *ns;
	struct nvme_read_cdw12 *cdw12;
	uint64_t lba_address;
	uint32_t lba_count;
@@ -338,13 +299,6 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
	response->status.sc = SPDK_NVME_SC_SUCCESS;
	response->cid = cmd->cid;

	/* verify subsystem */
	if (subsystem == NULL) {
		SPDK_ERRLOG("Subsystem Not Initialized!\n");
		response->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR;
		return true;
	}

	/* verify that the contoller is ready to process commands */
	if (session->vcprop.csts.bits.rdy == 0) {
		SPDK_ERRLOG("Subsystem Controller Not Ready!\n");
@@ -352,22 +306,16 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
		return true;
	}

	/* verify namespace id */
	if (cmd->nsid == 0 || cmd->nsid > MAX_PER_SUBSYSTEM_NAMESPACES) {
		SPDK_ERRLOG("Invalid NS_ID %u\n", cmd->nsid);
	switch (cmd->opc) {
	case SPDK_NVME_OPC_READ:
	case SPDK_NVME_OPC_WRITE:
		ns = spdk_nvme_ctrlr_get_ns(subsystem->ctrlr, cmd->nsid);
		if (ns == NULL) {
			SPDK_ERRLOG("Invalid NS ID %u\n", cmd->nsid);
			response->status.sc = SPDK_NVME_SC_INVALID_NAMESPACE_OR_FORMAT;
			return true;
		}

	nvmf_ns = &subsystem->ns_list_map[cmd->nsid - 1];
	ctrlr = nvmf_ns->ctrlr;
	nsid = nvmf_ns->nvme_ns_id;
	ns = nvmf_ns->ns;
	qpair = nvmf_ns->qpair;

	switch (cmd->opc) {
	case SPDK_NVME_OPC_READ:
	case SPDK_NVME_OPC_WRITE:
		cdw12 = (struct nvme_read_cdw12 *)&cmd->cdw12;
		/* NVMe library read/write interface expects non-0based lba_count value */
		lba_count = cdw12->nlb + 1;
@@ -379,7 +327,7 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Read LBA 0x%" PRIx64 ", 0x%x blocks\n",
				      lba_address, lba_count);
			spdk_trace_record(TRACE_NVMF_LIB_READ_START, 0, 0, (uint64_t)req, 0);
			rc = spdk_nvme_ns_cmd_read(ns, qpair,
			rc = spdk_nvme_ns_cmd_read(ns, subsystem->io_qpair,
						   req->data, lba_address, lba_count,
						   nvmf_complete_cmd,
						   req, io_flags);
@@ -387,7 +335,7 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Write LBA 0x%" PRIx64 ", 0x%x blocks\n",
				      lba_address, lba_count);
			spdk_trace_record(TRACE_NVMF_LIB_WRITE_START, 0, 0, (uint64_t)req, 0);
			rc = spdk_nvme_ns_cmd_write(ns, qpair,
			rc = spdk_nvme_ns_cmd_write(ns, subsystem->io_qpair,
						    req->data, lba_address, lba_count,
						    nvmf_complete_cmd,
						    req, io_flags);
@@ -395,8 +343,7 @@ nvmf_process_io_cmd(struct spdk_nvmf_request *req)
		break;
	default:
		SPDK_TRACELOG(SPDK_TRACE_NVMF, "io_cmd passthrough: opc 0x%02x\n", cmd->opc);
		cmd->nsid = nsid;
		rc = spdk_nvme_ctrlr_cmd_io_raw(ctrlr, qpair,
		rc = spdk_nvme_ctrlr_cmd_io_raw(subsystem->ctrlr, subsystem->io_qpair,
						cmd,
						req->data, req->length,
						nvmf_complete_cmd,
+5 −33
Original line number Diff line number Diff line
@@ -122,8 +122,6 @@ nvmf_init_discovery_session_properties(struct nvmf_session *session)
static void
nvmf_init_nvme_session_properties(struct nvmf_session *session, int aq_depth)
{
	/* for now base virtual controller properties on first namespace controller */
	struct spdk_nvme_ctrlr *ctrlr = session->subsys->ns_list_map[0].ctrlr;
	const struct spdk_nvme_ctrlr_data	*cdata;
	struct spdk_nvmf_extended_identify_ctrlr_data *nvmfdata;

@@ -134,14 +132,9 @@ nvmf_init_nvme_session_properties(struct nvmf_session *session, int aq_depth)
	*/

	/* Init the virtual controller details using actual HW details */
	cdata = spdk_nvme_ctrlr_get_data(ctrlr);
	cdata = spdk_nvme_ctrlr_get_data(session->subsys->ctrlr);
	memcpy((char *)&session->vcdata, (char *)cdata, sizeof(struct spdk_nvme_ctrlr_data));

	/* update virtual controller data to represent merge of
	   controllers for all namespaces
	*/
	session->vcdata.nn = session->subsys->ns_count;

	/* indicate support for only a single AER */
	session->vcdata.aerl = 0;

@@ -547,35 +540,14 @@ nvmf_property_set(struct nvmf_session *session,
void
nvmf_check_admin_completions(struct nvmf_session *session)
{
	struct spdk_nvmf_subsystem *subsystem = session->subsys;
	struct spdk_nvme_ctrlr *ctrlr, *prev_ctrlr = NULL;
	int i;

	for (i = 0; i < MAX_PER_SUBSYSTEM_NAMESPACES; i++) {
		ctrlr = subsystem->ns_list_map[i].ctrlr;
		if (ctrlr == NULL)
			continue;
		if (ctrlr != NULL && ctrlr != prev_ctrlr) {
			spdk_nvme_ctrlr_process_admin_completions(ctrlr);
			prev_ctrlr = ctrlr;
		}
	/* Discovery subsystem won't have a real NVMe controller, so check ctrlr first */
	if (session->subsys->ctrlr) {
		spdk_nvme_ctrlr_process_admin_completions(session->subsys->ctrlr);
	}
}

void
nvmf_check_io_completions(struct nvmf_session *session)
{
	struct spdk_nvmf_subsystem *subsystem = session->subsys;
	struct spdk_nvme_qpair *qpair, *prev_qpair = NULL;
	int i;

	for (i = 0; i < MAX_PER_SUBSYSTEM_NAMESPACES; i++) {
		qpair = subsystem->ns_list_map[i].qpair;
		if (qpair == NULL)
			continue;
		if (qpair != NULL && qpair != prev_qpair) {
			spdk_nvme_qpair_process_completions(qpair, 0);
			prev_qpair = qpair;
		}
	}
	spdk_nvme_qpair_process_completions(session->subsys->io_qpair, 0);
}
+24 −63
Original line number Diff line number Diff line
@@ -44,7 +44,6 @@
#include "spdk/trace.h"
#include "spdk/nvmf_spec.h"

#define MAX_TMPBUF 1024
#define SPDK_CN_TAG_MAX 0x0000ffff

static TAILQ_HEAD(, spdk_nvmf_subsystem_grp) g_ssg_head = TAILQ_HEAD_INITIALIZER(g_ssg_head);
@@ -116,44 +115,19 @@ nvmf_delete_subsystem(struct spdk_nvmf_subsystem *subsystem)
	return 0;
}

int
nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem,
static int
nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem,
			 struct spdk_nvme_ctrlr *ctrlr)
{
	int i, count, total_ns;
	struct spdk_nvme_qpair *qpair;
	struct spdk_nvmf_namespace *nvmf_ns;

	total_ns = spdk_nvme_ctrlr_get_num_ns(ctrlr);
	subsystem->ctrlr = ctrlr;

	/* Assume that all I/O will be handled on one thread for now */
	qpair = spdk_nvme_ctrlr_alloc_io_qpair(ctrlr, 0);
	if (qpair == NULL) {
	subsystem->io_qpair = spdk_nvme_ctrlr_alloc_io_qpair(ctrlr, 0);
	if (subsystem->io_qpair == NULL) {
		SPDK_ERRLOG("spdk_nvme_ctrlr_alloc_io_qpair() failed\n");
		return -1;
	}

	SPDK_TRACELOG(SPDK_TRACE_NVMF, "Adding %d namespaces from ctrlr %p to subsystem %s\n",
		      total_ns, ctrlr, subsystem->subnqn);

	count = 0;
	for (i = 0; i < MAX_PER_SUBSYSTEM_NAMESPACES; i++) {
		if (count == total_ns) {
			break;
		}
		nvmf_ns = &subsystem->ns_list_map[i];
		if (nvmf_ns->ctrlr == NULL) {
			SPDK_TRACELOG(SPDK_TRACE_NVMF, "Adding namespace %d to subsystem %s\n", count + 1,
				      subsystem->subnqn);
			nvmf_ns->ctrlr = ctrlr;
			nvmf_ns->qpair = qpair;
			nvmf_ns->nvme_ns_id = count + 1;
			nvmf_ns->ns = spdk_nvme_ctrlr_get_ns(ctrlr, count + 1);
			subsystem->ns_count++;
			count++;
		}
	}

	return 0;
}

@@ -246,7 +220,6 @@ spdk_nvmf_subsystem_add_map(struct spdk_nvmf_subsystem_grp *ss_group,
static int
spdk_cf_add_nvmf_subsystem(struct spdk_conf_section *sp)
{
	char buf[MAX_TMPBUF];
	struct spdk_nvmf_subsystem_grp *ss_group;
	const char *port_tag, *ig_tag;
	const char *val, *name;
@@ -340,39 +313,27 @@ spdk_cf_add_nvmf_subsystem(struct spdk_conf_section *sp)
		goto err0;
	}

	/* add controllers into the subsystem */
	for (i = 0; i < MAX_PER_SUBSYSTEM_NAMESPACES; i++) {
		snprintf(buf, sizeof(buf), "Controller%d", i);
		val = spdk_conf_section_get_val(sp, buf);
	val = spdk_conf_section_get_val(sp, "Controller");
	if (val == NULL) {
			break;
		}

		val = spdk_conf_section_get_nmval(sp, buf, 0, 0);
		if (val == NULL) {
			SPDK_ERRLOG("No name specified for Controller%d\n", i);
		SPDK_ERRLOG("Subsystem %d: missing Controller\n", ss_group->num);
		goto err0;
	}

	/* claim this controller from the available controller list */
	nvmf_ctrlr = spdk_nvmf_ctrlr_claim(val);
	if (nvmf_ctrlr == NULL) {
			SPDK_TRACELOG(SPDK_TRACE_DEBUG, "nvme controller %s not found\n", val);
			continue;
		SPDK_ERRLOG("Subsystem %d: NVMe controller %s not found\n", ss_group->num, val);
		goto err0;
	}

		/* notify nvmf library to add this device namespace
		   to this subsystem.
		 */
		ret = nvmf_subsystem_add_ns(ss_group->subsystem, nvmf_ctrlr->ctrlr);
	ret = nvmf_subsystem_add_ctrlr(ss_group->subsystem, nvmf_ctrlr->ctrlr);
	if (ret < 0) {
			SPDK_ERRLOG("nvmf library add namespace failed!\n");
		SPDK_ERRLOG("Subsystem %d: adding controller %s failed\n", ss_group->num, val);
		goto err0;
	}

	SPDK_TRACELOG(SPDK_TRACE_DEBUG, "    NVMf Subsystem: Nvme Controller: %s , %p\n",
		      nvmf_ctrlr->name, nvmf_ctrlr->ctrlr);
	}

	TAILQ_INSERT_TAIL(&g_ssg_head, ss_group, tailq);

+2 −14
Original line number Diff line number Diff line
@@ -40,16 +40,8 @@
struct spdk_nvmf_conn;

#define MAX_PER_SUBSYSTEM_ACCESS_MAP 2
#define MAX_PER_SUBSYSTEM_NAMESPACES 32
#define MAX_NQN_SIZE 255

struct spdk_nvmf_namespace {
	int nvme_ns_id;
	struct spdk_nvme_ns *ns;
	struct spdk_nvme_ctrlr *ctrlr;
	struct spdk_nvme_qpair *qpair;
};

/*
 * The NVMf subsystem, as indicated in the specification, is a collection
 * of virtual controller sessions.  Any individual controller session has
@@ -61,8 +53,8 @@ struct spdk_nvmf_subsystem {
	int num_sessions;
	enum spdk_nvmf_subsystem_types subtype;
	TAILQ_HEAD(session_q, nvmf_session) sessions;
	struct spdk_nvmf_namespace ns_list_map[MAX_PER_SUBSYSTEM_NAMESPACES];
	int ns_count;
	struct spdk_nvme_ctrlr *ctrlr;
	struct spdk_nvme_qpair *io_qpair;

	TAILQ_ENTRY(spdk_nvmf_subsystem) entries;
};
@@ -87,10 +79,6 @@ nvmf_create_subsystem(int num, char *name, enum spdk_nvmf_subsystem_types sub_ty
int
nvmf_delete_subsystem(struct spdk_nvmf_subsystem *subsystem);

int
nvmf_subsystem_add_ns(struct spdk_nvmf_subsystem *subsystem,
		      struct spdk_nvme_ctrlr *ctrlr);

struct spdk_nvmf_subsystem *
nvmf_find_subsystem(const char *subnqn);

Loading