Re: [Idr] Working group last call for draft-ietf-idr-tunnel-encaps-04

[Lou Berger <[e-mail]>]

On 05/23/2017 11:04 AM, Eric C Rosen wrote:
> On 5/23/2017 8:45 AM, Lou Berger wrote:
>>
>> On 5/22/2017 10:51 AM, Eric C Rosen wrote:
>>> On 5/21/2017 3:58 PM, Lou Berger wrote:
>>>> Overall I support this document being published, but after addressing
>>>> some comments, most importantly the first one:
>>>>
>>>> 1. I think this document needs to cover its impact on RF5566.  My
>>>> personal preference is for this document to subsume/obsolete 5566, but
>>>> the types defined in 5566 shouldn't be lost. (BTW I previously provided
>>>> authors with text to allow this draft to obsolete 5566 as well, and can
>>>> do so to the list if it would be helpful.)
>>> It is true that this document orphans RFC 5566 by obsoleting a document
>>> on which 5566 has a normative dependency.  However, incorporating the
>>> security-related material from 5566 in a way that makes it useful is
>>> going to be a fair amount of work, and the rest of the document
>>> shouldn;'t be held up waiting for that.
>> Well I did make this comment ~ 2 years ago, so you shouldn't be
>> surprised that I'm raising it now.  I'm not saying the WG has to follow
>> my preferred path (of having this doc replace 5566 as well), but I think
>> something has to be done to not leave 5566 "orphaned".
>>
>>> If you have some simple and
>>> non-controversial text to suggest, let's consider it.  (I checked my old
>>> emails on this topic but couldn't find it.)
>> my memory is that I put together a rough google doc at the time we
>> discussed whether your doc would deprecate 5512 or not.  (See
>> https://docs.google.com/document/d/1PSVTwf_QaRVWqhmkfaoRuP2JoGq_Nq9mlICUv6Z2BOQ/edit)
>>
>>
>>
>> To have tunnel-encaps deprecate 5566 as well, I think the following is
>> needed:
>> - Include the 5566 defined tunnel types (particularly 4, 5 and 6)
>> - include section 3.4.1 and 3.6 from the google doc (which is just edit
>> text from 5566 sections 3 and 4)
>> - perhaps include the 2nd paragraph of the Security Considerations
>> section
>>
>> Given that the text is mostly lifted from 5566, I'd hope this qualifies
>> as "simple and non-controversial".
> 
> RFC 5566 assumes that the IPsec tunnel info and the authenticator
> sub-TLV is conveyed on an update of the encapsulation SAFI.   I'm not
> convinced that there are no addtional security issues to be considered
> if this information is carried on updates of other address families. 

I think the assumption is that the sub-TLVs are conveyed with the
attribute.  I don't see how changing the SAFI impacts this.

> Also, note that in RFC 5512, the tunnels only extend to the BGP next
> hop, while in the tunnel-encaps draft the tunnel endpoints are not
> necessarily the BGP next hop.  This might also change some of the
> security properties.  I think this would have to be thought out fairly
> thoroughly, and I don't think that work's been done yet.

So leaving 5566 as is introduces this issue already (due to the
introduction of the remote endpoint sub-tlv).  If the we deprecate 5566
with this document, this can be trivially addressed by saying that the
endpoint Authenticator sub-tlv applies to the endpoint identified in the
Remote Endpoint Sub-TLV.
> Given that no one's been motivated to do the necessary work over the
> past two years, I don't think the draft should be held up waiting for it.
> 
Well the document was dormant for a time and had I realized it was
coming up for LC I would have offered to do the work yet again.  I guess
that's part of the reason to do a LC.

>> (new issue 6) In looking at the google doc, I was reminded of a
>> deficiency of  in 5512 that still exists in the wg draft.  Neither ever
>> define the format of the Tunnel TLV.  Don't you think it should?
>> Obviously, I do. Feel free to lift Section 3 of the google doc.
> 
> Figure 1 is not satisfactory?

Yes it is - I should have had another cup of coffee before sending the
message!

> 
>>
>>>> 2. WRT the Encapsulation Extended Community.  I find the following rule
>>>> very hard to parse:
>>>>
>>>>         A Tunnel Encapsulation attribute MUST NOT include a barebones
>>>>         Tunnel TLV.  Instead of placing such a TLV in the Tunnel
>>>>         Encapsulation attribute attached to a particular route, the
>>>>         corresponding Encapsulation Extended Community MUST be
>>>> attached to
>>>>         the route.
>>>>
>>>> Are you saying the extended community MUST be "barebones".  If so, I
>>>> agree as this matches 5512 formatting.  If you want this extended
>>>> community to carry sub-tlv information, then I see no backwards
>>>> compatibility basis for this and don't support this.  Either way, I
>>>> think this rule needs to be clarified.
>>> I'm not sure I understand what you are objecting to.  An extended
>>> community cannot carry sub-TLVs.
>> Perfect.  That's what I was hoping for/expecting, but I couldn't parse
>> your text in a way that made sense (at least to me). I also think the
>> current phasing goes beyond what some implementations do, e.g., send a
>> barebones EC all the time.
>>
>>> A number of existing applications exist that use the Encapsulation EC to
>>> specify a tunnel type, with no parameters.  The above-quoted paragraph
>>> attempts to encourage backwards compatibility with those applications by
>>> saying that if you want to specify a single tunnel type with no
>>> parameters, use the Encapsulation EC.  The Tunnel Encaps attribute would
>>> be used whenever the Encapsulation EC  is not sufficient.
>> How about replacing the quoted paragraph with:
>>       An Encapsulation Extended Community MUST be included when a
>> barebones
>>     Tunnel TLV is used to identify a tunnel endpoint. The corresponding
>> barebones
>>     Tunnel Encapsulation attribute MAY be omitted in this case.
>>
>> BTW our implementation always includes a "barebones" Encapsulation
>> Extended Community ;-)
> 
> For anyone trying to follow this thread, the Encapsulation EC was
> defined in RFC5512 as a simple way to specify a tunnel whose endpoint is
> the BGP next hop, and for which no encapsulation information needs to be
> provided.  That is, only the tunnel type is identified, and nothing is
> said about how to form the encapsulation for that tunnel, how to choose
> which packets go through that tunnel, etc.  In the draft, a "barebones
> Tunnel TLV" is defined to be a Tunnel TLV that that only specifies the
> tunnel type, and hence could just as well be encoded in the
> Encapsulation EC.
> 

