Re: [storm] WG Last Call - iSER - comments

[Michael Ko <[e-mail]>]

David,

Thanks for the review.  My comments  are inline.

Mike
----- Original Message ----- 
From: [e-mail]
To: [e-mail]
Sent: Friday, December 02, 2011 4:00 PM
Subject: [storm] WG Last Call - iSER - comments


This email is sent with WG chair hat off - all of these comments are for 
further discussion,
but they do need to be dealt with.

In addition to my previous comment on references:

> The iSER draft currently references RFC 3720 for iSCSI - that reference 
> will need to be
> updated to at least the new consolidated iSCSI draft, and a reference to 
> the iSCSI new
> features (SAM) draft should probably be added.

[mk] Done.

I've now done a relatively thorough review of draft-ietf-storm-iser-05.  It 
looks very good
overall, but I did find a number of additional things that need attention 
(items 5,6 and 8
are tagged as Major items):

1) Nit - In section 1.1's definition of connection, change "logical circuit" 
to "logical
bi-directional communication channel".

[mk] Done

2) The SAM-2 reference needs to be updated to the version of SAM-5 that the 
iSCSI SAM
new features draft uses, and that new features draft does need to be added 
as a normative
reference.

[mk] Done

3) There's a discussion of markers in Section 2.1.  I'd prefer that it be 
removed, but I
could live with it remaining, although it would have to informatively 
reference RFC 3720,
not the new consolidated iSCSI draft.

[mk] I have removed all references to markers.

4) In section 2.3, items 1 and 2 are inconsistent, as they use "session" and 
"connection"
for essentially the same concept.  I suggest changing "session" to 
"connection in item 1.


[mk] iSER-assisted mode is negotiated using the RDMAExtensions key in the 
leading connection, as stated in sections 5.1 and 6.3, to ensure that "an 
entire iSCSI session can only operate in one mode".  Hence the choice of the 
words "session" and "connection" in items 1 and 2 are correct.  I have added 
"in the leading connection" before "for each session" in the first sentence 
in item 1.

5) Major: There's something wrong with the discussion of how to send 
unsolicited data.
Item 4 in section 2.3 requires use of tagged buffers (RDMA), but the second 
paragraph in
Section 2.6 requires use of Send (untagged buffers, not RDMA), the new key 
in 6.9 appears
to allow unsolicited data in a tagged buffer (RDMA), but the next to last 
paragraph in
7.3.4 requires use of Send (untagged buffers, not RDMA).

[mk] Item 4 states that the control and all data information associated with 
the iSCSI data-type PDUs are handled in iSER using RDMA Write, RDMA Read 
Request and RDMA Read Response Messages.  The reader is referred to section 
7.1 for the meaning of "iSCSI data-type PDUs", but item 4 itself does not 
mention tagged buffers.  I have reworded item 4 for clarity.  The new key in 
6.9 is intended to resolve the issue of how the I/O buffer is used.  In 
iSCSI (RFC3270), the I/O buffer is used to contain all the data associated 
with the SCSI operation.  Some of this data can be transferred in an 
unsolicited fashion (using Send Messages), while the rest is transferred 
using RDMA.  This new key restricts of the use of this buffer for solicited 
data only, as stated in 6.9.  2.6 and 7.3.4 are correct as stated.

6) Major: I think something is missing in Section 5 to explains how to 
conduct iSCSI
negotiation when the connections start up in iSER-assisted mode.  I assume 
that this is
done via Send messages and the resource allocation referred to is the 
resources for RDMA,
but that does need to be explained.

[mk] In this version, iSER is changed to remove the requirement that the 
connection transitions from TCP mode to RDMA mode.  It does not require that 
login negotiations be done using Send Messages.

7) The first paragraph of Section 8.2 describes what happens when 
iSERHelloRequired is
negotiated to "Yes" - add a few sentence to explain what happens when it's 
negotiated
to "No", which is the typical case for implementations.  A similar problem 
occurs in
10.1.3.4 - that one's easily handled by saying that these errors cannot 
occur in the "No"
case.  Please check for all other dependencies on the negotiated value of
iSERHelloRequired, and make sure that both the "Yes" and "No" cases are 
covered.

[mk] Done

8) Major: The IANA Considerations section is empty.  That is wrong - the new 
keys defined
in sections 6.8 - 6.10 need to be registered in the iSCSI Login/Text Keys 
registry at: http://www.iana.org/assignments/iscsi-parameters
IANA also should be requested to update the registrations of the other 4 
iSER keys in that
registry to reference the RFC number of this draft when it is published as 
an RFC.

[mk] Done.

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

_______________________________________________
storm mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/storm