iSCSI: Seven minor draft 10 editorial comments

[Daniel Lee <dan@danielslee.com>]

Hi Julian,

Here are some more comments on the draft 10 PDF file.

Minor Editorial Comment #1

On page 105 the "9.4" section heading is missing from
the "Sync and Steering Layer and Performance" section.
 As a result, the next section entitled "Unsolicited
Data and Performance" is mislabled as "9.4" instead of
"9.5".

Minor Editorial Comment #2

There's a typo in Section 2.2.1, in the second
paragraph on page 26.  I've extracted the paragraph
below, the typo is beginning of the 4th sentence.

<ParagraphWithTypo>
Across all connections within a session, an initiator
sees one "target image". All target identifying
elements, such as LUN, are the same. In
addition, a target sees one "initiator image" across
all connections within a session. Initiator that
identifying elements, such as the Initiator Task Tag,
can be used to identify the same entity regardless of
the connection on which they are sent or received.
</ParagraphWithTypo>

I recommend rewriting the paragraph as follows:

<RecommendedReWrite>
An initiator sees one "target image" across all
connections within a session.  All target identifying
elements, such as LUN, are identical regardless of the
connection on which they are sent or received.  In
addition, a target sees one "initiator image" across
all connections within a session.  Initiator
identifying elements, such as the Initiator Task Tag,
are identical regardless of the connection on which
they are sent or received.
</RecommendedReWrite>

Minor Editorial Comment #3

Typo on page 234: 'Success' is misspelled as 'Sucess'

Minor Editorial Comment #4

For consistency and clarity reasons I recommend
changing the following pseudocode at the top of page
230 as follows.

<originalCode>
if (current StatSN is not expected) {
 Recover-Status-if-Possible(Connection, CurrentPDU);
}
if (current ExpCmdSN is not our NextCmdSN) {
 Retransmit-Command-if-Possible(Connection,
 CurrentPDU.ExpCmdSN);
}
</originalCode>

<suggestedChange>
if (CurrentPDU.StatSN is not expected) {
 Recover-Status-if-Possible(Connection, CurrentPDU);
}
if (CurrentPDU.ExpCmdSN != Session.NextCmdSN) {
 Retransmit-Command-if-Possible(Connection,
 CurrentPDU.ExpCmdSN);
}
</suggestedChange>

In addition, the 'Session.NextCmdSN' should probably
be 'Connection.SessionReference.NextCmdSN', but it's
not used this way elsewhere in the pseudocode and it's
easy enough to understand the way it's currently used.

Minor Editorial Comment #5

In the pseudocode for the previous Editorial Comment
(on page 230) and in the 'struct Session' definition
on page 222, it might make sense to change 'NextCmdSN'
to just 'CmdSN' for consistency reasons (since on page
28 it says 'CmdSn always contains the number to be
assigned next').  

Minor Editorial Comment #6

The 'n' in 'CmdSn' on page 28 (referenced in Editorial
Comment #4) should be capitalized.

Minor Editorial Comment #7
(Note: I'll stop my Editorial Comments with this one
since 7 is a lucky number.)

I suggest the following line on page 29 be either
modified as follows or removed completely to make it
less confusing to first time readers of the spec.

<originalText>
-If the PDU MaxCmdSN is less than the PDU ExpCmdSN-1
(in Serial Arithmetic Sense), they are both ignored.
</originalText>

<suggestedChange>
-If the PDU MaxCmdSN is less than the PDU ExpCmdSN-1
(in Serial Arithmetic Sense), the target has made an
error generating this PDU so both fields are ignored.
</suggestedChange>

The reason I suggest this change is that it isn't
until a few lines down the in the spec that you learn
'The target MUST NOT transmit a MaxCmdSN that is less
than the last ExpCmdSN.'  (note: the previous line
might be clearer as 'The target MUST NOT transmit a
MaxCmdSN that is less than ExpCmdSN-1 (in Serial
Arithmetic Sense)'.

Regards,

Daniel Lee
Unemployed Person
dan@danielslee.com
"iSCSI, therefore I am"


__________________________________________________
Do You Yahoo!?
Send FREE Valentine eCards with Yahoo! Greetings!
http://greetings.yahoo.com