Commit 18192cd4 authored by Pawel Baldysiak's avatar Pawel Baldysiak Committed by Tomasz Zawadzki
Browse files

nvmf_tgt: Add passthru support for get/set_features OPC 



This patch adds basic passthru support for
get/set_feature OPCs. It aligns feat_id_and_effects log page
as well. It is disabled by default, can be enabled by RPC.
It uses already existing generic passthru handler,
which makes sure that ctrlr-scope admin cmds are passthru
only for subsystem with single namespace.

Not all features should be handled directly by the drive.
Exclude them from passthru handling.

Feat_id_and_effect log page should contains feature
supported by the drive + the one explicitely handled by SPDK.

Change-Id: I870fe69c958abf7a0574641e39adf727f9cf6712
Signed-off-by: default avatarPawel Baldysiak <pawel.baldysiak@dell.com>
Reviewed-on: https://review.spdk.io/c/spdk/spdk/+/26458


Tested-by: default avatarSPDK Automated Test System <spdkbot@gmail.com>
Reviewed-by: default avatarJim Harris <jim.harris@nvidia.com>
Community-CI: Mellanox Build Bot
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
parent da574235

doc/jsonrpc.md.jinja2

+6 −5
Original line number Diff line number Diff line
@@ -8586,9 +8586,10 @@ have been initialized. 
##### admin_cmd_passthru {#spdk_nvmf_admin_passthru_conf}



 Name             | Optional   | Type   | Description

--------------- | ---------- | ------ | --------------------------------------------------------------------------------------------------------------

----------------- | ---------- | ------ | --------------------------------------------------------------------------------------------------------------

 identify_ctrlr   | Optional   | bool   | If true, enables custom identify handler that reports some identify attributes from the underlying NVMe drive

 get_log_page     | Optional   | bool   | If true, enables passthru get log page command handlers for underlying NVMe drive (for non-fabrics related log pages)

 get_set_features | Optional   | bool   | If true, enables passthru get/set_feature OPC handlers for underlying NVMe drive

 vendor_specific  | Optional   | bool   | If true, enables passthru command handlers for underlying NVMe drive for vendor specific cmds (OPCs: 0xC0-0xFF)



#### Example

module/event/subsystems/nvmf/event_nvmf.h

+1 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ 
struct spdk_nvmf_admin_passthru_conf {

	bool identify_ctrlr;

	bool get_log_page;

	bool get_set_features;

	bool vendor_specific;

};

module/event/subsystems/nvmf/nvmf_rpc.c

+1 −0
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ SPDK_RPC_REGISTER("nvmf_set_max_subsystems", rpc_nvmf_set_max_subsystems, 
static const struct spdk_json_object_decoder admin_passthru_decoder[] = {

	{"identify_ctrlr", offsetof(struct spdk_nvmf_admin_passthru_conf, identify_ctrlr), spdk_json_decode_bool, true},

	{"get_log_page", offsetof(struct spdk_nvmf_admin_passthru_conf, get_log_page), spdk_json_decode_bool, true},

	{"get_set_features", offsetof(struct spdk_nvmf_admin_passthru_conf, get_set_features), spdk_json_decode_bool, true},

	{"vendor_specific", offsetof(struct spdk_nvmf_admin_passthru_conf, vendor_specific), spdk_json_decode_bool, true},

};

module/event/subsystems/nvmf/nvmf_tgt.c

+91 −2
Original line number Diff line number Diff line
@@ -57,6 +57,7 @@ struct spdk_nvmf_tgt_conf g_spdk_nvmf_tgt_conf = { 
	},

	.admin_passthru.identify_ctrlr = false,

	.admin_passthru.get_log_page = false,

	.admin_passthru.get_set_features = false,

	.admin_passthru.vendor_specific = false

};
@@ -455,6 +456,53 @@ fixup_get_supported_log_pages(struct spdk_nvmf_request *req) 
	spdk_nvmf_request_copy_from_buf(req, (uint8_t *) &nvme_log_data + offset, datalen);

}



static void

fixup_get_feature_ids_effects_log_page(struct spdk_nvmf_request *req)

