Re: [storm] WG Last Call - iSER - through December 12 - comments

["Hemal Shah" <[e-mail]>]

Mike,

Thanks! I replied to David's reply to my original message. I'm fine with your responses to 1-10 below. For 11-17, please look at my response to David's reply. Let us resolve those items using that email thread.

Hemal

From: Michael Ko [mailto:[e-mail]]
Sent: Wednesday, December 14, 2011 1:37 PM
To: Hemal Shah; [e-mail]; [e-mail]
Subject: Re: [storm] WG Last Call - iSER - through December 12 - comments

Hemal,

Thank you for the extensive reivew.  My comments are inline.

Mike
----- Original Message -----
From: Hemal Shah<mailto:[e-mail]>
To: '[e-mail]'<mailto:'[e-mail]'> ; [e-mail]<mailto:[e-mail]>
Sent: Monday, December 12, 2011 5:06 PM
Subject: Re: [storm] WG Last Call - iSER - through December 12 - comments

David and Mike,

Below are my comments on the iSER draft.

Minor


 1.  Section 1.1: iSCSI data-type PDU definition should clearly say at what layer the data transfer is happening. For example, the following sentence "An iSCSI data-type PDU is defined as an iSCSI PDU that causes data transfer, transparent to the remote iSCSI Layer, to take place between the peer iSCSI nodes on a full feature phase iSCSI connection." should be rephrased (added text in red) as "An iSCSI data-type PDU is defined as an iSCSI PDU that causes data transfer via RDMA operations at the iSER layer, transparent to the remote iSCSI Layer, to take place between the peer iSCSI nodes on a full feature phase iSCSI connection. I believe that the definition is already being revised based on David's comments.  [mk] OK if no one else objects.
 2.  Section 1.1: The definition of iSCSI/iSER session needs to have further description. The current description is insufficient. Suggest to add the following to the definition "All connections of an iSCSI/iSER session are iSCSI/iSER connections."  [mk] OK if no one else objects.
 3.  SNIC term is not defined but used in this draft. This needs to be addressed.  [mk] Simpler just to delete the reference to SNIC.
 4.  Sections 2.4.3 and 2.4.4 use terms "sending side", "receiving side", "remote node", etc. It is confusing to use multiple terms (some of them are not defined in the draft) in these two sections. Suggest to use "Local Peer/Remote Peer" or "Data Source/Data Sink" terms consistently in these two sections.  [mk] OK if no one else objects.
 5.  The use of term input qualifiers in Section 3 is misleading as we are actually referring to the input parameters for the operational primitives. Suggest to replace term "input qualifier" with "input parameter" in Section 3.  [mk] "Qualifier" is a term used in the Datamover Architecture document referenced in the first paragraph of section 3.  iSER uses the abstract model as defined in the Datamover Architecture.
 6.  Section 5.1.3: In the following sentence the second "iSER-ORD" should be replaced with "ORD". "Upon receiving the iSER Hello Message, the iSER Layer at the target MUST set the iSER-ORD value to the minimum of the iSER-ORD value at the target and the iSER-IRD value declared by the initiator."  [mk] iSER-ORD is a variable while ORD is a concept.  Its usage here is correct.  See the definition in section 1.1 on iSER-ORD and ORD.
 7.  Section 9.2: Do WSV and RSV flags indicate the validity of WBO and RBO fields? The description of these flags should state it.  [mk] OK.
 8.  Section 15 should be titled as "iWARP Message Formats for iSER".  [mk] Current title is correct.
 9.  Sections 15.3-15.5 should be formatted as iWARP messages.  [mk] The intent is to show the iSER message format for the generic RCaP, not just iWARP.
 10. I do not understand the reason behind removing the iWARP Message Format for RDMA Read Request, iWARP Message Format for Solicited SCSI Write Data, and iWARP Message Format for SCSI Response PDU in Section 15. They should be kept in Section 15.  [mk] The intent is to show the iSER message format for the generic RCaP, not just iWARP.

