SORT BY:

LIST ORDER
THREAD
AUTHOR
SUBJECT


SEARCH

IPS HOME


    [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

    Re: Comments on v12 of iSCSI Specification



    
    Thanks for tanking your time to do such a thorough review.
    
    my comments in text -Thanks again - Julo
    
    
                                                                                                                                                       
                          "BARRY,BOB                                                                                                                   
                          (HP-Roseville,ex1        To:       "'ips@ece.cmu.edu'" <ips@ece.cmu.edu>                                                     
                          )"                       cc:                                                                                                 
                          <bob_barry@hp.com        Subject:  Comments on v12 of iSCSI Specification                                                    
                          >                                                                                                                            
                          Sent by:                                                                                                                     
                          owner-ips@ece.cmu                                                                                                            
                          .edu                                                                                                                         
                                                                                                                                                       
                                                                                                                                                       
                          05/03/2002 08:03                                                                                                             
                          PM                                                                                                                           
                          Please respond to                                                                                                            
                          "BARRY,BOB                                                                                                                   
                          (HP-Roseville,ex1                                                                                                            
                          )"                                                                                                                           
                                                                                                                                                       
                                                                                                                                                       
    
    
    
    The following comments are submitted against the April 17, 2002,
    draft-ietf-ips-iSCSI-12.txt.
    
    Bob Barry
    ====================================================
    
    General Comments
       1)        An acronym section would make it easier to read
                 this document.  Acronyms such as SW for session
                 wide, IO for initialize-only, and others are not
                 immediately obvious.
    +++ If you volunteer to do it, I will volunteer to review it and add it
    with proper acknowledgements +++
    
    
       2)        A copyright notice be part of this document.
    
    +++ There is one on the last page as customary for RFCs +++
    
       3)        In chapter 9, each PDU should have a complete
                 description provided.  Yes, this means a duplication
                 of the field descriptions, but having to search back
                 in the document to find field meanings does not
                 make sense.
    
    +++ NO - it will add too much volume and no additional meaning +++
    
       4)        Each table should be numbered and titled.  Currently,
                 there is no way to reference an individual table, accept
                 a page number which may change over revisions.
    
    +++ All references - are made with a tool that takes care of it.
    
       5)        Either "data is" or "data are" are considered correct
                 for technical documentation, however, only one should
                 be used in a document for consistency.  Examples:
                   p 35: last paragraph, first line          "data is"
                   p 36: third paragraph, first line         "data are"
                   p 36: third paragraph, third line        "data are"
                   p 36: seventh paragraph, fourth line  "data is"
    
    +++ fixed +++
    
       6)        Word "since" used instead of "because".  "Since"
                 implies a temporal relationship, "because" implies a
                 cause-effect" relationship.  Some examples
                   p 31: third paragraph from bottom, first line;
                   p 37: last paragraph, second line;
                   p 44: second paragraph, seventh line;
                   p 47: last paragraph, third line
                   p 108: second paragraph, second line
                   p 111: first paragraph after bullet item, first line
    +++ fixed +++              p 141: bullet item c) in upper list
    
       6)        The word "null" is used throughout this document to
                 mean "zero", or "zero valued".  This needs to be
                 clearly stated.
    +++ I think that this is common in computing but I will see where I can add
    something +++
    
    Comments regarding draft contents
       1)        p 22: "iSCSI Initiator Node" and "iSCSI Target Node"
                 are circular and provide no insight into what is being
                 defined.
    +++ what do you suggest? The notions hep us map iSCSI architecurally to
    SCSI +++
    
       2)        p 31: second paragraph, third line: "from ExpCmdSN
                 to MaxCmdSN" should be "from ExpCmdSN to
                 MaxCmdSN inclusive"; or "in the closed interval
                 [ ExpCmdSN . . MaxCmdSN ]"
    
    +++ fixed +++
    
       3)        p 31: second paragraph, sixth line, how the
                 parathesized example applies to the preceding
                 sentence is not immediately obvous.
    +++ wht do you suggest? +++
    
       4)        p 31: last paragraph, last 2 lines: use of the words
                 "and" and "or" could be ambiguously interpreted.
    +++ I reread it twice - and the parsing is obvious +++
       5)        p 34: second paragraph, line 3: "the TSIH is null".
                 The word "null" should be replaced with "equal to
                 zero".
    +++ null is commonly used and its meaning is clear in the context. However,
    I will replace it in this instance+++
    
       6)        p 36: third paragraph, third line: the sentence ends
                 right in the middle; need to complete the thought.
    +++ I can't locate it - context would help ++
    
       7)        p 37: second paragraph: "Unsolicited data MUST
                 be sent on every connection in the same order in
                 which commands are sent."  This sentence needs
                 better wording to get the intended point.
    +++ It is clear to me and my friends. If you have a suggestion please make
    it explicit
    Better is unbounded :-). +++
    
       8)        p 39: item c) in list: where are "displayable" and
                 and "whitespace" characters defined?
    +++ The common meanings are well understood that is why they appear in
    requirements (i have removed the MUST from this part of the text the
    normative text follows in the encoding part. Precise references are
    stringprep, and mime. +++
    
       9)        p 50: item e), some of the references to "Node" should
                 be references to "Entity".
    +++ which ones ? +++
    
      10)        p 52: last paragraph, fifth bullet item: "3 null bytes".
                 The word "null" should be replaced with "zero".
    +++ that is the log of changes part +++
    
      11)        p 63: third paragraph of 4.1, last line: "(comma or
                 null)" should be "(comma or zero byte)"
    +++ OK +++
      12)        p 72: fourth paragraph, second line: does the phrase
                 "MUST be sent after the parameters" mean later in
                 the current request, or could they be sent in a later
                 request.  The same comment can be applied on p 75,
    
    paragaph 4.
    
    +++ either - by omission +++
    
      13)        p 72: fifth paragraph, second line: "a null TSIH" should
                 be "a zero-valued TSIH"
    +++ OK +++
    
      14)        p 78: in the state table, it appears that exit from S8 is
                 impossible.  Actually, this is explained on p 84.  An
                 explanation of what is going on on p 78 would be helpful.
                 The same can be said of the state table on p 80.
    +++ It is just the internal activity and/or time that makess the transition
    out of S8 not an even of interest
    to iSCSI. Hard to show any of those in a crowded ASCII table or figure and
    the text is clear. +++
    
      15)        p 118: section 9 - why are some bits marked as reserved
                 and some marked as '0' or '1' in the PDU definitions.  If
                 the bits marked as '0' or '1' in the PDUs will never change,
                 then this needs to be stated.  In other words, there are
                 not treated the same as reserved bits.
    
    +++ ??? +++
    
      16)        p 122 and 123: why doesn't AHS-Specific data begin on
                 a 4-byte boundary?  Doesn't this lead to an extra operation
                 when using the AHSLength?
    
    +++ 4 byte boundaries are somewhat artificial - no boundary is needed on
    the wire. We introduced it when frame markers where introduced marker and
    we use it only when strictly necessary +++
    
      17)        p 147: it should be explicitly stated that the O and U bits
    are
                 valid only of the S bit is 1.
    +++ added it - although the statement about usage indicates affinity to
    SCSI response +++
    
      18)        p 150: first paragraph, last line: the requirement should be a
                 MUST, not a SHOULD; it is not obvious what an initiator
                 should do if a transfer length of 0 is received.  If the
    SHOULD
                 requirement is maintained, then explain what is an initiator
    to do
                 with an R2T containing a zero transfer length.
    +++ fixed +++
    
      19)        p 152: for code 1 -- "Close the connection" (if not the only
                 connection) OR "Close the session" to close all connections --
                 What must be sent for a single connection session?
    +++ if you do not intend to close the session send "close connection" and
    then you are free (within the llowed time) to login on a new connection. If
    you close the session the session is gone. +++
    
      20)        p 155: second paragraph: the requirement of MUST with the
                 current wording requires that one text request must be
                 outstanding at any given time.  This requirement should be
                 MAY, or the wording changed to "An initiator MUST have
                 at most one outstanding Text Request on a connection at
                 any given time."
    +++ fixed +++
      21)        p 156 (and other areas): next to last paragraph beginning with
                 "A target MAY reset its internal state".  What should it be
    reset
                 to, the original or default settings, the previous state prior
    to
    this
                 sequence of text requests, or ...  A similar reference is made
                 on p 159 in each of the last 2 paragraphs.  Doesn't it say
                 somewhere in the spec something about a stateless exchange?
                 If so, what state are you talking about?
    +++ the current negotiation state - (related to the current ITT) I will
    state that ++
      22)        p 162: section 9.12.4: in this section, the version of the
    current
                 draft is defined.  Shouldn't this info be in a more obvious
    location
                 in the draft?  How would someone reading this spec know to
                 come to this section to find out this info?
    +++ It is only for the login to state the version. I will add this to the
    front matter +++
    Formatting and wording issues
       1)        p 26: second paragraph, last line: is the phase 'an
                 individual I/O device is called a "logical unit" LU'
                 consistent with the definition of "CSSI device" earlier
                 in the document.
    
    +++ NO - for details see SAM This paragraph is meant as an level setting
    for an overvie the SCSI Device is a formal element in SCSI lingo+++
    
       2)        p 27: first paragraph of 2.2, fourth line, "the request
                 response mechanism" should be "the request-response
    
           mechanism".
    +++ fixed +++
       3)        p 28: paragraph preceding section 2.2.2, last sentence,
                 "For error recovery purposes, targets ... during recovery"
                 contains some redundancy.  Remove the words "during
                 recovery" from the end of the sentence.
    +++ the statement stresses tha 2 connection may have to be supported ONLY
    during recovery +++
       4)        p 34: third paragraph, third line: the phrase "later in
                 this part" is used.  What is the "part" that is being
                 referenced?
    +++ section - fixed +++
       5)        p 35: second paragraph, third line: the use of the phrase
                 "by default" implies a non-default behavior.  This phrase
                 should be deleted.
    +++ fixed +++
       6)        p 36: last paragraph, third line, does the phrase "these
                 tags" refer to initiator tags or target tags?  The current
                 wording could be ambiguous.
    +++ fixed - target tags +++
       7)        p 39: item c) in list, fourth line: "would be identical except
    
                 for their case" could be "are case sensitive".
    +++ fixed removed also MUSTs as this part is a requirement description part
    not an encoding norm +++
       8)        p 39: third paragraph from bottom, fourth line: "NIC or
                 HBA card".  The word "card" is redundant.
    +++ fixed +++
       9)        p 53: first paragraph after section 2.4.3, last line: "with a
                 given session" should be "with the same session".
    +++ fixed +++
      10)        p 54: last paragraph, first line: "via one iSCSI node" should
                 be "via one iSCSI target node"
    +++ fixed +++
      11)        p 66: last paragraph, fifth line: "key=value pair TargetName"
                 What does this mean?
    +++ I've made it TargetName key=value pair. Is this better? +++
      12)        p 82: for -T6 and -T7: the individual cases should be
                 maintained in a table-like list, or at least separated by
                 semi-colons.  -T15, T16 on page 83 has a nice table-
                 like list.  Why not use this format for consistency.
    +++ did i prompted by an earlier mail +++
      13)        p 97: after last paragraph: formatting (indentation) would
                 assist in understanding.
                             - item a) additional blanks near end of first line
                             - between item a) and i), there is an extra '-'
                             - at end of ii), the "[OR]" should refer to a) or
    b),
                                         however, the placement could indicate
                                         ii) or b).  This could lead to an
    ambiguous
                                         interpretation.
    
    +++ fixed (I think :-) +++
     14)         p 104: bullet item near top beginning with "- N.B. The
    logout".
                 Should this bullet item be here?
    
    +++ fixed +++
    
      15)        p 108: first and third paragraphs, first line in each:
    paragraph
                 begins with "With this mechanism".  What do you mean?
                 Yes, I know, but you need to say it. :-)
    +++ fixed +++
      16)        p 108: third paragraph, line 4: the sentence ends in the
                 middle: "received in clear"
    +++ does not look this way to me - it says that without IPsec .... +++
      17)        p 118: section 9.2, first paragraph "may" should be
                 capitalized, similarly in second paragraph, second line.
    
    +++ fixed +++
    
      18)        p 119: first paragraph, last line "SHOULD be sent as 0" should
                 be "SHOULD be set to zero".  Also, why is SHOULD used here
                 instead of MUST?
    +++ sent means on the wire and as the receiver must not check them we felt
    SHOULD is enough +++
    
      19)        p 125: in SCSI Command PDU, very last field: "(DataSegment
                 - Command Data + Data Digest (if any))" should be
                 "(DataSegment or Command Data, Data Digest) (if any)"
    +++ fixed ++
    
      20)        p 126: paragraph preceding 9.3.2: "Expected Data Transfer
                 Lengths are" should be "Expected Data Transfer Length and
                 Bidirectional Read Expected Data Transfer Length are"
    
    +++ it said Lenths - but I made it explicit as you suggested ++++
    
      21)        p 144: last paragraph on page, line 5: "in this later case"
                 should be "in this latter case".
    +++ fixed - I will suggest we write the next RFC in Hebrew - at list
    spelling is simpler :-) +++
    
      22)        p 153: last paragraph: "Data Segment" should be
                 "DataSegment".
    +++ fixed +++
      23)        p 155: last paragraph: the word "commands" should be
                 replaced by "Text Requests"
    +++ fixed +++
      24)        p 162: the list in the middle of the page: why was the number
                 2 skipped in numbering the items in this list?
    +++ just a coding choice - it is not a bullet number - but a code ++
      25)        p 164: next to last line: "All login requests within the login
                 phase" should be "All login requests within a login phase".
    +++ fixed +++
      26)        p 182: third paragraph, second line: should the "must" be
                 capitalized?
    +++ more of a SCSI requirement but I have capitalized it +++
      27)        p 184: last line: "Data Segment Length" should be
                 "DataSegmentLength"
    +++ fixed +++
      28)        p 188: the info below the table at the top of the page should
                 be incorporated into the table.
    +++  fixed +++
      29)        p 188: last paragraph, first line: "the target MUST answer"
                 could be "the target MUST respond" (keeping with the
                 language of the spec.
    +++ fixed +++
      30)        p 193: section 11, this info would be nice in a table, and
                 also in an acronym table.
    +++ are you volunteering ? +++
      31)        p 198, 199, 200, 203, 203: what do
                             Result function is OR                     and
                             Result function is AND
                 mean?
    +++ means that the result an AND or OR boolean function of the offer and
    what the responder would have to say +++
      32)        p 200: third paragraph, last line "MUST reject them with".
                 What does "them" refer to?
    +++ I've replaced them with unsolicited data +++
      33)        p 201: Third paragraph "This is a connection specific
                 parameter".  This statement is redundant because the
                 scope is CO.
    +++ removed +++
      34)        p 201: last paragraph of section 11.14: what "two numbers"
                 are being referenced?  Same question can be asked on
                 p 202, second paragraph from top, and also in section
                 11.16, second last paragraph.
    +++ Great - this was from the time when selection was done by "observer"
    not responder.
    I have changed it to:
    
    The responder MUST select a value that does not exceed the offered value.
    ++++
    
    Simple issues
    
       1)        p 24: last paragraph, second line, "session ID that is
                 tuple" should be session ID that is a tuple".
    +++ fixed +++
       2)        p 25: definition of "SCSI Port Name", get rid of the
                 word "basically".  Use of this adverb implies there
                 are additional items that make up the name.
    +++ fixed +++
       3)        p 25: definition of "TSIH", third line, change "the target
                 is generating" to "the target generates".
    +++ fixed +++
       4)        p 32: section 2.2.2.2, first paragraph, line 5: "32bit"
                 should be "32-bit" (the '-' could be a blank).
    +++ fixed +++
       5)        p 35: third paragraph, second line: "the status for a
                 command" should be "the status for the command"
    +++ fixed +++
       6)        p 55: last paragraph, need a blank line between paragraphs.
    +++ fixed +++
       7)        p 56: third paragraph, last 2 lines (occurs twice): "at
                 target" should be "at the target".
    +++ fixed +++
       8)        p 56: third paragraph, second line: "Data-In PDU" should
                 be "Data-In PDUs".
    +++ fixed +++
       9)        p 57: last paragraph, third line: "status indicated
    termination"
                 should be "statue indicates termination".
    +++ fixed +++
      10)        p 60: first paagraph after 2.5.3.4, fourth line: "the type is
                 indicate in" should be "the type is indicated in"
    +++ fixed +++
      11)        p 63: second paragraph, next to last line: "range is a a set"
                 should be "range is a set".
    +++ text changed +++
      12)        P 64: fifth paragraph, second line: "a single literal constant
                 a Boolean value" should be "a single literal constand, a
                 Boolean value".
    +++ text changed +++
      13)        p 65: after first paragraph there is an extra blank line.
    
    +++ text changed +++
      14)        p 68: paragraph preceding 4.3.1, second line: period '.'
                 missing after "once during login".
    +++ fixed +++
      15)        p 73: fourth paragraph, first line: "implicit logout
    connection
                 reinstatement is" should be "implicit logout connection,
                 reinstatement is"
    +++ fixed +++
      16)        p 74: first paragraph before 4.3.6, last line: there is some
                 garbage at the end of the paragraph.
    +++ fixed +++
      17)        p 82: for transition -T8, second line: "on a another
    connection"
                 should be "on another connection"
    +++ fixed +++
      18)        p 93: First paragraph, fifth line: "assumed that at target"
                 should be "assumed that at the targer"
    +++ fixed +++
      19)        p 95: throughout the text, words like "Reject" is used in
                 referring to a PDU type.  The problem is the use of the
                 plural "Rejects"; this would be better written as "Reject's".
    +++ fixed only 2 occurences! +++
      20)        p 96: first paragraph after 6.3.2, third line: "(section
    9.15)in"
                 should be "section 9.15) in".
    +++ fixed +++
      21)        page 100: second paragraph after 6.9, second line: "errors
                 can be only be" should be "errors can only be"
    +++ fixed +++
      22)        p 104: third line on page: "not received for a response"
                 should be "not received a response".
    +++ fixed +++
      23)        p 105: bullet item b) near top of page, last line: "in
    resource
                 requirement" should be "in resource requirements".
    +++ fixed +++
      24)        p 107: first paragraph, fourth line: "via a subnet distinctly"
                 could be "via a SAN distinctly".
    +++ fixed +++
      25)        p 107: first paragraph, line line 6: "such as SCSI, over IP
                 networks requires" should be "such as SCSI over IP,
                 requires"
    +++ I've made it IP-networks to disambiguate +++
      26)        p 108: after paragraph 4, there is an extra blank line
    +++ removed +++
      27)        p 108: sixth paragraph, fourth line: it looks like an extra
                 <CR> was added at the end of this line.
    +++ removed +++
      28)        p 113: first paragraph after 8.1.2, first line: it looks like
    an
                 extra <CR> was added at the end of this line.
    +++ fixed +++
      29)        p 114: third paragraph, second line: it looks like an extra
                 <CR> was added at the end of this line.
    +++ fixed +++
      30)        p 115: first paragraph after 8.3, fourth line:
    "acknowledgements
                 etc.)" should be "acknowledgements, etc.)"
    +++ fixed +++
      31)        p 122: section 9.2.1.7, first paragraph, second line:
    "identify it"
                 should be "identify the task".
    +++ fixed +++
      32)        p 128: last line of page: "the b Bidirectional" should be
                 "the Bidirectional".
    +++ fixed +++
      33)        p 129: for bit 5 at the top, third line: "Transfer length"
    should
                 be "Transfer Length".
    +++ fixed +++
      34)        p 129: for bit 6  at the top, third line: "bytes that expected
    to
    be"
                 should be "bytes that were expected to be".
    +++ fixed +++
      35)        p 130: third line from top of page, "sequence, before" should
    be
                 "sequence before"
    +++ fixed +++
      36)        p 130: third line from bottom of page: use "greater than"
    rather
                 than "higher than".  "Higher" could imply some level of
                 positioning
    +++ fixed +++
      37)        p 134: the fields in the number list at the bottom of the page
                 should be lined up.
    +++ I will try to make clear that it is a list of codes and not a table
    with fields +++
      38)        p 138: why is this page blank?
    +++ fixed +++
      39)        p 145: second paragraph of 9.7.3, third line: "SNACK of,  type
    "
                 should be "SNACK of type".
    +++ fixed +++
      40)        p 147: last paragraph: "and they values are as define in"
    should
    +++ fixed +++            be "and the values are as defined in"
    
      41)        p 151: PDU diagram, align the right side
    +++ tool problem +++
      42)        p 154: PDU diagram, align the right side
    +++ tool problem +++
      43)        p 156: first paragraph, first line: it looks like an extra
                 <CR> was added at the end of this line.
    +++ fixed +++
      44)        p 159: section 9.11.2, first line: "initial Text request"
    should be
                 "initial Text Request".  Similarly, in last paragraph, second
                 line, "Text request" should be "Text Request".
    +++ fixed +++
      45)        p 162: section 9.12.2, first paragraph, next to last line, it
                 looks like an extra <CR> was added at the end of this line.
    +++ fixed +++
      46)        p 164: section 9.12.6, first paragraph, fifth line: "to the
    target,
                 the associated" should be "to the target the associated"
    +++ fixed +++
      47)        p 165: last paragraph: missing a blank line after line 3.
    +++ fixed +++
      48)        p 165:last paragraph: "Keys in Chapter 10, only need" should
                 be "Keys in Chapter 10 only need"
    +++ fixed +++
      49)        p 167: section 9.13.3, fifth line: "andd" should be "and".
    +++ fixed +++
      50)        p 169: for codes 0101 and 0201, remove the <CR> in the
                 description fields.
    +++ ???? +++
      51)        p 178: first 2 paragraphs: what do "its" refer to -- "its
    flags"
    +++ the two paragraphs read now:
    The numbered-response(s) or R2T(s), requested by a SNACK, MUST be delivered
    as exact replicas of the ones the initiator missed except for the fields
    ExpCmdSN, MaxCmdSN and ExpDataSN which MUST carry the current values.
    
           The numbered Data-In PDUs, requested by a SNACK with a RunLength
           dif-ferent from 0, MUST be delivered as exact replicas of the ones
           the initiator missed except the fields ExpCmdSN and MaxCmdSN which
           MUST carry the current values.  Data-In PDUs requested with
           RunLength 0 (meaning all PDUs after this number) may be different
           from the ones originally sent, in order to reflect changes in
           MaxRecvPDULength.
           ++++
             52)                p 179: section 9.16.3, second paragraph, second
           line: "or
                      greater to BegRun" should be "or greater than BegRun"
           +++ fixed +++
             53)                p 181: code 0x04 in table, the explanation for
           this code is
                      missing a right paren at the end.
           +++ fixed +++
             54)                p 186: second paragraph, first line: "When a
           target send a
                      NOP-In" should be When a target sends a NOP-In".
           +++ fixed +++
             55)                p 193: bottom: position entire table on one
           page.
           +++ done +++
             56)                p 196: missing blank line between paragraphs at
           top of page.
           +++ fixed +++
             57)                p 199: first paragraph on top of page, it looks
           like an extra
                      <CR> was added in the middle.
           +++ fixed +++
             58)                p 201: section 11.14, second to last paragraph,
           second line:
                      "bytes in an Data-In" should be "bytes in a Data-In".
           Also,
                      the last phrase in this paragraph is not a sentence.
           +++ fixed +++
             59)                p 201: last line of page: "to the target,
           during the" should be
                      "to the target during the
           +++ fixed +++
    
    
    
    
    
    
    
    
    
    
    
    
    


Home

Last updated: Mon May 13 12:18:37 2002
10090 messages in chronological order