Subject: iSCSI : Review Feedback on iSCSI Rev 06

From: Santosh Rao <[email-masked]>
Date: Thu, 3 May 2001 19:01:02 -0700 (PDT)

Julian & All,

Enclosed is my review feedback on the iSCSI draft Rev 06.

Regards,
Santosh

--------------------------------------------------------------------

1) Suggest re-naming the following in the interests of better
clarity : 

a) Rename the ExpDataSN in SCSI Response PDU to EndDataSN as was used in 
the previous version of the draft. (indicative of being the final Data sequence
number.)

b) Rename DataSN in the R2T PDU to R2TSN. (This is really an R2T seqeunce
number.)

c)  Rename R2TExpDataSn to EndR2TSN. (indicative of being the final R2T
sequence number.)

2) If a target issues a REJECT response to any received PDU within a SCSI
Command operation, does this imply the target has internally aborted the I/O as
well ? Will the target send a SCSI Response in addition following the Reject ?

What behaviour must an initiator expect when it sees a reject in response to
some PDU within a SCSI command ? Does it treat this as a termination of the I/O
or should it await a SCSI Response ? 

Also, if the target does terminate the command on sending REJECT, can a "retry"
be issued ?

In the interests of clearing all these obscurities with the REJECT, I'd like to
suggest the following model :

- SCSI Command be always responded with a SCSI Response.
- SCSI Task Mgmt Command be always responded with a SCSI Task Mgmt response.
- Only non-SCSI PDUs may be errored back with a REJECT PDU. 

The above is also semantically aligned with the FC LS_RJT/BA_RJT model.

3) Why does the new draft limit the size of text commands and nop commands to
4096 bytes ? What is the rationale behind the magic number selection of 4096 ? 

Since DataPDULength is defined in units of 512 bytes, how does iSCSI deal with
fragmentation caused due to the usage of a DataPDULength < 4K and an attempt to
perform a 4K text , login or nop operation ?

I believe such scenarios were discussed in an earlier thread
titled "Fragmentation & Reassembly issues in iSCSI." 
(http://ips.pdl.cs.cmu.edu/mail/msg03031.html) and the resolution was to
enforce a restriction that the size of a login, nop & text operation be 
limited to DataPDULength (?).

On a seperate note, what happened to PingMaxReplyLength ? It seems to have been
removed from the draft. How can the 2 end points now negotiate a max limit on
nop ping operations ? (for ex : how can a target enforce that the nop ping
operation data buffer not exceed 512 bytes ?).

4) As mentioned in a seperate mail, the login or text negotiation can span
several command/response exchanges each using the same initiator task tag. The
addition of a "Target Task Tag" field in the login response, text command and
text response would allow targets to perform a quick loopup on their T.T.T for
this negotiation state information, instead of having to lookup based on
(Initiator iSCSI Name + ISID).

5)  Section 2.8.3
"The data length of a text command or response SHOULD be less than 
   4096 bytes."

suggest re-wording the above to :
"The data length of a login, nop & text command or response MUST be less than 
 DataPDULength. For the leading session login, the length of the login command
 MUST be less than 512 bytes."

The above will ensure that login, text & nop operations will not result in
fragmentation.

6) Section 2.8.3
"A session may have only one outstanding text command or text command 
   sequence at any given time."

The above is contradicting itself. Is it only one outstanding text command or
one outstanding text sequence.

Suggest re-wording the above to :
" A session may have only one outstanding text command at any given time."

7) Section 2.11.5
"The TSID is an initiator identifying tag set by the target.  It is 
   valid only if the connection is accepted."
		     ^^^^^^^^^^

The above needs to be re-worded as :
"only if the login is accepted"

8) Section 2.13.1

"A target may also issue a NOP-In on its own to carry a changed CmdSN 
  							        ^^^^^
   and/or MaxCmdSN if there is no other PDU to carry them for a long 
      time."

The above reference to changed CmdSN must be re-worded to "changed ExpCmdSN".

9) Section 2.18.1

"Target requests logout".

Suggest removal of the above option. A target that needs to logout would use
either :
"Target will drop the connection"
or
"Target will drop all connections"

The "Target requests logout" also does not allow the target to specify which
flavor of logout it wishes to receive. It is better to remove this option and
use only a single scheme.

10) Section 2.19

The section 2.19 seems out of place being in the midst of descriptions of other
iSCSI PDUs. It should be moved out to a seperate location since it has nothing
to do with PDU descriptions.

11) As was brought up at the Nashua interim meeting, Status SNACK must be made
optional to support. Simple implementations as also gateway and bridging
products may choose not to [or are unable to] support SNACK mechanisms. Non-use
of Status SNACK must be accompanied by setting StatSN to 0 on all status PDUs.

