FW: ISCSI: comments on iSCSI 14. (resend)

["Elizabeth G. Rodriguez" <Elizabeth.G.Rodriguez@123mail.net>]

My comments do not seem to have made it to the list.
Used wrong email account initially.
Resend...
Apologies if you receive twice.

Elizabeth

-----Original Message-----
From: Elizabeth Rodriguez [mailto:ElizabethRodriguez@ieee.org] 
Sent: Sunday, July 07, 2002 4:15 PM
To: 'ips@ece.cmu.edu'
Cc: 'Julian Satran (Actcom)'; John Hufferd (hufferd@us.ibm.com); David
Black (black_david@emc.com); 'Julian_Satran@il.ibm.com'
Subject: ISCSI: comments on iSCSI 14.


Here are my working group last call on the iSCSI document.
This was a review of version 14 of the document.
All page and section numbers refer to that document.
All comments are my own as an individual contributor, as opposed to WG
co-chair.

Thanks,

Elizabeth Rodriguez



Technical

1) Acknowledgements, pg 3, para 3
  This document has to be considered together with the "Naming & Dis-
  covery"[NDT], "Boot"[BOOT] and "Securing iSCSI, iFCP and FCIP"[SEC-
  IPS] documents.

EGR What about the requirements doc, string profile, SLP and iSNS?
Think the first should be present in this list; not certain about the
others in this list in particular.

2) Section 2.2.1. p. 34, last paragraph
   iSCSI targets and initiators MUST support at least one TCP connec-
   tion and MAY support several connections in a session. For error 
   recovery purposes, targets and initiators that support a single 
   active connection in a session may have to support two connections 
   during recovery.

The may in the last sentence (...session may have...) needs to be "MAY
need".  Assuming MAY, since if the device only supports
ErrorRecoveryLevel 0, will not need to support multiple connections.

3) Section 2.2.2.1 p 36, para 2, line 3

Immediate commands can be rejected by the iSCSI target 
  due to lack of resources.

Believe the 'can' needs to be a MAY, due to the fact that the initiator
must be able to support the target rejecting the immediate command.

4) section 2.2.2.1, pg 36, para 3 
With the exception of the commands marked for immediate delivery, the 
  iSCSI target layer MUST deliver the commands for execution in the 
  order specified by CmdSN. Commands marked for immediate delivery may 
  be handed over by the iSCSI target layer for execution as soon as 
  detected. iSCSI may avoid delivering some commands for execution if 
  required by a prior SCSI or iSCSI action (e.g., CLEAR TASK SET Task 
  Management request received before all the commands on which it was 
  supposed to act). Delivery for execution means delivery to the SCSI 
  execution engine or an iSCSI-SCSI protocol specific execution engine 
  (e.g., for text requests).

a) may needs to be MAY.  "Commands marked for immediate delivery may"
b) may needs to be MUST in "iSCSI may avoid"..., since cannot deliver
immediate command prior to the command execution itself.  

Alternatively, can change paragraph to state something to the effect of
"Commands marked for immediate delivery MUST be handed over by the iSCSI
target layer for execution as soon as detected, unless required not to
by a prior SCSI or iSCSI action..."

5) section 2.2.4, pg 41, para 2

  For an iSCSI request issued over a TCP connection, the corresponding 
  response and/or requested PDU(s) MUST be sent over the same connec-
  tion. We call this "connection allegiance". If the original connec-
  tion fails before the command is completed, the connection allegiance 
  of the command may be explicitly reassigned to a different transport 
  connection as described in detail in Section 6.1 Retry and Reassign 
  in Recovery.

a) Since an exception follows the MUST, probably should note that here,
e.g. following "same connection" add "(connection allegiance) unless a
connection failure has occurred and connection allegiance has been
reassigned. " and delete the following sentence.

b) may in last sentence should be MAY -- connection allegiance of the
command may be ...

6) Section 2.2.6.1, pg 44, bullet b

b)  iSCSI names must be permanent.  An iSCSI initiator or target 
    has the same name for its lifetime.

Recommend this be changed to ...iSCSI initiator node or target node...

7) Section 2.2.6.3.2, pg 48

   The IEEE Registration Authority provides a service for assigning glo-
   bally unique identifiers [EUI].  The EUI-64 format is in use as a 
   global identifier in other network protocols such as Fibre Channel. 
   See http://standards.ieee.org/regauth/oui/index.shtml - for more 
   information on registering for EUI identifiers.

