Commit 4ff0eba8 authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvmf: merge connect handling into single function 



Combine nvmf_process_connect() into spdk_nvmf_ctrlr_connect() to
simplify the logic and keep it all together in one function.

This also allows us to move the invalid connect helper function and
macros back out of the header into a static function in one file.

Change-Id: Ia3bd80d0309392520d51bf8f5830d3f23332c5e3
Signed-off-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-on: https://review.gerrithub.io/378016


Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
parent 6b8096d8

lib/nvmf/ctrlr.c

+66 −12
Original line number Diff line number Diff line
@@ -152,15 +152,39 @@ spdk_nvmf_ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr) 
	ctrlr_destruct(ctrlr);

}



static inline void

spdk_nvmf_invalid_connect_response(struct spdk_nvmf_fabric_connect_rsp *rsp,

				   uint8_t iattr, uint16_t ipo)

{

	rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

	rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_PARAM;

	rsp->status_code_specific.invalid.iattr = iattr;

	rsp->status_code_specific.invalid.ipo = ipo;

}



#define SPDK_NVMF_INVALID_CONNECT_CMD(rsp, field)	\

	spdk_nvmf_invalid_connect_response(rsp, 0, offsetof(struct spdk_nvmf_fabric_connect_cmd, field))

#define SPDK_NVMF_INVALID_CONNECT_DATA(rsp, field)	\

	spdk_nvmf_invalid_connect_response(rsp, 1, offsetof(struct spdk_nvmf_fabric_connect_data, field))



void

spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair,

			struct spdk_nvmf_fabric_connect_cmd *cmd,

			struct spdk_nvmf_fabric_connect_data *data,

			struct spdk_nvmf_fabric_connect_rsp *rsp)

spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req)

{

	struct spdk_nvmf_tgt *tgt;

	struct spdk_nvmf_fabric_connect_data *data = req->data;

	struct spdk_nvmf_fabric_connect_cmd *cmd = &req->cmd->connect_cmd;

	struct spdk_nvmf_fabric_connect_rsp *rsp = &req->rsp->connect_rsp;

	struct spdk_nvmf_qpair *qpair = req->qpair;

	struct spdk_nvmf_tgt *tgt = qpair->transport->tgt;

	struct spdk_nvmf_ctrlr *ctrlr;

	struct spdk_nvmf_subsystem *subsystem;

	const char *subnqn, *hostnqn;

	void *end;



	if (req->length < sizeof(struct spdk_nvmf_fabric_connect_data)) {

		SPDK_ERRLOG("Connect command data length 0x%x too small\n", req->length);

		rsp->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return;

	}



	SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "recfmt 0x%x qid %u sqsize %u\n",

		      cmd->recfmt, cmd->qid, cmd->sqsize);
