Re: [Idr] AD Review of draft-ietf-idr-tunnel-encaps-17

[John Scudder <[e-mail]>]

Alvaro,

Hi! Detailed replies in line below.

WG: Note this change:

OLD:
   If the Length field of a Color sub-TLV has a value other than 8, or
   the first two octets of its value field are not 0x030b, the sub-TLV
   should be treated as if it were an unrecognized sub-TLV (see
   Section 12).

NEW:
   If the Length field of a Color sub-TLV has a value other than 8, or
   the first two octets of its value field are not 0x030b, the sub-TLV
   MUST be treated as if it were an unrecognized sub-TLV (see
   Section 12).

If there are any objections to this strengthening of the requirement, please raise them ASAP.

Also note the change to Section 3.1.1 discussed about halfway down the message. 

> On Aug 4, 2020, at 4:01 PM, Alvaro Retana <[e-mail]> wrote:
> 
> [External Email. Be cautious of content]
> 
> 
> Hi!
> 
> These are in-line comments to complement the thread related to -15.
> 
> Thanks!!
> 
> Alvaro.
> 
> 
> [Line numbers from idnits.]
> 
> 
> ...
> 277     1.4.  Use Case for The Tunnel Encapsulation Attribute
> 
> 279        Consider the case of a router R1 forwarding an IP packet P.  Let D be
> 280        P's IP destination address.  R1 must look up D in its forwarding
> 281        table.  Suppose that the "best match" route for D is route Q, where Q
> 282        is a BGP-distributed route whose "BGP next hop" is router R2.  And
> 283        suppose further that the routers along the path from R1 to R2 have
> 284        entries for R2 in their forwarding tables, but do NOT have entries
> 285        for D in their forwarding tables.  For example, the path from R1 to
> 286        R2 may be part of a "BGP-free core", where there are no BGP-
> 287        distributed routes at all in the core.  Or, as in [RFC5565], D may be
> 288        an IPv4 address while the intermediate routers along the path from R1
> 289        to R2 may support only IPv6.
> 
> [minor] s/NOT/not/g
> This is not a keyword -- even if simply used to emphasize, you'll be
> asked to change the case.

Changed.

> 
> 
> ...
> 307        This document specifies a way in which BGP itself can be used by a
> 308        given BGP speaker to tell other BGP speakers, "if you need to
> 309        encapsulate packets to be sent to me, here's the information you need
> 310        to properly form the encapsulation header".  A BGP speaker signals
> 311        this information to other BGP speakers by using a new BGP attribute
> 312        type value, the BGP Tunnel Encapsulation Attribute.  The Tunnel
> 313        Encapsulation attribute MAY be used in any BGP UPDATE message whose
> 314        AFI/SAFI is 1/1 (IPv4 Unicast), 2/1 (IPv6 Unicast), 1/4 (IPv4 Labeled
> 315        Unicast), 2/4 (IPv6 Labeled Unicast), 1/128 (VPN-IPv4 Labeled
> 316        Unicast), 2/128 (VPN-IPv6 Labeled Unicast), or 25/70 (Ethernet VPN,
> 317        usually known as EVPN)).
> 
> [major] "The Tunnel Encapsulation attribute MAY be used..."  Please
> keep normative language out of the use case description.  Also, this

> is the same text as in §6 -- specify things only once.

I’m a little confused — as you rightly post out, a use case isn’t normative, it doesn’t specify protocol. Therefore, by definition we’re not specifying things twice. No? On the other hand, it probably doesn’t add anything to get so detailed in the use case, so I’ve removed that sentence.

> 
> 
> 319        In a given BGP update, the encapsulation information is specified in
> 320        the BGP Tunnel Encapsulation Attribute.  This attribute specifies the
> 321        encapsulation protocols that may be used as well as whatever
> 322        additional information (if any) is needed in order to properly use
> 323        those protocols.  Other attributes, e.g., communities or extended
> 324        communities, may also be included.
> 
> [nit] This last paragraph seems to repeat information from the
> previous one...consider merging.

With the removal of the laundry list of AFI/SAFIs, it was the natural thing to merge the paragraphs.

