Re: iscsi: technical comments to iSCSI 05:

[julian_satran@il.ibm.com]

Answers in text.

Regards,
Julo

Matt Wakeley <matt_wakeley@agilent.com> on 23/03/2001 01:02:14

Please respond to Matt Wakeley <matt_wakeley@agilent.com>

To:   Julian Satran/Haifa/IBM@IBMIL
cc:
Subject:  Re: iscsi: technical comments to iSCSI 05:




Julian,

Why haven't you responded to my comments?  You seem to respond to everyone
elses.  Does no response mean you agree?

-Matt

Matt Wakeley wrote:
>
>
>
>
------------------------------------------------------------------------------

> technical comments to iSCSI 05:
>
> Global: (especially in section 6) timeouts are not defined.  We went
>     through a lot of work defining time-out values for FCP-2.  I think
>     that time-out values need to be specified for interoperability.

+++ probably. Please note also that we had ages ago some timers and people
did not find them usefull.  Considering the variability in network
diameters
we will have to either have large values or set them based or RTT (complex)
+++
>
> 1.2, page 10:
>     "The iSCSI protocol is a mapping of the SCSI remote procedure
>      invocation model on top of the TCP protocol."
>     is iSCSI limited to TCP? Seems like it could be implemented over
>     other transports, such as SCTP with no modification...
>
+++ I am sure we can but on SCTP other mappings would be more effective
(numbering, digests, marking etc.).  In SCTP many of those are already in.
Some others- like security would have been more complex +++

>
> 1.2.3, page 14:
>     After the sentance "Otherwise, it sends a response with
>     a "login reject" ..."
>     add that the connection is terminated.
>
+++ did +++

> 1.2.5, page 16 (middle) and 6.7.1, page 84:
>     The initiator should not be required ("MUST") to honor R2Ts.
>     Conditions may have occurred that might prevent the initiator
>     from processing the R2T, in which case appropriate error recovery
>     should be performed.  I suggest "SHOULD if at all possible...".
>
+++ did +++

> 1.2.5, page 16 (bottom):
>     "Unsolicited data MUST be sent on every connection in the same order
>     in which commands were sent." - must the data be sent immediately
after
>     the command, or can other commands be sent before the data?
>
+++ "were sent" does not imply that commands precede the data and that is
the only ordering required - i.e., other commands can be sent before the
data +++
> 1.2.7, page 19, middle of page
>     must "iSCSI://..." be caps?  is "iscsi://..." allowed?
>
+++ the syntax is the same as for URNs -- see the NDT doc +++



> 2.2.3, page 28:
>     Are "what's next" fields included in the header digest?  It's not
>     stated that they are.
>
>
+++ will dissapear as a separate entity - and yes they are included +++

> 2.7.2, page 45:
>     Why must a LUN be associated with a Target Transfer Tag?
>
+++ I though we had this settled. The reason is to allow targets to set
their Tags independent of each other and help the target steer the data
without looking at Tags.
if the device server is located with the LU. The alternative would be to
declare the whole LU+Tag in R2T and Data Out as an extended tag and let
every target use it as they wish - no mandatory LU correctness check +++

> 2.7.4, page 45:
>     It is not clear if the DataSN restarts from zero for each R2T.
>
+++ will fix wording +++
> 2.7.6, page 46:
>     It is not stated, but seems implied that a bi-di commands must
>     send status in a separate response PDU.  If this is so, please
>     state it.
>
+++ will fix although not very relevant now as Bidi are supposed to be
input-followed-by-output - but this may change +++

> 2.10.9, page 52:
>     What is the default maximum length for login parameters?

+++ the default text command length currently defined as one DataPDU and
one data PDULength is defaulted to 8K +++
>
> 2.15, page 63:
>     What is the initiator supposed to do when it receives a code of
>     "cleanup failed"?  It seems like it's the target's problem and
>     should not be reported to the initiator.
>
+++ It is supposed to do a reset +++