@@ -175,21 +199,48 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, 
		      data->hostid[9],

		      ntohs(*(uint16_t *)&data->hostid[10]),

		      ntohl(*(uint32_t *)&data->hostid[12]));

	SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "  subnqn: \"%s\"\n", data->subnqn);

	SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "  hostnqn: \"%s\"\n", data->hostnqn);



	assert(qpair->thread == NULL);

	qpair->thread = spdk_get_thread();

	if (cmd->recfmt != 0) {

		SPDK_ERRLOG("Connect command unsupported RECFMT %u\n", cmd->recfmt);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INCOMPATIBLE_FORMAT;

		return;

	}



	tgt = qpair->transport->tgt;

	/* Ensure that subnqn is null terminated */

	end = memchr(data->subnqn, '\0', SPDK_NVMF_NQN_MAX_LEN + 1);

	if (!end) {

		SPDK_ERRLOG("Connect SUBNQN is not null terminated\n");

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, subnqn);

		return;

	}

	subnqn = data->subnqn;

	SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "  subnqn: \"%s\"\n", subnqn);



	subsystem = spdk_nvmf_tgt_find_subsystem(tgt, data->subnqn);

	subsystem = spdk_nvmf_tgt_find_subsystem(tgt, subnqn);

	if (subsystem == NULL) {

		SPDK_ERRLOG("Could not find subsystem '%s'\n", data->subnqn);

		SPDK_ERRLOG("Could not find subsystem '%s'\n", subnqn);

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, subnqn);

		return;

	}



	/* Ensure that hostnqn is null terminated */

	end = memchr(data->hostnqn, '\0', SPDK_NVMF_NQN_MAX_LEN + 1);

	if (!end) {

		SPDK_ERRLOG("Connect HOSTNQN is not null terminated\n");

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, hostnqn);

		return;

	}

	hostnqn = data->hostnqn;

	SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "  hostnqn: \"%s\"\n", hostnqn);



	if (!spdk_nvmf_subsystem_host_allowed(subsystem, hostnqn)) {

		SPDK_ERRLOG("Subsystem '%s' does not allow host '%s'\n", subnqn, hostnqn);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_HOST;

		return;

	}



	/*

	 * SQSIZE is a 0-based value, so it must be at least 1 (minimum queue depth is 2) and

	 *  strictly less than max_queue_depth.
@@ -273,6 +324,9 @@ spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair, 
		}

	}



	assert(qpair->thread == NULL);

	qpair->thread = spdk_get_thread();



	ctrlr->num_qpairs++;

	TAILQ_INSERT_HEAD(&ctrlr->qpairs, qpair, link);

	qpair->ctrlr = ctrlr;

lib/nvmf/nvmf_internal.h

+1 −19
Original line number Diff line number Diff line
@@ -222,10 +222,7 @@ void spdk_nvmf_property_set(struct spdk_nvmf_ctrlr *ctrlr, 
			    struct spdk_nvmf_fabric_prop_set_cmd *cmd,

			    struct spdk_nvme_cpl *rsp);



void spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair,

			     struct spdk_nvmf_fabric_connect_cmd *cmd,

			     struct spdk_nvmf_fabric_connect_data *data,

			     struct spdk_nvmf_fabric_connect_rsp *rsp);

void spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req);

struct spdk_nvmf_qpair *spdk_nvmf_ctrlr_get_qpair(struct spdk_nvmf_ctrlr *ctrlr, uint16_t qid);

int spdk_nvmf_ctrlr_poll(struct spdk_nvmf_ctrlr *ctrlr);

void spdk_nvmf_ctrlr_destruct(struct spdk_nvmf_ctrlr *ctrlr);
@@ -263,21 +260,6 @@ _spdk_nvmf_subsystem_get_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid 
	return ns;

}



static inline void

spdk_nvmf_invalid_connect_response(struct spdk_nvmf_fabric_connect_rsp *rsp,

				   uint8_t iattr, uint16_t ipo)

{

	rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

	rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_PARAM;

	rsp->status_code_specific.invalid.iattr = iattr;

	rsp->status_code_specific.invalid.ipo = ipo;

}



#define SPDK_NVMF_INVALID_CONNECT_CMD(rsp, field)	\

	spdk_nvmf_invalid_connect_response(rsp, 0, offsetof(struct spdk_nvmf_fabric_connect_cmd, field))

#define SPDK_NVMF_INVALID_CONNECT_DATA(rsp, field)	\

	spdk_nvmf_invalid_connect_response(rsp, 1, offsetof(struct spdk_nvmf_fabric_connect_data, field))



#define OBJECT_NVMF_IO				0x30



#define TRACE_GROUP_NVMF			0x3

lib/nvmf/request.c

+2 −61
Original line number Diff line number Diff line
@@ -109,66 +109,6 @@ nvmf_process_property_set(struct spdk_nvmf_request *req) 
	return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

}



static spdk_nvmf_request_exec_status

nvmf_process_connect(struct spdk_nvmf_request *req)

{

	struct spdk_nvmf_tgt		*tgt;

	struct spdk_nvmf_subsystem	*subsystem;

	struct spdk_nvmf_fabric_connect_data *data = (struct spdk_nvmf_fabric_connect_data *)

			req->data;

	struct spdk_nvmf_fabric_connect_cmd *cmd = &req->cmd->connect_cmd;

	struct spdk_nvmf_fabric_connect_rsp *rsp = &req->rsp->connect_rsp;

	void *end;



	if (cmd->recfmt != 0) {

		SPDK_ERRLOG("Connect command unsupported RECFMT %u\n", cmd->recfmt);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INCOMPATIBLE_FORMAT;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	if (req->length < sizeof(struct spdk_nvmf_fabric_connect_data)) {

		SPDK_ERRLOG("Connect command data length 0x%x too small\n", req->length);

		req->rsp->nvme_cpl.status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	/* Ensure that subnqn and hostnqn are null terminated */

	end = memchr(data->subnqn, '\0', SPDK_NVMF_NQN_MAX_LEN + 1);

	if (!end) {

		SPDK_ERRLOG("Connect SUBNQN is not null terminated\n");

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, subnqn);

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	end = memchr(data->hostnqn, '\0', SPDK_NVMF_NQN_MAX_LEN + 1);

	if (!end) {

		SPDK_ERRLOG("Connect HOSTNQN is not null terminated\n");

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, hostnqn);

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	tgt = req->qpair->transport->tgt;



	subsystem = spdk_nvmf_tgt_find_subsystem(tgt, data->subnqn);

	if (subsystem == NULL) {

		SPDK_ERRLOG("Could not find subsystem '%s'\n", data->subnqn);

		SPDK_NVMF_INVALID_CONNECT_DATA(rsp, subnqn);

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	if (!spdk_nvmf_subsystem_host_allowed(subsystem, data->hostnqn)) {

		SPDK_ERRLOG("Subsystem '%s' does not allow host '%s'\n", data->subnqn, data->hostnqn);

		rsp->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		rsp->status.sc = SPDK_NVMF_FABRIC_SC_INVALID_HOST;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	spdk_nvmf_ctrlr_connect(req->qpair, cmd, req->data, rsp);



	return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

}



static spdk_nvmf_request_exec_status

nvmf_process_fabrics_command(struct spdk_nvmf_request *req)

{
@@ -180,7 +120,8 @@ nvmf_process_fabrics_command(struct spdk_nvmf_request *req) 
	if (qpair->ctrlr == NULL) {

		/* No ctrlr established yet; the only valid command is Connect */

		if (cap_hdr->fctype == SPDK_NVMF_FABRIC_COMMAND_CONNECT) {

			return nvmf_process_connect(req);

			spdk_nvmf_ctrlr_connect(req);

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		} else {

			SPDK_DEBUGLOG(SPDK_TRACE_NVMF, "Got fctype 0x%x, expected Connect\n",

				      cap_hdr->fctype);

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

+6 −0
Original line number Diff line number Diff line
@@ -115,6 +115,12 @@ spdk_nvmf_subsystem_get_next_ns(struct spdk_nvmf_subsystem *subsystem, struct sp 
	return NULL;

}



bool

spdk_nvmf_subsystem_host_allowed(struct spdk_nvmf_subsystem *subsystem, const char *hostnqn)

{

	return true;

}



int

spdk_nvmf_subsystem_add_ctrlr(struct spdk_nvmf_subsystem *subsystem, struct spdk_nvmf_ctrlr *ctrlr)

{

test/unit/lib/nvmf/request.c/request_ut.c

+1 −17
Original line number Diff line number Diff line
@@ -51,10 +51,7 @@ spdk_nvmf_transport_req_complete(struct spdk_nvmf_request *req) 
}



void

spdk_nvmf_ctrlr_connect(struct spdk_nvmf_qpair *qpair,

			struct spdk_nvmf_fabric_connect_cmd *cmd,

			struct spdk_nvmf_fabric_connect_data *data,

			struct spdk_nvmf_fabric_connect_rsp *rsp)

spdk_nvmf_ctrlr_connect(struct spdk_nvmf_request *req)

{

}
@@ -139,19 +136,6 @@ spdk_nvmf_get_discovery_log_page(struct spdk_nvmf_tgt *tgt, void *buffer, 
{

}



struct spdk_nvmf_subsystem *

spdk_nvmf_tgt_find_subsystem(struct spdk_nvmf_tgt *tgt, const char *subnqn)

{

	return NULL;

}



bool

spdk_nvmf_subsystem_host_allowed(struct spdk_nvmf_subsystem *subsystem, const char *hostnqn)

{

	return false;

}





static void

test_nvmf_process_fabrics_cmd(void)

{