> 
> 
> ...
> 390     3.1.  The Tunnel Egress Endpoint Sub-TLV
> 
> [major] The type of this sub-TLV is not indicated inline...

Fixed.

> 
> 
> ...
> 425        If the Address Family subfield contains the value for IPv4, the
> 426        address subfield MUST contain an IPv4 address (a /32 IPv4 prefix).
> 
> 428        If the Address Family subfield contains the value for IPv6, the
> 429        address subfield MUST contain an IPv6 address (a /128 IPv6 prefix).
> 
> [major] What should the receiver do if the address is not a host prefix?

Let’s consider the possibilities. Keep in mind there’s no length field for the address field, so it has to be 4 or 16 octets. (That is, it’s not really a prefix except in the pedantic sense that a /32 or a /128 is a “prefix” that covers the entire address.) If the sender put in too many octets, or too few, it’s a simple length error, and the action is already specified. If the sender put in the right number of octets, but the content is nonsense, the nonsense might be so nonsensical that it fails the special-purpose registry (i.e., Martian) check, which is again a covered case. If the nonsense is syntactically ok enough, but it’s not currently forwardable, then the tunnel wouldn’t be considered feasible, which we’ve already discussed at length. If the nonsense just happens to be a feasible host route, it’s a little hard to call it “nonsense”. I can’t think of any other “not a host prefix” cases. So, I think this is already covered in the document.

> 
> 
> ...
> 466           *  0, if the Address Family subfield contains the value zero.
> 
> [major] s/0/6

Ouch. Thanks!

> 
> 
> 468        o  The IP address in the sub-TLV's address subfield is listed in the
> 469           relevant Special-Purpose IP Address Registry [RFC6890] as either
> 470           not a valid destination, or not forwardable.
> 
> [] I don't think that pointing at what is listed in a registry is the
> best.  Suggestion:
> 
>   The IP address in the sub-TLV's address subfield is either not
>   Forwardable or not a valid Destination as defined in the Special-Purpose
>   Address Registries [RFC6890].

I disagree. RFC 6890 has two relevant categories: “destination” can be true or false, and “forwardable” can be true or false. Our text is not talking about whether we have a forwarding route for the address, that’s dealt with in the feasibility condition. The text is talking about whether, according to RFC 6890, the address is a Martian, and the term RFC 6890 chose to use for this is “forwardable". I’ve tentatively rewritten as "The IP address in the sub-TLV's address subfield lies within a block listed in the relevant Special-Purpose IP Address Registry [RFC6890] with either a “destination” attribute value or a “forwardable” attribute value of “false”. (Such routes are sometimes colloquially known as “Martians".)” The new text is more ponderous but hopefully even less ambiguous.

> 
> 
> ...
> 477        If the Tunnel Egress Endpoint sub-TLV is malformed, the TLV
> 478        containing it is also considered to be malformed.  However, the
> 479        Tunnel Encapsulation attribute MUST NOT be considered to be malformed
> 480        in this case; other TLVs in the attribute MUST be processed (if they
> 481        can be parsed correctly).
> 
> 483        Error Handling is detailed in Section 12.
> 
> [] Yes, Error Handling is described later; please remove the previous
> paragraph ("If the Tunnel Egress Endpoint sub-TLV is malformed...") as
> this behavior is already specified there.

Done.

> 
> 
> ...
> 489     3.1.1.  Validating the Address Field
> 
> 491        This section details a procedure that MAY be applied to validate that
> 492        when traffic is sent to the IP address depicted in the Address Field,
> 493        it will go to the same AS as it would go to if the Tunnel
> 494        Encapsulation Attribute were not present.  See Section 13 for
> 495        discussion of the limitations of this procedure.
> 
> [minor] s/Section 13/Section 14

Thanks.

