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

[Adam Roach <[e-mail]>]

Thanks! The changes look great, and I believe that all of my comments 
have been addressed.

/a

On 2/25/18 4:44 PM, Donald Eastlake wrote:
> Hi Adam,
>
> The just posted revision -07 is believed to resolve your comments.
>
> Thanks,
> Donald
> ===============================
>  Donald E. Eastlake 3rd   [phone] (cell)
>  155 Beaver Street, Milford, MA 01757 USA
> [e-mail] <mailto:[e-mail]>
>
> On Fri, Feb 23, 2018 at 6:57 PM, Bob Briscoe <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>     @Adam, Thx for your patience. (It turned out I didn't see your
>     DISCUSS cos I had created a bug in my own e-mail filters - sry for
>     that).
>
>     @Donald (as editor), Adam's right that we risk implementers doing
>     their own thing if they judge that Table 3 might be wrong, because
>     its apparent illogicality is not explained without reading the
>     references. So, I've suggested one further change to text, inline...
>
>
>
>     On 20/02/18 22:22, Adam Roach wrote:
>>     Bob --
>>
>>     Thanks for taking the time to explain the history and rationale
>>     of Table 3 to me. I'm going to clear my DISCUSS based on this
>>     explanation, since it seems that the technical solution in the
>>     document is correct, if a bit non-obvious. Some additional
>>     comments inline.
>>
>>     [For those of you who did not see Bob's response: it appears that
>>     my original DISCUSS email did not reach him,  so he replied to a
>>     reduced audience. I have restored the traditional IESG review
>>     distribution to this email. For this reason, I am eliding less of
>>     Bob's response than I usually would]
>>
>>     On 2/20/18 12:51 PM, Bob Briscoe wrote:
>>>
>>>>>>
>>>>>>             On Thu, Feb 8, 2018 at 2:15 PM, Adam Roach
>>>>>>             <[e-mail] <mailto:[e-mail]>> wrote:
>>>>>>
>>>>>>                 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:
>>>>>>
>>>     [BB] These states are not tagged (*) cos they can arise with
>>>     variants of ECN marking behaviour referred to in Section 4
>>>     (explained beneath each bullet below). By design (explained
>>>     further below), a consistent encap and decap behaviour is
>>>     intended to support all variants of ECN, because subnet ingress
>>>     and egress nodes will very probably be deployed independently of
>>>     each other and of intermediate nodes and/or L4 endpoints.
>>>
>>>     Nonetheless, you're right that this is completely non-obvious to
>>>     a reader coming to this new. So let's change the text:
>>>     CURRENT:
>>>         An asterisk in the above table indicates a currently unused
>>>         combination that SHOULD be logged. In contrast to [RFC6040 <https://tools.ietf.org/html/rfc6040>], in TRILL
>>>         the drop condition is the result of a valid combination of events and
>>>         need not be logged.
>     SUGGESTED (2nd attempt):
>
>         An asterisk in the above table indicates a combination that is currently
>         unused in all variants of ECN marking (see Section 4) and therefore
>         SHOULD be logged.
>
>         With one exception, the mappings in Table 3 are consistent with those
>         for IP-in-IP tunnels [RFC6040], which ensures backward compatibility with
>         all current and past variants of ECN marking (see Section 4). It also
>         ensures forward compatibility with any future form of ECN marking that
>         complies with the guidelines in [ECNencapGuide], including cases where
>         ECT(1) represents a second level of marking severity below CE.
>
>         The one exception is that the drop condition in Table 3 need not be
>         logged because, with TRILL, it is the result of a valid combination of
>         events.
>
>
>>
>>     Thanks. That definitely helps; but see my comments below.
>>
>>>>>>>                 >
>>>>>>>                 >   - Native==CE && TRILL==ECT(0)
>>>>>>
>>>     [BB] You have indeed highlighted an awkward wrinkle here - we
>>>     could have tagged this combination with '(*)', cos it is
>>>     strictly currently unused by trill. However, I'm going to argue
>>>     against a '(*)'...
>>>
>>>     My explanation starts with some history: Back in 2001 when ECN
>>>     in IP was first defined [RFC3168], an IP-in-IP tunnel ingress
>>>     set the outer to ECT(0) if ECN in the native header was anything
>>>     but Not-ECT (0b00). This was intended to close off a perceived
>>>     covert channel in IPsec. However, since then the security area
>>>     agreed that this added over-paranoid complexity. So, since
>>>     [RFC6040] an IP-in-IP tunnel ingress now just copies the native
>>>     ECN to the outer, and TRILL ECN follows this new simpler model.
>>>
>>>     Altho this spec does not allow a TRILL ingress RBrIdge to
>>>     exhibit the old IP-in-IP legacy behaviour, I think it best not
>>>     to tag this combination as 'currently unused'. Because, wherever
>>>     possible, we want trill's ECN encap and decap  to mimic IP-in-IP
>>>     (a principle explained further down this email).
>>>
>>>     A more specific reason: there is already a congestion monitoring
>>>     technique being proposed that fakes the legacy IP-in-IP
>>>     behaviour to measure congestion across a tunnel
>>>     (draft-ietf-tsvwg-tunnel-congestion-feedback). Before it becomes
>>>     an RFC, I wouldn't be surprised if that draft gets extended from
>>>     IP-in-IP to IP-in-TRILL and other L2 protocols as well. If that
>>>     likely event happened, we would have to update this trill-ecn
>>>     spec to remove the '(*)' again. In the mean time, there's little
>>>     harm in not logging a combination that is not harmful, even tho
>>>     it is stricty 'currently unused'.
>>>
>>>     (sry about the triple negative!)
>>
>>     This is a great explanation. I understand the risks of putting
>>     forward-looking speculation in RFCs, but could I convince you to
>>     at least explain in the document that the reason for this state
>>     not being considered out of the ordinary is to mimic legacy
>>     IP-in-IP ECN behavior? Any acknowledgement of why this case is
>>     being treated the way it is would be sufficient to prevent
>>     implementors from deciding that the table is wrong.
>>
>>>
>>>>>>>                 >
>>>>>>>                 >   - Native==ECT(0) && TRILL==ECT(1)
>>>>>>
>>>     [BB] The standards track PCN variant of ECN [RFC6660] uses
>>>     ECT(1) as a lower severity of congestion notification than CE.
>>>     Hence the asymmetry where a native ECT(0) header can have an
>>>     ECT(1) outer, but not vice versa.
>>
>>     As with the case above, it would be good (inasmuch as it would
>>     serve to un-confuse implementors) to include an explanation that
>>     this case makes sense due to RFC 6660's handling of ECT(0) versus
>>     ECT(1). Such a comment would also address my concern quoted below.
>>
>>>
>>>>>>>
>>>>>>>                 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.
>>>>>>
>>>     [BB]
>>>     * PCN is standards track, hence PCN-specific combinations are
>>>     not tagged 'currently unused' in (normative) Section 3.
>>>     * L4S is experimental track, but it doesn't use any different
>>>     combinations of ECN than standard ECN, so it doesn't alter the
>>>     CU asterisks in section 3.
>>>
>>>     Section 4 is informative, only because it doesn't define the
>>>     protocol spec; it merely explains how the normative spec in
>>>     Section 3 allows for ECN variants (whether stds track or
>>>     experimental).
>>
>>     Thanks. The answer to this that I find compelling is your
>>     explanation below about RFC 6040's relative ordering of ECN
>>     markings, which I think should be at least mentioned in passing here.
>>
>>>
>>>
>>>>>>
>>>>>>                 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.
>>>>>>
>>>     As above, the possibility that ECT(1) is a higher severity than
>>>     ECT(0) is already on the standards track (PCN [RFC6660]).
>>>
>>>     As far as possible, we want trill subnet endpoints to follow the
>>>     same ECN behaviour as IP-in-IP tunnel endpoints [RFC6040]. The
>>>     explicitly stated philosophy of RFC6040 and [ECNencapGuide]
>>>     (which is referred to from the Intro to this trill spec), is for
>>>     respectively all tunnel endpoints and subnet endpoints to have
>>>     the same mechanical, dumb ECN behaviour, whatever the variant of
>>>     ECN in use. Because in the incrementally deployed public
>>>     Internet, one cannot know what tunnels and subnets might be
>>>     encountered across different paths.
>>>
>>>
>>>>>>
>>>>>>                 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.
>>>>>>
>>>     Nope. By design, all variants of ECN use the same encap and
>>>     decap behaviours, otherwise they would be undeployable to all
>>>     intents and purposes. Perhaps we should make that clearer in
>>>     this sentence introducing Section 4:
>>>
>>>     CURRENT:
>>>         The ECN wire protocol for TRILL (Section 2
>>>     <https://tools.ietf.org/html/draft-ietf-trill-ecn-support-05#section-2>) has been designed to
>>>         support the other known variants of ECN,
>>>     SUGGESTED:
>>>         The ECN wire protocol for TRILL (Section 2
>>>     <https://tools.ietf.org/html/draft-ietf-trill-ecn-support-05#section-2>) and the ingress (Section 3.1)
>>>         and egress (Section 3.3) ECN behaviours have been designed to
>>>         support the other known variants of ECN,
>>
>>     This sounds good. It might be worth mentioning that this behavior
>>     is intended to also be forward-compatible with future ECN
>>     variants, as long as they conform to RFC 6040's notion of
>>     relative codepoint priorities.
>>
>>>
>>>>>>
>>>>>>                 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).
>>>>>>
>>>     [BB] I hope you're happy with the additional text I've suggested
>>>     for why Table 3 was like it was. In summary, consistency with
>>>     IP-in-IP is paramount wherever possible.
>>
>>     That's exactly what the additional text should say. :)  I don't
>>     see that in your current proposal.
>>
>>>
>>>>>>                 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.
>>>>>>
>>>     [BB] That's already catered for in [RFC6040] (stds track) for
>>>     IP-in-IP, quoting:
>>>         o  In all other cases where the inner supports ECN, the decapsulator
>>>            MUST set the outgoing ECN field to the more severe marking of the
>>>            outer and inner ECN fields, where the ranking of severity from
>>>            highest to lowest is CE, ECT(1), ECT(0), Not-ECT.  This in no way
>>>            precludes cases where ECT(1) and ECT(0) have the same severity;
>>>
>>>     and in [ECNencapGuide] (intended BCP) for IP-in-any-L2, quoting:
>>>         4.  Congestion indications may be encoded by a severity level.  For
>>>             instance increasing levels of congestion might be encoded by
>>>             numerically increasing indications, e.g. pre-congestion
>>>             notification (PCN) can be encoded in each PDU at three severity
>>>             levels in IP or MPLS [RFC6660 <https://tools.ietf.org/html/rfc6660>].
>>>
>>>             If the arriving inner header is an ECN-PDU, where the inner and
>>>             outer headers carry indications of congestion of different
>>>             severity, the more severe indication SHOULD be forwarded in
>>>             preference to the less severe.
>>>
>>>
>>>>>>
>>>>>>                 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
>>>>>>
>>>     The way ECN encap & decap has been designed already follows your
>>>     'approach 2'; where the same consistent behaviour at all encaps
>>>     and decaps can be exploited in both backward and forward
>>>     compatible ways.
>>
>>     Given your explanation above, I agree that such is the case. I
>>     think including enough of that explanation that implementors
>>     don't find themselves double-guessing the specification in Table
>>     3 would ultimately aid in interoperability.
>>
>>>
>>>     Indeed, here's proof that this forward compatibility is not just
>>>     theoretical...
>>>
>>>     L4S came along after all this, and required drop probability to
>>>     be the square of the marking probability, but only for one of
>>>     the ECN codepoints. We managed to contrive the marking behaviour
>>>     of an intermediate trill RBridge that supports L4S so that any
>>>     trill egress will achieve this precise squaring, even an egress
>>>     without any ECN or L4S logic, and no squaring logic either (see
>>>     Appendix A).
>>>
>>>     Cheers, and thx again for noticing all this.
>>>
>>>
>>>
>>>     Bob
>>
>>
>>     _______________________________________________
>>     trill mailing list
>>     [e-mail] <mailto:[e-mail]>
>>     https://www.ietf.org/mailman/listinfo/trill
>>     <https://www.ietf.org/mailman/listinfo/trill>
>
>     -- 
>     ________________________________________________________________
>     Bob Briscoehttp://bobbriscoe.net/
>
>