> 2.17, page 66:
>     *if* as you state in 2.7.2 a LUN must be sent when a target transfer
>     tag is specified, shouldn't the LUN be also specified in the R2T?
>
+++ see above - the target may have device servers that do not need to know
what they are (they may get this information from the command but they
don't need it). On the other hand the initiator has always this
information. +++

> 2.20, page 71:
>     remove "reserved fields not 0".  This makes it sound like
>     reserved fields must be checked, which they should not must
>     be checked.

+++ I did. But this raises an interesting side-question. It was said that
checking for reserved fiels being 0 is a good practice. It enables easy
migration from version to version +++

>
> 2.20, page 71:
>     add "data digest error" to the list of reject reasons.

+++ it is there as cause 3. Is the name confusing? +++
>
> 3.1.2 and 3.1.3, page 72:
>     I don't care what SCSI has defined for it's mode pages, for iSCSI,
>     these fields should be byte values, not multiples of 512.
>
+++ the current field size is 16 bit. Do we want to limit ourselves to 64k
PDUs?
+++

> 4.3, page 78:
>     It may not be clear to everyone that a target can send a text or
>     login response to a login or text command.  That is, a target could
>     respond to a text command with a login response.  This should be
>     made more clear.
>
> 6.2, page 81, last paragraph:
>     Can a "restart" also be issued in this case?
>

+++ made it lowercase must -:)+++
> Appendix A: 01, page 94:
>     I don't think that crc-64 "MUST" be implemented.
>     I also don't agree with the phrase "Cyclic codes are particularly
>     well suited for hardware implementations."  This is not true if
>     iSCSI PDUs span TCP segments and arrive out of order.
>
+++ fine on crc64 - on the rest if you do on the DMA to host it is still
decent+++


> Appendix A: 01, page 94:
>     "Implementations MAY also negotiate digests with security
>      significance for data authentication and integrity as detailed
>      in the following table:" needs a lot more description.
>
+++ what for example? (copy the reference?) +++

> Appendix C: page 104:
>     change: "will leave at least" to "should leave at least".
>
+++ fixed +++

> Appendix C: 06, page 105:
>     change: "When reduced to iSCSI terms markers MUST point to..."
>     to: "When reduced to iSCSI terms markers MUST indicate the offset
to..."
>
+++ fixed ++
> Appendix C: 07, page 105:
>     "The marker-less interval is not less than 4 kbytes and the
>      default is 4 kbytes." It should be whatever the the receiver
>      needs it to be (not min 4K).
>
+++ how should a send know? we don't want this to kick in before end of
login +++

> Appendix C: 17,18 page 109:
>      The sender should send markers at whatever interval the reciever
>      requires them.  Not the "larger" of the sender and receiver.
>      Otherwise, the sender could say 0xffffffff and it would be of no
>      use to the receiver.  Also, the default should be "none", not 2050.
+++ imposed by receiver is also not palatable. How about the receiver
sending a min and a max and the receiver selecting beween them?  as in:

01   RFMarkInt

   RFMarkInt=<number-from-1-to-65535>[,<number-from-1-to-65535>]

   This is a connection specific parameter.

   The receiver indicates the minimum to maximum interval (in 4-byte words)
   the receiver wants the markers. The sender selects a value within the
   minimum and maximum the receiver requires or indicates through the
   FMarker key=value its inability to set markers. The interval is measured
   from the beginning of a marker to the beginning of the next marker. For
   example, a value of 1026 means 1026 words (4096 bytes of "pure" payload
   between markers).

   Default is 2050.

02   SFMarkInt

   SFMarkInt=<number-from-1-to-65535>

   This is a connection specific parameter.

   Indicates at what interval (in 4-byte words) the sender accepts to send
   the markers. The number MUST be within the range required by the
   receiver. The interval is measured from the beginning of a marker to the
   beginning of the next marker. For example, a value of 1026 means 1026
   words (4096 bytes of "pure" payload between markers).

   Default is 2050.
   +++

>
> Appendix C: 22 page 111:
>      Indicate what "no,no" and "yes,no" means.
>
+++ OK +++
> Appendix C: 23 page 111:
>      This parameter should be in bytes, not multiples of 512,
>      especially since framing mechanisms may require that a PDU not be
>      larger than MSS. (perhaps -1 can be used to indicate MSS)
>
+++ the field length in the mode page is limited +++
> Appendix C: 26 page 112:
>      This should be deleted, since CmdSN is now mandatory.

+++ that is the SAM2 CmdRN +++
>
> Appendix C: 27 page 112:
>      PingMaxReplyLength is both a target and initiator negotiated value.
>      Therefore, text saying "the target MUST..." should be change to
>      indicate both target and initiator.
+++ fixed - this holds also for textmaxlength +++