Message ID | 20210120102700.5514-1-d.bogdanov@yadro.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] scsi: target: core: check SR field in REPORT LUNS | expand |
Hi, On Wed, 20 Jan 2021 13:27:00 +0300, Dmitry Bogdanov wrote: > Now REPORT LUNS for software device servers always reports all luns > regardless of SELECT REPORT field. > Add handling of that field according to SPC-4: > * accept known values, > * reject unknown values. We currently advertise SPC-3 VERSION compliance via standard INQUIRY data, so I think we should either support SPC-3 SELECT REPORT values or bump the VERSION field (SPC-4 behaviour is already scattered throughout LIO). Out of curiosity, do you know of any initiators which use this field? Cheers, David
On Fri, Jan 22, 2021 at 11:42:51PM +0100, David Disseldorp wrote: > On Wed, 20 Jan 2021 13:27:00 +0300, Dmitry Bogdanov wrote: > > > Now REPORT LUNS for software device servers always reports all luns > > regardless of SELECT REPORT field. > > Add handling of that field according to SPC-4: > > * accept known values, > > * reject unknown values. > > We currently advertise SPC-3 VERSION compliance via standard INQUIRY > data, so I think we should either support SPC-3 SELECT REPORT values or > bump the VERSION field (SPC-4 behaviour is already scattered throughout > LIO). > Out of curiosity, do you know of any initiators which use this field? > Hi David, SELECT REPORT field can be used for vVOL (LU conglomerate) discovery and for well-known lun listing. The field is used by VMware ESXi: https://support.purestorage.com/Solutions/VMware_Platform_Guide/User_Guides_for_VMware_Solutions/Virtual_Volumes_User_Guide/vVols_User_Guide%3A_Protocol_Endpoints "PEs greatly extend the number of vVols that can be connected to an ESXi cluster; each PE can have up to 16,383 vVols per host bound to it simultaneously. Moreover, a new binding does not require a complete I/O rescan. Instead, ESXi issues a REPORT_LUNS SCSI command with SELECT REPORT to the PE to which the sub-lun is bound. The PE returns a list of sub-lun IDs for the vVols bound to that host. In large clusters, REPORT_LUNS is significantly faster than a full I/O rescan because it is more precisely targeted." The post also confirms that: https://sourceforge.net/p/scst/mailman/message/33030432/ A few more targets that support SELECT REPORT field below. Sun/Oracle tape library: https://docs.oracle.com/en/storage/tape-storage/storagetek-sl150-modular-tape-library/slorm/report-luns-a0h.html#GUID-4140F40D-BD9A-495C-9A86-8BD7E91C985C IBM Flash Storage: https://www.ibm.com/support/pages/sites/default/files/support/ssg/ssgdocs.nsf/0/95d7115d7eb428e485257f80005cc3a7/%24FILE/FlashSystem_840_SCSI_Interface_Manual_1.2.pdf With regards to bumping TCM to SPC-4, are there any objections if we submit a separate patch for that? Or resubmit a series with the patch? Thanks, Roman
Dmitry, > + switch (sr) { > + case SCSI_SELECT_WELLKNOWN: > + case SCSI_SELECT_ADMINISTRATIVE: > + case SCSI_SELECT_SUBSIDIARY: > + /* report empty lun list */ > + goto out; I'm a bit concerned about things inadvertently breaking if we return an empty list for the well known LUNs.
Hi Martin, >> + switch (sr) { >> + case SCSI_SELECT_WELLKNOWN: >> + case SCSI_SELECT_ADMINISTRATIVE: >> + case SCSI_SELECT_SUBSIDIARY: >> + /* report empty lun list */ >> + goto out; > I'm a bit concerned about things inadvertently breaking if we return an empty list for the well known LUNs. According to SPC we shall report an empty list if there is no well-known LUNS. FreeBSD has the same logic in REPORT LUNS handling. SCST does not support SELECT_WELLKNOWN case at all. I don't know the history of the existing behaviour to send always LUN0 instead of empty list. Probably it was for the SCSI_SELECT_ALL_ACCESSIBLE(0x02) case, where SPC allows LUN0. My patch keeps it for the 0x00, 0x02, 0x11 cases. Thus, I believe it does not break the backward compatibility. BR, Dmitry
Hi Roman, On Tue, 26 Jan 2021 12:13:44 +0300, Roman Bolshakov wrote: > Hi David, > > SELECT REPORT field can be used for vVOL (LU conglomerate) discovery and > for well-known lun listing. > > The field is used by VMware ESXi: > https://support.purestorage.com/Solutions/VMware_Platform_Guide/User_Guides_for_VMware_Solutions/Virtual_Volumes_User_Guide/vVols_User_Guide%3A_Protocol_Endpoints > > "PEs greatly extend the number of vVols that can be connected to an ESXi > cluster; each PE can have up to 16,383 vVols per host bound to it > simultaneously. Moreover, a new binding does not require a complete I/O > rescan. Instead, ESXi issues a REPORT_LUNS SCSI command with SELECT > REPORT to the PE to which the sub-lun is bound. The PE returns a list of > sub-lun IDs for the vVols bound to that host. In large clusters, > REPORT_LUNS is significantly faster than a full I/O rescan because it is > more precisely targeted." Interesting, thanks. ... > With regards to bumping TCM to SPC-4, are there any objections if we > submit a separate patch for that? Or resubmit a series with the patch? I don't object to that. FWIW, the following crude metrics could be seen as an argument in favour of SPC-4 versioning: linux> git grep -ic -e "SPC4" -e "SPC-4" -- drivers/target/ drivers/target/target_core_alua.c:7 drivers/target/target_core_alua.h:5 drivers/target/target_core_device.c:1 drivers/target/target_core_fabric_lib.c:5 drivers/target/target_core_pr.c:37 drivers/target/target_core_spc.c:19 drivers/target/target_core_tmr.c:3 drivers/target/target_core_transport.c:2 drivers/target/target_core_ua.c:1 drivers/target/target_core_ua.h:1 drivers/target/target_core_xcopy.c:3 drivers/target/target_core_xcopy.h:1 linux> git grep -ic -e "SPC3" -e "SPC-3" -- drivers/target/ drivers/target/target_core_alua.c:1 drivers/target/target_core_configfs.c:14 drivers/target/target_core_pr.c:75 drivers/target/target_core_spc.c:4 drivers/target/target_core_tmr.c:1 drivers/target/target_core_transport.c:4 drivers/target/target_core_ua.c:1 Most of the SPC3 target_core_pr and target_core_configfs matches above are debug/error messages, rather than spec references. Cheers, David
On 1/27/21 6:45 AM, Dmitriy Bogdanov wrote: > Hi Martin, > >>> + switch (sr) { >>> + case SCSI_SELECT_WELLKNOWN: >>> + case SCSI_SELECT_ADMINISTRATIVE: >>> + case SCSI_SELECT_SUBSIDIARY: >>> + /* report empty lun list */ >>> + goto out; > >> I'm a bit concerned about things inadvertently breaking if we return an empty list for the well known LUNs. > > According to SPC we shall report an empty list if there is no well-known LUNS. > FreeBSD has the same logic in REPORT LUNS handling. SCST does not support SELECT_WELLKNOWN case at all. > > I don't know the history of the existing behaviour to send always LUN0 instead of empty list. Probably it was > for the SCSI_SELECT_ALL_ACCESSIBLE(0x02) case, where SPC allows LUN0. My patch keeps it for the 0x00, 0x02, 0x11 cases. > Thus, I believe it does not break the backward compatibility. Will this change require users to update their LUN configuration? Some initiator operating systems require presence of a dummy LUN 0. Although I agree that it is cleaner not to provide a hardcoded LUN 0, I think Martin is concerned about this patch potentially breaking existing configurations and causing frustration among LIO users. Bart.
Hi, >>> I'm a bit concerned about things inadvertently breaking if we return an empty list for the well known LUNs. > > >> According to SPC we shall report an empty list if there is no well-known LUNS. >> FreeBSD has the same logic in REPORT LUNS handling. SCST does not support SELECT_WELLKNOWN case at all. >> >> I don't know the history of the existing behaviour to send always LUN0 instead of empty list. Probably it was >> for the SCSI_SELECT_ALL_ACCESSIBLE(0x02) case, where SPC allows LUN0. My patch keeps it for the 0x00, 0x02, 0x11 cases. >> Thus, I believe it does not break the backward compatibility. >Will this change require users to update their LUN configuration? Some >initiator operating systems require presence of a dummy LUN 0. Although >I agree that it is cleaner not to provide a hardcoded LUN 0, I think >Martin is concerned about this patch potentially breaking existing >configurations and causing frustration among LIO users. No reconfiguration on initiator side is required. W-LUN is a specific LUN for the specific SCSI target device that is well known for the Initiator. Generic Linux TCM does not have W-LUNs. Some storage systems based on Linux TCM may have W-LUNs. But then they shall / already have its own handling of REPORT LUNS command. Basically, it is an error to report LUN0 as W-LUN for the Initiator that expects some other numbers. BR, Dmitry
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c index ca5579ebc81d..044ac45cdf47 100644 --- a/drivers/target/target_core_spc.c +++ b/drivers/target/target_core_spc.c @@ -1210,10 +1210,12 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd) { struct se_dev_entry *deve; struct se_session *sess = cmd->se_sess; + unsigned char *cdb = cmd->t_task_cdb; struct se_node_acl *nacl; struct scsi_lun slun; unsigned char *buf; u32 lun_count = 0, offset = 8; + u8 sr = cdb[2]; __be32 len; buf = transport_kmap_data_sg(cmd); @@ -1230,6 +1232,28 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd) nacl = sess->se_node_acl; + switch (sr) { + case SCSI_SELECT_WELLKNOWN: + case SCSI_SELECT_ADMINISTRATIVE: + case SCSI_SELECT_SUBSIDIARY: + /* report empty lun list */ + goto out; + case SCSI_SELECT_TOP_LEVEL: + if (cmd->se_lun->unpacked_lun != 0) + goto out; + fallthrough; + case SCSI_SELECT_REGULAR: + case SCSI_SELECT_ALL_ACCESSIBLE: + break; + default: + pr_debug("TARGET_CORE[%s]: Invalid REPORT LUNS with unsupported " + "SELECT REPORT %#x for 0x%08llx from %s\n", + cmd->se_tfo->fabric_name, sr, cmd->se_lun->unpacked_lun, + sess->se_node_acl->initiatorname); + transport_kunmap_data_sg(cmd); + return TCM_INVALID_CDB_FIELD; + } + rcu_read_lock(); hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) { /* @@ -1252,6 +1276,8 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd) * See SPC3 r07, page 159. */ done: + if ((sr != SCSI_SELECT_REGULAR) && (sr != SCSI_SELECT_ALL_ACCESSIBLE)) + goto out; /* * If no LUNs are accessible, report virtual LUN 0. */ @@ -1263,6 +1289,7 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd) lun_count = 1; } +out: if (buf) { len = cpu_to_be32(lun_count * 8); memcpy(buf, &len, min_t(int, sizeof len, cmd->data_length)); diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h index c36860111932..280169c75d85 100644 --- a/include/scsi/scsi_proto.h +++ b/include/scsi/scsi_proto.h @@ -341,4 +341,14 @@ enum zbc_zone_cond { ZBC_ZONE_COND_OFFLINE = 0xf, }; +/* Select Report fot REPORT LUNS */ +enum scsi_select_report { + SCSI_SELECT_REGULAR = 0x0, + SCSI_SELECT_WELLKNOWN = 0x1, + SCSI_SELECT_ALL_ACCESSIBLE = 0x2, + SCSI_SELECT_ADMINISTRATIVE = 0x10, + SCSI_SELECT_TOP_LEVEL = 0x11, + SCSI_SELECT_SUBSIDIARY = 0x12, +}; + #endif /* _SCSI_PROTO_H_ */