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

From: Alvaro Retana <[email-masked]>
Date: Tue, 4 Aug 2020 15:01:57 -0500

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.

...
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.

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.

...
390	3.1.  The Tunnel Egress Endpoint Sub-TLV

[major] The type of this sub-TLV is not indicated inline...

...
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?

...
466	      *  0, if the Address Family subfield contains the value zero.

[major] s/0/6

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].

...
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.

...
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

[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.

[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"

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.

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...
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...

[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.

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

...
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

...
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

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?

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

...
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

[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.

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

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.

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.

...
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

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.

...
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.

...
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.

...
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.