Major

 1.  According to the iSCSI spec, an iSCSI connection has an underlying TCP connection. Based on that, an iSER-assisted iSCSI connection requires an underlying TCP connection to be present. For iWARP, an iSER-assisted iSCSI connection has an underlying TCP connection. For iSER-assisted iSCSI connection over InfiniBand transports, there is no underlying TCP connection. In this case, what does an iSER-assisted iSCSI connection mean? What IB transport service types are used for the iSER-assisted iSCSI connection? The draft is not clear on what an iSER-assisted iSCSI connection mean for the InfiniBand transports. The draft needs to clearly define iSER-assisted iSCSI connection for InfiniBand transports and its mapping over underlying InfiniBand transport connections and transport service types.  [mk] The support for iSER in Infiniband can be found in Annex A12 of the Infiniband Architecture Specification which appeared as an Informative Reference in section 13.2.
 2.  Section 4.1: "For a transport layer that provides message delivery capability such as [IB], the RCaP implementation supports the use of the messaging capability by the iSCSI Layer directly for the Login phase after connection establishment before enabling iSER-assisted mode." What is the meaning of this sentence? What message delivery capability is being referred here? Does this sentence infer that for IB transports, IB messages are allowed prior to enabling the iSER-assisted mode? The spec is not clear on how the iSCSI PDUs are transferred over IB connections prior to enabling iSER-assisted mode. Are iSCSI PDUs transferred over iSCSI/TCP/IPoIB connection or iSCSI/IB Reliable Connection prior to enabling iSER-assisted mode (I think it is the later)?  [mk] The support for iSER in Infiniband can be found in Annex A12 of the Infiniband Architecture Specification which appeared as an Informative Reference in section 13.2.
 3.  Section 5.1 has the following text "If the RDMAExtensions key is not negotiated to Yes, then for some RCaP implementation (such as [IB]), the connection may need to be re-established in TCP capable mode.  (For InfiniBand this will require an [IPoIB] type connection.) " What is the implication of re-establishing connection in TCP capable mode for InfiniBand? Does it require tearing down the exisiting iSCSI/IB connection, establishing a new TCP/IPoIB connection, and performing iSCSI login without RDMAExtensions key on that TCP connection? The spec does not describe this behavior clearly.  The support for iSER in Infiniband can be found in Annex A12 of the Infiniband Architecture Specification which appeared as an Informative Reference in section 13.2.
 4.  Section 9.2: iSER header and iSER messages defined in Section 9.2 are not backward compatible with RFC 5046 defined iSER messages. I consider this as a significant issue. I suggest to add a single flag (or two flags) in iSCSI-control type PDU to indicate the presence of WBO and RBO by using one (or two) of the reserved bits. If this flag is set to 1, then the format is as defined in Figure 3 of this draft. If this flag is cleared, then the format of the iSER header for iSCSI control-type PDU is same as the one defined in RFC5046. I strongly suggest to keep the formats of iSER Hello and HelloReply Messages to be identical to the formats defined in RFC5046. These proposed modifications to the header formats in Section 9.2 make this draft backward compatible with RFC5046 when WBO and RBO are not used.  [mk] Will change the text in section 14 as suggested by David Black.
 5.  The headers in Sections 9.3 and 9.4 need to be reformatted if the change suggested in the previous bullet is made.
 6.  We should add an appendix for "InfiniBand Message Formats for iSER" because they are different from "iWARP Message Formats for iSER".  [mk] The support for iSER in Infiniband can be found in Annex A12 of the Infiniband Architecture Specification which appeared as an Informative Reference in section 13.2.
 7.  Solicited Event: RFC5046 describes the use of Send with Solicited Event (SendSE) and Send with Solicited Event and Invalidate (SendSEInv) Messages. This draft removes the use of SendSE and SendSEInv Messages. What was the rationale behind this removal? The draft should allow the use of SendSE and SendSEInv unless we have good reasons not to.  [mk] The intent is to describe how iSER is supposed to work over a generic RCaP.  For iWARP, I can restore the references to SendSE and SendSEInv where applicable by including them inside parenthesis.

Regards,

Hemal

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of [e-mail]
Sent: Monday, November 21, 2011 11:32 AM
To: [e-mail]
Subject: [storm] WG Last Call - iSER - through December 12
Importance: High

This message is to announce the STORM working group Last Call on the following document:

                  iSCSI Extensions for RDMA Specification
                       draft-ietf-storm-iser-05.txt

http://datatracker.ietf.org/doc/draft-ietf-storm-iser/

Last Call comments are due by Monday, December 12 at midnight Eastern time.

Please send all technical comments to the storm mailing list: [e-mail]
Editorial comments may be sent directly to the draft authors: [e-mail]

I'll even start this Working Group Last Call with an initial comment:

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.

There's no need for a new version of the draft now, but a new version will be needed before the RFC publication request can be submitted.

Thanks in advance for the WG's prompt review. :-)

--David (storm WG co-chair)
----------------------------------------------------
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


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