Fibre Channel does not support the EUI-64 directly.  Instead, it has a
method of encoding an EUI-64 into the WWN.   But I know of no place that
Fibre Channel uses an EUI-64 directly.

8) Section 2.2.6.3.2, pg 49

Should a caution be added to this section on use of EUI format, since
names are not tied to HW.  Actuall applies to both formats, so maybe
should go in 2.2.6.3.

9) Section 2.2.8.2, pg 51, para following table

An implementation may choose to place Sync and 
  Steering somewhere else in the stack...

May should be MAY.  

10) Section 2.2.8.2, pgs 51-52

  The Sync and Steering Layer (which is OPTIONAL) MUST retain the PDU 
  end address within the stream for every delivered iSCSI PDU.
  To enable the Sync and Steering operation to perform Steering, addi-
  tional information, including identifying tags and buffer offsets, 
  MUST also be retained for every sent PDU. The Sync and Steering Layer 
  is required to add enough information to every sent data item (IP 
  packet, TCP packet or some other superstructure) to enable the 
  receiver to steer it to a memory location independent of any other 
  piece.

This paragraph needs to be clarified.  Sync and steering is optional --
on both send and receive?
Is this negotiated somewhere if sync and steering is being used?
We have an optional function that has MUSTs associated with it.

11) Section 4.2, pgs 72-

Talk about F and T bits through-out this section, but no description of
what F and T bits are, no intro, does not point to further information. 

12) Section 6, pg 103, para 2

It is further assumed that a target will keep the "status & sense" 
   for a command it has executed if it supports status retransmission.

Wording should be changed to something along the lines of "If OPTIONAL
status retransmission is supported, (or better yet, since status
retransmission only occurs as part of ErrorRecoveryLevel 2, correct?,
"If ErrorRecoveryLevel 2 is supported, ) then the device MUST keep
status and sense data for a previously completed command."  This then
leads to the question "For how long must this data be kept around?"
Some multiple of Time2Wait or Time2Retain?.  

13) Section 6.1.2, p. 104

In reassigning connection allegiance for a command, the targets 
   SHOULD continue the command from its current state. For example, when

   reassigning read commands, the target SHOULD take advantage of Exp-
   DataSN field provided by the Task Management Function Request (which 
   must be set to zero if there was no data transfer) and bring the read

   command to completion by sending the remaining data and sending (or 
   resending) the status.  However, targets MAY choose to send/receive 
   the entire data on a reassignment of connection allegiance, and it is

   not considered an error.  For all types of commands, a reassignment 
   request implies that the task is still considered in progress by the 
   initiator and the target must conclude the task appropriately.  This 
   might possibly involve retransmission of data/R2T/status PDUs as nec-
   essary.

The SHOULDs and MAY need to be re-examined.  The paragraph indicates
that the command SHOULD be continued from its current state but that it
MAY instead choose to send/receive the entire data reassignment, and it
will not be in error.  That seems to contradict the SHOULD that precedes
it.  Perhaps change the two SHOULDs in this statement to lower case
should.

14 Section 6.1.2, p. 104, last para

   It is optional for targets to support the allegiance reassignment.  

This should be OPTIONAL.

15. Section 6.12.3, pg 114

There are MUSTs/SHOULDs/MAYs listed in this section that are only valid
if Connection Recovery is supported.
While this is noted elsewhere, it needs to be noted in this section as
well. 

16) Section 7.2, pg 119

During login the target authenticates the initiator and the initia-
   tor optionally authenticates the target. The authentication is per-
   formed on every new iSCSI connection by an exchange of iSCSI Login 
   PDUs using a negotiated authentication method.

Since this is a requirement, think this should be "During login the
target MUST authenticate the initiator.  The Initiator MAY OPTIONALLY
authenticate the target."

17) Section 8.1.1, pg 124

To both minimize the disruption of legacy applications and to better 
   facilitate the SCSI features that rely on persistent names for SCSI 
   ports, iSCSI implementations should attempt to provide a stable pre-
   sentation of SCSI Initiator Ports (both to the upper OS-layers and to

should needs to be SHOULD, since this is a requirement.

18) Section 8.1.1, p. 125

In other words, 
   the same ISID should be used in the Login process to multiple target

Again, needs to be SHOULD, since this is a requirement.