12) Suggest the addition of new login keys to negotiate support for Data SNACK
& Status SNACK. If a target does not support Status SNACK, it must set StatSN
to 0 for all Status PDUs.

13) Section 3.2
" 3.2 iSCSI Logical Unit Control Mode Page 
    
       The following outlines the iSCSI Port mode page: "

The above needs to be re-worded to :
"The following outlines the iSCSI LUN Control Mode Page: ".

As suggested earlier, perhaps, suggest using the SPC-2 terms
"protocol specific port page" and "protocol specific LUN page" for these mode
pages.

14)  Section 3.3.3
LoginLogoutMinTime

The wording must be clarified to indicate that this delay is only in the case
the prior logout was due to a target originated event. There is no need for an
initiator to delay after a logout originated by itself.

For example, while doing open, read/write, close type of operations on a single
thread of activity for the entire session, it may be typical to see iSCSI level
on-the-wire activity of login, read/write, logout....

In the above type of scenarios, LoginLogoutMinTime is not applicable and the
initiator must not delay prior to issuing logins.

15) Section 4.3
"If the target responds to a text or Login command with the F bit set 
 to 1, with a text response with the F bit set to 0, or a login 
 response with the text bit set to 0, the initiator must keep sending 
 		   ^^^^^^^^
 the text command (even empty) with the F bit set to 1 until it gets 
 the Login Response with the F bit set to 1."

The above must be re-worded to :
"or a login response with the F bit set to 0, ...".

16) Section 6
"It is further assumed that a target will keep the "status & sense" 
 for a command it has executed while the total number of outstanding 
 commands and executed commands does not exceed its limit and status 
 has not been acknowledged."

What if the target has exceeded its limit on resources ? Can it drop these
stale resources or must it close its command window and not accept further new
commands ?

If it is the latter, then, iSCSI is optimizing for recovery paths and is
impacting performance paths by limiting the ability of the target to accept new
I/Os due to having to hold onto stale resources awaiting ExpStatSN updates.

17) Section 6.7.3
"N.B. As an alternative to Logout and reissue commands, the 
 initiator MAY instead reset the target and terminate all 
 outstanding commands with a service response indicating 
 Delivery Subsystem Failure. The initiator MUST perform one of 
 the two actions."

 As was brought up at the Nashua interim meeting, suggest removal of the above
 sentence since it may lead implementors to use Target Reset as a form of error
 recovery while dealing with session-level error recovery.

 An alternate form of error recovery in its place would be to use :
 "logout - cleans the connection"
 or 
 "logout - cleans the session"

18) Section 7.3

This section outlines how deterministic clean-up of tasks can be achieved
during task mgmt command execution. However, in the absence of mandating this
behaviour for targets, initiators cannot rely on the same for reliable
deterministic clean-up of pending tasks and will need to go ahead and implement
their own forms of abort task based cleanup.

This section is meaningless in the absence of a mandate that forces targets to
do as described. In its absence, initiators will be forced to implement their
own schemes for clean up.

19) Appendix E

"15 AccessID 
 Use: ALL 
 Who: Initiator 
 AccessID=<SCSI-AccessID-value> 
 Deliver a SCSI AccessID to the target "

What is a SCSI Access ID ? Can further descriptions be added to this definition
and a reference be added to SAM2 or some other SCSI document, if appropriate.

20) Appendix E
In several places, IO seems to have been used instead of LO. Is this a typo or
a new acronym ? 

Speaking of acronyms, can we add an acronym section to the draft ?

21) Appendix E
the description of ImmediateData is describing the case :
"ImmediateData=YES & InitialR2T=YES" twice in differing manners. One of them
needs fixing [or removal].

---------------------------------------------------------------------------

-- 
#################################
Santosh Rao
Software Design Engineer,
HP, Cupertino.
email : [email-masked]
Phone : 408-447-3751
#################################

---

From: Stephen Bailey <[email-masked]>
Date: Mon, 07 May 2001 09:00:22 -0400

Santosh,

> 4096 bytes ? What is the rationale behind the magic number selection of 4096 

The reason for a limit like this (I won't argue that we've done it
quite right here, in fact, I may argue against it a little bit below),
is for to allow the receiving endpoints to have only extremely modest,
tightly bounded resources for handling this non-nominal (or, at least,
non-performance) traffic.

DataPDULength is way too large a limit.  I'm still optimistic that the
maximum DataPDULength limit will be relaxed back towards 4G, even
though I know that y'all have decided against it and everybody out
there thinks I'm on dust (you'll see...).

