iSCSI Corrections and Clarifications draft - review comments

[email protected] Fri, 23 Feb 2007 16:12:25 -0500
Newsgroups gmane.ietf.ips
Message-ID <F222151D3323874393F83102D614E055068B8F00@CORPUSMX20A.corp.emc.com>
Here are my review comments on this draft (-06 version).
I think all of these are minor, except for the IANA issue
(see previous message).  These need to be picked up in
the next version of the draft, but can be treated as
initial IETF Last Call comments if the IANA issue does
not require a new version of the draft.

Thanks, --David

Abstract:

OLD:
     iSCSI is a SCSI transport protocol and maps the SCSI family 
     of application protocols onto TCP/IP
NEW:
     iSCSI is a SCSI transport protocol that maps the SCSI
     architecture and command sets onto TCP/IP

SCSI consists of command sets, not protocols.

Section 3.3.2:

OLD:
     b)  If it is a multi-connection session, target iSCSI layer 
        takes note of last-sent and unacknowledged StatSN on each 
        of the connections in the iSCSI session, and waits for 
        acknowledgement (SHOULD solicit for acknowledgement by way 
        of a Nop-In) of each such StatSN to clear the fence. 
NEW:
     b)  If it is a multi-connection session, target iSCSI layer 
        takes note of last-sent and unacknowledged StatSN on each 
        of the connections in the iSCSI session, and waits for 
        acknowledgement (Nop-In PDUs MAY be used to solicit
        acknowledgements as needed in order to accelerate this
        process) of each such StatSN to clear the fence.

The "SHOULD" was poorly phrased - a literal reading would encourage
implementations to always send Nop-In PDUs, even when not needed.

OLD:
        SCSI 
        response with the Response Fence flag must be sent to the 
        initiator only after receiving acknowledgements for each of 
        the unacknowledged StatSNs.
NEW:
        The SCSI response requiring Response Fence behavior MUST NOT
        be sent to the initiator before acknowledgements are received
        for each of the unacknowledged StatSNs.

Need to eliminate "flag" word, and rephrased for clarity.

     c)  Target iSCSI layer must wait for an acknowledgement of the 
        SCSI Response PDU that carried the response which the 
        target SCSI layer marked with the Response Fence flag.  The 
        fence must be considered cleared after receiving the 
        acknowledgement. 

Change "must" to "MUST" in the above text.

     d)  All further status processing for the LU is resumed only 
        after clearing the fence.  If any new responses for the 
        I_T_L nexus are received from the SCSI layer before the 
        fence is cleared, those Response PDUs must be held and 
        queued at the iSCSI layer until the fence is cleared. 

Change "must" to "MUST" in the above text.

Section 3.3.3:

	This section lists the fenced response use cases that iSCSI 
	implementations must comply with.

Change "must" to "MUST" in the above text.

OLD:
	Whenever the TaskReporting key (section 9.1) is negotiated to 
	ResponseFence or FastAbort for an iSCSI session, target iSCSI 
	layer MUST assume that Response Fence flag is set by the target 
	SCSI layer on the following SCSI completion messages handed down

	to it:
NEW:
	Whenever the TaskReporting key (section 9.1) is negotiated to 
	ResponseFence or FastAbort for an iSCSI session, target iSCSI 
	layer MUST assume that Response Fence behavior is required for 
	the following SCSI completion messages:

Remove the "flag" word.

  1. The first completion message carrying the UA after the 
       multi-task abort on issuing and third-party sessions. 

The above text needs to cite a SAM-2 section for this or list
the iSCSI TMFs that can cause it.  A forward reference to
Section 4.1.1 may suffice.

Section 4.1.2 (initiator):

OLD:
     b. Should receive any responses that the target may provide 
          for some tasks among the affected tasks (may process them 
          as usual because they are guaranteed to have 
          chronologically originated prior to the TMF response). 
NEW:
     b. SHOULD process any responses received for affected tasks
          in the normal fashion.  This is acceptable because the
          responses are guaranteed to have been sent prior to the
          TMF response. 

Clarify requirements language.  Same change applies to section 4.1.3

     c. Should receive the TMF Response concluding all the tasks in 
          the set of affected tasks.

