Hi Sumit, Apologies for the delayed response. Comments below..
On Sun, 2016-07-03 at 19:24 +0530, Sumit Rai wrote: > > 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. > Applied to target-pending/for-next here. https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commit/?h=for-next&id=577d8f97b4f911881dfebe4c619b6eb9f087f042 Thanks Sumit + Ajay for tracking this down. -- 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