The limit that makes slightly more sense from an engineering
standpoint, but makes no sense whatsoever from a standards standpoint,
is FirstBurstSize.  In a hardware implementation, DataPDULength is a
property associated with the hardware data handling path.  The data
arriving in DataPDULength units is destined for buffers which are
known to have already been allocated and supplied from the upper
level.  The resource cost for these buffers is a direct function of
the application, as opposed to iSCSI overhead.

The Login[Response], Text and Ping PDUs, and probably (but not
definitely) unsolicited data, flow through a path where the buffers
are iSCSI implementation overhead.

iSCSI must provide a mechanism for reducing this resource overhead to
an arbitrary, manageable size.  The alternative is to require an iSCSI
implementation to allocate a gratuitously large quantity of memory to
handle occurences which a) may rarely (probably never) happen b)
don't provide the implementation anything at all worthwhile in return
for that memory.

Because this is a non-performance path, having this parameter being
negotiable is also a waste of effort.

The iSCSI negotiation protocol should be (it mostly already is)
designed to tightly bound the worst-case (largest) text payload in a
Text or Login[Response].  To do this we must:
  1) determine the maximum text payload size which contains all
    `fixed' length keys 
  2) create an iterator protocol for arbitrary, or even just large
     text messages.  This includes X-* parameters---an X-* exchange
     must be structured to work within the max text PDU limits.  In
     addition the X-* parameter definitions must create their own
     iterator if necessary---iSCSI only needs to handle iSCSI
     specified keys.

Looking over the present set of text keys, the security exchanges
already have tight size bounds, and clearly aren't going to be the
limiting factor in the maximal text payload.  That leaves the
operation parameters.  Most of the operational PDUs also have a small
maximum fixed size.  The exceptions are:
  InitiatorName
  InitiatorAlias
  TargetName
  TargetAddress
  TargetAlias

We can probably put a modest bound on each of these keys individually
(e.g. 512 bytes), which means that the maximum text payload size
within a Login exchange is tightly bounded and 4K seems adequate.

The problem comes with SendTargets=.  That is why I proposed an
iterator protocol for SendTargets= instead of having the target
information gush back from the target uncontrolled.  Simply bounding
the maximum text payload and allowing and arbitrary sequence of text
payloads to be sent in response to a single request doesn't cut it.
We need to be able to bound the total amount of data returned from
each particular request.

SendTargets= actually has two problems.  1) an arbitrary number of
targets per endpoint, 2) an arbitrary number of addresses per target.
Given the second problem, unfortunately, it's not adequate to merely
have a target iterator, since the data for each target may be
arbitrarily long.  Therefore, here's another proposal which might be
close to the mark:

  Initiator keys:
    SendTargetStart=
       Send first PDU of target list information
       If already in the midst of a target list exchange, restart at
         the beginning
    SendTargetMore=
       Send next PDU of target list information
    SendTargetEnd=
       Terminate a target list exchange

  Target Keys:
    TargetName=name
       one per target
    TargetAddress=address
       one or more per target
    TargetAlias=alias
       one per target (may be empty)
    TargetMore=
       more target information remaining within the present target

A target list exchange always terminates with a Text PDU with final=1.
In the case that the exchange is ended by a SendTargetEnd=, the
target's Text PDU with final=1 would have no payload.

We would specify at most one target list request outstanding per
session.  Personally, I'd like to be even more restrictive and say
that target list exchanges are only allowed on single connection
sessions where a TargetName was not specified at login.  However, Mark
Bakke seemed to think that rather than a session with no TargetName,
it should be a session with TargetName=iSCSI.  Either way is fine with
me, assuming that we do not want to allow a regular, operational login
with TargetName=iSCSI (e.g. for boot, or simple initiators).  I happen
to think that operational login with TargetName=iSCSI is useful.  The
fact is, many boxes, even big ones, will have only a single target,
but many LUNs.  This corresponds to present storage boxes (as opposed
to bridges).

Steph

---

From: Santosh Rao <[email-masked]>
Date: Mon, 07 May 2001 17:33:20 -0700

> there thinks I'm on dust (you'll see...).

Steph,

I agree that an upper limit is required on the text/login/nop commands.
What I was pointing out was that this upper limit needs to be lower than
DataPDULength, failing which fragmentation can occur.

If it is felt that explicitly defined keys that constitute such limits
is over-kill due to these PDUs not being in performance path, that's
fine. All we then need is to say :

non-scsi command PDUs commands MUST be limited to a size of :
min(4096, DataPDULength).

This ensures that no fragmentation occurs when DataPDULength is < 4K.

Regarding the separate problem that arises from an unbound response to a
command PDU [such as a SendTargets], the iterator proposal results in
more "wire traffic" during target discovery than the use of a single
text command and target controlling the data returned. Multiple sets of
data from the target can then be re-assembled into a single host buffer.

This is the way the FC-GS-2/FC-GS-3 GID_FT command operates and it
allows for a faster target discovery than the iterative use of other
FC-GS-2/GS-3 name server queries such as GA_NXT (GAN).

However, both methods could be specified by the draft with
implementations choosing either an iterator based scheme or a bulk
response scheme.

Regards,
Santosh

---

From: Stephen Bailey <[email-masked]>
Date: Sun, 13 May 2001 22:54:29 -0400

Santosh,

> DataPDULength, failing which fragmentation can occur.

I see what you're saying now, but I don't agree that the iSCSI spec
requires this fragmentation.  Data PDUs are not NOP, Text or
Login[Response] PDUs.  The DataPDULength limit applies only to data
PDU length (or, we should change the name to PDULength).

The most important point to make is that both the initiator and the
target be able to control the total size of any PDU sequence received
(therefore, sent) during an operational session.

If we want to punt completely on iterators for SendTarget response and
use transport-level flow control to handle an arbitrary amount of data
(i.e. no iterator for SendTarget), so be it.  However, it is essential
that if we do this, SendTarget be prohibited during operational
sessions.

Outside of a SendTarget interaction, I do not see any way that an
iSCSI-specified text interaction would result in a gush larger than
EITHER 4K OR the minimum DataPDULength of 512 bytes (not that this
matters, as mentioned above).  However, we do allow extended
interactions with the X-*, and in order to ensure that these extended
interactions function properly across implementations, we need to set
a bound on the resources that the transport implementation must
provide for these interactions.

> FC-GS-2/GS-3 name server queries such as GA_NXT (GAN).

Do people actually use GID_FT?  It doesn't seem wise.  I always used
GA_NXT, and it seemed that all the other FCP implementors I asked did
as well (it's been a while).

The problem is precisely analogous to what you see FCP.  Data from an
operation like GID_FT is going to go into buffers which are allocated
locally to the FCP driver.  This pool is going to be sized based upon
operational assumptions (e.g. # of outstanding commands), and if you
ask the name server for the entire name list at once, you're probably
going to consume all those buffers and engage link-layer flow control
before you can wade through the results.  In a fabric, this can clog
everything up pretty good.

Fortunately, in the iSCSI/TCP case, engaging flow control like this
will not block the channel, so it's actually marginally better to do
this in iSCSI than in FCP.  As long as you are not performing storage
operations, AND the target is somewhat sophisiticated about returning
the data (big IF---it must not consume all it's local resources trying
to SEND the target list data which is going to drizzle out slowly
under TCP flow control), everybody will get along just fine.

This underscores another argument for iterators.  For large targets,
simply SENDING the entire target list at once might be a very resource
intensive operation.  You could suggest that the target just send a
little at a time, and that's sorta OK, except that the initiator has
to set some timeout on receiving the response, and if the targets are
allowed to drizzle the data at an arbitrary rate, it's hard to chose a
timeout value.

I can argue both sides of this (I have, right?), but given that
nobody besides me seems to believe in iterators, we at least have to
make sure that the big gush approach works properly.

Steph

---

From: Santosh Rao <[email-masked]>
Date: Thu, 17 May 2001 18:04:51 -0700

Steph,

I've asked this question a long time back on whether DataPDULength
controls the length of all Data PDUs or only the Data PDUs and have'nt
heard a definitive answer on this yet.

If it controls the length of all Data PDUs, it must be re-named to
MaxPDULength. If it does not control the length of all PDUs, then, a
seperate Max PDU Length must be negotiated for all the non-scsi PDUs.
(text, login, nop). 

> (therefore, sent) during an operational session.

Agreed. 

<snip snip>

> as well (it's been a while).

Well, you've just met your first GID_FT implementor in that case. It
saves us a lot of on-the-wire traffic by replacing 'n' GA_NXTs with a
single GID_FT as long as we can handle the buffer allocation [which is
ok with us].

In any case, that's a separate discussion... :}

Regards,
Santosh

---

From: Santosh Rao <[email-masked]>
Date: Thu, 17 May 2001 18:15:41 -0700

Julian,

I did'nt hear back on any of these review comments on Rev 06 and hence,
am raising these again for comments. Could you perhaps clarify on these
so that we get closure on the same.

Regards,
Santosh

---

From: Santosh Rao <[email-masked]>
Date: Thu, 17 May 2001 18:17:15 -0700

> controls the length of all Data PDUs or only the Data PDUs and have'nt
                        ^^^^^^^^^^^^^^^

the above should read "of all PDUs".