Re: iSCSI-08 Comments 1 through 10

["Julian Satran" <Julian_Satran@il.ibm.com>]

Comments in text.  Thanx, Julo

Thomas Dineen <tdineen@redswitch.com> Sent by: owner-ips@ece.cmu.edu 03-10-01 03:15 Please respond to Thomas Dineen

To:        ips@ece.cmu.edu, Thomas Dineen <tdineen@redswitch.com>         cc:                 Subject:        iSCSI-08 Comments 1 through 10

Comment 1:

Section 3.17 Page 101:

"Reject is used to indicate an iSCSI error condition (protocol,
unsupported option etc.)."

- Please enhance the description of the Reject PDU. Please specify
exactly what it is used for, when, and why. I realize that there is
a forward reference on the following page, but feel that a more
robust definition of the reject functionality also belongs here. 
+++ The next revision is also an editorial "redo".  However if you have something specific in mind please feel free to forward an Internet Draft for consideration by the list or a short text here if you feel this is more appropriate +++
Comment 2:

Section 3.18 on Page 104:

- Please add a definition of the I Bit. I assume it is the same meaning
as the previous definition in other PDUs of I and Immediate. I feel
these fields should be specified for each PDU.

+++ Common fields are specified once +++

Comment 3:

Section 3.19.1 on Page 

"3.19.1 Target Transfer Tag"

"Whenever the NOP-In is not issued in response to a NOP-Out the StatSN
field will contain as usual the next StatSN but StatSN for this
connection is not advanced."

- Why is the above text in a section titled "3.19.1 Target Transfer
Tag"?
- It almost appears the this text belongs in NOP-Out?
- Please elaborate on the meaning and proper placement for this text.

+++ StatSN belongs to Nop-IN +++

Comment 4:

Section 5.1 on Page 112:

"-Login Response with Login Accept as a final response (T bit
set to 1 and the NSG in both command [and] response are set to
FullFeaturePhase). The response includes the protocol version
supported by the target and the session ID and may include
iSCSI operational or security parameters (depending on the
current stage)."

Change "a' to "and".

+++ will do +++

Comment 5:

Section 5.2 on Page 115:

"If security is established in the login phase then after the security
negotiation is complete, each iSCSI PDU MUST include the appropriate
digest field if any."

"All PDUs sent after the security negotiation stage MUST be built
using the agreed security."

- The two sets of quoted text shown above are a bit redundant, or at
least could be combined into a single paragraph. By the way I do realize
there are two distinct phases here.

- Could we be a little more specific in the second statement? Something
like "MUST be composed of the fields or tuples specified by the agreed
upon protocol specification".

+++ I will combine them into:
If security is established in the login phase then after the security negotiation is complete, each iSCSI PDU MUST be built using the agreed security and include the appropriate integrity digest fields if any.
+++
Comment 6:

Section 5.2 on Page 115:

"If the security negotiation fails at the target then the target MUST
send the appropriate Login Response PDU. If the security negotiation
fails at the initiator, the initiator [shall] drop the connection."

- Do you really mean "shall"? Shall is IEEE terminology do you not mean
"MUST"? This is a recurring problem in many places in the draft, for
brevity I would suggest a text search and examination of each "shall".

+++ SHALL is also IETF - I will make it uppercase where needed +++

Comment 7:

Section 5.3 on Page 115:
"Session specific parameters can be specified only during the login
phase begun by a login command [containing a null TSID] (e.g., the
maximum number of connections that can be used for this session).
Connection specific parameters, if any, can be specified during the
login phase begun by any login command. Thus, a session is
operational once it has at least one connection."

- Do you not mean the first or leading login? If true please add this
terminology.
+++ will try +++
Comment 8:

Section 6 on Page 116:

"If the target responds to a text request with the F bit set to 1,
with a text response with the F bit set to 0, the initiator [must] keep
sending the text request (even empty) with the F bit set to 1 until
it gets the text response with the F bit set to 1. Responding to a
text request with the F bit set to 1 with an empty (no key=value
pairs) is not an error but is discouraged."

- "must" versus "MUST"! Do you not mean "MUST"? This is a recurring
problem in many places in the draft, for brevity I would suggest a
text search and examination of each "must'.
+++ Not every must is a MUST. The selection is sometimes intentional (like in the example) +++
Comment 9:

Section 8.11 on Page 131:

"Usage of within a connection and within a command recovery classes
MUST NOT be attempted before the connection is in full feature phase."

- I think there is a english problem here, or maybe dropped words.
+++ will fix +++
Comment 10:

Section 8.11.4 on Page 134:
"Session recovery implies closing of all [the] TCP connections, aborting
at
target all executing and queued tasks for the given initiator,
terminating at initiator all [the] outstanding SCSI commands with an
appropriate SCSI service response and restarting a session on a new
connection set (TCP connection establishment and login on all new
connections)."

- Minor english suggest adding "the" in two places.

+++ sounds right in my English but will see what our technical writers have to say about it :-) +++