19) Section 8.1.2, p. 125

For targets, because of the closed environment, implementation of 
   this entity should be straightforward. However, vendors of iSCSI 
   hardware (e.g., NICs or HBAs) intended for targets, should provide 
   mechanisms for configuration of the iSCSI Node Name across the por-
   tal groups instantiated by multiple instances of these components 
   within a target.

Should needs to be SHOULD.

20) Section 8.1.2, p. 126

A vendor of iSCSI hardware (e.g., NICs or HBAs) intended for use in 
  the initiators must allow

Since a requirement, must needs to be MUST.

21) 9.1, pg. 130

iSCSI PDUs are padded to the closest integer number of four byte 
   words. The padding bytes SHOULD be 0.

Needs to be a MUST instead of a SHOULD

22) 9.2, pg. 131

   All PDU segments and digests are padded to the closest integer num-
   ber of four byte words - i.e., all PDU segments and the digests start

   at a four byte word boundary and the padding ranges from 0 to 3 
   bytes. The padding bytes SHOULD be sent as 0.

Needs to be a MUST instead of a SHOULD.

23) Section 9.2.1.2, pg. 132

   Initiators MUST NOT use target opcodes and targets MUST NOT use ini-
   tiator opcodes.

Do we need a note here indicating that a single device may function in
both roles, but must adhere to the rules applicable to each role as
independent entities?

24) Section 9.4.6

iSCSI targets MUST support and enable autosense. If Status is CHECK 
   CONDITION (0x02), then the Data Segment contains sense data for the 
   failed command.

Contains needs to be changed to "MUST contain"

25) Section 9.5.1, p 148

The TARGET WARM RESET function MAY also be subject 
  to SCSI access controls (see [SPC3]) on the requesting initiator

Change 'MAY' to 'is'.  This is a SCSI function, not an iSCSI one.  It is
subject to SCSI access controls.

26) Section 9.5.1, p. 148


  When executing the TARGET WARM RESET and TARGET COLD RESET func-
  tions, the target cancels all pending operations.

Should add "across all logical units" of the target device".  May also
want to add strong warning on Target Reset functions.

27) Section 9.6.1, p 152

For the TARGET COLD RESET and TARGET WARM RESET functions, the tar-
  get cancels all pending operations.  

Should add "across all logical units" of the target device". 

28) Section 9.7.2, pg. 156

Need to add statement to this section that states "the A bit MUST NOT be
set in any sessions in which the ErrorRecoveryLevel is 0".  

29) Section 9.7.3

On incoming data, the Target Transfer Tag MUST be provided by the 
   target if the A bit is set to 1.

This is only applicable if the ErrorRecoveryLevel is set to 1 or
greater.  This should be noted. Also, what action should be taken (error
returned, termination, etc)  if the A bit is set, and the
ErrorRecoveryLevel is 0?

30) Section 9.11.1, pg. 172

A Text Response with the F bit set to 1 MUST NOT contain key=value 
   pairs that may require additional answers from the initiator.

The 'may' should be eliminated.

31) Sections 9.14.1, 9.14.2, pg. 175

Recommend some change here.  As of this version of the document, the
only value that may be used for either Version-Max or Version-Min is
0x00.
That is not clear here.

32) Section 9.14.1, pg 189

If the tasks terminated in any of the above cases are SCSI tasks, 
   they MUST be internally terminated with CHECK CONDITION status with a

   sense key of unit attention and ASC/ASCQ values of 0x6E/0x00 (COM-
   MAND TO LOGICAL UNIT FAILED).  Note that this status is meaningful 
   only for appropriately handling the internal SCSI state aspects such 
   as queued commands because this status is never communicated back as 
   a terminating status to the initiator.

I believe this MUST should be a lower case should.  This is defining
internal SCSI behavior, not iSCSI behavior.  If the SCSI and iSCSI
device are self contained, the device may be able to address the issue
in some other manner, and should not be in violation of the
specification for internal behavior.

33) 9.16.1, pg. 194

Advisable to come up with table, related to ErrorRecoveryLevels/recovery
mechanisms as defined in 6.12, that indicates which of these SNACK
functions MAY be supported and which are REQUIRED to be supported at the
various ErrorRecoveryLevels/mechanisms..  E.g. DataACK is required for
ErrorRecoveryLevel 1, but Status ACK only seems to be required for
ErrorRecoveryLevel 2, further qualified to if it supports within
connection recovery. 