> I don't see how it could be correct for an implementation to always
> include an Encapsulation EC.  Sometimes you need to specify more than
> the tunnel type in order to construct the encapsulation properly.

It doesn't "hurt" to include it always and there is an implementation
that does this, so why not allow it?

> 
> I don't think your suggested text is accurate, because a barebones
> Tunnel TLV does not identify a tunnel endpoint.

then just s/endpoint/type

> 
>>
>>>> 3. I'm not sure why the color sub-tlv is still needed. why not just use
>>>> the Color Extended Community alone?  (Is there really a case where the
>>>> recursive lookup in section 7 would have a different color?)
>>> The color sub-TLV is per-tunnel, the color EC is per route.  You could
>>> have multiple tunnels to a given endpoint, each with a different color.
>> okay, this works -- we solved this using multiple updates.   That said,
>> do you think the complexity of multiple tunnel tlvs in the same update
>> is needed or really used today (vs just using multiple/per tunnel tlv
>> updates)?  The encap safi certainly resulted in some pretty complex code
>> in order to "optimize" update sizes.  It seems that this too is a
>> similar trade off, and would be a good additional area of complexity
>> that can be simplified.  If it's not used today, I think we should
>> restrict the Tunnel Encapsulation Attribute to one Tunnel-TLV.
> 
> The use of multiple tunnels between a pair of endpoints seems to make
> sense, especially if the tunnels have different characteristics, and if
> color can be used to map particular traffic flows to particular
> tunnels.    This is something that has not changed from RFC 5512.  Note
> also that even the Encapsulation EC allows multiple tunnels to be
> specifiied (if they are of different types), since an EC attribute can
> contain multiple instances of the same extended community.

I'm not arguing it that it's not a real use case, but rather that it can
be solved using other existing, albeit more verbose, mechanisms.  Just
like encap safi itself, if no one is using this, I think it would be
better to simplify the solution and associated code.  If you are already
making use of this in deployed code, then I withdraw the proposed
simplification.

> 
>> If this is kept, it seems like there's some special aggregation rules
>> that could apply.
> 
> I'm not sure I understand this point.  If you receive two routes, and
> want to aggregate them so that you onluy need to propagate one upstream,
> and the two routes have different tunnel-encaps attributes, you need
> some policy to decide what to do.  But that's true even if the
> tunnel-encaps attribute only specifies one tunnel.
> 
Sure, and the policy may be to merge the tlv lists.  This is the kind of
information I'm suggesting should be added.

>...

Lou