Re: iscsi: technical comments to iSCSI 05:

[Matt Wakeley <matt_wakeley@agilent.com>]

> > 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)
> +++

The timers you had ages ago were overkill and per I/O to support gateways to
unreliable networks. In 6.6, you need to specify the amount of time after
connection failure that a session is terminated.

> > 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 +++

How about a simple yes or no...

> > 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.

That's the point of a tag - to help steer data.
Fibre Channel doesn't have a LUN field in the headers of data transferred, why
does iSCSI require it?

> 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 +++

No - just use the target task tag and nothing else, just like fibre channel.

> > 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. +++

I restate that I don't think LUN should be used as a steering mechanism.

> > 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 +++

No it doesn't.  If some future version makes use of a reserved field, it makes
older versions puke on the new messages.

> > 2.20, page 71:
> >     add "data digest error" to the list of reject reasons.
> 
> +++ it is there as cause 3. Is the name confusing? +++

Ok I see it - you should call it data digest error.

> > 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?
> +++

See my comment below on the max PDU size.
 
> > 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.

You didn't reply to this.

> >
> > 6.2, page 81, last paragraph:
> >     Can a "restart" also be issued in this case?
> >
> 
> +++ made it lowercase must -:)+++
good.  do you allow a restart?


> > 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+++

My point was that I don't think that phrase should be in the document.  It
adds no value.

> 
> > 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?) +++

Well for example, the "what's next" field doesn't seem to have any encoding
for a security digest.  There is no format of the digest (what it contains)
and how it's validated.

> > 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 +++

I agree - but if login only takes 1K, then the above statement says that I
can't have a marker for another 3k.  It should simply say that the markers are
disabled until after login.  And then the interval will be whatever was
negotiated.

> > 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 +++

The login parameter should be able to negotiate any value, including some
indication to use the current MSS.  The value stored in the mode page can be
an approximation of the real value. (This raises the question, why does iSCSI
need mode pages when the values can be negotiated and discovered using the
key:value pairs?)

The whole purpose of the WARP proposal is to be able to have a fully self
describing iSCSI PDU per TCP segment.  Therefore, there must be a way to
specify that the max sized iSCSI PDU is the MSS less whatever size is required
for the WARP header.

-Matt Wakeley
Agilent Technologies