Q:  ErrorRecoveryLevel2 is comprised of both within Connection and
Within Command recovery, correct?  Is there any relationship as to how
these are supported.  E.g. is within-connection recovery support
required for within-command recovery?  Vice Versa?  Must both be
supported at ErrorRecoveryLevel2?

34) Section 9.16.1, p 195

   An iSCSI target that does not support recovery within connection MAY 
   reject the status SNACK with a Reject PDU. If the target supports 
   recovery within connection, it MAY reject the SNACK after which it 
   MUST issue an Asynchronous Message PDU with an iSCSI event that indi-
   cates "Request Logout".

If an initiator operates at ErrorRecoveryLevel 1 or higher, it MUST 
   issue a SNACK of type DataACK after receiving a Data-In PDU with the 
   A bit set to 1.  However, if the initiator has detected holes in the 
   input sequence, it MUST postpone issuing the SNACK of type DataACK 
   until the holes are filled. An initiator MAY ignore the A bit if it 
   deems that the bit is being set aggressively by the target (i.e.,

   before the MaxBurstLength limit is reached).

This section needs clarification.  SNACK support is required for
ErrorRecoveryLevels 1 and 2.  DataACK is required for recovery level 1 &
2 (see 2nd para).  The 1st paragraph seems to indicate that if
ErrorRecoveryLevel 1 only is supported, the target may reject the status
SNACK, so that means that Status SNACK may be supported at level 1, but
not required?  

35) Section 10.1, pg. 205

The authentication exchange authenticates the initiator to the tar-
   get, and optionally, the target to the initiator.  Authentication is 
   not mandatory to use but must be supported by the target and initia-
   tor.

Need a MUST here instead of must.

36) Section 10.5, pg 209

   To guarantee interoperability, initiators SHOULD always offer it as 
   one of the proposed algorithms.

The SHOULD needs to be changed to MUST.

37) Section 11.7, pg 214

   If an initiator has been configured with a human-readable name or 
   description, it may be communicated to the target during a Login 
   Request PDU. If not, the host name can be used instead. This string 
   is not used as an identifier, but can be displayed by the target's 
   user interface in a list of initiators to which it is connected.

   This key SHOULD be sent by an initiator within the Login Phase, if 
   available.

First paragraph indicates may be communicated. Second indicates it
SHOULD be communicated.  Change first paragraph to ". it SHOULD be
communicated.

38) Section 11.12, pg 217

The initiator and target negotiate support for immediate data. To 
   turn immediate data off, the initiator or target must state its 
   desire to do so. ImmediateData can be turned on if both the initia-
   tor and target have ImmediateData=Yes.

Believe must needs to be a MUST.

39) Section 11.12, pg. 217

   If ImmediateData is set to Yes and InitialR2T is set to Yes 
   (default), then only immediate data are accepted in the first burst.

   If ImmediateData is set to No and InitialR2T is set to Yes, then the 
   initiator MUST NOT send unsolicited data and the target MUST reject 
   unsolicited data with the corresponding response code. 

   If ImmediateData is set to No and InitialR2T is set to No, then the 
   initiator MUST NOT send unsolicited immediate data, but MAY send one 
   unsolicited burst of Data-OUT PDUs.

   If ImmediateData is set to Yes and InitialR2T is set to No, then the 
   initiator MAY send unsolicited immediate data and/or one unsolicited 
   burst of Data-OUT PDUs.

The above are the expected actions of the combination of InitialR2T and
Immediate Data.  Need to clarify that, either in this section, or move
these examples elsewhere.  In other words, prior to this set of
statements, put in a statement something to the following.
"ImmediateData and InitialR2T(see 11.10) settings result in the
following possible operations data flow:




Editorial
1.  General - For the version to be submitted to the IESG, we need to
make sure the formatting in the txt version is good.  Not sure what you
are using to generate txt version, erratic in format, especially
spacing.

2)  (nice to haves) 
a) Number figures and tables.  
b) Generate table of figures and table of tables following table of
contents.

3. Sync and Steering/Synch and steering.  
Need consistency.  Some places have Sync and Steering, where as other
have Synch and steering.  Consistent spelling for sync needed. 

