On Thu, Apr 02, 2015 at 09:43AM, Andy Grover wrote:
> On 04/02/2015 06:18 AM, Ilias Tsitsimpis wrote:
> >I am not sure I understand what you are suggesting. How can we find
> >where the Data-Out section starts without requiring the new field? Even
> >when req.iov[] is sized as max(in, out), don't we need to pass both
> >Data-In and Data-Out buffer lengths? Can you elaborate a bit?
>
> The buffer lengths are already given in the CDB. Still using XDWRITEREAD as
> an example (see sbc-3), the transfer length field in the cdb gives us the
> size in blocks, which userspace can multiply with the block size to get the
> overall buffer length. TCMU guarantees req.iov[] is equal to that.
Hello Andy,
If I understand correctly, what you are proposing is to get the Data-Out
and/or Data-In buffer lengths from the CDB. I believe this is not
feasible for the following reasons:
1) The SCSI Architecture Model (SAM) defines the Execute Command
procedure call as follows:
Service Response =
Execute Command (IN(I_T_L Nexus, Command Identifier, CDB,
Task Attribute, [Data-In Buffer Size],
[Data-Out Buffer], [Data-Out Buffer Size],
[CRN], [Command Priority]),
OUT([Data-In Buffer], [Sense Data],
[Sense Data Length], Status,
[Status Qualifier]))
Both Data-In/Data-Out buffer length are given and they are not retrieved
from the CDB. I believe that staying as compliant as possible with the
SAM will make TCMU more extensible in the feature.
2) In many cases, TCMU is going to be used as a simple proxy (i.e.
taking a SCSI command from the kernel and forwarding it to another
subsystem). For example, one can easily implement an iSCSI initiator on
top of TCMU. In such cases, the TCMU proxy should not have to understand
or be able to decode the SCSI command (CDB) in order to get the
Data-Out/Data-In buffer lengths. It should be able to just forward CDBs
without looking at them, so having distinct Data-In/Data-Out lengths is
important.
3) TCMU *does not* guarantee that the length of req.iov[] is equal to
the value of the transfer length field in the CDB. The user (in our
case, the kernel) may pass a bigger buffer to TCMU (req.iov[]) than what
is actually needed. In fact, it is fairly easy to do so using the SG_IO
ioctl and creating a SCSI command where the buffer size is not equal to
the value of the transfer length field in the CDB. Essentially, there is
no guaranteed relation between buffer lengths as specified in the TCMU
header and the length fields as specified in the CDB. This means that
TCMU user cannot find the buffer length if it is not explicitly given.
> For bidi commands if we double that to put the data-in buffer after
> the data-out buffer, then walking the iovec for data_out_bytes will
> give us the start of the data-in section. Not the most elegant, but we
> need to balance against the rare usage (yes?) of bidi commands, and
> the additional implementation work, because of the flaw I mentioned in
> handling new tcmu_opcodes.
BIDI commands are not as rare as they look like. For example, all the
Object-Based Storage Devices Commands (OSD) are bidirectional. In fact,
the kernel has an implementation for an OSD initiator and this is what I
use to test my code.
I understand that handling new tcmu_opcodes requires additional
implementation work, but I believe we should head in that direction. I
am afraid that adding new *features* without fixing this now, will lead
to a less elegant approach, which will increase code complexity and make
debugging harder later on.
Cheers,
Ilias
Attachment:
signature.asc
Description: Digital signature
