Subject: iSCSI: draft-10 more editorial

From: "Martin, Nick" <[email-masked]>
Date: Fri, 8 Feb 2002 21:15:30 -0600

Julian,

The following are some more comments about the iSCSI draft 10 document.
These are primarily editorial comments.  

Thanks,
Nick

As previously noted, the section numbers and page numbers are different
between the text and pdf versions of the document.  All my comments are
based on the pdf version.

Page 26 "Initiator that identifying elements, such as..."  The word
"that" does not fit.  One of the words "unique", "defined", "specific"
or "assigned" may have been intended, or perhaps it can just be removed.

On page 33, The capitalization of the quoted keyword "None" is confusing
"(literal constants which may include "None")".  I have previously
believed that keywords were case sensitive and that "none" (not "None")
was the correct string.  I note however that nearly all other keywords
have been capitalized ("Reject", "NotUnderstood", etc.).  For
consistency with other keywords perhaps we should use "None".  (As
someone else noted, "yes" and "no" are also not currently capitalized.)

On page 35, I presume the text "(up to the mode page limit)" is obsolete
since iSCSI mode pages have been removed.

On page 36 "It is also an error ..., than the SCSI limit for first
burst."  Should "SCSI" not be "iSCSI"?

In the next sentence, which mentions "MAY request to send data blocks
and a first burst of any size;..."  This looks like it might have been a
reference to 0 meaning any value which is now removed.  

On page 44, the sentence "During login, each end of the iSCSI session
specifies the maximum iSCSI PDU size it will accept."  This sounds like
the PDU size could be negotiated differently in each direction.  This is
not how I think it works.

On page 87, one paragraph starts with "LS:When".  I presume the LS:
should be removed.

This may be more of an issue than editorial:
On page 89, section 7.7 Negotiation Failures, I believe another reason
needs to be listed.
-	The text request was answered with Irrelevant.  
Alternatively perhaps Irrelevant was added as a way to avoid sending a
reject.  If a negotiation ending in Irrelevant is to be considered a
successful negotiation, this needs to be clearly stated.

On page 115, in "than no more data PDUs are expected", "than" should be
"then".

On page 124, in the section Referenced Task Tag, ABORT TASK is referred
to as TASK ABORT.

On page 134, in the section R2TSN, the maximum number of R2Ts is given
in hexadecimal 0xffffffff.  In most other cases (like DataSN on page
131) the limits are expressed in decimal 2**32 or 2**32-1.  The form
0xffffffff is generally used when specifying a reserved value.

On page 141, for the list of characters valid in a text value, I would
suggest also allowing the character '@' as would be used in e-mail
addresses.

On page 141, it is explained that binary items may be expressed in
decimal, hex, or Base64.  Are the protocol defined binary values, such
as MaxRecvPDULength required to be accepted in all these formats?

Request for clarification/restriction of text key=value continuations:
Are there any special rules for text key=value pairs spanning PDUs?  Is
a pair of length less than MaxRecvPDULength permitted to span PDUs?  Is
the sender required to fill MaxRecvPDULength before continuing to the
next PDU?  Can a key=value pair which will require continuation begin
somewhere in the middle of a PDU text buffer due to other key=value
pairs already in the buffer?  If so, can a key=value pair be continued
before the "=" ?  Is the text buffer to be continued required to fill a
multiple of 4 bytes in the current text buffer (no padding required)?  
I would personally prefer to see key=value continuation restricted to
cases where the key=value string exceeds MaxRecvPDULength.  In other
cases when the next key=value will not fit in the remaining space in the
current PDU, one could put the entire key=value in the next text PDU.
This would eliminate the possibility that one could be expected to
correctly parse small strings such as ImmediateData=yes or
MaxBurstSize=16384 with PDU breaks within them
(MaxBurstSize=16<PDU-break>384).

On page 149, in the section Login Parameters, the text appears
contradictory.  "All keys in Chapter 12, except for the X- extension
format, MUST be supported by iSCSI initiators and targets.  Keys in
Chapter 12, only need to be supported when the function to which they
refer is mandatory to implement."

On page 173, there is no section header (and number) for the beginning
of CHAP protocol.

On page 179, an example for TargetAlias uses an apostrophe (') although
this is not among the permitted characters (on page 141).

On page 181, InitialR2T has no section number.

On page 184, LogoutLoginMaxTime has no section number.

---

From: "Mallikarjun C." <[email-masked]>
Date: Sun, 10 Feb 2002 21:13:24 -0800

Nick,

I assume Julian will answer the rest, but let me comment on one.

> successful negotiation, this needs to be clearly stated.

The reference to Reject in section 7.7 is describing when the entire text
*request* is being rejected via a Reject PDU. Since it is somewhat
drastic for the negotiation in progress, it is categorized under "negotiation
failures".

Rejecting one text *key* with "Reject" key value is not as severe, and I
think it's best not to kill the entire negotiation.

I suggest that we change the constant key value "Reject" in section 
2.2.4 to "Declined" to make this distinction explicit.  Comments?
--
Mallikarjun

Mallikarjun Chadalapaka
Networked Storage Architecture
Network Storage Solutions Organization
Hewlett-Packard MS 5668 
Roseville CA 95747