[storm] iscsi-sam draft: David Black's WG Last Call comments

["Black, David" <[e-mail]>]

This draft is in reasonably good shape - it's been reviewed enough
times ;-).

The IETF reviewers (e.g., Sec-Dir, Gen-ART) aren't acknowledged
in the acknowledgements section - I'll leave the decision about
whether to acknowledge them (and the ADs who made comments) up
to the authors.

I found 7 things that need attention, most of them minor.

-- Two significant issues:

[1] Section 7.1.1 still contains the "logical superset" text:

     Negotiation of the iSCSIProtocolLevel key to a value
     corresponding to an RFC indicates that both negotiating parties
     are compliant to the RFC in question, and agree to support the
     corresponding PDU formats and semantics on that iSCSI session.
     An operational value of iSCSI ProtocolLevel = "x" on an iSCSI
     session requires that the iSCSI protocol semantics on that iSCSI
     session be a logical superset of the capabilities in all RFCs
     that have claimed values of an iSCSIProtocolLevel less than "x".

The above paragraph needs to be removed in favor of the paragraph in
the IANA considerations section that begins with the following three
sentences:

     Special care must be taken in the assignment of new values in
     this registry. Compatibility and interoperability will be
     adversely impacted if proper care is not exercised. Features
     using this key are expected to be cumulative.

That third sentence above captures the requirement without causing
problems if T10 decides to reduce functionality (e.g., hypothetically,
T10 could decide in the future to obsolete one of the new task
management functions for which this draft specifies iSCSI support).

The effect should be the same, as we're going to rely on the expert
for the registry to ensure that the proverbial "right thing" happens.

[2] Some changes are needed to the IANA registry for the
iSCSI Protocol Level values.

The proximate reason for these changes is that T10 is going to
require that the words "No version claimed" be associated with
the value 0 in order to use these values to generate iSCSI
version descriptors for use in standard inquiry data (see
SPC-4, 6.6.2).  Those words need to appear in the registry.

OLD
     Fields to record in the registry: Assigned value, and its
     associated RFC reference.

     0, [RFCxxx]

     1, [RFC-cons]

     2, [RFCxxx]

NEW

     Fields to record in the registry: Assigned value, description
     (text string) and associated RFC reference.

     0, No version claimed, [RFCxxx]

     1, RFC-cons, [RFCxxx]

     2, RFCxxx features, [RFCxxx]

The reference for the value 1 is now this RFC, even though the features
that the value refers to are defined in RFC-cons, so that's another
reason to have both a description string and a reference.

The RFC EDITORS note needs some slight revision to deal with the fact that
I didn't put [square brackets] around the RFC numbers in the description.

-- One Procedural item

[3] The text in Section 4.2 on SCSI version descriptors requires that
T10 approve the associated changes (probably as a proposal adopted for
incorporation into SPC-5) before this draft is published as an RFC.
The easiest way to deal with this is to insert an RFC EDITORS NOTE at
the end of Section 4.2:

      --------------------------------------------------------
      RFC EDITORS NOTE: The specification text in this section requires
	corresponding changes in a SCSI standard (SPC-4 or SPC-5)
	that is developed by INCITS Technical Committee T10. Confirmation
	that these T10 changes have been made is necessary before
	publishing this draft as an RFC; the contacts for obtaining this
	confirmation are the draft authors and storm WG chairs.
      --------------------------------------------------------

If these changes wind up going into SPC-5, this draft may need to reference
SPC-5 - that can be dealt with when this draft is ready for RFC publication.

-- Editorial Nits:

[4] Section 4.1 contains this sentence twice:

     The iSCSIProtocolLevel operational text key (see 7.1.1)
     containing a value of "2" MUST be negotiated to enable the use of
     features described in this RFC.

Once instance will suffice ;-).

[5] Near end of 2nd paragraph in Section 4.1:

     sufficient for hse of the SCSI capabilities enabled by the iSCSI
     features in this RFC.

"sufficient for hse" -> "sufficient for use"

[6] At the end of section 5.1.1

  See [SAM5] for special considerations on the use of the priority
  field.

The meaning of "special" is unclear in this context - "additional"
would be better.

[7] Abstract

Silly rule - reference citations are forbidden in the Abstract.
Change both instances of "[draft-ietf-storm-iscsi-cons-xx]" to
"RFCxxx" and make the corresponding change to the RFC EDITORS
NOTE.

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
[phone]             FAX: [phone]
[e-mail]        Mobile: [phone]
----------------------------------------------------