On 10/10/19 5:07 AM, Bodo Stroesser wrote:
On 10/10/19 05:38, Mike Christie wrote:
On 10/08/2019 03:20 PM, bodo.stroesser@xxxxxxxxxxxxxx wrote:
Hi,
We use TCMUser to run userspace tape and changer emulations.
Current tests with a new version of backup SW failed, due to that
application
using SECURITY PROTOCOL IN / OUT SCSI commands.
The CDBs of these commands in byte 1 contain relevant information that
is overwritten in passthrough_parse_cdb() / target_core_device.c
This is the related part of the code:
/*
* Clear a lun set in the cdb if the initiator talking to
use spoke
* and old standards version, as we can't assume the
underlying device
* won't choke up on it.
*/
switch (cdb[0]) {
case READ_10: /* SBC - RDProtect */
case READ_12: /* SBC - RDProtect */
case READ_16: /* SBC - RDProtect */
case SEND_DIAGNOSTIC: /* SPC - SELF-TEST Code */
case VERIFY: /* SBC - VRProtect */
case VERIFY_16: /* SBC - VRProtect */
case WRITE_VERIFY: /* SBC - VRProtect */
case WRITE_VERIFY_12: /* SBC - VRProtect */
case MAINTENANCE_IN: /* SPC - Parameter Data Format for SA
RTPG */
break;
default:
cdb[1] &= 0x1f; /* clear logical unit number */
break;
}
Obviously the list of command codes for which bits 5,6,7 of byte 1
are _not_ reset
is not complete. Now I'm wondering what would be the right fix:
1) Scan the SCSI specs and add all other missing command codes to the
list of
Codes to skip
2) Create a list of commands that definitely have the LUN field in
byte 1 and
reset the bits only in those. (Might be better than 1), because I
expect new
commands to not have the LUN field.)
3) Remove the code entirely, because it is no longer needed / useful (?)
My preference would be to remove it like Bart suggested. Maybe we should
make it configurable via a device attribute or backend module flag.
For the 2 users:
pscsi - It seems this code was there from the beginning via
transport_generic_prepare_cdb in the original commit. Nick must have
found some initiator where the workaround was needed and I am not sure
if we would break them or not now. Based on the original code's comment
about iscsi my guess is there was some misc iscsi initiator probably in
a boot firmware or some offload card that supported old commands.
tcmu - We want the raw cdb. I think masking out the above bits was an
accident. It looks like when Andy converted tcmu to use common code the
behavior got brought in for tcmu.
I think, regarding TCMU you both are right, we should skip the code.
To avoid the need for an attribute, I'd prefer a new flag in the backend
ops. So pscsi can stay unchanged for the moment.
If you agree, I would prepare a patch.
SCSI-2 was introduced in 1994 and SCSI-3 was introduced in 1996
according to https://en.wikipedia.org/wiki/Parallel_SCSI. Is embedding
the LUN in byte one of the CDB something that is only relevant for
SCSI-2 parallel adapters? Since the SCSI target code supports receiving
SCSI commands over iSCSI, FC and SRP but not through a parallel port,
can the SCSI target code ever receive a CDB with a LUN number in byte
one of the CDB?
Thanks,
Bart.
