Alexander,
You are right, I misread/misremembered rfc5048/3.1
I have re-sent a patch that only fixes the swapped parameters.
I added a test for residuals and they work correctly as is :
$ ./bin/iscsi-test-cu iscsi://127.0.0.1/iqn.ronnie.test/1 --test
SCSI.ModeSense6.Residuals -V
CUnit - A Unit testing framework for C - Version 2.1-0
http://cunit.sourceforge.net/
Suite: ModeSense6
Test: Residuals ...
Test of MODESENSE6 Residuals
MODESENSE6 command should not result in any residuals
Try a MODESENSE6 command with 4 bytes of transfer length and
verify that we dont get residuals.
[SUCCESS] All Pages fetched.
Verify that we got at most 4 bytes of DATA-IN
[SUCCESS] <= 4 bytes of DATA-IN received.
Verify residual overflow flag not set
Try a MODESENSE6 command with 255 bytes of transfer length and
verify that we get residuals if the target returns less than the
requested amount of data.
[SUCCESS] All Pages fetched.
We got less than the requested 255 bytes from the target. Verify
that underflow is set.
[SUCCESS] Residual underflow is set
passed
--Run Summary: Type Total Ran Passed Failed
suites 1 1 n/a 0
tests 1 1 1 0
asserts 3 3 3 0
Tests completed with return value: 0
On Fri, May 31, 2013 at 4:50 AM, Alexander Nezhinsky
<nezhinsky@xxxxxxxxx> wrote:
> On Fri, May 31, 2013 at 8:30 AM, Ronnie Sahlberg
> <ronniesahlberg@xxxxxxxxx> wrote:
>
>> diff --git a/usr/spc.c b/usr/spc.c
>> index 074fdad..197e611 100644
>> --- a/usr/spc.c
>> +++ b/usr/spc.c
>> @@ -622,7 +622,7 @@ static int build_mode_page(uint8_t *data, struct mode_pg *pg,
>> * Set a byte at the given index within dst buffer to val,
>> * not exceeding dst_len bytes available at dst.
>> */
>> -void set_byte_safe(uint8_t *dst, uint32_t dst_len, uint32_t index, int val)
>> +void set_byte_safe(uint8_t *dst, uint32_t index, uint32_t dst_len, int val)
>> {
>> if (index < dst_len)
>> dst[index] = (uint8_t)val;
>
> Oops, you are right! My fault, i swapped the arguments when calling
> the function.
>
>> @@ -713,10 +713,8 @@ int spc_mode_sense(int host_no, struct scsi_cmd *cmd)
>> set_byte_safe(data, 7, alloc_len, blk_desc_len & 0xff);
>> }
>>
>> - scsi_set_in_resid_by_actual(cmd, actual_len);
>> return SAM_STAT_GOOD;
>> sense:
>> - scsi_set_in_resid_by_actual(cmd, 0);
>> sense_data_build(cmd, key, asc);
>> return SAM_STAT_CHECK_CONDITION;
>
> I guess we shouldn't remove these calls as the following reasoning:
> > 2, These commands never return residual under/overflow so dont set
> the residuals for them
> does not exactly apply here.
>
> Truncation of Data-IN buffer to ALLOC_LENGTH is not reflected in residual count,
> but if ALLOC_LENGTH is less than EDTL, it should produce a non-zero
> residual count.
> That is one of the purposes of scsi_set_in_resid_by_actual().
> Also, please refer to the commit log of d7af3dc1 which explains this logic.
> Is spc_mode_sense() really a special case which requires a different treatment?
>
> Alexander
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