> 
> [major] "a procedure that MAY be applied"  If a sub-TLV is considered
> malformed, then there are mandatory actions to be taken (§12).  Making
> this procedure optional creates a Normative conflict.  IOW, if the
> process is optional then the criteria should not be used to determine
> if the sub-TLV is malformed...if some implementations do it this way
> and other don't, or chose something different then the behavior
> becomes non-deterministic.
> 
> I made a similar comment before:
> 
>>> [major] "one way" I hope that it is the MTI way -- otherwise, the
>>> determination of the sub-TLV being malformed is not deterministic.
> 
> ...and your answer...
> 
>> This is not relevant any more, however 3.1.1 as written is optional, so
>> not “MTI". I don’t think it’s non-deterministic, though — you either do it
>> as written, or you don’t.
> 
> Of course...you either do it or don't...
> 
> However, it introduces potential interoperability issues: given the
> same information, the actions of two different routers may be
> different.

I’m having a hard time accepting that this is a major issue. Yes, routers with different feature sets may exhibit different behaviors when presented with the same inputs. This has always been true and will always be true. The question we should ask ourselves is, will this cause a problem in the field? You say “potential interoperability issues”. I don’t see them: if you send the same update to two routers, if one doesn’t install a tunnel due to failing 3.1.1 and the other does install the tunnel due to not implementing 3.1.1 (or having it disabled in configuration), the network doesn’t melt down. The second one forwards packets using the tunnel, the first does not. That’s not what we normally mean by an “interoperability issue”, that term makes me think more of “the session might reset” or similar. It’s inconsistent forwarding, true, but this is something tunneled networks excel at handling without drama.

This is exactly the same kind of issue one might see with, say, route origin validation, if one router implements it and one doesn’t. For that matter, it’s the same kind of issue we can create with simple policy. And the behavior isn’t “non-deterministic”. It’s completely deterministic, if you know what software and configurations the network is running.

If we made 3.1.1 mandatory to implement, it would raise an additional bar to implementations, some of which probably don’t need the functionality, for example if they’re only going to be used in a closed network. Also, if we made 3.1.1 mandatory, I think that unless the WG grants an exception to the IDR implementation policy, we have to hold up progress of the document until we have implementation — and I’m feeling the heat to get this thing shipped, because of the number of documents stuck in downref waiting on us. If we have to hold it up, so be it, but if so let’s do it for good reasons. (The co-chair in me says: yes, John, but the fact you just found and fixed a bug in 3.1.1 — see below — means that maybe you DO need some implementation experience. :-( It’s hard when you have to argue with yourself.)

On the other hand, if we remove 3.1.1 entirely and just say “sorry, there’s no attempt to do this at all” that gets rid of your MTI complaint, but it doesn’t seem like a good thing to do, and probably we end up going down a security rabbit hole instead.

Finally, I think that if we made 3.1.1 MTI, it’s nearly pointless unless we make RFC 6811 support mandatory also… which seems like a slippery slope.


> 
> 
> [major] "validate that when traffic is sent to the IP address depicted
> in the Address Field, it will go to the same AS as it would go to if
> the Tunnel Encapsulation Attribute were not present."  Not only is
> this process not validating what happens to the traffic, but the
> purpose mentioned is not the same as what is proposed in §3.1: "that
> the IP address in the sub-TLV's address subfield does not belong to
> the Autonomous System (AS) that originated the route that contains the
> attribute"

You’re technically right of course. I’m loth to completely delete the paragraph though, because when read with -Wno-pedantic, I think it provides valuable information to the reader about WHY we are doing this. Now it says:

"This section details a procedure that MAY be applied to validate that the IP address in the sub-TLV's address subfield belongs to the AS that originated the route that contains the attribute. Doing this is thought to increase confidence that when traffic is sent to the IP address depicted in the Address Field, it will go to the same AS as it would go to if the Tunnel Encapsulation Attribute were not present, although of course it cannot guarantee it.”

> 
> 
> 
> 497        The Route Origin ASN (Autonomous System Number) of a BGP route that
> 498        includes a Tunnel Encapsulation Attribute can be determined by
> 499        inspection of the AS_PATH attribute, according to the procedure
> 500        specified in [RFC6811] section 2.  Call this value Route_AS.
> 
> 502        In order to determine the Route Origin ASN of the address depicted in
> 503        the Address Field of the Tunnel Egress Endpoint sub-TLV, it is
> 504        necessary to determine the forwarding route, that is, the route
> 505        installed in the Forwarding Information Base that will be used to
> 506        forward traffic toward that address.  The Address Field's Route
> 507        Origin ASN is the Route Origin ASN of that route, or the
> 508        distinguished value "NONE2" if the forwarding route has no AS Path,
> 509        for example if that route's source is a protocol other than BGP.
> 510        (Note that this is a distinct case from a route that has an empty AS
> 511        Path.)  Call this value Egress_AS.
> 
> [minor] s/Forwarding Information Base/Routing Table
> For consistency with rfc4271.
> 
> [major] If multiple routing tables exist, which should be used?  This
> point is related to the reachability question in (now) §6.

That is EXACTLY what we were trying to finesse by saying “FIB” and not “RIB”. I’ve split the difference by removing “FIB” but not pasting “RIB” in, so it now reads "that is, the route that will be used to forward traffic toward that address”. The closer you get to the metal in a router, the more our nice abstractions break down. All we really want to say here is, “how ya gonna get the packet there? What route are you using? That’s the one you need to check.”


> 
> It seems to me that the BGP UPDATE
> 
> 
> [major] "The Address Field's Route Origin ASN is the Route Origin ASN
> of that route..."  This sounds like a circular reference...it is not,
> but there seems to be an overload of the Route Origin ASN term...

I made this go away when I rewrote the paragraph, see below.

> Also, consider starting with explaining that the origin AS is the
> indicator of "belonging".  For example:
> 
>   An address belongs to an AS if the route that covers it was originated in it.
> 
>   For BGP...Route Origin ASN [rfc6811]...
> 
>   For routes from other sources...

Seems as though if I did too much of that, a careful reviewer might say something like “please only specify things once” :-) so I just added "The notion of "belonging to" an AS is expanded on below”. 

