iSCSI : Review Feedback on iSCSI Rev 06

[Santosh Rao <santoshr@cup.hp.com>]

Julian & All,

Enclosed is my review feedback on the iSCSI draft Rev 06.

Regards,
Santosh

--------------------------------------------------------------------

1) Suggest re-naming the following in the interests of better
clarity : 

a) Rename the ExpDataSN in SCSI Response PDU to EndDataSN as was used in 
the previous version of the draft. (indicative of being the final Data sequence
number.)

b) Rename DataSN in the R2T PDU to R2TSN. (This is really an R2T seqeunce
number.)

c)  Rename R2TExpDataSn to EndR2TSN. (indicative of being the final R2T
sequence number.)


2) If a target issues a REJECT response to any received PDU within a SCSI
Command operation, does this imply the target has internally aborted the I/O as
well ? Will the target send a SCSI Response in addition following the Reject ?

What behaviour must an initiator expect when it sees a reject in response to
some PDU within a SCSI command ? Does it treat this as a termination of the I/O
or should it await a SCSI Response ? 

Also, if the target does terminate the command on sending REJECT, can a "retry"
be issued ?

In the interests of clearing all these obscurities with the REJECT, I'd like to
suggest the following model :

- SCSI Command be always responded with a SCSI Response.
- SCSI Task Mgmt Command be always responded with a SCSI Task Mgmt response.
- Only non-SCSI PDUs may be errored back with a REJECT PDU. 

The above is also semantically aligned with the FC LS_RJT/BA_RJT model.


3) Why does the new draft limit the size of text commands and nop commands to
4096 bytes ? What is the rationale behind the magic number selection of 4096 ? 

Since DataPDULength is defined in units of 512 bytes, how does iSCSI deal with
fragmentation caused due to the usage of a DataPDULength < 4K and an attempt to
perform a 4K text , login or nop operation ?

I believe such scenarios were discussed in an earlier thread
titled "Fragmentation & Reassembly issues in iSCSI." 
(http://ips.pdl.cs.cmu.edu/mail/msg03031.html) and the resolution was to
enforce a restriction that the size of a login, nop & text operation be 
limited to DataPDULength (?).

On a seperate note, what happened to PingMaxReplyLength ? It seems to have been
removed from the draft. How can the 2 end points now negotiate a max limit on
nop ping operations ? (for ex : how can a target enforce that the nop ping
operation data buffer not exceed 512 bytes ?).


4) As mentioned in a seperate mail, the login or text negotiation can span
several command/response exchanges each using the same initiator task tag. The
addition of a "Target Task Tag" field in the login response, text command and
text response would allow targets to perform a quick loopup on their T.T.T for
this negotiation state information, instead of having to lookup based on
(Initiator iSCSI Name + ISID).


5)  Section 2.8.3
"The data length of a text command or response SHOULD be less than 
   4096 bytes."

suggest re-wording the above to :
"The data length of a login, nop & text command or response MUST be less than 
 DataPDULength. For the leading session login, the length of the login command
 MUST be less than 512 bytes."

The above will ensure that login, text & nop operations will not result in
fragmentation.


6) Section 2.8.3
"A session may have only one outstanding text command or text command 
   sequence at any given time."

The above is contradicting itself. Is it only one outstanding text command or
one outstanding text sequence.

Suggest re-wording the above to :
" A session may have only one outstanding text command at any given time."


7) Section 2.11.5
"The TSID is an initiator identifying tag set by the target.  It is 
   valid only if the connection is accepted."
		     ^^^^^^^^^^

The above needs to be re-worded as :
"only if the login is accepted"


8) Section 2.13.1

"A target may also issue a NOP-In on its own to carry a changed CmdSN 
  							        ^^^^^
   and/or MaxCmdSN if there is no other PDU to carry them for a long 
      time."

The above reference to changed CmdSN must be re-worded to "changed ExpCmdSN".


9) Section 2.18.1

"Target requests logout".

Suggest removal of the above option. A target that needs to logout would use
either :
"Target will drop the connection"
or
"Target will drop all connections"

The "Target requests logout" also does not allow the target to specify which
flavor of logout it wishes to receive. It is better to remove this option and
use only a single scheme.


10) Section 2.19