4. Abstract

   The Small Computer Systems Interface (SCSI) is a popular family of 
   protocols for communicating with I/O devices, especially storage 
   devices. This document describes a transport protocol for SCSI that 
   works on top of TCP. The iSCSI protocol aims to be fully compliant 
   with the rules laid out in the SCSI Architecture Model - 2 [SAM2] 
   document. The current version of iSCSI is 0.

EGR: The last sentence needs clarification.  I know that for version
numbers, we are currently using 0.  But, it does not really make sense
in the contect of the abstract.  

5.  Acknowledgements

a) NuSpeed is now Cisco.  

b) Believe Paul Koning spells his name as listed here.  Believe he is
with EqualLogic.

c) This document has to be considered together with the "Naming & Dis-
  covery"[NDT], "Boot"[BOOT] and "Securing iSCSI, iFCP and FCIP"[SEC-
  IPS] documents.

EGR:  Suggested rewording to "In addition to this document, the
following must be considered in order to get a full understanding of the
iSCSI specification", then list documents.  

Insert "iSCSI" before "Naming & Discovery"

Should be "Bootstrapping Clients using the iSCSI Protocol" instead of
"Boot"

Security draft name change to "Securing Block Storage Protocols over IP"

d) We are grateful to all them for their good work and for helping us 
   correlate this document with the ones they produced.

EGR:  Insert "of" between "to all" and "them"

6) Change Log
EGR:  This section needs to be eliminated once we get through WG last
call, prior to forwarding to IESG. 

7) Section 1.2 Acronyms, pg 28
Gbps - GigaBits per Second.  

EGR:  Do not believe B should be capitalized in GigaBits.

8) Section 2.1, pg 32, para 3
   SCSI is a client-server architecture. Clients of a SCSI interface are

   called "initiators". Initiators issue SCSI "commands" to request ser-
   vice from a logical unit. The "device server" on the logical unit 
   accepts SCSI commands and processes them.

EGR:  Since define clients here as initiators, and cannot find similar
association for targets, recommend that you add a sentence after
..."called "initiators".", such as:

Servers of a SCSI interface are called "Targets".

Might also want to put in a description similar to what is in the
security draft that the concept of clients and servers and resources
associated with each is not necessarily the same in the storage world as
it is in many other areas.

9) section 2.2.6.2, p 45, 1st line in 2nd to last para

 Note that in most cases, the Stringprep process does not need to be    
   implemented if the names are generated using only lower-case (any 
   character set) alpha-numeric characters.

Stringprep should be lower case.  

10) section 2.2.6.3, p. 47, para 2

The type designator strings that may currently be used are:

Change to "The type designator strings currently defined are".  Think
may in the current sentence is too weak, since one of the two MUST be
used, as defined in following paragraph.  

11) section 4.2, pg 70, iSCSI-local-name-value

Typo -- nul should be null.

12) Section 4.2.1, pg 72, para 1

However, None is a valid selection only if it is explicitly 
   offered.

Need "" around None.

13) Section 5.1.  pgs 87-94

Decipher of state diagrams in sections 5.1.1 and 5.1.2 without 5.1.3 is
virtually impossible.  Confused as to why transition numbers were
missing, until saw 5.1.3.  Recommend either moving section 5.1.3 move
before 5.1.1, or adding intro on layout

14) Is there a reason for a blank page 117?

15) Section 7, pg 118

Although technically possible, iSCSI SHOULD NOT be configured with-
   out security. iSCSI without security should be confined, in extreme 
   cases, to closed environments without any security risk.

Suggestion:  Add "configured" between "iSCSI" and "without" in second
sentence.  Though correct, this stresses the configured part, since
mandatory to implement.

16) Section 7.2.2, pg 121, para 1

   Well- known SRP

Extraneous '-'

17) Section 7.3, pg. 121
Carriage Return missing prior to this section

18) section 9.5.1, p 148, para 4

Extraneous carriage return

19) Section 11.8, pg. 215

If the TCP port is not specified, it is assumed to be the IANA-
   assigned default port for iSCSI (3260)

Since the default port number  may be changed in the future, recommend
referring to the IANA considerations section for the default port
number, so that it needs to be updated in a single location in the
future, if it is changed

20) Full Copyright Statement.

Need to add the following boilerplate:

"The IETF has been notified of intellectual property rights claimed in
regard to some or all of the specification contained in this document.
For more information consult the online list of claimed rights."

Per Allison, this can be added directly to the end of the Full Copyright
Statement.