> 
> 
> [major] "or the distinguished value "NONE2" if the forwarding route
> has no AS Path, for example if that route's source is a protocol other
> than BGP. (Note that this is a distinct case from a route that has an
> empty AS Path.)"
> 
> An IGP route, for example, covers prefixes belonging to the local AS.
> An iBGP route originated at the local AS would have an empty AS_PATH.
> These 2 routes wouldn't match based on the criteria above, even though
> clearly the intent of the route belonging to the same AS is satisfied.
> IOW, the use of NONE2 would not allow tunnels to be set up within the
> local AS, or inside a confederation.

Thanks for catching that, fixed. It turns out RFC 6811 mostly covers this case already, I copied a bit of text from there. But, working through it exposed another bonehead error: the text as written would almost always have you testing against an IGP route to determine the value of Egress_AS if you followed it to the letter, and an IGP route doesn’t have an AS. Here’s the rewrite of the paragraph:

   In order to determine the Route Origin ASN of the address depicted in
   the Address Field of the Tunnel Egress Endpoint sub-TLV, it is
   necessary to consider the forwarding route, that is, the route that
   will be used to forward traffic toward that address.  This route is
   determined by a recursive route lookup operation for that address, as
   discussed in [RFC4271] Section 5.1.3.  The relevant AS Path to
   consider is the last one encountered while performing the recursive
   lookup; the procedures of RFC6811 Section 2 are applied to that AS
   Path to determine the Route Origin ASN.  If no AS Path is encountered
   at all, for example if that route's source is a protocol other than
   BGP, the Route Origin ASN is the BGP speaker's own AS number.  Call
   this value Egress_AS.



> 
> 
> 513        If Route_AS does not equal Egress_AS, then the Tunnel Egress Endpoint
> 514        sub-TLV is considered not to be valid.  In some cases a network
> 515        operator who controls a set of Autonomous Systems might wish to allow
> 516        a Tunnel Egress Endpoint to reside in an AS other than Route_AS;
> 517        configuration MAY allow for such a case, in which case the check
> 518        becomes, if Egress_AS is not within the configured set of permitted
> 519        AS numbers, then the Tunnel Egress Endpoint sub-TLV is considered not
> 520        to be valid.
> 
> [major] s/considered not to be valid/considered to be malformed

Done.