{

	struct spdk_nvme_feature_ids_effects_log_page nvme_log_data = {};

	struct spdk_nvme_feature_ids_effects_log_page nvmf_log_data = {};

	struct spdk_nvmf_ctrlr *ctrlr = spdk_nvmf_request_get_ctrlr(req);

	uint32_t page_size = sizeof(struct spdk_nvme_feature_ids_effects_log_page);

	uint64_t offset;

	size_t datalen, copy_len = 0;



	get_log_page_offset_and_len(req, page_size, &offset, &copy_len);



	if (copy_len == 0) {

		return;

	}



	/* This is supported features log page from the NVMe drive */

	datalen = spdk_nvmf_request_copy_to_buf(req, (uint8_t *) &nvme_log_data + offset,

						copy_len);



	/* This is supported features log page from SPDK */

	spdk_nvmf_get_feature_ids_effects_log_page(ctrlr, &nvmf_log_data);



	if (!g_spdk_nvmf_tgt_conf.admin_passthru.get_set_features) {

		/* Passthru support to the drive is disabled for get/set features OPC

		 * Report only SPDK supported features.

		 */

		spdk_nvmf_request_copy_from_buf(req, (uint8_t *) &nvmf_log_data + offset, datalen);

		return;

	}



	/* Make sure that features not handled by custom handler are added as well */

	nvme_log_data.fis[SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION] =

		nvmf_log_data.fis[SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION];

	nvme_log_data.fis[SPDK_NVME_FEAT_KEEP_ALIVE_TIMER] =

		nvmf_log_data.fis[SPDK_NVME_FEAT_KEEP_ALIVE_TIMER];

	nvme_log_data.fis[SPDK_NVME_FEAT_NUMBER_OF_QUEUES] =

		nvmf_log_data.fis[SPDK_NVME_FEAT_NUMBER_OF_QUEUES];

	nvme_log_data.fis[SPDK_NVME_FEAT_HOST_RESERVE_MASK] =

		nvmf_log_data.fis[SPDK_NVME_FEAT_HOST_RESERVE_MASK];

	nvme_log_data.fis[SPDK_NVME_FEAT_HOST_RESERVE_PERSIST] =

		nvmf_log_data.fis[SPDK_NVME_FEAT_HOST_RESERVE_PERSIST];



	/* Copy the data back to the request */

	spdk_nvmf_request_copy_from_buf(req, (uint8_t *) &nvme_log_data + offset, datalen);

}



static void

fixup_identify_ctrlr(struct spdk_nvmf_request *req)

{
@@ -562,9 +610,9 @@ nvmf_custom_get_log_page_hdlr(struct spdk_nvmf_request *req) 
	 */

	case SPDK_NVME_LOG_ASYMMETRIC_NAMESPACE_ACCESS:

	case SPDK_NVME_LOG_CHANGED_NS_LIST:

	/* SPDK is not supporting get/set_feature passthru, so disable log page with drive supported features */

	case SPDK_NVME_LOG_FEATURE_IDS_EFFECTS:

		return -1;

	case SPDK_NVME_LOG_FEATURE_IDS_EFFECTS:

		return nvmf_admin_passthru_generic_hdlr(req, fixup_get_feature_ids_effects_log_page);

	case SPDK_NVME_LOG_COMMAND_EFFECTS_LOG:

		return nvmf_admin_passthru_generic_hdlr(req, fixup_get_cmds_and_effects_log_page);

	case SPDK_NVME_LOG_SUPPORTED_LOG_PAGES:
@@ -574,6 +622,40 @@ nvmf_custom_get_log_page_hdlr(struct spdk_nvmf_request *req) 
	}

}



static int

nvmf_custom_set_features(struct spdk_nvmf_request *req)

{

	struct spdk_nvme_cmd *cmd = spdk_nvmf_request_get_cmd(req);



	switch (cmd->cdw10_bits.set_features.fid) {

	case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER:

	case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION:

	case SPDK_NVME_FEAT_NUMBER_OF_QUEUES:

	case SPDK_NVME_FEAT_HOST_RESERVE_MASK:

	case SPDK_NVME_FEAT_HOST_RESERVE_PERSIST:

		return -1;

	default:

		return nvmf_admin_passthru_generic_hdlr(req, NULL);

	}

}



