Re: [PATCH v2 1/3] scsi_cmnd: Introduce scsi_transfer_length helper

Sagi Grimberg
2014-08-06T12:15:51Z

On 7/27/2014 12:11 PM, Boaz Harrosh wrote:
> On 06/25/2014 01:32 PM, Sagi Grimberg wrote:
>> On 6/25/2014 11:48 AM, Sagi Grimberg wrote:
>>
>> <SNIP>
>>>
>>>> I made the patch below which should fix both bidi
>>>> support in iscsi and also WRITE_SAME (and similar commands) support.
>>>
>>> I'm a bit confused, for all commands (bidi or not) the iscsi header
>>> data_length
>>> should describe the scsi_data_buffer length, bidi information will lie
>>> in AHS header.
>>> (in case bidi will ever co-exist with PI, we might need another helper
>>> that will look
>>> in req->special + PI, something like scsi_bidi_transfer_length)
>>>
>>> So why not keep scsi_transfer_length to work on sdb length (take
>>> scsi_bufflen(scmnd) or
>>> scsi_out(scmnd)->length as MKP suggested) and that's it - don't touch
>>> libiscsi.
>>>
>>> Let me test that.
>>>
>>
>> So I tested a bidirectional command using:
>> $ sg_raw --infile=/root/filex --send=1024 --request=1024
>> --outfile=/root/filex "/dev/bsg/7:0:0:0" 53 00 00 00 00 00 00 00 02 00
>>
>> And I see:
>> kernel: session1: iscsi_prep_scsi_cmd_pdu iscsi prep [bidirectional cid
>> 0 sc ffff880468ca1e00 cdb 0x53 itt 0x16 len 1024 bidi_len 1024 cmdsn 223
>> win 64]
>>
>
> This is a very bad example and tested nothing, since len && bidi_len
> are the same. So even if you had a bug and took length from the
> wrong side it would come out the same.
>
> You must test with a bidi command that has two different lengths for
> each side
>

Yes, I thought the same thing right after I sent this, so I tested it
with different lengths and it does work. I guess I was lazy in replying
it on top...

Sagi.