Re: iSCSI: need for new data SNACK code?

["Mallikarjun C." <cbm@rose.hp.com>]

> Well - I tend to agree with Mallikarjun that this new code was not strictly
> required.
> But I added it  nevertheless for several reason:
>
>    it does really no harm

Julian, it never was my argument that David's proposal is unworkable - of course
it does work.  It however continues to be my position that the change is *not* needed
for technical correctness despite David's arguments to the contrary.  Let's not
add in features that lack sufficient grounds.  Based on the reflector arguments
and Yokohama discussions, I am sure the WG makes the appropriate decision.

There are three crucial aspects to David's arguments (at least that I understand) -
    a) one Data SNACK code leads to ambiguity on either side
        (several references to - "if in doubt, negotiate" maxim)
    b) if multiple I/Os are in progress when a PDU size is redeclared,
        it is unclear when the change is effective, or somehow the situation is
        far more complicated than with one I/O.
     c) the proposed new code invokes a different "algorithm" that requires
         a different behavior on target, so is justified.

[ Let me acknowledge that David was correct about the case of target-initiated
  MaxRecvDataSegmentLength change being irrelevant for the current discussion.
  I can concoct a scenario with bidirectional commands and certain implementation
  constraints, but it's too hypothetical to worth taking reflector bandwidth....and
  certainly not what I had in mind when I thought it was relevant.  ]

Brief comments on the three crucial aspects -
    a)  There is *no ambiguity* wrt when the PDU size changed for any I/O if the
          following simple initiator rules are followed.
                    0. Do not start the text negotiation if any data SNACKs are
                        pending.  (Negotiation may start, however, as soon as those I/Os
                        are planned to be aborted - for whatever reason.)
                    1. Once a text negotiation is underway, for each I/O that requires the
                         issuance of a data SNACK, hold off doing so until the negotiation is
                         complete.  Mark it for subsequent status drop (whenever the status arrives).
                    2. Once the negotiation concludes, issue the appropriate data SNACK(s)
                        with the required BegRun (doesn't have to be 0), but RunLength must
                        be zero.
                    3. Issue the status SNACK eventually.
                    4. If the PDU change happens multiple times *and* the initiator intends
                        to continue one I/O across these changes, repeat steps 0-3 multiple times.
         On the target side, the rule is simple.
                    0. Reject a data SNACK w/ RunLength != 0, if the SNACK is for a task
                        across a PDU size change.  Or else, resegment per the current
                        MaxRecvDataSegmentLength, as needed.  (Note: I'm suggesting a rule
                        stronger than requiring RunLength=0 for the *first* data SNACK - what's
                        stated today.)

        b) Multiple I/Os do not change the situation in any qualitative way, as noted above.
            If the I/O doesn't need a data SNACK, well and good - signal the completion to
            SCSI.  If it does, follow (a) above.

        c) Assigning an existing "algorithm" for a new code in itself is not a justification for a new
            code.  We need to analyze starting from a single code, which can automatically
            and correctly pick the right "algorithm" based on the negotiation state, and then
            question the need for the second (see Marjorie's email about Occam's Razor).
            I don't see the need.

Other quick responses to David's message -

> The only obvious way I see to eliminate this "metadata explosion"
> is to require BegRun to be zero if there's any possibility of
> resegmentation.

Please see my response to Randy Jennings.  I believe we can get away without
the metadata explosion that I feared from your description, even for BegRun != 0.

> Irrelevant - iSCSI does not require, recommend, or even describe
> ULPDU containment.  TCP ULP framing was removed from the iSCSI
> draft many months ago after a long controversial discussion.  I
> strongly object to attempting to sneak it back in.  Besides,
> adaptation to PMTU degradation is the transport's job, and iSCSI
> should not be trying to do the transport's work for it, as it
> in general does not have direct access to PMTU information.

I'm sorry that an ulterior motive is being ascribed to the reference to "ULPDU
containment".   No need for recounting layering principles either.  I was merely
attempting to come up with a good reason why an initiator may change the max PDU
size in the first place for an operational iSCSI connection.  If you have one that
has nothing to do with ULPDU containment, please let me know, and I'll use that
in future.
--
Mallikarjun

Mallikarjun Chadalapaka
Networked Storage Architecture
Network Storage Solutions
Hewlett-Packard MS 5668
Roseville CA 95747
cbm@rose.hp.com

----- Original Message -----
From: "Julian Satran" <Julian_Satran@il.ibm.com>
To: <Black_David@emc.com>
Cc: <cbm@rose.hp.com>; <ips@ece.cmu.edu>; <owner-ips@ece.cmu.edu>
Sent: Saturday, July 13, 2002 4:26 PM
Subject: RE: iSCSI: need for new data SNACK code?


> Well - I tend to agree with Mallikarjun that this new code was not strictly
> required.
> But I added it  nevertheless for several reason:
>
>    it does really no harm
>    The "settling time" of a change in MaxRecvPDULength vs. data is not
>    synchronized with command execution or even between connections so
>    setting clear expectations is not bad. It also helps building
>    sophisticated initiators that don't have to synchronize different
>    threads.
>    the burden of resending status went on the transmitter and that would
>    have been confusing. Please pay attention to the fact that initiator has
>    now to ask for the status if it wants it again. That is not really
>    related but easier to do now and it was one technical issue that David
>    raised that I would have also called technical and not syntactic sugar.
>    All the plugfests have not shown yet a large number of within command
>    recovery (I counted 2) although interest was expressed in private by
>    many
>
> The new text takes also care about all the issues related to rejects (i.e.
> - it is tolerant).
> I suggest we close this thread and will fix the appendix soon.
>
> Julo
>
>
>
>
>                       Black_David@emc.c
>                       om                       To:       cbm@rose.hp.com
>                       Sent by:                 cc:       ips@ece.cmu.edu
>                       owner-ips@ece.cmu        Subject:  RE: iSCSI: need for new data SNACK code?
>                       .edu
>
>
>                       07/12/2002 12:40
>                       AM