static int

nvmf_custom_get_features(struct spdk_nvmf_request *req)

{

	struct spdk_nvme_cmd *cmd = spdk_nvmf_request_get_cmd(req);



	switch (cmd->cdw10_bits.get_features.fid) {

	case SPDK_NVME_FEAT_KEEP_ALIVE_TIMER:

	case SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION:

	case SPDK_NVME_FEAT_NUMBER_OF_QUEUES:

	case SPDK_NVME_FEAT_HOST_RESERVE_MASK:

	case SPDK_NVME_FEAT_HOST_RESERVE_PERSIST:

		return -1;

	default:

		return nvmf_admin_passthru_generic_hdlr(req, NULL);

	}

}



static void

nvmf_tgt_advance_state(void)

{
@@ -603,6 +685,11 @@ nvmf_tgt_advance_state(void) 
				SPDK_NOTICELOG("Custom get log page handler enabled\n");

				spdk_nvmf_set_custom_admin_cmd_hdlr(SPDK_NVME_OPC_GET_LOG_PAGE, nvmf_custom_get_log_page_hdlr);

			}

			if (g_spdk_nvmf_tgt_conf.admin_passthru.get_set_features) {

				SPDK_NOTICELOG("Custom get/set_feature commands handlers enabled\n");

				spdk_nvmf_set_custom_admin_cmd_hdlr(SPDK_NVME_OPC_SET_FEATURES, nvmf_custom_set_features);

				spdk_nvmf_set_custom_admin_cmd_hdlr(SPDK_NVME_OPC_GET_FEATURES, nvmf_custom_get_features);

			}

			if (g_spdk_nvmf_tgt_conf.admin_passthru.vendor_specific) {

				int i;

				SPDK_NOTICELOG("Custom vendor specific commands handlers enabled\n");
@@ -725,6 +812,8 @@ nvmf_subsystem_write_config_json(struct spdk_json_write_ctx *w) 
				   g_spdk_nvmf_tgt_conf.admin_passthru.identify_ctrlr);

	spdk_json_write_named_bool(w, "get_log_page",

				   g_spdk_nvmf_tgt_conf.admin_passthru.get_log_page);

	spdk_json_write_named_bool(w, "get_set_features",

				   g_spdk_nvmf_tgt_conf.admin_passthru.get_set_features);

	spdk_json_write_named_bool(w, "vendor_specific",

				   g_spdk_nvmf_tgt_conf.admin_passthru.vendor_specific);

	spdk_json_write_object_end(w);

python/spdk/cli/nvmf.py

+3 −2
Original line number Diff line number Diff line
@@ -26,7 +26,7 @@ def add_parser(subparsers): 
            print('WARNING: -i|--passthru-identify-ctrlr is deprecated, please use -p|--passthru-admin-cmds identify_ctrlr.',

                  file=sys.stderr)

            admin_cmd_passthru.append('identify_ctrlr')

        all_admin_cmd_passthru = ('identify_ctrlr', 'get_log_page', 'vendor_specific')

        all_admin_cmd_passthru = ('identify_ctrlr', 'get_log_page', 'get_set_features', 'vendor_specific')

        if 'all' in admin_cmd_passthru:

            admin_cmd_passthru = {cmd: True for cmd in all_admin_cmd_passthru}

        else:
@@ -42,7 +42,8 @@ def add_parser(subparsers): 
    when the controller has a single namespace that is an NVMe bdev (deprecated)""", action='store_true')

    p.add_argument('-p', '--passthru-admin-cmds', help="""Comma-separated list of admin commands to be passthru

                   when the controller has a single namespace that is an NVMe bdev.

                   Available options are: all, identify_ctrlr, get_log_page, vendor_specific""", type=lambda d: d.split(','))

                   Available options are: all, identify_ctrlr, get_log_page, get_set_features, vendor_specific""",

                   type=lambda d: d.split(','))

    p.add_argument('-m', '--poll-groups-mask', help='Set cpumask for NVMf poll groups (optional)', type=str)

    p.add_argument('-d', '--discovery-filter', help="""Set discovery filter (optional), possible values are: `match_any` (default) or

         comma separated values: `transport`, `address`, `svcid`""", type=str)