The section 2.19 seems out of place being in the midst of descriptions of other
iSCSI PDUs. It should be moved out to a seperate location since it has nothing
to do with PDU descriptions.


11) As was brought up at the Nashua interim meeting, Status SNACK must be made
optional to support. Simple implementations as also gateway and bridging
products may choose not to [or are unable to] support SNACK mechanisms. Non-use
of Status SNACK must be accompanied by setting StatSN to 0 on all status PDUs.


12) Suggest the addition of new login keys to negotiate support for Data SNACK
& Status SNACK. If a target does not support Status SNACK, it must set StatSN
to 0 for all Status PDUs.


13) Section 3.2
" 3.2 iSCSI Logical Unit Control Mode Page 
    
       The following outlines the iSCSI Port mode page: "

The above needs to be re-worded to :
"The following outlines the iSCSI LUN Control Mode Page: ".

As suggested earlier, perhaps, suggest using the SPC-2 terms
"protocol specific port page" and "protocol specific LUN page" for these mode
pages.


14)  Section 3.3.3
LoginLogoutMinTime

The wording must be clarified to indicate that this delay is only in the case
the prior logout was due to a target originated event. There is no need for an
initiator to delay after a logout originated by itself.

For example, while doing open, read/write, close type of operations on a single
thread of activity for the entire session, it may be typical to see iSCSI level
on-the-wire activity of login, read/write, logout....

In the above type of scenarios, LoginLogoutMinTime is not applicable and the
initiator must not delay prior to issuing logins.


15) Section 4.3
"If the target responds to a text or Login command with the F bit set 
 to 1, with a text response with the F bit set to 0, or a login 
 response with the text bit set to 0, the initiator must keep sending 
 		   ^^^^^^^^
 the text command (even empty) with the F bit set to 1 until it gets 
 the Login Response with the F bit set to 1."

The above must be re-worded to :
"or a login response with the F bit set to 0, ...".


16) Section 6
"It is further assumed that a target will keep the "status & sense" 
 for a command it has executed while the total number of outstanding 
 commands and executed commands does not exceed its limit and status 
 has not been acknowledged."

What if the target has exceeded its limit on resources ? Can it drop these
stale resources or must it close its command window and not accept further new
commands ?

If it is the latter, then, iSCSI is optimizing for recovery paths and is
impacting performance paths by limiting the ability of the target to accept new
I/Os due to having to hold onto stale resources awaiting ExpStatSN updates.


17) Section 6.7.3
"N.B. As an alternative to Logout and reissue commands, the 
 initiator MAY instead reset the target and terminate all 
 outstanding commands with a service response indicating 
 Delivery Subsystem Failure. The initiator MUST perform one of 
 the two actions."

 As was brought up at the Nashua interim meeting, suggest removal of the above
 sentence since it may lead implementors to use Target Reset as a form of error
 recovery while dealing with session-level error recovery.

 An alternate form of error recovery in its place would be to use :
 "logout - cleans the connection"
 or 
 "logout - cleans the session"


18) Section 7.3

This section outlines how deterministic clean-up of tasks can be achieved
during task mgmt command execution. However, in the absence of mandating this
behaviour for targets, initiators cannot rely on the same for reliable
deterministic clean-up of pending tasks and will need to go ahead and implement
their own forms of abort task based cleanup.

This section is meaningless in the absence of a mandate that forces targets to
do as described. In its absence, initiators will be forced to implement their
own schemes for clean up.


19) Appendix E

"15 AccessID 
 Use: ALL 
 Who: Initiator 
 AccessID=<SCSI-AccessID-value> 
 Deliver a SCSI AccessID to the target "

What is a SCSI Access ID ? Can further descriptions be added to this definition
and a reference be added to SAM2 or some other SCSI document, if appropriate.


20) Appendix E
In several places, IO seems to have been used instead of LO. Is this a typo or
a new acronym ? 

Speaking of acronyms, can we add an acronym section to the draft ?


21) Appendix E
the description of ImmediateData is describing the case :
"ImmediateData=YES & InitialR2T=YES" twice in differing manners. One of them
needs fixing [or removal].


---------------------------------------------------------------------------



-- 
#################################
Santosh Rao
Software Design Engineer,
HP, Cupertino.
email : santoshr@cup.hp.com
Phone : 408-447-3751
#################################