>                       Please respond to
>                       Black_David
>
>
>
>
>
> Mallikarjun,
>
> > I am still unclear about the need to add the new data SNACK
> > code.  IMHO, this can only be confusing to both initiator and target
> > implementers - to have two codes that have identical semantics
> > on the wire.
> >
> > The only remaining reason that I see from David (attached below)
> > is that the new code makes the initiator aware that it must request
> > status retransmission.  IOW, you're expecting the initiator
> > to be careful
> > enough to use the new code, but somehow you don't expect it to be
> > attentive enough to discard the status PDU unless it had used
> > this new
> > code in an *outbound* SNACK.  I completely miss this rationale.
>
> I disagree with the "careful enough" characterization.  The new code
> provides the initiator with a more robust way to detect resegmentation
> by requiring the initiator to explicitly ask for it.  The initiator
> can take a simple approach of always starting with the existing
> Data SNACK code that does not resegment and only using the new code
> when the non-resegmenting SNACK doesn't work.  If the target makes
> its own choice to resegment, and the initiator doesn't think the
> target resegmented, there are error scenarios that combine this with
> corrupt Data PDU headers to cause the initiator to successfully
> complete a SCSI command that has not delivered all its data
> (the resegmented PDUs caused the Data PDU count to match the ExpDataSN
> value in the response that should have been discarded, but wasn't).
> While these should be rare, their consequences can be catastrophic.
>
> > I'd buy the new code if it's meant to convey something different to
> > *targets* because they need to process these SNACK PDUs.  I see
> > nothing that the targets need to different in satisfying either SNACK.
> > Both parties are aware of the fact of PDU size change, it
> > had happened.
>
> It is conveying the Initiator's instructions that resegmentation is
> permitted.  I am not comfortable with the last sentence above that assumes
> that the Initiator and Target will always have identical views of all of
> the effects of a full feature phase PDU size change - (which is
> a rare event to begin with, and hence likely to involve code that
> isn't well exercised/tested).
>
> > The only two changes from the rev14 text that I propose are that we add:
> >
> >    a) The first status PDU must always be dropped after a
> >                        MaxRecvDataSegmentLength change, if ever a data
> SNACK is
> >                        employed for the task.
>
> When does this obligation to drop the first status PDU expire?  I think
> the Initiator has to mark all commands that are outstanding or become
> outstanding between the time it starts the negotiation that changes
> MaxRecvDataSegmentLength and the time that it gets the final Text Response
> of that negotiation from the target.  This requires additional Initiator
> state per command for something that almost never happens, and if it
> gets one of these markings wrong, it's vulnerable to failing to deliver
> all the data for a SCSI command in a compound error situation.  An
> alternative with the new code could involve a single bit per connection
> that records whether the PDU size was ever changed (if so, retry any
> failed Data SNACK as a resegmenting Data SNACK).
>
> >                        Initiator MUST issue a status SNACK to recover the
> >                        status PDU (i.e. move the onus of retransmitting
> >                        status from the target to the initiator).
> >     b) A SNACK requesting an R2T, Data or Status PDU for a task MUST be
> >           issued before the status for the task is acknowledged.
>
> I have no problem with these two.
>
> > I'll be glad to see any technical reasons that I am
> > overlooking, that require two codes.
>
> See above.  This is somewhat analogous to the "if in doubt, negotiate
> it" principle for login - telling the other side *exactly* what is wanted
> is more robust than assuming that it will do what is wanted, and in
> this resegmenting Data SNACK case, there are potentially nasty
> consequences to an incorrect assumption.  Does this make any sense?
>
> > I'd assume Elizabeth would be the process co-chair, if need be.
>
> Yes.
>
> Thanks,
> --David
>
>
> > T.30 (Resegmenting may come as a surprize - suggested a
> > different request)
> > Ouch - the new SNACK type code was inserted in the middle of the
> > sequence making this change not backwards compatible with earlier
> > versions. Please use 3 for the new R-Data SNACK and leave 0, 1
> > and 2 unchanged from -14.
> >
> > I am ok with the Initiator deciding whether it needs a new
> > Response and using the existing Status SNACK mechanism to get it -
> > that's definitely cleaner than my proposed abuse of the service
> > response code.  The current working draft uses a MAY for the
> > initiator requesting a status retransmit - I prefer Mallikarjun's
> > proposal on the list that the initiator MUST discard the first
> > Response PDU and MUST use a Status SNACK to get one that is certain
> > to reflect any resegmentation.  I still disagree with Mallikarjun
> > about the new R-Data SNACK type code - I would prefer to see this
> > code used so that the initiator is clearly aware that it is in a
> > situation where it MUST request status retransmission.  Getting
> > this wrong risks returning incomplete data on a READ.
>
>
>
>
>