> From: Sumit Rai <sumit....@calsoftinc.com>
> Subject: LIO seems to use SCSI Allocation Length instead of SPDTL for
> calculating ResidualCount
> Date: June 22, 2016 at 7:43:50 PM GMT+5:30
> To: target-de...@vger.kernel.org
>
> We are trying to test if LIO target sets ResidualCount correctly in case of
> ResidualOverflow for Data-In by following the below Steps:
> 1. Send SCSI Inquiry command to iSCSI target, if there are no exceptions in
> the response (and no residual overflow) we are able to determine how many
> bytes the target SCSI layer on target side attempted to transfer to target
> iSCSI layer. On the initiator side when response is received: in the absence
> of any exceptions or residual overflow we assume all the data was
> successfully transferred and is equal to iSCSI DataSegmentLength. We will use
> this value as SPDTL for SCSI Inquiry command.
>
> Note: SPDTL is detailed in RFC 7143 11.4.5.2 (and you may also refer to
> discussion on T10 mailing list:
> http://www.t10.org/pipermail/t10/2014-June/017367.html).
>
> 2. Send out the same SCSI inquiry command as in Step 1. but set iSCSI EDTL to
> a value lower than SPDTL.
>
> Assumption: Since we are not making any changes to iSCSI target LU
> configuration in our setup b/w Step 1. and 2, we assume amount of Inquiry
> Data target generates will be same of 1. and 2. since inquiry command is the
> same. During our testing we find this assumption to be true.
>
> Expected Result:
> Since in Step 2, SPDTL > EDTL, iSCSI target is expected to set
> ResidualOverflow flag and ResidualCount to SPDTL - EDTL.
> In the attached PCAP text file, frame no. 11 (SCSI Inquiry Req.) and 12
> (Inquiry Response) are for Step 1. As explained, SPDTL = 0x24 or 36D.
> Frame no. 14 is for (SCSI Inq. Req.) Step 2. EDTL = 0x08.
> Hence, expected ResidualCount = 0x1C (28D) i.e. 0x24 (36D) - 0x08 (8D)
>
> Observed Result:
> iSCSI target sets the Residual Overflow flag correctly but value of
> ResidualCount doesn’t match expected value.
> Target is setting, ResidualCount to 0x3f8 (1016D) as seen in frame no 16. In
> the frame no. 14, in the SCSI Inquiry CDB allocation length (SPC 5r01 Section
> 4.2.5.6) was set to 0x0400 (1024D) and target seems to be using this value to
> calculate ResidualCount i.e. 0x0400 (1024D) - 0x08.
> To further verify this we changed the allocation length value in SCSI
> Inquiry CDB to 0x0200 (512D) bytes instead, we get ResidualCount of 0x1f8
> (504D).
>
> Allocation Length is the maximum value of the SPDTL and not substitute for
> it, hence it shouldn’t be used to calculate ResidualCount except for cases
> where SPDTL > Allocation Length and Data is truncated (in that case both
> Alloc Len and SPDTL are same). (SPC 5r01 Section 4.2.5.6).
>
> LIO Version: Datera Inc. iSCSI Target v4.1.0
> Linux Kernel: 4.4.0-22-generic
>
> RFC 7143:
>
> 11.4.5.2. Residuals Concepts Overview
>
>
> "SCSI-Presented Data Transfer Length (SPDTL)" is the term this
> document uses (see
> Section 2.2
> for definition) to represent the
> aggregate data length that the target SCSI layer attempts to transfer
> using the local iSCSI layer for a task. "Expected Data Transfer
>
> Length (EDTL)" is the iSCSI term that represents the length of data
> that the iSCSI layer expects to transfer for a task. EDTL is
> specified in the SCSI Command PDU.
>
> When SPDTL = EDTL for a task, the target iSCSI layer completes the
> task with no residuals. Whenever SPDTL differs from EDTL for a task,
> that task is said to have a residual.
>
> If SPDTL > EDTL for a task, iSCSI Overflow MUST be signaled in the
> SCSI Response PDU as specified in
> Section 11.4.5.1
> . The Residual
> Count MUST be set to the numerical value of (SPDTL - EDTL).
>
> If SPDTL < EDTL for a task, iSCSI Underflow MUST be signaled in the
> SCSI Response PDU as specified in
> Section 11.4.5.1
> . The Residual
> Count MUST be set to the numerical value of (EDTL - SPDTL).
>
> Note that the Overflow and Underflow scenarios are independent of
> Data-In and Data-Out. Either scenario is logically possible in
>
>
> SPC 5r01
> 4.2.5.6 Allocation length
>
> The ALLOCATION LENGTH field specifies the maximum number of bytes or blocks
> that an application client has allocated in the Data-In Buffer. The
> ALLOCATION LENGTH field specifies bytes unless a different requirement is
> stated in the command definition.
>
> An allocation length of zero specifies that no data shall be transferred.
> This condition shall not be considered an error.
>
> The device server shall terminate transfers to the Data-In Buffer when the
> number of bytes or blocks specified by the ALLOCATION LENGTH field have been
> transferred or when all available data have been transferred, whichever is
> less. The allocation length is used to limit the maximum amount of variable
> length data (e.g., mode data, log data, diagnostic data) returned to an
> application client. If the information being transferred to the Data-In
> Buffer includes fields containing counts of the number of bytes in some or
> all of the data (e.g., a PARAMETER DATA LENGTH field, a PAGE LENGTH field, a
> DESCRIPTOR LENGTH field, an AVAILABLE DATA field), then the contents of these
> fields shall not be altered to reflect the truncation, if any, that results
> from an insufficient ALLOCATION LENGTH value, unless the standard that
> describes the Data-In Buffer format states otherwise.
>
> If the amount of information that is available to be transferred exceeds the
> maximum value that the ALLOCATION LENGTH field in combination with other
> fields in the CDB is capable of specifying, then no data shall be transferred
> and the command shall be terminated with CHECK CONDITION status, with the
> sense key set to ILLEGAL REQUEST, and the additional sense code set to
> INVALID FIELD IN CDB.
I havn't received any comments on this issue, I guess the maintainer(s) are
occpied with
other high priority issues, so I tried to fix the issue. I have successfully
tested
the patch and it calculates the residual count as expected. I would appreciate
it if someone
can do a review of the below patch:
--- linux/drivers/target/target_core_transport.c.orig 2016-07-03
18:09:28.949281611 +0530
+++ linux/drivers/target/target_core_transport.c 2016-07-03
18:10:44.007293696 +0530
@@ -754,7 +754,15 @@ EXPORT_SYMBOL(target_complete_cmd);
void target_complete_cmd_with_length(struct se_cmd *cmd, u8 scsi_status, int
length)
{
- if (scsi_status == SAM_STAT_GOOD && length < cmd->data_length) {
+ if (scsi_status != SAM_STAT_GOOD) {
+ return;
+ }
+
+ /*
+ * Calculate new residual count based upon length of SCSI data
+ * transferred.
+ */
+ if (length < cmd->data_length) {
if (cmd->se_cmd_flags & SCF_UNDERFLOW_BIT) {
cmd->residual_count += cmd->data_length - length;
} else {
@@ -763,6 +771,12 @@ void target_complete_cmd_with_length(str
}
cmd->data_length = length;
+ } else if (length > cmd->data_length) {
+ cmd->se_cmd_flags |= SCF_OVERFLOW_BIT;
+ cmd->residual_count = length - cmd->data_length;
+ } else {
+ cmd->se_cmd_flags &= ~(SCF_OVERFLOW_BIT | SCF_UNDERFLOW_BIT);
+ cmd->residual_count = 0;
}
target_complete_cmd(cmd, scsi_status);
Signed-off-by: Sumit Rai <sumitra...@gmail.com>
Thanks to Ajay Nair in assisting with this patch.
Thanks,
Sumit Rai
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
