Subject: Re: [storm] iSER - what to do

From: <[email-masked]>
Date: Mon, 9 Jul 2012 20:13:26 -0400

<WG chair hat on>

I think we have a couple of choices.

- A revised draft this week as outlined in this thread below.
- Pick up discussion again in August or September after the Vancouver meetings.

At this point, I think we're headed for August or September.

Thanks,
--David

From: [email-masked] [mailto:[email-masked]] On Behalf Of [email-masked]
Sent: Tuesday, June 26, 2012 3:02 PM
To: [email-masked]
Cc: [email-masked]
Subject: Re: [storm] iSER - what to do

Mike,

<WG chair hat off>

Thanks for responding - we will eventually get to a conclusion in this discussion :-).

I'm adding two comments to try to sharpen the discussion of the current solution:

> The current solution is for the initiator to post an extra buffer before
> sending the last Login Request as the target most probably sends the next
> NOP-IN only after some timeout period measuring in seconds or hundreds of
> milliseconds.  But this is not a foolproof solution as the target MAY
> theoretically send up to MaxOustandingUnexpectedPUDs to the initiator.
Right - my thought about how to make it fool-resistant is to require that
the target MUST NOT send the next unsolicited PDU until at least 200ms
after the first one or after receipt of a full-phase PDU from the initiator
(that requirement could be a "SHOULD NOT" with a warning that sending
sooner risks the initiator closing the RCaP connection).  I pulled 200ms
out of my proverbial "hat" and would like to see comments on what a
good number would be for both IB and iWarp.

> But if we look at the current solution, the initiator is already
> posting an extra buffer before sending the last Login Request.  Given
> that the existing implementation is broken and has to be fixed anyway,
> how difficult would it be for it to post MaxOustandingUnexpectedPDU
> instead of just an extra one before sending the last Login Request?

That's a question for Alexander and other implementers.  My guess is that
we're talking about quite a few buffers, and there may be resource management
advantages to not allocating that full set of resources until the initiator
is committed to the connection.

Thanks,
--David

From: Michael Ko [mailto:[email-masked]]
Sent: Tuesday, June 26, 2012 2:45 PM
To: Black, David
Cc: [email-masked]
Subject: Re: [storm] iSER - what to do

The last minute problem as related by Alex stems from the fact that the iSER initiator posts its RX buffers for that connection only after the final Login Response arrives.  During that time (after the target had sent the Last Login Response but before
the Full Featured phase related RX-buffers are posted on the initiator side) the target may send the first NOP-IN as it considers the connection in Full Featured phase already and MaxOutstandingUnexpectedPDUs has been negotiated to a non-zero value.

The current solution is for the initiator to post an extra buffer before sending the last Login Request as the target most probably sends the next NOP-IN only after some timeout period measuring in seconds or hundreds of milliseconds.  But this is not a foolproof solution as the target MAY theoretically send up to MaxOustandingUnexpectedPUDs to the initiator.

The new proposed solution is to require iSER Hello exchanges in order to delay the onset of the iSER assisted Full Featured phase until the iSERHelloReply PDU has arrived at the target.  This allows the initiator to delay posting its RX buffers until after receiving the final LoginResponse returns but before sending the iSER Hello PDU.  To support this new proposed solution, the iSERHelloRequired key must be negotiated to Yes.  This require both sides support the new iSERHelloRequired key, leading to the need for clarifying texts suggested by David in the spec in order to specify what should be done otherwise.

But if we look at the current solution, the initiator is already posting an extra buffer before sending the last Login Request.  Given that the existing implementation is broken and has to be fixed anyway, how difficult would it be for it to post MaxOustandingUnexpectedPDU instead of just an extra one before sending the last Login Request?  This complies with the spec and is guaranteed to work.  The alternative is to require the use of and implement iSER Hello exchanges.  If the partner does not support the new iSERHelloRequired key, the solution fails anyway.  It seems to me it makes more sense (and a cleaner soluton besides) for the initiator to allocate the iSER resources, i.e., MaxOustandingUnexpectedPDUs, before sending the last Login Request as stated in the spec than requiring both sides to support iSER Hello exchanges and negotiate the new key.

Mike
On Tue, Jun 26, 2012 at 6:16 AM, <[email-masked]<mailto:[email-masked]>> wrote:
<WG chair hat on>

The discussion about whether the buffer behavior of the Linux iSER
implementation complies with RFC 5046 is missing at least one
important point - to my knowledge, there are *no* iSER
implementations that comply with RFC 5046.