I think this may be misstated.  I believe the actual requirement is
"MUST treat the TMF response as terminating all affected tasks for
which responses have not been received, and MUST discard any responses
for affected tasks received after the TMF response is passed to the
SCSI layer."  This also applies to initiator item d. in section 4.1.3.

Section 4.1.2 (target):

OLD:
     d. MUST address the Response Fence flag on the TMF Response on 
          issuing session as defined in 3.3.2.

     e. MUST address the Response Fence flag on the first post-TMF 
          Response on third-party sessions as defined in 3.3.2.
NEW:
     d. MUST provide Response Fence behavior for the TMF Response on 
          the issuing session as specified in section 3.3.2.

     e. MUST provide Response Fence behavior for the first post-TMF 
          Response on third-party sessions as defined in 3.3.2.

Yet more "flag" removal.  This also applies to items e. and f. in
section 4.1.3.

	Implementation note: Technically, the TMF servicing is complete 

Remove "Implementation note:" as there's a MUST NOT in this paragraph.
This also applies to Section 4.1.3 .

Section 4.1.6

There are several lower case uses of "must" in this section.  As lower
case, they do not state requirements with the force of RFC 2119.  I
think that is ok and is what was intended, but wanted to double-check.

	6.3  Understanding NotUnderstood 

This is noteworthy as one of the best section names I've ever
seen in an Internet-Draft, doubly so as it happens to be a
technically correct and meaningful section name.

OLD:
	The proper way to handle a NotUnderstood response varies 
	depending on the lineage and type of the key.
NEW:
	The proper way to handle a NotUnderstood response depends
	on where the key is specified and whether the key is
	declarative vs. negotiated.

Clarity improvement.

Section 8.1:

This section needs to refer to the new negotiation key defined
in Section 9.1 that prevents this new Async Message type (5)
from being seen by initiators that aren't expecting it (e.g.,
initiators that are only compliant to RFC 3720).
 
Section 8.2:

OLD: 
	Thus reason code 0x0b MUST be considered as 
	deprecated and MUST NOT be used by any new implementations.
NEW:
	Thus reason code 0x0b MUST be considered as 
	deprecated and MUST NOT be sent by implementations that comply
	with the requirements of this document.  An implementation
	receiving reason code 0x0b MUST treat it as a negotiation
	failure that terminates the Login phase and the TCP connection,
	as specified in Section 6.10 of [RFC3720].

	Section 5.4 of [RFC3720] states:

   Neither the initiator nor the target should attempt to declare or
   negotiate a parameter more than once during any negotiation sequence
   without an intervening operational parameter negotiation reset,
   except for responses to specific keys that explicitly allow repeated
   key declarations (e.g., TargetAddress).

	The removal of reason code 0x0b eliminates the possibility of
	an operational parameter negotiation reset, causing the
	phrase "without an intervening operational parameter
	negotiation reset" to refer to an impossible event.  The quoted
	phrase SHOULD be ignored by a receivers that handle
	reason code 0x0b in the manner specified in this section.

Need to cover receiver behavior and address some now-redundant text
in RFC 3720.

Section 9.1:

OLD: 
	When TaskReporting is not negotiated to FastAbort, the default 
	behavior is to use the [RFC3720] TMF semantics as clarified in 
	section 4.1.2.
NEW:
	When TaskReporting is not negotiated to FastAbort, the [RFC3720]
	TMF semantics as clarified in section 4.1.2 of this document
	MUST be used.

Tighten up requirements language.

Section 11: IANA Considerations 

	This draft does not have any specific IANA considerations.

At the risk of opening Pandora's box, do we want to create IANA
registries into which we can put the TaskReporting key and the
type 5 async message in order to establish a precedent for
future standards-track extensions to iSCSI?

Section 12.1: Normative References

SPC-3 needs to be cited as "ANSI INCITS 408-2005".

Section 14: Acknowledgements 

I'd prefer a list of names without the additional parenthetical
text listing their technical contributions, but I don't feel
strongly about this.


_______________________________________________
Ips mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/ips