> 
> 
> 
> ...
> 545     3.2.1.  VXLAN
> ...
> 564           V: This bit is set to 1 to indicate that a VN-ID (Virtual Network
> 565           Identifier) is present in the Encapsulation sub-TLV.  If set to 0,
> 566           the VN-ID field is disregarded.  Please see Section 8.
> 
> [] s/Section 8/Section 9

Fixed, for this and all the other places you noted.

> 
> 
> ...
> 613        o  Section 8 describes how the VNI field of the VXLAN encapsulation
> 614           header is set.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 622     3.2.2.  VXLAN GPE
> ...
> 646           V: This bit is set to 1 to indicate that a VN-ID is present in the
> 647           Encapsulation sub-TLV.  If set to 0, the VN-ID field is
> 648           disregarded.  Please see Section 8.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 669        o  Section 8 describes how the VNI field of the VXLAN GPE
> 670           encapsulation header is set.
> 
> [] s/Section 8/Section 9
> 
> 
> 672     3.2.3.  NVGRE
> ...
> 691           V: This bit is set to 1 to indicate that a VN-ID is present in the
> 692           Encapsulation sub-TLV.  If set to 0, the VN-ID field is
> 693           disregarded.  Please see Section 8.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 740        o  Section 8 describes how the VSID (Virtual Subnet Identifier) field
> 741           of the NVGRE encapsulation header is set.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 898     3.4.2.  Color Sub-TLV
> 
> 900        The Color sub-TLV, whose type code is 4, MAY be used as a way to
> 901        "color" the corresponding Tunnel TLV.  The value field of the sub-TLV
> 902        is eight octets long, and consists of a Color Extended Community, as
> 903        defined in Section 4.3.  For the use of this sub-TLV and Extended
> 904        Community, please see Section 7.
> 
> [] s/Section 7/Section 8

Fixed, for this and the other place you noted.

> 
> 
> 906        If the Length field of a Color sub-TLV has a value other than 8, or
> 907        the first two octets of its value field are not 0x030b, the sub-TLV
> 908        should be treated as if it were an unrecognized sub-TLV (see
> 909        Section 12).
> 
> [major] "should be treated as if it were an unrecognized sub-TLV"
> Should that be Normative?  Why is this not a required action?  When
> would it be ok to not treat the sub-TV as unrecognized?

Changed to MUST. 

> 
> 
> 911     3.5.  Embedded Label Handling Sub-TLV
> ...
> 935        The Embedded Label Handling sub-TLV, whose type code is 9, may be
> 936        included in any TLV of the Tunnel Encapsulation attribute.  If the
> 937        Tunnel Encapsulation attribute is attached to an UPDATE of a non-
> 938        labeled address family, then the sub-TLV MUST be disregarded.  If the
> 939        sub-TLV is contained in a TLV whose Tunnel Type does not have a
> 940        virtual network identifier in its encapsulation header, the sub-TLV
> 941        MUST be disregared.  In those cases where the sub-TLV is ignored, it
> 942        SHOULD NOT be stripped from the TLV before the route is propagated.
> 
> [nit] s/disregared/disregarded

Done.

> 
> 
> ...
> 954        Please see Section 8 for the details of how this sub-TLV is used when
> 955        it is carried by an UPDATE of a labeled address family.
> 
> [] s/Section 8/Section 9
> 
> 
> 957     3.6.  MPLS Label Stack Sub-TLV
> ...
> 990        If the MPLS Label Stack sub-TLV is included in a TLV identifying a
> 991        Tunnel Type that uses virtual network identifiers (see Section 8),
> 992        the contents of the MPLS Label Stack sub-TLV MUST be pushed onto the
> 993        packet before the procedures of Section 8 are applied.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 1029    3.7.  Prefix-SID Sub-TLV
> ...
> 1040       [RFC8669] only defines behavior when the Prefix-SID Attribute is
> 1041       attached to routes of type IPv4/IPv6 Labeled Unicast ([RFC4760],
> 1042       [RFC8277]), and it only defines values of the Prefix-SID Attribute
> 1043       when attached to routes of those types.  Therefore, similar
> 1044       limitations exist for the Prefix-SID sub-TLV: although it MAY be
> 1045       encoded in any BGP UPDATE message where the Tunnel Encapsulation
> 1046       attribute is allowed (see Section 5), the encoded information MUST be
> 1047       ignored just as the base specification that defines the encoding
> 1048       requires.  So, in the case of the values specified in [RFC8669], they
> 1049       MUST be ignored if received with routes of type other than IPv4/IPv6
> 1050       Labeled Unicast.
> 
> [] s/Section 5/Section 6

