Commit 89d35cef authored by Daniel Verkamp's avatar Daniel Verkamp
Browse files

nvmf: refactor get_log_page into a common function 



Both regular NVM controllers and discovery controllers implement the Get
Log Page command; combine the implementations into one in ctrlr.c.

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


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
parent 2d1a2926

lib/nvmf/ctrlr.c

+69 −0
Original line number Diff line number Diff line
@@ -818,3 +818,72 @@ spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req) 
	ctrlr->aer_req = req;

	return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS;

}



int

spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req)

{

	struct spdk_nvmf_subsystem *subsystem = req->qpair->ctrlr->subsys;

	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;

	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;

	uint64_t offset, len;

	uint32_t numdl, numdu;

	uint8_t lid;



	if (req->data == NULL) {

		SPDK_ERRLOG("get log command with no buffer\n");

		response->status.sct = SPDK_NVME_SCT_GENERIC;

		response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	memset(req->data, 0, req->length);



	offset = (uint64_t)cmd->cdw12 | ((uint64_t)cmd->cdw13 << 32);

	if (offset & 3) {

		SPDK_ERRLOG("Invalid log page offset 0x%" PRIx64 "\n", offset);

		response->status.sct = SPDK_NVME_SCT_GENERIC;

		response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	numdl = (cmd->cdw10 >> 16) & 0xFFFFu;

	numdu = (cmd->cdw11) & 0xFFFFu;

	len = ((numdu << 16) + numdl + (uint64_t)1) * 4;

	if (len > req->length) {

		SPDK_ERRLOG("Get log page: len (%" PRIu64 ") > buf size (%u)\n",

			    len, req->length);

		response->status.sct = SPDK_NVME_SCT_GENERIC;

		response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	lid = cmd->cdw10 & 0xFF;

	SPDK_TRACELOG(SPDK_TRACE_NVMF, "Get log page: LID=0x%02X offset=0x%" PRIx64 " len=0x%" PRIx64 "\n",

		      lid, offset, len);



	if (subsystem->subtype == SPDK_NVMF_SUBTYPE_DISCOVERY) {

		switch (lid) {

		case SPDK_NVME_LOG_DISCOVERY:

			spdk_nvmf_get_discovery_log_page(req->data, offset, len);

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		default:

			goto invalid_log_page;

		}

	} else {

		switch (lid) {

		case SPDK_NVME_LOG_ERROR:

		case SPDK_NVME_LOG_HEALTH_INFORMATION:

		case SPDK_NVME_LOG_FIRMWARE_SLOT:

			/* TODO: actually fill out log page data */

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		default:

			goto invalid_log_page;

		}

	}



invalid_log_page:

	SPDK_ERRLOG("Unsupported Get Log Page 0x%02X\n", lid);

	response->status.sct = SPDK_NVME_SCT_GENERIC;

	response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

	return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

}

lib/nvmf/ctrlr.h

+2 −0
Original line number Diff line number Diff line
@@ -134,6 +134,8 @@ int spdk_nvmf_ctrlr_get_features_async_event_configuration(struct spdk_nvmf_requ 


int spdk_nvmf_ctrlr_async_event_request(struct spdk_nvmf_request *req);



int spdk_nvmf_ctrlr_get_log_page(struct spdk_nvmf_request *req);



void spdk_nvmf_ctrlr_set_dsm(struct spdk_nvmf_ctrlr *ctrlr);



#endif

lib/nvmf/ctrlr_bdev.c

+1 −38
Original line number Diff line number Diff line
@@ -105,43 +105,6 @@ nvmf_bdev_ctrlr_complete_cmd(struct spdk_bdev_io *bdev_io, bool success, 
	spdk_bdev_free_io(bdev_io);

}



static int

nvmf_bdev_ctrlr_get_log_page(struct spdk_nvmf_request *req)

{

	uint8_t lid;

	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;

	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;

	uint64_t log_page_offset;



	if (req->data == NULL) {

		SPDK_ERRLOG("get log command with no buffer\n");

		response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	memset(req->data, 0, req->length);



	log_page_offset = (uint64_t)cmd->cdw12 | ((uint64_t)cmd->cdw13 << 32);

	if (log_page_offset & 3) {

		SPDK_ERRLOG("Invalid log page offset 0x%" PRIx64 "\n", log_page_offset);

		response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	lid = cmd->cdw10 & 0xFF;

	switch (lid) {

	case SPDK_NVME_LOG_ERROR:

	case SPDK_NVME_LOG_HEALTH_INFORMATION:

	case SPDK_NVME_LOG_FIRMWARE_SLOT:

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	default:

		SPDK_ERRLOG("Unsupported Get Log Page 0x%02X\n", lid);

		response->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC;

		response->status.sc = SPDK_NVME_SC_INVALID_LOG_PAGE;

		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}

}



static int

identify_ns(struct spdk_nvmf_subsystem *subsystem,

	    struct spdk_nvme_cmd *cmd,
@@ -357,7 +320,7 @@ nvmf_bdev_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) 


	switch (cmd->opc) {

	case SPDK_NVME_OPC_GET_LOG_PAGE:

		return nvmf_bdev_ctrlr_get_log_page(req);

		return spdk_nvmf_ctrlr_get_log_page(req);

	case SPDK_NVME_OPC_IDENTIFY:

		return nvmf_bdev_ctrlr_identify(req);

	case SPDK_NVME_OPC_ABORT:

lib/nvmf/ctrlr_discovery.c

+1 −34
Original line number Diff line number Diff line
@@ -144,22 +144,12 @@ spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length) 
	assert(copy_len + zero_len == length);

}



static inline uint32_t

nvmf_get_log_page_len(struct spdk_nvme_cmd *cmd)

{

	uint32_t numdl = (cmd->cdw10 >> 16) & 0xFFFFu;

	uint32_t numdu = (cmd->cdw11) & 0xFFFFu;

	return ((numdu << 16) + numdl + 1) * sizeof(uint32_t);

}



static int

nvmf_discovery_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req)

{

	struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr;

	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;

	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;

	uint64_t log_page_offset;

	uint32_t len;



	/* pre-set response details for this command */

	response->status.sc = SPDK_NVME_SC_SUCCESS;
@@ -184,30 +174,7 @@ nvmf_discovery_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req) 
		}

		break;

	case SPDK_NVME_OPC_GET_LOG_PAGE:

		log_page_offset = (uint64_t)cmd->cdw12 | ((uint64_t)cmd->cdw13 << 32);

		if (log_page_offset & 3) {

			SPDK_ERRLOG("Invalid log page offset 0x%" PRIx64 "\n", log_page_offset);

			response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		}



		len = nvmf_get_log_page_len(cmd);

		if (len > req->length) {

			SPDK_ERRLOG("Get log page: len (%u) > buf size (%u)\n",

				    len, req->length);

			response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		}



		if ((cmd->cdw10 & 0xFF) == SPDK_NVME_LOG_DISCOVERY) {

			spdk_nvmf_get_discovery_log_page(req->data, log_page_offset, len);

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		} else {

			SPDK_ERRLOG("Unsupported log page %u\n", cmd->cdw10 & 0xFF);

			response->status.sc = SPDK_NVME_SC_INVALID_FIELD;

			return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

		}

		break;

		return spdk_nvmf_ctrlr_get_log_page(req);

	default:

		SPDK_ERRLOG("Unsupported Opcode 0x%x for Discovery service\n", cmd->opc);

		response->status.sc = SPDK_NVME_SC_INVALID_OPCODE;

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

+67 −3
Original line number Diff line number Diff line
@@ -108,9 +108,10 @@ spdk_nvmf_ctrlr_set_dsm(struct spdk_nvmf_ctrlr *ctrlr) 
	abort();

}



static void

test_foobar(void)

void

spdk_nvmf_get_discovery_log_page(void *buffer, uint64_t offset, uint32_t length)

{

	abort();

}



int
@@ -119,6 +120,69 @@ spdk_nvmf_request_complete(struct spdk_nvmf_request *req) 
	return -1;

}



static void

test_get_log_page(void)

{

	struct spdk_nvmf_subsystem subsystem = {};

	struct spdk_nvmf_request req = {};

	struct spdk_nvmf_qpair qpair = {};

	struct spdk_nvmf_ctrlr ctrlr = {};

	union nvmf_h2c_msg cmd = {};

	union nvmf_c2h_msg rsp = {};

	char data[4096];



	subsystem.subtype = SPDK_NVMF_SUBTYPE_NVME;



	ctrlr.subsys = &subsystem;



	qpair.ctrlr = &ctrlr;



	req.qpair = &qpair;

	req.cmd = &cmd;

	req.rsp = &rsp;

	req.data = &data;

	req.length = sizeof(data);



	/* Get Log Page - all valid */

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

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

	cmd.nvme_cmd.opc = SPDK_NVME_OPC_GET_LOG_PAGE;

	cmd.nvme_cmd.cdw10 = SPDK_NVME_LOG_ERROR | (req.length / 4 - 1) << 16;

	CU_ASSERT(spdk_nvmf_ctrlr_get_log_page(&req) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(req.rsp->nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);

	CU_ASSERT(req.rsp->nvme_cpl.status.sc == SPDK_NVME_SC_SUCCESS);



	/* Get Log Page with invalid log ID */

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

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

	cmd.nvme_cmd.opc = SPDK_NVME_OPC_GET_LOG_PAGE;

	cmd.nvme_cmd.cdw10 = 0;

	CU_ASSERT(spdk_nvmf_ctrlr_get_log_page(&req) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(req.rsp->nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);

	CU_ASSERT(req.rsp->nvme_cpl.status.sc == SPDK_NVME_SC_INVALID_FIELD);



	/* Get Log Page with invalid offset (not dword aligned) */

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

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

	cmd.nvme_cmd.opc = SPDK_NVME_OPC_GET_LOG_PAGE;

	cmd.nvme_cmd.cdw10 = SPDK_NVME_LOG_ERROR | (req.length / 4 - 1) << 16;

	cmd.nvme_cmd.cdw12 = 2;

	CU_ASSERT(spdk_nvmf_ctrlr_get_log_page(&req) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(req.rsp->nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);

	CU_ASSERT(req.rsp->nvme_cpl.status.sc == SPDK_NVME_SC_INVALID_FIELD);



	/* Get Log Page without data buffer */

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

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

	req.data = NULL;

	cmd.nvme_cmd.opc = SPDK_NVME_OPC_GET_LOG_PAGE;

	cmd.nvme_cmd.cdw10 = SPDK_NVME_LOG_ERROR | (req.length / 4 - 1) << 16;

	CU_ASSERT(spdk_nvmf_ctrlr_get_log_page(&req) == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);

	CU_ASSERT(req.rsp->nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC);

	CU_ASSERT(req.rsp->nvme_cpl.status.sc == SPDK_NVME_SC_INVALID_FIELD);

	req.data = data;

}



int main(int argc, char **argv)

{

	CU_pSuite	suite = NULL;
@@ -135,7 +199,7 @@ int main(int argc, char **argv) 
	}



	if (

		CU_add_test(suite, "foobar", test_foobar) == NULL) {

		CU_add_test(suite, "get_log_page", test_get_log_page) == NULL) {

		CU_cleanup_registry();

		return CU_get_error();

	}