Subject: [storm] Comments on draft-ietf-storm-iscsi-sam-00.txt

From: <[email-masked]>
Date: Wed, 24 Feb 2010 17:33:35 -0500

<WG Co-Chair hat OFF>

With the WG last call on the iFCP-related drafts completed (summary message coming soon from Tom), it's time to turn attention to this draft.  This draft has been subject to significant review from a number of iSCSI-knowledgeable people prior to submission, so it's in very good shape for a -00 draft.

I have a few comments, mostly intended to start discussion.  To respond to this message, please pick one of the comments below to respond to in a single email message, and CHANGE THE SUBJECT LINE of the email.

[1] UML, model and terminology mapping.

The UML, model and terminology mapping in Sections 3-5 need to be done, and in particular it's crucial to permit the device structure example shown at the end of Section 5 (RFC 3720 does not permit this, but I suspect that to be an oversight as opposed to intentional).  I wonder whether this material would be better moved to the consolidated iSCSI draft, as that may be a more appropriate home for it; that decision does not need to be made now.

[2] RFC 3720 change summary

I would like to see a "Summary of Changes to RFC 3720" section, this could be accomplished by expanding Section 2.3 to list the specific changes, which are:

- iSCSI Node may contain both initiator and target functionality (Section 5)

- PRI field for SCSI Command Priority added to SCSI Command PDU (Section 7.1.1)
- Status Qualifier field added to SCSI Response PDU (Section 7.2.1)
- Sense data may be returned (via Autosense) for any SCSI status, not
	just CHECK CONDITION (Section 7.2.2)
- Four new task management functions: QUERY TASK, QUERY TASK SET, I_T NEXUS RESET,
	QUERY ASYNCHRONOUS EVENT (Section 8.2).
- New "function succeeded" response code for the new task management functions (Section 8.3.1)
- New PDUFormatForSAMUpdate text key to negotiate iSCSI support for the above features (except for "iSCSI node may contain both initiator and target functionality", which should be allowed no matter what, IMHO).

[3] Section 9.1.1.  I don't like the phrase "RFC compliance level" and suggest replacing the "This key ..." paragraph with

	This key is used to negotiate presence of iSCSI support for the features defined in this RFC.

I strongly suggest moving the "Note that ..." paragraph from Section 9.1.1 to Section 6.  I would also add an example. Here's a possible example that should help illuminate the distinction between SCSI and iSCSI errors:
- An iSCSI implementation may negotiate this new key to "Yes" but respond to the new task management functions with a "Task management function not supported" (that response reports a SCSI error that prevents the function from being performed).
- In contrast but if that key is negotiated to "Yes", an iSCSI implementation must not reject a Task Management Function Request PDU that requests one of the new task management functions (that reject reports an iSCSI protocol error).

[4] Nit: "compliant"

I don't like use of this word in RFCs, but this is an editorial matter of taste.  Here are a couple of suggested rewordings that I think also improve clarity:

- Section 7.1.1
OLD
   As defined in Section 10, iSCSI PDU Formats of [RFC3720],
   compliant senders already set this field to zero.
NEW
   Section 10 of [RFC3720] requires that senders set this field to zero.

- Section 8.2
OLD
   Any compliant sender MUST NOT send these task management function
   requests unless ...
NEW
   These task management function request MUST NOT be sent unless ...

[5] Minor Nit: text key name

While PDUFormatForSAMUpdate is not the key name I would have chosen, I didn't put in all the time and effort to write this draft ;-).  That key name is ok with me.

I'll plan to review this draft again when it gets to WG Last Call ...

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953             FAX: +1 (508) 293-7786
[email-masked]        Mobile: +1 (978) 394-7754
----------------------------------------------------