After RFC 5046 was issued, implementers looked at its requirements and
decided to do a number of things differently - the important difference
for this discussion is that the iSER Hello exchange is not implemented,
even though RFC 5046 requires that it be used.  There are a number of
other "running code" changes in the current iSER draft that reflect
implementations and that are incompatible with RFC 5046, see Appendix A
of the draft for details.

The iSER work item was added to the storm WG's charter in order to produce
an RFC specification of iSER that matches the "running code" and hence will
interoperate with existing implementations.  Here's the text from the
WG charter:

       (6) iSER: Experience with Infiniband implementations suggest
       a few minor updates to reflect what has been done in practice.

As WG chair, I'm going to insist on the following:
1) The iSER draft has to match the implementations in order to be
       published as an RFC.  Replacing RFC 5046 with another RFC that
       won't interoperate with the implementations is pointless.
2) The issue encountered by the Linux iSER implementation cannot be
       ignored by the WG.  The workaround (1 extra buffer, at most
       one unsolicited PDU until initiator has time to deploy
       resources) at least needs to be explained in the draft.
3) While requiring the iSER Hello exchange may be the proverbial
       "right thing to do" going forward, it is necessary to provide
       at least guidance on how to interoperate with current iSER
       implementations that don't support the Hello exchange.

In contrast, as WG chair, I'm not going to insist that we adopt the
workaround as part of the standard.  For example, it's possible to
recommend (SHOULD) use of the iSER Hello exchange (explicitly negotiated
using the new key) and explain the workaround as what should be
done in order to interoperate if that key results in a NotUnderstood
response.  For this approach to the workaround, I think a lower case
"should" may be acceptable (e.g., as opposed to a "SHOULD" or "MUST",
but that's subject to further discussion.

Thanks,
--David
----------------------------------------------------
David L. Black, Distinguished Engineer
EMC Corporation, 176 South St., Hopkinton, MA  01748
+1 (508) 293-7953<tel:%2B1%20%28508%29%20293-7953>             FAX: +1 (508) 293-7786<tel:%2B1%20%28508%29%20293-7786>
[email-masked]<mailto:[email-masked]>        Mobile: +1 (978) 394-7754<tel:%2B1%20%28978%29%20394-7754>
----------------------------------------------------

_______________________________________________
storm mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/storm

---

From: Alexander Nezhinsky <[email-masked]>
Date: Sun, 15 Jul 2012 10:27:24 +0300

Hi, all

Sorry for a late answer (again).

I have been thinking over this issue hesitantly for a long time being
close to just agree with the latest set of suggestions.
But then I realized there is a simple counter-argument which
complicates things even more.

When the initiator sends its final Login Request it is not guaranteed
that the next Login Response it receives is the "final" one, too.
If the target has more text data to send than the hardcoded 8KB, it
will split it into two (or more) PDUs by raising Continue bit in all
its responses except the last.

This is a rare event but it means that to be fully compliant and
full-proof the initiator can't just post another N buffers to anticipate
all "unexpected" PDUs from target.

It posts one 8KB buffer for the next Login Response, but it should be
ready for the case where the response contains C=1. In such case
it would post another 8KB buffer and answer ok to continue.

Regular initiator rx-buffers are much smaller than 8KB.
Implementation-wise they are usually allocated from a separate pool or
some other kind of discrimination is made between the login and
full-featured-phase buffers.

As there is no acceptable way to reclaim the buffers after they have
been posted, the only way out is to post a few 8KB buffers, but it will
make the implementation even more complicated and cumbersome.

All in all, I suggest that we bite the bullet, complete the spec and head
towards fully spec-compliant implementations of both initiator and target
as soon as possible.
On pratical grounds we can address the distro maintainers to employ all
possible means to distribute compliant updates sooner than later,
as those will represent a special, critical change.

To minimize the damages i suggest taking the following path:

1. iSERHelloRequired remains defined as is, with default=No.

2. It becomes *mandatory* for a fully-spec compliant initiator
   implementation to communicate iSERHelloRequired=Yes.
   * If this key is not sent then the "new" target knows that it has
     encountered an "old" initiator.
   * If the initiator sends  iSERHelloRequired=No, it means it choses
     (for some bizarre reasons) to behave as an "old" one - while
     such behavior is strongly discouraged.
     I guess the requirement that:
     "the initiator SHOULD send iSERHelloRequired=Yes"
     reflects the situation, correct me if i'm wrong.

3. "New" initiator will recognize an "old" target by receiving
   "NotUnderstood" in response to iSERHelloRequired=Yes.
   Then it can either refuse to deal with it, or to employ a range of
   tricky means used until now.
   We can describe those means as the guidelines, e.g. :
   * posting one or better MaxOutstandingUnexpectedPDUs buffers
   * to be really on the safe side, having those buffers at least 8KB long.

   As we are trying to neutralize the shortcomings of the existing
   targets, the initiator can bet that the target won't send split
   login responses, as it regularly does not do so today.

4. "New" target will recognize an "old" initiator by having received
   iSERHelloRequired=No either implicitly or explicitly. Then it must
   ignore the iSERHello absense and may also take some precautions,
   like:
   * delaying sending any "unexpected" PDUs until the first PDU is
     received from the initiator after the final login response
     has been sent
   * taking a reasonable timeout, say a second (the exact value
     does not matter as the initiator can't count on it anyway and
     no value will solve the problem in full, theoretically).
   * doing both, that is waiting for the first incoming PDU and
     taking a timer to start sending NOP-INs in case no PDUs arrived
     during the timeout period, to be able to detect silent connection
     failures.

5. "New" target and "new" initiator will count on ISERHello as the
   guarantee of proper buffer posting

6. "Old" target and "old" initiator will work as they do now, in their
   double bliss of ignorance.

By the way, the initiator patch alleviating the problem by posting one
additional login buffer was submitted relatively recently and all previous
deployed implementations of the initiator are exposed.
Eventually, the new better code is making its way to the users of all distros.
This is a common situation encountered by the linux kernel community
quite often. Let's take this as a working example, make the spec fool-proof
and advise the implementors how to minimize the damages with the old
software, while keeping everything as simple as possible under these
already over-complicated circumstances.

Alexander

---

From: <[email-masked]>
Date: Mon, 16 Jul 2012 21:20:44 -0400

Alexander,

Thank you for the response.

> On practical grounds we can address the distro maintainers to employ all

We can update the spec now - what do you think a reasonable timeframe
is to push out that code?

> 1. iSERHelloRequired remains defined as is, with default=No.

Ok.

>    * If the initiator sends  iSERHelloRequired=No, it means it chooses
>      reflects the situation, correct me if I'm wrong.

Ok, that seems correct ... rewriting using my own words ...

Specifically, a spec-compliant initiator SHOULD begin the negotiation
with a "Yes" value.  In addition, a spec-compliant target SHOULD
respond to iSERHelloRequired=Yes with iSERHelloRequired=Yes.

A spec-compliant target MUST treat non-receipt of
iSERHelloRequired and receipt of iSERHelloRequired=No in the same
fashion - they both indicate an old initiator. [old = does not use
iSER Hello].

Further, a spec-compliant initiator SHOULD be prepared to receive
both iSERHelloRequired=NotUnderstood and iSERHelloRequired=No responses,
and MUST treat both responses in the same fashion - they both
indicate an old target [old = does not use iSER Hello].

>    "NotUnderstood" in response to iSERHelloRequired=Yes.

Or by receiving iSERHelloRequired=No in response.

>    * to be really on the safe side, having those buffers at least 8KB long.

I would say "SHOULD post" at least one or more buffers that are sized
for negotiation (and not say 8KB explicitly), explain this reduces the
possibility that early unsolicited PDUs will cause the RCaP connection
to close, and state that this technique is known to work with existing
InfiniBand iSER targets.
 
>    login responses, as it regularly does not do so today.

Ok, this does need to be stated as something that existing targets
do not do.

>    iSERHelloRequired=No either implicitly or explicitly.

"implicitly or explicitly" means either iSERHelloRequired was not
received or iSERHelloRequired=No was received.  Both cases MUST be
treated in the same fashion.

>    Then it must
>    ignore the iSERHello absence 

Those aren't the right words.  The target knows that iSERHello will
not be used - it can choose to terminate the negotiation without setting
up iSER.  If it does not ...

and may also take some precautions,

>      during the timeout period, to be able to detect silent connection

I believe the second and third bullets ought to be combined, in that
receipt of a PDU is sufficient reason to end the timeout period before
it would otherwise expire.

> 5. "New" target and "new" initiator will count on iSERHello as the

We also need to issue a warning about the latter combination risking
RCaP session termination if unsolicited PDUs show up from the target
before the initiator is ready.

If the above looks close to correct, we can start working on text for
the draft ...

Thanks,
--David

> -----Original Message-----
> From: Alexander Nezhinsky [mailto:[email-masked]]
> Sent: Sunday, July 15, 2012 3:27 AM
> To: [email-masked]; Black, David; Mike Ko; Paul Koning; Mallikarjun
> Chadalapaka; Or Gerlitz; Mike Christie

---

From: Alexander Nezhinsky <[email-masked]>
Date: Tue, 17 Jul 2012 21:45:46 +0300

> is to push out that code?

I can estimate that the target part can be ready within a month.
It's harder for me to make a correct estimation about the initiator.
But I would bet on a similar time frame.

>From there it should be taken up by the distributions. Each one has
its own pace.
At least RHEL 6.3 was released a couple of weeks ago, so if all goes
well we can
be pretty sure that the changes are included in RH 6.4.

> Ok, this does need to be stated as something that existing targets

Hmm, my second thought is that this may be incorrect in general.
They don't do it in a "normal" setup, where the initiator sends a
small number of keys,
so the target answers with the response of a comparable size.
I need to check how well split responses are generated and tolerated right now.
Probably we should not bet on their absence anyway.

> up iSER.

At least in theory, there could be an "old" home-grown initiator implementation
which will not send  iSERHelloRequired=Yes but will send iSERHello.
Should not we allow this ?

> it would otherwise expire.

I agree

> before the initiator is ready.

Ok

> If the above looks close to correct, we can start working on text for

I'm ok with this

Alexander

---

From: Michael Ko <[email-masked]>
Date: Tue, 17 Jul 2012 12:06:46 -0700

An ""old" home-grown initiator implementation which will not send
iSERHelloRequired=Yes but will send iSERHello" should not cause any
problems.  An old target which does not support iSERHello will fail.  A new
target will assume it is dealing with an old initiator which does not use
iSERHello and will act according to the new spec for dealing with old
initiators.  So when iSERHello shows up, it will follows the new spec for
dealing with new initiators.

There are two other issues which need to be resolved.

1. Should the spec say anything about the timeout value?  I agree with Alex
that there is no practical value that will work in all cases.  So how about
simply stating that the timeout values is outside the scope of the document?

2. In the current iSER spec, section 5.1.1 states that:

"If the outcome of the iSCSI negotiation is to enable iSER-assisted mode,
then on the initiator side, prior to sending the Login Request with the T
(Transit) bit set to 1 and the NSG (Next Stage) field set to
FullFeaturePhase, the iSCSI Layer MUST request the iSER Layer to allocate
the connection resources necessary to support RCaP by
invoking the Allocate_Connection_Resources Operational Primitive."

Should the MUST requirement be weakened to a SHOULD as
MaxOutstandingUnexpectedPDUs will not be allocated beofre Login Request is
sent?

Mike

On Tue, Jul 17, 2012 at 11:45 AM, Alexander Nezhinsky
<[email-masked]>wrote:

> [email-masked]

---

From: <[email-masked]>
Date: Tue, 17 Jul 2012 15:57:51 -0400

Mike,

In particular, we need to say that a new target MUST respond to iSERHello with iSERHello when the iSERHelloRequired key was omitted from the negotiation.  OTOH, if iSERHelloRequired was negotiated to No, target receipt of iSERHello indicates a protocol error by the initiator - the target MAY choose to treat that as an error (e.g., and close the RCaP connection) or the target MAY choose to continue by responding with an iSERHello.

We need to state a recommended timeout value, at least as a default.  1 second seems like a reasonable default, and it could be adjusted via other means (e.g., system configuration).  There's no need to state a single value that MUST be used in all implementations.

I believe the quoted text has to be changed to align with this discussion.

Thanks,
--David

From: Michael Ko [mailto:[email-masked]]
Sent: Tuesday, July 17, 2012 3:07 PM
To: Alexander Nezhinsky
Cc: Black, David; [email-masked]
Subject: Re: [storm] iSER - what to do

An ""old" home-grown initiator implementation which will not send iSERHelloRequired=Yes but will send iSERHello" should not cause any problems.  An old target which does not support iSERHello will fail.  A new target will assume it is dealing with an old initiator which does not use iSERHello and will act according to the new spec for dealing with old initiators.  So when iSERHello shows up, it will follows the new spec for dealing with new initiators.

1. Should the spec say anything about the timeout value?  I agree with Alex that there is no practical value that will work in all cases.  So how about simply stating that the timeout values is outside the scope of the document?

"If the outcome of the iSCSI negotiation is to enable iSER-assisted mode, then on the initiator side, prior to sending the Login Request with the T (Transit) bit set to 1 and the NSG (Next Stage) field set to FullFeaturePhase, the iSCSI Layer MUST request the iSER Layer to allocate the connection resources necessary to support RCaP by
invoking the Allocate_Connection_Resources Operational Primitive."

Should the MUST requirement be weakened to a SHOULD as MaxOutstandingUnexpectedPDUs will not be allocated beofre Login Request is sent?

Mike
On Tue, Jul 17, 2012 at 11:45 AM, Alexander Nezhinsky <[email-masked]<mailto:[email-masked]>> wrote:

its own pace.

I agree

Ok

I'm ok with this

Alexander