From 7c59dace7e8e8cc75815f11882805368e5d2a807 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Fri, 24 Apr 2020 13:39:13 +0200 Subject: scsi: target: iscsi: Remove the iscsi_data_count structure This patch removes the iscsi_data_count structure and the iscsit_do_rx_data() function because they are used only by rx_data() Link: https://lore.kernel.org/r/20200424113913.17237-1-mlombard@redhat.com Reviewed-by: Mike Christie Signed-off-by: Maurizio Lombardi Signed-off-by: Martin K. Petersen --- drivers/target/iscsi/iscsi_target_util.c | 30 ++++++------------------------ 1 file changed, 6 insertions(+), 24 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c index 89183b3b178f..45ba07c6ec27 100644 --- a/drivers/target/iscsi/iscsi_target_util.c +++ b/drivers/target/iscsi/iscsi_target_util.c @@ -1228,18 +1228,20 @@ void iscsit_print_session_params(struct iscsi_session *sess) iscsi_dump_sess_ops(sess->sess_ops); } -static int iscsit_do_rx_data( +int rx_data( struct iscsi_conn *conn, - struct iscsi_data_count *count) + struct kvec *iov, + int iov_count, + int data) { - int data = count->data_length, rx_loop = 0, total_rx = 0; + int rx_loop = 0, total_rx = 0; struct msghdr msg; if (!conn || !conn->sock || !conn->conn_ops) return -1; memset(&msg, 0, sizeof(struct msghdr)); - iov_iter_kvec(&msg.msg_iter, READ, count->iov, count->iov_count, data); + iov_iter_kvec(&msg.msg_iter, READ, iov, iov_count, data); while (msg_data_left(&msg)) { rx_loop = sock_recvmsg(conn->sock, &msg, MSG_WAITALL); @@ -1256,26 +1258,6 @@ static int iscsit_do_rx_data( return total_rx; } -int rx_data( - struct iscsi_conn *conn, - struct kvec *iov, - int iov_count, - int data) -{ - struct iscsi_data_count c; - - if (!conn || !conn->sock || !conn->conn_ops) - return -1; - - memset(&c, 0, sizeof(struct iscsi_data_count)); - c.iov = iov; - c.iov_count = iov_count; - c.data_length = data; - c.type = ISCSI_RX_DATA; - - return iscsit_do_rx_data(conn, &c); -} - int tx_data( struct iscsi_conn *conn, struct kvec *iov, -- cgit v1.2.3 From 9299941716a3082e5335fe751ce433cdb62b26d0 Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Mon, 27 Apr 2020 17:08:20 +0200 Subject: scsi: target: Add missing emulate_pr attribute to passthrough backends In commit b49d6f788530 ("scsi: target: add emulate_pr backstore attr to toggle PR support") the new attribute emulate_pr was added. passthrough_parse_cdb() uses the attribute's value to distinguish whether reservation commands should be rejected or not. But the new attribute was not added to passthrough_attrib_attrs, so in pscsi and tcmu - the users of passthrough_parse_cdb() - the attribute is not available to change parser's behavior. Link: https://lore.kernel.org/r/20200427150823.15350-2-bstroesser@ts.fujitsu.com Reviewed-by: Mike Christie Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index ff82b21fdcce..d8589bb39c53 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1203,6 +1203,7 @@ struct configfs_attribute *passthrough_attrib_attrs[] = { &attr_hw_block_size, &attr_hw_max_sectors, &attr_hw_queue_depth, + &attr_emulate_pr, &attr_alua_support, &attr_pgr_support, NULL, -- cgit v1.2.3 From 4703b6252b338eb312ba61c5129d872cfe58759f Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Mon, 27 Apr 2020 17:08:21 +0200 Subject: scsi: target: tcmu: Add attributes enforce_pr_isids and force_pr_aptpl tcmu has not set TRANSPORT_FLAG_PASSTHROUGH_PGR. Therefore the in-core pr emulation is active by default, but there are some attributes for configuration missing. Add them. Link: https://lore.kernel.org/r/20200427150823.15350-3-bstroesser@ts.fujitsu.com Reviewed-by: Mike Christie Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 11 +++++++++++ drivers/target/target_core_user.c | 19 +++++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index d8589bb39c53..efa92d836946 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1210,6 +1210,17 @@ struct configfs_attribute *passthrough_attrib_attrs[] = { }; EXPORT_SYMBOL(passthrough_attrib_attrs); +/* + * pr related dev_attrib attributes for devices passing through CDBs, + * but allowing in core pr emulation. + */ +struct configfs_attribute *passthrough_pr_attrib_attrs[] = { + &attr_enforce_pr_isids, + &attr_force_pr_aptpl, + NULL, +}; +EXPORT_SYMBOL(passthrough_pr_attrib_attrs); + TB_CIT_SETUP_DRV(dev_attrib, NULL, NULL); TB_CIT_SETUP_DRV(dev_action, NULL, NULL); diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 0b9dfa6b17bc..4b190d600d2f 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -2752,12 +2752,12 @@ static int __init tcmu_module_init(void) goto out_unreg_device; } - for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) { + for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) len += sizeof(struct configfs_attribute *); - } - for (i = 0; tcmu_attrib_attrs[i] != NULL; i++) { + for (i = 0; passthrough_pr_attrib_attrs[i] != NULL; i++) + len += sizeof(struct configfs_attribute *); + for (i = 0; tcmu_attrib_attrs[i] != NULL; i++) len += sizeof(struct configfs_attribute *); - } len += sizeof(struct configfs_attribute *); tcmu_attrs = kzalloc(len, GFP_KERNEL); @@ -2766,13 +2766,12 @@ static int __init tcmu_module_init(void) goto out_unreg_genl; } - for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) { + for (i = 0; passthrough_attrib_attrs[i] != NULL; i++) tcmu_attrs[i] = passthrough_attrib_attrs[i]; - } - for (k = 0; tcmu_attrib_attrs[k] != NULL; k++) { - tcmu_attrs[i] = tcmu_attrib_attrs[k]; - i++; - } + for (k = 0; passthrough_pr_attrib_attrs[k] != NULL; k++) + tcmu_attrs[i++] = passthrough_pr_attrib_attrs[k]; + for (k = 0; tcmu_attrib_attrs[k] != NULL; k++) + tcmu_attrs[i++] = tcmu_attrib_attrs[k]; tcmu_ops.tb_dev_attrib_attrs = tcmu_attrs; ret = transport_backend_register(&tcmu_ops); -- cgit v1.2.3 From 69088a049488171bc05394799b048c8536e7dbab Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Mon, 27 Apr 2020 17:08:22 +0200 Subject: scsi: target: Make transport_flags per device pgr_support and alua_support device attributes show the inverted value of the transport_flags: * TRANSPORT_FLAG_PASSTHROUGH_PGR * TRANSPORT_FLAG_PASSTHROUGH_ALUA These attributes are per device, while the flags are per backend. Rename the transport_flags in backend/transport to transport_flags_default and use this value to initialize the new transport_flags field in the se_device structure. Now data and attribute both are per se_device. Link: https://lore.kernel.org/r/20200427150823.15350-4-bstroesser@ts.fujitsu.com Reviewed-by: Mike Christie Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/target_core_alua.c | 10 +++++----- drivers/target/target_core_configfs.c | 14 +++++++------- drivers/target/target_core_device.c | 3 ++- drivers/target/target_core_pr.c | 2 +- drivers/target/target_core_pscsi.c | 6 +++--- drivers/target/target_core_tpg.c | 3 +-- drivers/target/target_core_transport.c | 6 +++--- drivers/target/target_core_user.c | 2 +- 8 files changed, 23 insertions(+), 23 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 385e4cf9cfa6..6b72afee2f8b 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -677,7 +677,7 @@ target_alua_state_check(struct se_cmd *cmd) if (dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE) return 0; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) return 0; /* @@ -1090,7 +1090,7 @@ int core_alua_do_port_transition( struct t10_alua_tg_pt_gp *tg_pt_gp; int primary, valid_states, rc = 0; - if (l_dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) + if (l_dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) return -ENODEV; valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states; @@ -1920,7 +1920,7 @@ ssize_t core_alua_store_tg_pt_gp_info( unsigned char buf[TG_PT_GROUP_NAME_BUF]; int move = 0; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA || + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA || (dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)) return -ENODEV; @@ -2177,7 +2177,7 @@ ssize_t core_alua_store_offline_bit( unsigned long tmp; int ret; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA || + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA || (dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)) return -ENODEV; @@ -2263,7 +2263,7 @@ ssize_t core_alua_store_secondary_write_metadata( int core_setup_alua(struct se_device *dev) { - if (!(dev->transport->transport_flags & + if (!(dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) && !(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)) { struct t10_alua_lu_gp_member *lu_gp_mem; diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index efa92d836946..279989e32e64 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1099,7 +1099,7 @@ static ssize_t block_size_store(struct config_item *item, static ssize_t alua_support_show(struct config_item *item, char *page) { struct se_dev_attrib *da = to_attrib(item); - u8 flags = da->da_dev->transport->transport_flags; + u8 flags = da->da_dev->transport_flags; return snprintf(page, PAGE_SIZE, "%d\n", flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA ? 0 : 1); @@ -1108,7 +1108,7 @@ static ssize_t alua_support_show(struct config_item *item, char *page) static ssize_t pgr_support_show(struct config_item *item, char *page) { struct se_dev_attrib *da = to_attrib(item); - u8 flags = da->da_dev->transport->transport_flags; + u8 flags = da->da_dev->transport_flags; return snprintf(page, PAGE_SIZE, "%d\n", flags & TRANSPORT_FLAG_PASSTHROUGH_PGR ? 0 : 1); @@ -1654,7 +1654,7 @@ static ssize_t target_pr_res_holder_show(struct config_item *item, char *page) if (!dev->dev_attrib.emulate_pr) return sprintf(page, "SPC_RESERVATIONS_DISABLED\n"); - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) return sprintf(page, "Passthrough\n"); spin_lock(&dev->dev_reservation_lock); @@ -1796,7 +1796,7 @@ static ssize_t target_pr_res_type_show(struct config_item *item, char *page) if (!dev->dev_attrib.emulate_pr) return sprintf(page, "SPC_RESERVATIONS_DISABLED\n"); - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) return sprintf(page, "SPC_PASSTHROUGH\n"); if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS) return sprintf(page, "SPC2_RESERVATIONS\n"); @@ -1810,7 +1810,7 @@ static ssize_t target_pr_res_aptpl_active_show(struct config_item *item, struct se_device *dev = pr_to_dev(item); if (!dev->dev_attrib.emulate_pr || - (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) + (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) return 0; return sprintf(page, "APTPL Bit Status: %s\n", @@ -1823,7 +1823,7 @@ static ssize_t target_pr_res_aptpl_metadata_show(struct config_item *item, struct se_device *dev = pr_to_dev(item); if (!dev->dev_attrib.emulate_pr || - (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) + (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) return 0; return sprintf(page, "Ready to process PR APTPL metadata..\n"); @@ -1870,7 +1870,7 @@ static ssize_t target_pr_res_aptpl_metadata_store(struct config_item *item, u8 type = 0; if (!dev->dev_attrib.emulate_pr || - (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) + (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) return count; if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS) return count; diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 4cee1138284b..d5800cf22df7 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -732,6 +732,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name) dev->se_hba = hba; dev->transport = hba->backend->ops; + dev->transport_flags = dev->transport->transport_flags_default; dev->prot_length = sizeof(struct t10_pi_tuple); dev->hba_index = hba->hba_index; @@ -1100,7 +1101,7 @@ passthrough_parse_cdb(struct se_cmd *cmd, * emulate the response, since tcmu does not have the information * required to process these commands. */ - if (!(dev->transport->transport_flags & + if (!(dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR)) { if (cdb[0] == PERSISTENT_RESERVE_IN) { cmd->execute_cmd = target_scsi3_emulate_pr_in; diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 5e931690e697..91e41cc55704 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -4086,7 +4086,7 @@ target_check_reservation(struct se_cmd *cmd) return 0; if (!dev->dev_attrib.emulate_pr) return 0; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_PGR) return 0; spin_lock(&dev->dev_reservation_lock); diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c index c9d92b3e777d..4e37fa9b409d 100644 --- a/drivers/target/target_core_pscsi.c +++ b/drivers/target/target_core_pscsi.c @@ -1070,9 +1070,9 @@ static void pscsi_req_done(struct request *req, blk_status_t status) static const struct target_backend_ops pscsi_ops = { .name = "pscsi", .owner = THIS_MODULE, - .transport_flags = TRANSPORT_FLAG_PASSTHROUGH | - TRANSPORT_FLAG_PASSTHROUGH_ALUA | - TRANSPORT_FLAG_PASSTHROUGH_PGR, + .transport_flags_default = TRANSPORT_FLAG_PASSTHROUGH | + TRANSPORT_FLAG_PASSTHROUGH_ALUA | + TRANSPORT_FLAG_PASSTHROUGH_PGR, .attach_hba = pscsi_attach_hba, .detach_hba = pscsi_detach_hba, .pmode_enable_hba = pscsi_pmode_enable_hba, diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index d24e0a3ba3ff..62aa5fa63ac0 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -582,8 +582,7 @@ int core_tpg_add_lun( if (ret) goto out_kill_ref; - if (!(dev->transport->transport_flags & - TRANSPORT_FLAG_PASSTHROUGH_ALUA) && + if (!(dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA) && !(dev->se_hba->hba_flags & HBA_FLAGS_INTERNAL_USE)) target_attach_tg_pt_gp(lun, dev->t10_alua.default_tg_pt_gp); diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index 594b724bbf79..e6b448f43071 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1397,7 +1397,7 @@ transport_check_alloc_task_attr(struct se_cmd *cmd) * Check if SAM Task Attribute emulation is enabled for this * struct se_device storage object */ - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) return 0; if (cmd->sam_task_attr == TCM_ACA_TAG) { @@ -2012,7 +2012,7 @@ static bool target_handle_task_attr(struct se_cmd *cmd) { struct se_device *dev = cmd->se_dev; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) return false; cmd->se_cmd_flags |= SCF_TASK_ATTR_SET; @@ -2126,7 +2126,7 @@ static void transport_complete_task_attr(struct se_cmd *cmd) { struct se_device *dev = cmd->se_dev; - if (dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) + if (dev->transport_flags & TRANSPORT_FLAG_PASSTHROUGH) return; if (!(cmd->se_cmd_flags & SCF_TASK_ATTR_SET)) diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 4b190d600d2f..cb388b28f27e 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -2616,7 +2616,7 @@ static struct configfs_attribute *tcmu_action_attrs[] = { static struct target_backend_ops tcmu_ops = { .name = "user", .owner = THIS_MODULE, - .transport_flags = TRANSPORT_FLAG_PASSTHROUGH, + .transport_flags_default = TRANSPORT_FLAG_PASSTHROUGH, .attach_hba = tcmu_attach_hba, .detach_hba = tcmu_detach_hba, .alloc_device = tcmu_alloc_device, -- cgit v1.2.3 From 356ba2a8bc8d9f9bd2ee969df0e07b285aebb559 Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Mon, 27 Apr 2020 17:08:23 +0200 Subject: scsi: target: tcmu: Make pgr_support and alua_support attributes writable Currently in tcmu reservation commands are handled by core's pr implementation (default) or completely rejected (emulate_pr set to 0). We additionally want to be able to do full reservation handling in userspace. Therefore we need a way to set TRANSPORT_FLAG_PASSTHROUGH_PGR. The inverted flag is displayed by attribute pgr_support. Since we moved the flag from transport/backend to se_device in the previous commit, we now can make it changeable per device by allowing to write the attribute. The new field transport_flags_changeable in transport/backend is used to reject writing if not allowed for a backend. Regarding ALUA we also want to be able to passthrough commands to userspace in tcmu. Therefore we need TRANSPORT_FLAG_PASSTHROUGH_ALUA to be changeable, because by setting it we can switch off all ALUA checks in core. So we also set TRANSPORT_FLAG_PASSTHROUGH_ALUA in tcmu's transport_flags_changeable. Of course, ALUA and reservation handling in userspace will work only, if session/nexus information is sent to userspace along with every command. This will be object of a patch series announced by Mike Christie. Link: https://lore.kernel.org/r/20200427150823.15350-5-bstroesser@ts.fujitsu.com Reviewed-by: Mike Christie Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/target_core_configfs.c | 56 +++++++++++++++++++++++++++++++++-- drivers/target/target_core_user.c | 2 ++ 2 files changed, 56 insertions(+), 2 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index 279989e32e64..f04352285155 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -1105,6 +1105,32 @@ static ssize_t alua_support_show(struct config_item *item, char *page) flags & TRANSPORT_FLAG_PASSTHROUGH_ALUA ? 0 : 1); } +static ssize_t alua_support_store(struct config_item *item, + const char *page, size_t count) +{ + struct se_dev_attrib *da = to_attrib(item); + struct se_device *dev = da->da_dev; + bool flag; + int ret; + + if (!(dev->transport->transport_flags_changeable & + TRANSPORT_FLAG_PASSTHROUGH_ALUA)) { + pr_err("dev[%p]: Unable to change SE Device alua_support:" + " alua_support has fixed value\n", dev); + return -EINVAL; + } + + ret = strtobool(page, &flag); + if (ret < 0) + return ret; + + if (flag) + dev->transport_flags &= ~TRANSPORT_FLAG_PASSTHROUGH_ALUA; + else + dev->transport_flags |= TRANSPORT_FLAG_PASSTHROUGH_ALUA; + return count; +} + static ssize_t pgr_support_show(struct config_item *item, char *page) { struct se_dev_attrib *da = to_attrib(item); @@ -1114,6 +1140,32 @@ static ssize_t pgr_support_show(struct config_item *item, char *page) flags & TRANSPORT_FLAG_PASSTHROUGH_PGR ? 0 : 1); } +static ssize_t pgr_support_store(struct config_item *item, + const char *page, size_t count) +{ + struct se_dev_attrib *da = to_attrib(item); + struct se_device *dev = da->da_dev; + bool flag; + int ret; + + if (!(dev->transport->transport_flags_changeable & + TRANSPORT_FLAG_PASSTHROUGH_PGR)) { + pr_err("dev[%p]: Unable to change SE Device pgr_support:" + " pgr_support has fixed value\n", dev); + return -EINVAL; + } + + ret = strtobool(page, &flag); + if (ret < 0) + return ret; + + if (flag) + dev->transport_flags &= ~TRANSPORT_FLAG_PASSTHROUGH_PGR; + else + dev->transport_flags |= TRANSPORT_FLAG_PASSTHROUGH_PGR; + return count; +} + CONFIGFS_ATTR(, emulate_model_alias); CONFIGFS_ATTR(, emulate_dpo); CONFIGFS_ATTR(, emulate_fua_write); @@ -1146,8 +1198,8 @@ CONFIGFS_ATTR(, unmap_granularity); CONFIGFS_ATTR(, unmap_granularity_alignment); CONFIGFS_ATTR(, unmap_zeroes_data); CONFIGFS_ATTR(, max_write_same_len); -CONFIGFS_ATTR_RO(, alua_support); -CONFIGFS_ATTR_RO(, pgr_support); +CONFIGFS_ATTR(, alua_support); +CONFIGFS_ATTR(, pgr_support); /* * dev_attrib attributes for devices using the target core SBC/SPC diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index cb388b28f27e..72ffd359fc4d 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -2617,6 +2617,8 @@ static struct target_backend_ops tcmu_ops = { .name = "user", .owner = THIS_MODULE, .transport_flags_default = TRANSPORT_FLAG_PASSTHROUGH, + .transport_flags_changeable = TRANSPORT_FLAG_PASSTHROUGH_PGR | + TRANSPORT_FLAG_PASSTHROUGH_ALUA, .attach_hba = tcmu_attach_hba, .detach_hba = tcmu_detach_hba, .alloc_device = tcmu_alloc_device, -- cgit v1.2.3 From c68a56736c129f5dd1632856956f9c3e04bae200 Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Tue, 28 Apr 2020 20:26:17 +0200 Subject: scsi: target: loopback: Fix READ with data and sensebytes We use tcm_loop with tape emulations running on tcmu. In case application reads a short tape block with a longer READ, or a long tape block with a short READ, according to SCC spec data has to be tranferred _and_ sensebytes with ILI set and information field containing the residual count. Similar problem also exists when using fixed block size in READ. Up to now tcm_loop is not prepared to handle sensebytes if input data is provided, as in tcm_loop_queue_data_in() it only sets SAM_STAT_GOOD and, if necessary, the residual count. To fix the bug, the same handling for sensebytes as present in tcm_loop_queue_status() must be done in tcm_loop_queue_data_in() also. After adding this handling, the two function now are nearly identical, so I created a single function with two wrappers. Link: https://lore.kernel.org/r/20200428182617.32726-1-bstroesser@ts.fujitsu.com Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/loopback/tcm_loop.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index 3305b47fdf53..16d5a4e117a2 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -545,32 +545,15 @@ static int tcm_loop_write_pending(struct se_cmd *se_cmd) return 0; } -static int tcm_loop_queue_data_in(struct se_cmd *se_cmd) +static int tcm_loop_queue_data_or_status(const char *func, + struct se_cmd *se_cmd, u8 scsi_status) { struct tcm_loop_cmd *tl_cmd = container_of(se_cmd, struct tcm_loop_cmd, tl_se_cmd); struct scsi_cmnd *sc = tl_cmd->sc; pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n", - __func__, sc, sc->cmnd[0]); - - sc->result = SAM_STAT_GOOD; - set_host_byte(sc, DID_OK); - if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) || - (se_cmd->se_cmd_flags & SCF_UNDERFLOW_BIT)) - scsi_set_resid(sc, se_cmd->residual_count); - sc->scsi_done(sc); - return 0; -} - -static int tcm_loop_queue_status(struct se_cmd *se_cmd) -{ - struct tcm_loop_cmd *tl_cmd = container_of(se_cmd, - struct tcm_loop_cmd, tl_se_cmd); - struct scsi_cmnd *sc = tl_cmd->sc; - - pr_debug("%s() called for scsi_cmnd: %p cdb: 0x%02x\n", - __func__, sc, sc->cmnd[0]); + func, sc, sc->cmnd[0]); if (se_cmd->sense_buffer && ((se_cmd->se_cmd_flags & SCF_TRANSPORT_TASK_SENSE) || @@ -581,7 +564,7 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd) sc->result = SAM_STAT_CHECK_CONDITION; set_driver_byte(sc, DRIVER_SENSE); } else - sc->result = se_cmd->scsi_status; + sc->result = scsi_status; set_host_byte(sc, DID_OK); if ((se_cmd->se_cmd_flags & SCF_OVERFLOW_BIT) || @@ -591,6 +574,17 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd) return 0; } +static int tcm_loop_queue_data_in(struct se_cmd *se_cmd) +{ + return tcm_loop_queue_data_or_status(__func__, se_cmd, SAM_STAT_GOOD); +} + +static int tcm_loop_queue_status(struct se_cmd *se_cmd) +{ + return tcm_loop_queue_data_or_status(__func__, + se_cmd, se_cmd->scsi_status); +} + static void tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd) { struct tcm_loop_cmd *tl_cmd = container_of(se_cmd, -- cgit v1.2.3 From 5482d56bfedf1a2ce6450076e844676e20ab4e0a Mon Sep 17 00:00:00 2001 From: Lance Digby Date: Mon, 18 May 2020 11:02:16 +1000 Subject: scsi: target: core: Add initiatorname to NON_EXISTENT_LUN error The NON_EXISTENT_LUN error can be written without an error condition on the initiator responsible. Adding the initiatorname to this message will reduce the effort required to fix this when many initiators are supported by a target. This version ensures the initiator name is also printed on the same message in transport_lookup_tmr_lun for consistency. Link: https://lore.kernel.org/r/9b13bb2e1f52f1792cd81850ee95bf3781bb5363.1589759816.git.lance.digby@gmail.com Reviewed-by: Mike Christie Signed-off-by: Lance Digby Signed-off-by: Martin K. Petersen --- drivers/target/target_core_device.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index d5800cf22df7..46b0e1ceb77f 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -100,9 +100,10 @@ out_unlock: */ if (unpacked_lun != 0) { pr_err("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN" - " Access for 0x%08llx\n", + " Access for 0x%08llx from %s\n", se_cmd->se_tfo->fabric_name, - unpacked_lun); + unpacked_lun, + nacl->initiatorname); return TCM_NON_EXISTENT_LUN; } @@ -174,9 +175,10 @@ out_unlock: if (!se_lun) { pr_debug("TARGET_CORE[%s]: Detected NON_EXISTENT_LUN" - " Access for 0x%08llx\n", + " Access for 0x%08llx for %s\n", se_cmd->se_tfo->fabric_name, - unpacked_lun); + unpacked_lun, + nacl->initiatorname); return -ENODEV; } se_cmd->se_dev = rcu_dereference_raw(se_lun->lun_se_dev); -- cgit v1.2.3 From 61fb2482216679b9e1e797440c148bb143a5040a Mon Sep 17 00:00:00 2001 From: Bodo Stroesser Date: Mon, 18 May 2020 18:48:33 +0200 Subject: scsi: target: tcmu: Userspace must not complete queued commands When tcmu queues a new command - no matter whether in command ring or in qfull_queue - a cmd_id from IDR udev->commands is assigned to the command. If userspace sends a wrong command completion containing the cmd_id of a command on the qfull_queue, tcmu_handle_completions() finds the command in the IDR and calls tcmu_handle_completion() for it. This might do some nasty things because commands in qfull_queue do not have a valid dbi list. To fix this bug, we no longer add queued commands to the idr. Instead the cmd_id is assign when a command is written to the command ring. Due to this change I had to adapt the source code at several places where up to now an idr_for_each had been done. [mkp: fix checkpatch warnings] Link: https://lore.kernel.org/r/20200518164833.12775-1-bstroesser@ts.fujitsu.com Acked-by: Mike Christie Signed-off-by: Bodo Stroesser Signed-off-by: Martin K. Petersen --- drivers/target/target_core_user.c | 154 ++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 83 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 72ffd359fc4d..1d030d57c28c 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -882,41 +882,24 @@ static inline size_t tcmu_cmd_get_cmd_size(struct tcmu_cmd *tcmu_cmd, return command_size; } -static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo, - struct timer_list *timer) +static void tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo, + struct timer_list *timer) { - struct tcmu_dev *udev = tcmu_cmd->tcmu_dev; - int cmd_id; - - if (tcmu_cmd->cmd_id) - goto setup_timer; - - cmd_id = idr_alloc(&udev->commands, tcmu_cmd, 1, USHRT_MAX, GFP_NOWAIT); - if (cmd_id < 0) { - pr_err("tcmu: Could not allocate cmd id.\n"); - return cmd_id; - } - tcmu_cmd->cmd_id = cmd_id; - - pr_debug("allocated cmd %u for dev %s tmo %lu\n", tcmu_cmd->cmd_id, - udev->name, tmo / MSEC_PER_SEC); - -setup_timer: if (!tmo) - return 0; + return; tcmu_cmd->deadline = round_jiffies_up(jiffies + msecs_to_jiffies(tmo)); if (!timer_pending(timer)) mod_timer(timer, tcmu_cmd->deadline); - return 0; + pr_debug("Timeout set up for cmd %p, dev = %s, tmo = %lu\n", tcmu_cmd, + tcmu_cmd->tcmu_dev->name, tmo / MSEC_PER_SEC); } static int add_to_qfull_queue(struct tcmu_cmd *tcmu_cmd) { struct tcmu_dev *udev = tcmu_cmd->tcmu_dev; unsigned int tmo; - int ret; /* * For backwards compat if qfull_time_out is not set use @@ -931,13 +914,11 @@ static int add_to_qfull_queue(struct tcmu_cmd *tcmu_cmd) else tmo = TCMU_TIME_OUT; - ret = tcmu_setup_cmd_timer(tcmu_cmd, tmo, &udev->qfull_timer); - if (ret) - return ret; + tcmu_setup_cmd_timer(tcmu_cmd, tmo, &udev->qfull_timer); list_add_tail(&tcmu_cmd->queue_entry, &udev->qfull_queue); - pr_debug("adding cmd %u on dev %s to ring space wait queue\n", - tcmu_cmd->cmd_id, udev->name); + pr_debug("adding cmd %p on dev %s to ring space wait queue\n", + tcmu_cmd, udev->name); return 0; } @@ -959,7 +940,7 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err) struct tcmu_mailbox *mb; struct tcmu_cmd_entry *entry; struct iovec *iov; - int iov_cnt, ret; + int iov_cnt, cmd_id; uint32_t cmd_head; uint64_t cdb_off; bool copy_to_data_area; @@ -1060,14 +1041,21 @@ static int queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, sense_reason_t *scsi_err) } entry->req.iov_bidi_cnt = iov_cnt; - ret = tcmu_setup_cmd_timer(tcmu_cmd, udev->cmd_time_out, - &udev->cmd_timer); - if (ret) { - tcmu_cmd_free_data(tcmu_cmd, tcmu_cmd->dbi_cnt); + cmd_id = idr_alloc(&udev->commands, tcmu_cmd, 1, USHRT_MAX, GFP_NOWAIT); + if (cmd_id < 0) { + pr_err("tcmu: Could not allocate cmd id.\n"); + tcmu_cmd_free_data(tcmu_cmd, tcmu_cmd->dbi_cnt); *scsi_err = TCM_OUT_OF_RESOURCES; return -1; } + tcmu_cmd->cmd_id = cmd_id; + + pr_debug("allocated cmd id %u for cmd %p dev %s\n", tcmu_cmd->cmd_id, + tcmu_cmd, udev->name); + + tcmu_setup_cmd_timer(tcmu_cmd, udev->cmd_time_out, &udev->cmd_timer); + entry->hdr.cmd_id = tcmu_cmd->cmd_id; /* @@ -1279,50 +1267,39 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) return handled; } -static int tcmu_check_expired_cmd(int id, void *p, void *data) +static void tcmu_check_expired_ring_cmd(struct tcmu_cmd *cmd) { - struct tcmu_cmd *cmd = p; - struct tcmu_dev *udev = cmd->tcmu_dev; - u8 scsi_status; struct se_cmd *se_cmd; - bool is_running; - - if (test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)) - return 0; if (!time_after(jiffies, cmd->deadline)) - return 0; + return; - is_running = test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags); + set_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags); + list_del_init(&cmd->queue_entry); se_cmd = cmd->se_cmd; + cmd->se_cmd = NULL; - if (is_running) { - /* - * If cmd_time_out is disabled but qfull is set deadline - * will only reflect the qfull timeout. Ignore it. - */ - if (!udev->cmd_time_out) - return 0; + pr_debug("Timing out inflight cmd %u on dev %s.\n", + cmd->cmd_id, cmd->tcmu_dev->name); - set_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags); - /* - * target_complete_cmd will translate this to LUN COMM FAILURE - */ - scsi_status = SAM_STAT_CHECK_CONDITION; - list_del_init(&cmd->queue_entry); - cmd->se_cmd = NULL; - } else { - list_del_init(&cmd->queue_entry); - idr_remove(&udev->commands, id); - tcmu_free_cmd(cmd); - scsi_status = SAM_STAT_TASK_SET_FULL; - } + target_complete_cmd(se_cmd, SAM_STAT_CHECK_CONDITION); +} - pr_debug("Timing out cmd %u on dev %s that is %s.\n", - id, udev->name, is_running ? "inflight" : "queued"); +static void tcmu_check_expired_queue_cmd(struct tcmu_cmd *cmd) +{ + struct se_cmd *se_cmd; - target_complete_cmd(se_cmd, scsi_status); - return 0; + if (!time_after(jiffies, cmd->deadline)) + return; + + list_del_init(&cmd->queue_entry); + se_cmd = cmd->se_cmd; + tcmu_free_cmd(cmd); + + pr_debug("Timing out queued cmd %p on dev %s.\n", + cmd, cmd->tcmu_dev->name); + + target_complete_cmd(se_cmd, SAM_STAT_TASK_SET_FULL); } static void tcmu_device_timedout(struct tcmu_dev *udev) @@ -1407,16 +1384,15 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) return &udev->se_dev; } -static bool run_qfull_queue(struct tcmu_dev *udev, bool fail) +static void run_qfull_queue(struct tcmu_dev *udev, bool fail) { struct tcmu_cmd *tcmu_cmd, *tmp_cmd; LIST_HEAD(cmds); - bool drained = true; sense_reason_t scsi_ret; int ret; if (list_empty(&udev->qfull_queue)) - return true; + return; pr_debug("running %s's cmdr queue forcefail %d\n", udev->name, fail); @@ -1425,11 +1401,10 @@ static bool run_qfull_queue(struct tcmu_dev *udev, bool fail) list_for_each_entry_safe(tcmu_cmd, tmp_cmd, &cmds, queue_entry) { list_del_init(&tcmu_cmd->queue_entry); - pr_debug("removing cmd %u on dev %s from queue\n", - tcmu_cmd->cmd_id, udev->name); + pr_debug("removing cmd %p on dev %s from queue\n", + tcmu_cmd, udev->name); if (fail) { - idr_remove(&udev->commands, tcmu_cmd->cmd_id); /* * We were not able to even start the command, so * fail with busy to allow a retry in case runner @@ -1444,10 +1419,8 @@ static bool run_qfull_queue(struct tcmu_dev *udev, bool fail) ret = queue_cmd_ring(tcmu_cmd, &scsi_ret); if (ret < 0) { - pr_debug("cmd %u on dev %s failed with %u\n", - tcmu_cmd->cmd_id, udev->name, scsi_ret); - - idr_remove(&udev->commands, tcmu_cmd->cmd_id); + pr_debug("cmd %p on dev %s failed with %u\n", + tcmu_cmd, udev->name, scsi_ret); /* * Ignore scsi_ret for now. target_complete_cmd * drops it. @@ -1462,13 +1435,11 @@ static bool run_qfull_queue(struct tcmu_dev *udev, bool fail) * the queue */ list_splice_tail(&cmds, &udev->qfull_queue); - drained = false; break; } } tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer); - return drained; } static int tcmu_irqcontrol(struct uio_info *info, s32 irq_on) @@ -1652,6 +1623,8 @@ static void tcmu_dev_kref_release(struct kref *kref) if (tcmu_check_and_free_pending_cmd(cmd) != 0) all_expired = false; } + if (!list_empty(&udev->qfull_queue)) + all_expired = false; idr_destroy(&udev->commands); WARN_ON(!all_expired); @@ -2037,9 +2010,6 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level) mutex_lock(&udev->cmdr_lock); idr_for_each_entry(&udev->commands, cmd, i) { - if (!test_bit(TCMU_CMD_BIT_INFLIGHT, &cmd->flags)) - continue; - pr_debug("removing cmd %u on dev %s from ring (is expired %d)\n", cmd->cmd_id, udev->name, test_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags)); @@ -2076,6 +2046,8 @@ static void tcmu_reset_ring(struct tcmu_dev *udev, u8 err_level) del_timer(&udev->cmd_timer); + run_qfull_queue(udev, false); + mutex_unlock(&udev->cmdr_lock); } @@ -2699,6 +2671,7 @@ static void find_free_blocks(void) static void check_timedout_devices(void) { struct tcmu_dev *udev, *tmp_dev; + struct tcmu_cmd *cmd, *tmp_cmd; LIST_HEAD(devs); spin_lock_bh(&timed_out_udevs_lock); @@ -2709,9 +2682,24 @@ static void check_timedout_devices(void) spin_unlock_bh(&timed_out_udevs_lock); mutex_lock(&udev->cmdr_lock); - idr_for_each(&udev->commands, tcmu_check_expired_cmd, NULL); - tcmu_set_next_deadline(&udev->inflight_queue, &udev->cmd_timer); + /* + * If cmd_time_out is disabled but qfull is set deadline + * will only reflect the qfull timeout. Ignore it. + */ + if (udev->cmd_time_out) { + list_for_each_entry_safe(cmd, tmp_cmd, + &udev->inflight_queue, + queue_entry) { + tcmu_check_expired_ring_cmd(cmd); + } + tcmu_set_next_deadline(&udev->inflight_queue, + &udev->cmd_timer); + } + list_for_each_entry_safe(cmd, tmp_cmd, &udev->qfull_queue, + queue_entry) { + tcmu_check_expired_queue_cmd(cmd); + } tcmu_set_next_deadline(&udev->qfull_queue, &udev->qfull_timer); mutex_unlock(&udev->cmdr_lock); -- cgit v1.2.3 From 9d7464b18892332e35ff37f0b024429a1a9835e6 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Sat, 23 May 2020 13:11:29 +0300 Subject: scsi: target: tcmu: Fix a use after free in tcmu_check_expired_queue_cmd() The pr_debug() dereferences "cmd" after we already freed it by calling tcmu_free_cmd(cmd). The debug printk needs to be done earlier. Link: https://lore.kernel.org/r/20200523101129.GB98132@mwanda Fixes: 61fb24822166 ("scsi: target: tcmu: Userspace must not complete queued commands") Reviewed-by: Mike Christie Reviewed-by: David Disseldorp Signed-off-by: Dan Carpenter Signed-off-by: Martin K. Petersen --- drivers/target/target_core_user.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/target') diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 1d030d57c28c..0e281d30d81e 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -1292,13 +1292,13 @@ static void tcmu_check_expired_queue_cmd(struct tcmu_cmd *cmd) if (!time_after(jiffies, cmd->deadline)) return; + pr_debug("Timing out queued cmd %p on dev %s.\n", + cmd, cmd->tcmu_dev->name); + list_del_init(&cmd->queue_entry); se_cmd = cmd->se_cmd; tcmu_free_cmd(cmd); - pr_debug("Timing out queued cmd %p on dev %s.\n", - cmd, cmd->tcmu_dev->name); - target_complete_cmd(se_cmd, SAM_STAT_TASK_SET_FULL); } -- cgit v1.2.3