Re: [trill] Adam Roach's Discuss on draft-ietf-trill-ecn-support-05: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

On 2/8/18 11:53 AM, Donald Eastlake wrote:
> Hi Adam,
>
> On Wed, Feb 7, 2018 at 8:12 PM, Adam Roach <[e-mail] 
> <mailto:[e-mail]>> wrote:
> > Adam Roach has entered the following ballot position for
> > draft-ietf-trill-ecn-support-05: Discuss
> >
> > ...
> >
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > Thanks to the authors, chairs, shepherd, and working group for the 
> effort that
> > has been put into this document.
> >
> > I have concerns about some ambiguity and/or self-contradiction in this
> > specification, but I suspect these should be easy to fix. In 
> particular, the
> > behavior defined in Table 3 doesn't seem to be consistent with the 
> behavior
> > described in the prose.
> >
> > For easy reference, I've copied Table 3 here:
> >
> >> +---------+----------------------------------------------+
> >>       | Inner   |  Arriving TRILL 3-bit ECN Codepoint Name     |
> >>       | Native  +---------+------------+------------+----------+
> >>       | Header  | Not-ECT | ECT(0)     | ECT(1)     |     CE   |
> >> +---------+---------+------------+------------+----------+
> >>       | Not-ECT | Not-ECT | Not-ECT(*) | Not-ECT(*) |  <drop>  |
> >>       |  ECT(0) |  ECT(0) |  ECT(0)    |  ECT(1)    |     CE   |
> >>       |  ECT(1) |  ECT(1) |  ECT(1)(*) |  ECT(1)    |     CE   |
> >>       |    CE   |      CE |      CE    |      CE(*) |     CE   |
> >> +---------+---------+------------+------------+----------+
> >>
> >>                      Table 3. Egress ECN Behavior
> >>
> >>  An asterisk in the above table indicates a currently unused
> >>  combination that SHOULD be logged. In contrast to [RFC6040], in TRILL
> >>  the drop condition is the result of a valid combination of events and
> >>  need not be logged.
> >
> > The prose in this document indicates:
> >
> >  1. Ingress gateway either copies the native header value to the 
> TRILL ECN
> >     codepoint (resulting in any of the four values above) or doesn't 
> insert
> >     any ECN information in the TRILL header.
> >
> >  2. Intermediate gateways can set the CCE flag, resulting in "CE" in the
> >     table above.
> >
> > Based on the above, a packet arriving at an egress gateway can only 
> be in one of
> > the following states:
> >
> >  A. TRILL header is Not-ECT because no TRILL node inserted ECN 
> information.
> >
> >  B. TRILL header value == Native header value because the ingress 
> gateway
> >     copied it from native to TRILL.
> >
> >  C. TRILL header is "CE" because an intermediate node indicated 
> congestion.
>
> Sort of... But note that the TRILL header ECN bit s can indicate 
> non-ECT while the CCE bit is set making the overall TRILL Header "CE".

Right. That's part of case C. My states above assume application of 
Table 2 has already taken place.

>
> > If that's correct, I would think that any state other than those 
> three needs
> > to be marked with an (*). In particular, these two states fall into that
> > classification, and seem to require an asterisk:
> >
> >   - Native==CE && TRILL==ECT(0)
> >
> >   - Native==ECT(0) && TRILL==ECT(1)
>
> I would defer to my co-author Bob Briscoe but it looks to me like you 
> have a good point.

Thanks; I'll wait to hear from Bob.

> > In addition, the behavior this table defines for Native==ECT(0) && 
> TRILL==ECT(1)
> > is somewhat perplexing: for this case, the value in the TRILL header 
> takes
> > precedence; however, when Native==ECT(1) && TRILL==ECT(0) the Native 
> header
> > takes precedence. Or, put another way, this table defines ECT(1) to 
> always
> > override ECT(0). I don't find any prose in here to indicate why this 
> needs to be
> > treated differentially, so I'm left to conclude that this is a 
> typographical
> > error. If that's not the case, please add motivating text to Table 3 
> explaining
> > why ECT(1) is treated differently than ECT(0) for baseline ECN behavior.
>
> As noted in Section 4.1, there is an ECN variation where ECT(0) just 
> indicates ECT while ECT(1) indicates congestion of a lesser severity 
> level has been encountered than that indicated by CE. I believe the 
> dominance of ECT(1) over ECT(0) is designed to not interfere with this 
> variation.

Yes, and section 4 opens with the explanation that "Section 3 specifies 
interworking between TRILL and the original standardized form of ECN in 
IP [RFC3168]." Beyond that, I wouldn't expect any of the text in 
non-normative section 4 or its subsections to have bearing on the 
normative table in Section 3.

My reading of RFC 8311 is that it contemplates a series of experiments 
beyond those currently under development. It may well be that the 
current experiments consider ECT(1) to have a higher severity than 
ECT(0), but that this may not make sense for future experiments. Even if 
it does, I don't see guidance in RFC 8311 (or any other update to RFC 
3168) that suggests such a relationship between ECT(0) and ECT(1) exists.

On top of this (as implied by the existence of section 4), the TRILL 
handling for ECN will need to vary from experiment to experiment. It 
seems reasonable that part of this variability would include different 
mapping of ECN bits by the egress gateway.

Basically, I see two ways this can be resolved, although I'm happy to 
hear alternatives so long as they end up with the ECN and TRILL 
documents being in a consistent and future-proof state:

Approach 1: Revise Table 3 so that ECT(0) and ECT(1) are treated the 
same (i.e., pick one of "TRILL header wins" or "Native header wins" -- I 
would suggest "Native header wins," for maximal compatibility with older 
ECN clients but I'm not dogmatic on this point), and then include a 
normative statement that allows RFC 8311 experiments to override this 
mapping as makes sense for their design.

Approach 2: Leave the table as is, but add an explanation of why ECT(1) 
is given preferential treatment over ECT(0). Add a normative dependency 
from this document to a new document that updates RFC 8311 to add a 
requirement that any experiments that treat ECT(0) differently than 
ECT(1) MUST be designed such ECT(0) always indicates a lower severity of 
congestion than ECT(1). I presume that this new document would need to 
be done in coordination with TSVWG.

I think Approach 1 is more straightforward, but if there's a feeling in 
the working group that we need default egress behavior that is 
forwards-compatible with yet-undesigned experiments, then I think 
Approach 2 is what you need.

/a