On 10/10/19 17:41, Bart Van Assche wrote:
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?
Hmm. You are right. Ideally only SCSI-2 compliant initiators should
use the LUN field and they should run parallel SCSI only.
OTOH, like Mike already said, we can't know whether there is any SW, FW,
BIOS, ... out there, that still sends such old style CDBs.
For example: probably SW could send such CDBs by simply using SCSI
generic device on top of a modern initiator. (I hope that's true, I
didn't test ...)
That means, old code can produce old SCSI CDBs even when running
on top of modern HW.
Do we want to take the risk of breaking such "old stuff"?
Thank you,
Bodo
Thanks,
Bart.