Fixed.

> 
> [major] "Therefore, similar limitations exist..."  The rest of this
> sentence includes Normative language that I think is out of place
> because it sounds as stating a fact.  The first sentence in the next
> paragraph sort of repeats the same optional nature of the sub-TLV, but
> without Normative language and then it immediately goes into talking
> about how to interpret the information...without mentioning that the
> information can be ignored.  All this brings me to a suggestion (for
> the last and next paragraphs):
> 
> Suggestion>
> 
>   [RFC8669] only defines behavior when the Prefix-SID Attribute is attached
>   to routes of type IPv4/IPv6 Labeled Unicast, and it only defines values
>   of the Prefix-SID Attribute for those cases.  Therefore, similar
>   limitations exist for the Prefix-SID sub-TLV: it SHOULD only be included
>   in a BGP UPDATE message for one of the address-families defined in
>   [RFC8669].  If included in a BGP UPDATE for any other address-family then
>   it MUST be ignored.

Adopted this; thanks.

> 
>   If an Originator SRGB is specified in the sub-TLV...

I think you’re suggesting losing the first sentence, "The Prefix-SID sub-TLV can occur in a TLV identifying any type of tunnel.” I’m not vehemently opposed to removing it (any sub-TLV can occur in a TLV identifying any type of tunnel unless specified otherwise, so this is just emphasizing a fact already known). But, I’m not sure if the reason you’ve given for removing it is valid? You say that "The first sentence in the next paragraph sort of repeats the same optional nature of the sub-TLV” but I don’t see how that’s so. The previous paragraph (the one you rewrote) talks about how the attribute (and therefore the sub-TLV) only makes sense when attached to v4 or v6 routes. It’s about permitted pairings of sub-TLV:route. The sentence in question is talking about permitted pairings of sub-TLV:tunnel type. Since a route is very much not the same thing as a tunnel type, I think these are not redundant and it’s not immediately obvious that the sentence should be removed, so I’ve let it stand for now. 
 
