From 8305e49b07e9d762c80a9dcf472bc0aa6a27f254 Mon Sep 17 00:00:00 2001 From: Evgeniy Kochetov Date: Mon, 22 Aug 2022 16:37:13 +0300 Subject: [PATCH] nvmf: Add copy command support NVMf target reports copy command support if all bdevs in the subsystem support copy IO type. Maximum copy size is reported for each namespace independently in namespace identify data. For now we support just one source range. Note, that command support in the controller is initialized once on controller create. If another namespace which doesn't support copy command is added to the subsystem later, it will not be reflected in the controller data structure and will not be communicated to the initiator. Attempt to execute copy command on such namespace will fail. This issue is not specific to copy command and applies also to write zeroes and unmap (dataset management) commands. Signed-off-by: Evgeniy Kochetov Change-Id: I5f06564eb43d66d2852bf7eeda8b17830c53c9bc Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/14350 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: Aleksey Marchuk --- include/spdk/nvme_spec.h | 34 +++++++- lib/nvmf/Makefile | 2 +- lib/nvmf/ctrlr.c | 4 + lib/nvmf/ctrlr_bdev.c | 82 +++++++++++++++++++ lib/nvmf/nvmf_internal.h | 3 + test/make/check_so_deps.sh | 2 + test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c | 11 +++ .../lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c | 69 ++++++++++++++++ test/unit/lib/nvmf/tcp.c/tcp_ut.c | 11 +++ 9 files changed, 215 insertions(+), 3 deletions(-) diff --git a/include/spdk/nvme_spec.h b/include/spdk/nvme_spec.h index 55d7eb53a..9af3de4eb 100644 --- a/include/spdk/nvme_spec.h +++ b/include/spdk/nvme_spec.h @@ -1227,6 +1227,32 @@ union spdk_nvme_cmd_cdw11 { }; SPDK_STATIC_ASSERT(sizeof(union spdk_nvme_cmd_cdw11) == 4, "Incorrect size"); +union spdk_nvme_cmd_cdw12 { + uint32_t raw; + + struct { + /* Number of Ranges */ + uint32_t nr : 8; + /* Desciptor Format */ + uint32_t df : 4; + /* Protection Information Field Read */ + uint32_t prinfor : 4; + uint32_t reserved : 4; + /* Directive Type */ + uint32_t dtype : 4; + /* Storage Tag Check Write */ + uint32_t stcw : 1; + uint32_t reserved2 : 1; + /* Protection Information Field Write */ + uint32_t prinfow : 4; + /* Force Unit Access */ + uint32_t fua : 1; + /* Limited Retry */ + uint32_t lr : 1; + } copy; +}; +SPDK_STATIC_ASSERT(sizeof(union spdk_nvme_cmd_cdw12) == 4, "Incorrect size"); + struct spdk_nvme_cmd { /* dword 0 */ uint16_t opc : 8; /* opcode */ @@ -1265,8 +1291,12 @@ struct spdk_nvme_cmd { uint32_t cdw11; union spdk_nvme_cmd_cdw11 cdw11_bits; }; - /* dword 12-15 */ - uint32_t cdw12; /* command-specific */ + /* command-specific */ + union { + uint32_t cdw12; + union spdk_nvme_cmd_cdw12 cdw12_bits; + }; + /* dword 13-15 */ uint32_t cdw13; /* command-specific */ uint32_t cdw14; /* command-specific */ uint32_t cdw15; /* command-specific */ diff --git a/lib/nvmf/Makefile b/lib/nvmf/Makefile index b366e3289..97733da44 100644 --- a/lib/nvmf/Makefile +++ b/lib/nvmf/Makefile @@ -6,7 +6,7 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..) include $(SPDK_ROOT_DIR)/mk/spdk.common.mk -SO_VER := 13 +SO_VER := 14 SO_MINOR := 0 C_SRCS = ctrlr.c ctrlr_discovery.c ctrlr_bdev.c \ diff --git a/lib/nvmf/ctrlr.c b/lib/nvmf/ctrlr.c index a745cf84e..e28354e99 100644 --- a/lib/nvmf/ctrlr.c +++ b/lib/nvmf/ctrlr.c @@ -2740,6 +2740,8 @@ spdk_nvmf_ctrlr_identify_ctrlr(struct spdk_nvmf_ctrlr *ctrlr, struct spdk_nvme_c cdata->oncs.dsm = nvmf_ctrlr_dsm_supported(ctrlr); cdata->oncs.write_zeroes = nvmf_ctrlr_write_zeroes_supported(ctrlr); cdata->oncs.reservations = ctrlr->cdata.oncs.reservations; + cdata->oncs.copy = nvmf_ctrlr_copy_supported(ctrlr); + cdata->ocfs.copy_format0 = cdata->oncs.copy; if (subsystem->flags.ana_reporting) { /* Asymmetric Namespace Access Reporting is supported. */ cdata->cmic.ana_reporting = 1; @@ -4071,6 +4073,8 @@ nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req) case SPDK_NVME_OPC_RESERVATION_REPORT: spdk_thread_send_msg(ctrlr->subsys->thread, nvmf_ns_reservation_request, req); return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS; + case SPDK_NVME_OPC_COPY: + return nvmf_bdev_ctrlr_copy_cmd(bdev, desc, ch, req); default: return nvmf_bdev_ctrlr_nvme_passthru_io(bdev, desc, ch, req); } diff --git a/lib/nvmf/ctrlr_bdev.c b/lib/nvmf/ctrlr_bdev.c index dbfe019ea..aa26f7f32 100644 --- a/lib/nvmf/ctrlr_bdev.c +++ b/lib/nvmf/ctrlr_bdev.c @@ -60,6 +60,12 @@ nvmf_ctrlr_write_zeroes_supported(struct spdk_nvmf_ctrlr *ctrlr) return nvmf_subsystem_bdev_io_type_supported(ctrlr->subsys, SPDK_BDEV_IO_TYPE_WRITE_ZEROES); } +bool +nvmf_ctrlr_copy_supported(struct spdk_nvmf_ctrlr *ctrlr) +{ + return nvmf_subsystem_bdev_io_type_supported(ctrlr->subsys, SPDK_BDEV_IO_TYPE_COPY); +} + static void nvmf_bdev_ctrlr_complete_cmd(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) @@ -114,6 +120,7 @@ nvmf_bdev_ctrlr_identify_ns(struct spdk_nvmf_ns *ns, struct spdk_nvme_ns_data *n struct spdk_bdev *bdev = ns->bdev; uint64_t num_blocks; uint32_t phys_blocklen; + uint32_t max_copy; num_blocks = spdk_bdev_get_num_blocks(bdev); @@ -170,6 +177,21 @@ nvmf_bdev_ctrlr_identify_ns(struct spdk_nvmf_ns *ns, struct spdk_nvme_ns_data *n SPDK_STATIC_ASSERT(sizeof(nsdata->eui64) == sizeof(ns->opts.eui64), "size mismatch"); memcpy(&nsdata->eui64, ns->opts.eui64, sizeof(nsdata->eui64)); + + if (spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_COPY)) { + max_copy = spdk_bdev_get_max_copy(bdev); + if (max_copy == 0 || max_copy > UINT16_MAX) { + /* Zero means copy size is unlimited */ + nsdata->mcl = UINT16_MAX; + nsdata->mssrl = UINT16_MAX; + } else { + nsdata->mcl = max_copy; + nsdata->mssrl = max_copy; + } + + /* For now we support just one source range */ + nsdata->msrc = 0; + } } static void @@ -636,6 +658,66 @@ nvmf_bdev_ctrlr_dsm_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; } +int +nvmf_bdev_ctrlr_copy_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, + struct spdk_io_channel *ch, struct spdk_nvmf_request *req) +{ + struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; + struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl; + uint64_t sdlba = ((uint64_t)cmd->cdw11 << 32) + cmd->cdw10; + struct spdk_nvme_scc_source_range *range; + int rc; + + SPDK_DEBUGLOG(nvmf, "Copy command: SDLBA %lu, NR %u, desc format %u, PRINFOR %u, " + "DTYPE %u, STCW %u, PRINFOW %u, FUA %u, LR %u\n", + sdlba, + cmd->cdw12_bits.copy.nr, + cmd->cdw12_bits.copy.df, + cmd->cdw12_bits.copy.prinfor, + cmd->cdw12_bits.copy.dtype, + cmd->cdw12_bits.copy.stcw, + cmd->cdw12_bits.copy.prinfow, + cmd->cdw12_bits.copy.fua, + cmd->cdw12_bits.copy.lr); + assert(req->length == (cmd->cdw12_bits.copy.nr + 1) * sizeof(struct spdk_nvme_scc_source_range)); + + if (!spdk_bdev_io_type_supported(bdev, SPDK_BDEV_IO_TYPE_COPY)) { + SPDK_NOTICELOG("Copy command not supported by bdev\n"); + response->status.sct = SPDK_NVME_SCT_GENERIC; + response->status.sc = SPDK_NVME_SC_INVALID_OPCODE; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + /* We support only one source range */ + if (cmd->cdw12_bits.copy.nr > 0) { + response->status.sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; + response->status.sc = SPDK_NVME_SC_CMD_SIZE_LIMIT_SIZE_EXCEEDED; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + if (cmd->cdw12_bits.copy.df != 0) { + response->status.sct = SPDK_NVME_SCT_GENERIC; + response->status.sc = SPDK_NVME_SC_INVALID_FIELD; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + range = req->data; + rc = spdk_bdev_copy_blocks(desc, ch, sdlba, range->slba, range->nlb + 1, + nvmf_bdev_ctrlr_complete_cmd, req); + if (spdk_unlikely(rc)) { + if (rc == -ENOMEM) { + nvmf_bdev_ctrl_queue_io(req, bdev, ch, nvmf_ctrlr_process_io_cmd_resubmit, req); + return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS; + } + + response->status.sct = SPDK_NVME_SCT_GENERIC; + response->status.sc = SPDK_NVME_SC_INTERNAL_DEVICE_ERROR; + return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE; + } + + return SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS; +} + int nvmf_bdev_ctrlr_nvme_passthru_io(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, struct spdk_nvmf_request *req) diff --git a/lib/nvmf/nvmf_internal.h b/lib/nvmf/nvmf_internal.h index 1a10fb318..46c9eaac8 100644 --- a/lib/nvmf/nvmf_internal.h +++ b/lib/nvmf/nvmf_internal.h @@ -323,6 +323,7 @@ int nvmf_ctrlr_process_admin_cmd(struct spdk_nvmf_request *req); int nvmf_ctrlr_process_io_cmd(struct spdk_nvmf_request *req); bool nvmf_ctrlr_dsm_supported(struct spdk_nvmf_ctrlr *ctrlr); bool nvmf_ctrlr_write_zeroes_supported(struct spdk_nvmf_ctrlr *ctrlr); +bool nvmf_ctrlr_copy_supported(struct spdk_nvmf_ctrlr *ctrlr); void nvmf_ctrlr_ns_changed(struct spdk_nvmf_ctrlr *ctrlr, uint32_t nsid); bool nvmf_ctrlr_use_zcopy(struct spdk_nvmf_request *req); @@ -342,6 +343,8 @@ int nvmf_bdev_ctrlr_flush_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *des struct spdk_io_channel *ch, struct spdk_nvmf_request *req); int nvmf_bdev_ctrlr_dsm_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, struct spdk_nvmf_request *req); +int nvmf_bdev_ctrlr_copy_cmd(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, + struct spdk_io_channel *ch, struct spdk_nvmf_request *req); int nvmf_bdev_ctrlr_nvme_passthru_io(struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, struct spdk_nvmf_request *req); bool nvmf_bdev_ctrlr_get_dif_ctx(struct spdk_bdev *bdev, struct spdk_nvme_cmd *cmd, diff --git a/test/make/check_so_deps.sh b/test/make/check_so_deps.sh index e87097a25..fd63b819a 100755 --- a/test/make/check_so_deps.sh +++ b/test/make/check_so_deps.sh @@ -95,6 +95,8 @@ function confirm_abi_deps() { name = spdk_nvme_cdata_oacs [suppress_type] name = spdk_nvme_cdata_nvmf_specific +[suppress_type] + name = spdk_nvme_cmd [suppress_type] name = spdk_bs_opts [suppress_type] diff --git a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c index 0edfaa1e8..a01fcc70a 100644 --- a/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c +++ b/test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c @@ -74,6 +74,11 @@ DEFINE_STUB(nvmf_ctrlr_write_zeroes_supported, (struct spdk_nvmf_ctrlr *ctrlr), false); +DEFINE_STUB(nvmf_ctrlr_copy_supported, + bool, + (struct spdk_nvmf_ctrlr *ctrlr), + false); + DEFINE_STUB_V(nvmf_get_discovery_log_page, (struct spdk_nvmf_tgt *tgt, const char *hostnqn, struct iovec *iov, uint32_t iovcnt, uint64_t offset, uint32_t length, struct spdk_nvme_transport_id *cmd_src_trid)); @@ -136,6 +141,12 @@ DEFINE_STUB(nvmf_bdev_ctrlr_dsm_cmd, struct spdk_nvmf_request *req), 0); +DEFINE_STUB(nvmf_bdev_ctrlr_copy_cmd, + int, + (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, + struct spdk_nvmf_request *req), + 0); + DEFINE_STUB(nvmf_bdev_ctrlr_nvme_passthru_io, int, (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, diff --git a/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c b/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c index 7bdc84f19..c0bfe1cbf 100644 --- a/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c +++ b/test/unit/lib/nvmf/ctrlr_bdev.c/ctrlr_bdev_ut.c @@ -205,6 +205,14 @@ DEFINE_STUB(spdk_bdev_zcopy_end, int, spdk_bdev_io_completion_cb cb, void *cb_arg), 0); +DEFINE_STUB(spdk_bdev_copy_blocks, int, + (struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, + uint64_t dst_offset_blocks, uint64_t src_offset_blocks, uint64_t num_blocks, + spdk_bdev_io_completion_cb cb, void *cb_arg), + 0); + +DEFINE_STUB(spdk_bdev_get_max_copy, uint32_t, (const struct spdk_bdev *bdev), 0); + struct spdk_nvmf_ns * spdk_nvmf_subsystem_get_ns(struct spdk_nvmf_subsystem *subsystem, uint32_t nsid) { @@ -620,6 +628,7 @@ test_nvmf_bdev_ctrlr_cmd(void) struct spdk_nvmf_qpair qpair = {}; union nvmf_h2c_msg cmd = {}; union nvmf_c2h_msg rsp = {}; + struct spdk_nvme_scc_source_range range = {}; req.cmd = &cmd; req.rsp = &rsp; @@ -711,6 +720,66 @@ test_nvmf_bdev_ctrlr_cmd(void) CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC); CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_INTERNAL_DEVICE_ERROR); + + /* Copy blocks status asynchronous */ + MOCK_SET(spdk_bdev_io_type_supported, true); + cmd.nvme_cmd.cdw10 = 1024; + cmd.nvme_cmd.cdw11 = 0; + cmd.nvme_cmd.cdw12 = 0; + cmd.nvme_cmd.cdw12_bits.copy.nr = 0; + req.length = 32; + range.slba = 512; + range.nlb = 511; + req.data = ⦥ + rc = nvmf_bdev_ctrlr_copy_cmd(&bdev, NULL, &ch, &req); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_ASYNCHRONOUS); + + /* Copy command not supported */ + MOCK_SET(spdk_bdev_io_type_supported, false); + memset(&rsp, 0, sizeof(rsp)); + + rc = nvmf_bdev_ctrlr_copy_cmd(&bdev, NULL, &ch, &req); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); + CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC); + CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_INVALID_OPCODE); + + MOCK_SET(spdk_bdev_io_type_supported, true); + + /* Unsupported number of source ranges */ + cmd.nvme_cmd.cdw12_bits.copy.nr = 1; + req.length = 64; + memset(&rsp, 0, sizeof(rsp)); + + rc = nvmf_bdev_ctrlr_copy_cmd(&bdev, NULL, &ch, &req); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); + CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_COMMAND_SPECIFIC); + CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_CMD_SIZE_LIMIT_SIZE_EXCEEDED); + + cmd.nvme_cmd.cdw12_bits.copy.nr = 0; + req.length = 32; + + /* Unsupported source range descriptor format */ + cmd.nvme_cmd.cdw12_bits.copy.df = 1; + memset(&rsp, 0, sizeof(rsp)); + + rc = nvmf_bdev_ctrlr_copy_cmd(&bdev, NULL, &ch, &req); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); + CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC); + CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_INVALID_FIELD); + + cmd.nvme_cmd.cdw12_bits.copy.df = 0; + + /* Bdev copy command failed */ + MOCK_SET(spdk_bdev_copy_blocks, -1); + memset(&rsp, 0, sizeof(rsp)); + + rc = nvmf_bdev_ctrlr_copy_cmd(&bdev, NULL, &ch, &req); + CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE); + CU_ASSERT(rsp.nvme_cpl.status.sct == SPDK_NVME_SCT_GENERIC); + CU_ASSERT(rsp.nvme_cpl.status.sc == SPDK_NVME_SC_INTERNAL_DEVICE_ERROR); + + MOCK_CLEAR(spdk_bdev_copy_blocks); + MOCK_CLEAR(spdk_bdev_io_type_supported); } static void diff --git a/test/unit/lib/nvmf/tcp.c/tcp_ut.c b/test/unit/lib/nvmf/tcp.c/tcp_ut.c index 9a4ade7c2..9a17e8084 100644 --- a/test/unit/lib/nvmf/tcp.c/tcp_ut.c +++ b/test/unit/lib/nvmf/tcp.c/tcp_ut.c @@ -95,6 +95,11 @@ DEFINE_STUB(nvmf_ctrlr_write_zeroes_supported, (struct spdk_nvmf_ctrlr *ctrlr), false); +DEFINE_STUB(nvmf_ctrlr_copy_supported, + bool, + (struct spdk_nvmf_ctrlr *ctrlr), + false); + DEFINE_STUB(nvmf_bdev_ctrlr_read_cmd, int, (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, @@ -137,6 +142,12 @@ DEFINE_STUB(nvmf_bdev_ctrlr_dsm_cmd, struct spdk_nvmf_request *req), 0); +DEFINE_STUB(nvmf_bdev_ctrlr_copy_cmd, + int, + (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, + struct spdk_nvmf_request *req), + 0); + DEFINE_STUB(nvmf_bdev_ctrlr_nvme_passthru_io, int, (struct spdk_bdev *bdev, struct spdk_bdev_desc *desc, struct spdk_io_channel *ch,