> 
> 
> 1052       The Prefix-SID sub-TLV can occur in a TLV identifying any type of
> 1053       tunnel.  If an Originator SRGB is specified in the sub-TLV, that SRGB
> 1054       MUST be interpreted to be the SRGB used by the tunnel's egress
> 1055       endpoint.  The Label-Index, if present, is the Segment Routing SID
> 1056       that the tunnel's egress endpoint uses to represent the prefix
> 1057       appearing in the NLRI field of the BGP UPDATE to which the Tunnel
> 1058       Encapsulation attribute is attached.
> 
> 
> ...
> 1067       The corresponding MPLS label is pushed on after the processing of the
> 1068       MPLS Label Stack sub-TLV, if present, as specified in Section 3.6.
> 1069       It is pushed on before any other labels (e.g., a label embedded in
> 1070       UPDATE's NLRI, or a label determined by the procedures of Section 8,
> 1071       are pushed on the stack.
> 
> [] s/Section 8/Section 9
> 
> 
> ...
> 1163    4.3.  Color Extended Community
> ...
> 1178       The value of the high-order octet of the extended type field is 0x03,
> 1179       which indicates it is transitive.  The value of the low-order octet
> 1180       of the extended type field for this community is 0x0b.  The color
> 1181       value is user defined and configured locally.  No flags are defined
> 1182       in this document; this field MUST be set to zero by the originator
> 1183       and ignored by the receiver; the value MUST NOT be changed when
> 1184       propagating this Extended Community.  The Color Value field is
> 1185       encoded as 4 octet value by the administrator and is outside the
> 1186       scope of this document.  For the use of this Extended Community
> 1187       please see Section 7.
> 
> [] s/Section 7/Section 8
> 
> 
> 1189    5.  Special Considerations for IP-in-IP Tunnels
> 
> 1191       In certain situations with an IP fabric underlay, one could have a
> 1192       tunnel overlay with the tunnel type IP-in-IP.  The egress BGP speaker
> 1193       can advertise the IP-in-IP tunnel endpoint address in the Tunnel
> 1194       Egress Endpoint sub-TLV.  When the Tunnel type of the TLV is IP-in-
> 1195       IP, it will not have a Virtual Network Identifier.  However, the
> 1196       tunnel egress endpoint address can be used in identifying the
> 1197       forwarding table to use for making the forwarding decisions to
> 1198       forward the payload.  See the second bullet point of Section 9.1 for
> 1199       further discussion.
> 
> [] I might be lost...  I see no relationship between this section
> (about IP-in-IP Tunnels) and §9.1, which seems to talk about MPLS
> packets.

I wrote a long-winded explanation of how and why I think the text got to be that way, but the short form is: you’re right, that reference is just confusing, the section stands on its own without it. I deleted that sentence. Thanks.

As part of my review (and long-winded explanation writing) I also worked out that the change introduced to (what is now) Section 9.1 between versions 11 and 12 was mistaken, and I’ve reverted the text to what was in version 11. 

> 
> 
> 1201    6.  Semantics and Usage of the Tunnel Encapsulation attribute
> ...
> 1241          *  The tunnel is of a type that can be used to carry packet P
> 1242             (e.g., an MPLS-in-UDP tunnel would not be a feasible tunnel for
> 1243             carrying an IP packet, UNLESS the IP packet can first be
> 1244             encapsulated in a MPLS packet).
> 
> [minor] s/UNLESS/unless
> That is not a recognized key word.

I changed it. (Is there some actual mandate that words can’t be in all caps unless they’re RFC 2119 keywords? RFC 2119 itself doesn’t say so.)

> 
> 
> ...
> 1256       If the Tunnel Encapsulation attribute contains several TLVs (i.e., if
> 1257       it specifies several feasibile tunnels), router R may choose any one
> 1258       of those tunnels, based upon local policy.  If any Tunnel TLV
> 1259       contains one or more Color sub-TLVs (Section 3.4.2) and/or the
> 1260       Protocol Type sub-TLV (Section 3.4.1), the choice of tunnel may be
> 1261       influenced by these sub-TLVs.
> 
> [nit] s/feasibile/feasible

Done.

> 
> 
> 1263       The reachability to the address of the egress endpoint of the tunnel
> 1264       may change over time, directly impacting the feasibility of the
> 1265       tunnel.  A tunnel that is not feasible at some moment, may become
> 1266       feasible at a later time when its egress endpoint address is
> 1267       reachable.  The router MAY start using the newly feasible tunnel
> 1268       instead of an existing one.  How this decision is made is outside the
> 1269       scope of this document.
> 
> [] s/router MAY start/router may start
> The decision is called out of scope, so there's no Normative substance
> to test against.

I don’t understand your rationale, but I have no objection to the change; done.

> 
> 
> ...
> 1565    11.  Scoping
> 
> [major] This section focuses on the attribute...what about the
> communities?  I'm assuming that the same considerations apply for
> them, right?  Please be explicit.

Done. I trust the text I supplied is sufficiently explicit.

> 
> 
> ...
> 1662    13.  IANA Considerations
> ...
> 1673    13.2.  Subsequent Address Family Identifiers
> ...
> 1679       Because this document obsoletes RFC 5512, change all registration
> 1680       information that references [RFC5512] to instead reference this
> 1681       document.
> 
> [major] Because we want all the references from rfc5512 to move,
> please move this paragraph to be in the main section (13), and not one
> of the sub-sections.

Done.

> 
> 
> ...
> 1778    13.8.  Color Extended Community
> 
> 1780       Add this document as a reference for the "Color Extended Community"
> 1781       entry in the Transitive Opaque Extended Community Sub-Types registry.
> 
> [] Not needed because of the introductory text.

Section removed.

—John