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

Keyur Patel <keyur@arrcus.com> Tue, 25 February 2020 17:09 UTC

Return-Path: <keyur@arrcus.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6287C3A10BA; Tue, 25 Feb 2020 09:09:15 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=netorgft1331857.onmicrosoft.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mPIQBomqii1X; Tue, 25 Feb 2020 09:09:12 -0800 (PST)
Received: from NAM10-DM6-obe.outbound.protection.outlook.com (mail-dm6nam10on2082.outbound.protection.outlook.com [40.107.93.82]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 21A713A10B9; Tue, 25 Feb 2020 09:09:11 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MVNBqbWOaonq0urwzwADUmLb5fTSMb9t1I9hWXEQN4vk6rUNv7MsRn4NjLRrMOH9SryX30m76hBJp7A4IFl6xqHYtWEv4F6xBpCMWZebyJZGY2V35NvLbXIXrsykPRHRfgwCKFiAAXbWbnTXczq74SoJ6AE0rMJw63bfgYCwHUr+omlFN6tLHZVmGP9NM3kCx3P23i+uz7y9PJhqrfmCg5Yhpsttgl/9cO+JdLGE6UquWPhYqLW+zZ0UD9/Qj652ddahOIGV4FUosSj9+B444wyUtfnw0NzN23r5b7OJ6sx3Ee/0pltCvsZcjIwifHKR7YA4mVUD/rOt5WnsLARghQ==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3l4mPAD4+qrKzFPssrNVk8HnGCPkZJrFcbDlnlpoxzk=; b=J2tZ5hymx4K69X9wok8mtcBEJOZ3kWf6pGgNRCA3M9VaILwWWhtMt+KqroPG/EP5q7btNlAh7S1VP3KC1a3zgjyBSmqmzLsfkpb0W0XJp928gH6d8oTk4c3vs/ZO9//cf6nPF9gn1dCo76ASLXG2071Rw48L6j5AR5S4bZUFkfgugT1eGT5rU4C9Iwizh09wYrqqO28fhB4m4b2uw3waM8tbvCOG6TFXa0+WygXKKSvaGdopkLrCtGYE98rRaM8kpD1C5aoSqZFZ8YWsX485c1lG5VF0K9JoTvo8kPBSj3CJ6VJTYPRXAKIocI0XZTAGPqOZUUYg8J1FJl2jDXcEaQ==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arrcus.com; dmarc=pass action=none header.from=arrcus.com; dkim=pass header.d=arrcus.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=NETORGFT1331857.onmicrosoft.com; s=selector2-NETORGFT1331857-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=3l4mPAD4+qrKzFPssrNVk8HnGCPkZJrFcbDlnlpoxzk=; b=P/87sRkig+mcs+JOgNGboefEo+skLpKapaFvq0xzsHsk3b5MlXIPtVr3pbLkp08eTJOKRG3MLrep0T+IP9/2LYKPbkWG7990Ad1W+aBptZtgbenLkUWFZ9MDX+O6TAUTSUINsj9yMl8TdeozFtxXrFrmEXol2WNkMe3v40MSVBc=
Received: from BYAPR18MB2856.namprd18.prod.outlook.com (2603:10b6:a03:10e::30) by BYAPR18MB3047.namprd18.prod.outlook.com (2603:10b6:a03:105::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2750.21; Tue, 25 Feb 2020 17:09:08 +0000
Received: from BYAPR18MB2856.namprd18.prod.outlook.com ([fe80::9ca8:5fd:8296:7b05]) by BYAPR18MB2856.namprd18.prod.outlook.com ([fe80::9ca8:5fd:8296:7b05%4]) with mapi id 15.20.2750.021; Tue, 25 Feb 2020 17:09:08 +0000
From: Keyur Patel <keyur@arrcus.com>
To: Alvaro Retana <aretana.ietf@gmail.com>, "draft-ietf-idr-tunnel-encaps@ietf.org" <draft-ietf-idr-tunnel-encaps@ietf.org>
CC: John Scudder <jgs@juniper.net>, "idr-chairs@ietf.org" <idr-chairs@ietf.org>, "idr@ietf. org" <idr@ietf.org>
Thread-Topic: AD Review of draft-ietf-idr-tunnel-encaps-15
Thread-Index: AQHV6LUXjzaZiEFG+kyS9ZPP8NcJvKgrpQ4A
Date: Tue, 25 Feb 2020 17:09:08 +0000
Message-ID: <8BBF5B52-4CBE-4101-8FAD-CDB50D1149E3@arrcus.com>
References: <CAMMESsw09LGWWhqyJ_0=jRimUN+_UuCjaXHCdqF9zkpaxSQgVQ@mail.gmail.com>
In-Reply-To: <CAMMESsw09LGWWhqyJ_0=jRimUN+_UuCjaXHCdqF9zkpaxSQgVQ@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/10.22.0.200209
authentication-results: spf=none (sender IP is ) smtp.mailfrom=keyur@arrcus.com;
x-originating-ip: [70.234.233.187]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 4f8c3d7b-f399-4ad5-7538-08d7ba156d31
x-ms-traffictypediagnostic: BYAPR18MB3047:
x-microsoft-antispam-prvs: <BYAPR18MB30478F79D96D39B121B82813C1ED0@BYAPR18MB3047.namprd18.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:9508;
x-forefront-prvs: 0324C2C0E2
x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(376002)(39830400003)(136003)(366004)(346002)(396003)(199004)(189003)(6486002)(2906002)(81166006)(8676002)(71200400001)(81156014)(508600001)(4326008)(5660300002)(2616005)(66946007)(66476007)(54906003)(110136005)(316002)(6506007)(64756008)(66446008)(66556008)(8936002)(6512007)(33656002)(30864003)(86362001)(76116006)(66574012)(26005)(966005)(36756003)(186003)(579004); DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR18MB3047; H:BYAPR18MB2856.namprd18.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1;
received-spf: None (protection.outlook.com: arrcus.com does not designate permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: ptTrxCEOFZMD70z3O+4EyC+CRmNxcM1/KVTuW3X+uK5soP5gNqcUXEj6Z6SQqAwIXFkHg39LlkUcE0x16WyqEE3RMce2YxMX0tYaQlirrrVDZRhYOob9lcPQ2euXuTpEdWUs+vAuLxMkdHj17eIAsHY9vqmyTEDSGxRuNoM8/a2ypvkOlZ5OTf4Jz0+fOrOkOJInCAH8sJTKDItzS8UIB11QPL++5yvwOg615SUdIbYPsxRQXewuOGjn1ki9L9hMpdrMQxjKa3RrQ1ZCYDRjpKtXNShWAFmpVv9USfw4nOowW7u81GvyTGbrCDS6JI4/qe2IE6JM8bZrsrO7TNMvnt65KVBaADU11DXacGRkM78o/G01PXgUQkM6oHBBsZtuWDK4CAQbImauFbBp4h4R4n2BHPCHrXUPmG5iCMwpSxYrCdSJLlfH8q63TLZjEjSoEpRYSYqWwaV7exgM98FrmCIu2vYL7Rtg3AjLiTTfeFatg78UMd2hdU7HbN6dmm3PI+LEJqn8wnhilbV6V5I/eA==
x-ms-exchange-antispam-messagedata: cJQbGjXrQE2p4tZRrQWNTvPG8tb3UkcIJs5l5eOMihBAaOkIaKmXOu5VqGvnOJof9zBMsqY1uxYOGabXZ7qP6MZoUSTu/Vp6zPdFqE+aptVUjUzKcXfOyRZ5ENtUxqtYHdfTtU/6ylCJZfvUlG9bdA==
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="utf-8"
Content-ID: <209DF51AAC49724CA9C6C0FD664B1EC4@namprd18.prod.outlook.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-OriginatorOrg: arrcus.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4f8c3d7b-f399-4ad5-7538-08d7ba156d31
X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Feb 2020 17:09:08.4642 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 697b3529-5c2b-40cf-a019-193eb78f6820
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: hthMtSE0GNyu579LUo91mtFBLd/CI4brUZut8SJRvz3j+YthUQeUujWJSh+hShMfiO7NQg+ZPxjcJgQXohGnCg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR18MB3047
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/3spFJ3Gfeq87I2siNo7VADreYkk>
Subject: Re: [Idr] AD Review of draft-ietf-idr-tunnel-encaps-15
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 25 Feb 2020 17:09:16 -0000

Hi Alvaro,

Thanks for the detail review and comments. We will get back to you soon with our response.

Regards,
Keyur

On 2/21/20, 4:47 AM, "Alvaro Retana" <aretana.ietf@gmail.com> wrote:

    Dear authors:
    
    Here is my review of draft-ietf-idr-tunnel-encaps-15.  Thank you for
    all the work that you and the WG have put into it.
    
    While this is a good document, I do have a significant number of
    comments (see details in-line below).  I want to highlight a few of
    them up front.
    
    I will wait for the major issues to be addressed before starting the IETF LC.
    
    Thanks!!
    
    Alvaro.
    
    
    (1) Obsoleting other work
    
    This document deprecates the Encapsulation SAFI because it "has never
    been used in production".  What about other work that specifies the
    use of the Encapsulation SAFI?  Should they also be Obsoleted by this
    document?  [Yes, I know there was a long discussion on the list, which
    resulted in the text in §1.4.]
    
    Both rfc5566 (mentioned in §1.4) and rfc5640 specify the use of the
    Encapsulation SAFI.  Is there interest in Updating those RFCs?  If
    not, then it seems like the easy thing to do is to Obsolete that work
    with this document.  If there is, then I agree with the text in §1.4
    in that the effort would be out of scope of this document.  In either
    case, I think that §1.4 raises more questions than it answers and is
    not needed in this document.
    
    John: As Shepherd, can you please ping the WG (you may have to reach
    out to bess as well) about potential interest in Updating
    rfc5566/rfc5640.  I won't hold this document waiting for the
    resolution, but having an idea of any interest would be nice.
    Interest has been expressed for rfc5566bis [1], but didn't see
    anything related to rfc5640.  BTW, the mailing list contains
    discussion about other IPsec-related drafts that use tunnel-encap; I'm
    not sure of their relationship (if any) to rfc5566.
    
    [1] https://mailarchive.ietf.org/arch/msg/idr/yuIXr3HpPEp7v_k08XpdhXRN6GM
    
    
    (2) Underspecification.
    
    A big part of this document is dedicated to specifying sub-TLVs...but
    in none of those descriptions is the Type of the sub-TLV indicated!
    Please add all the basic description of a sub-TLV in one place, and
    don't force the user to search for the types elsewhere: in the IANA
    Considerations sometimes, but in the registry in others.  Please don't
    forget that this document Obsoletes rfc5512, so anything that was
    specified there needs to be re-specified here.
    
    
    (3) Use of Normative Language.
    
    While using rfc2119 keywords is not always necessary, they help
    clarify the behavior required for interoperability.  Cases where the
    keywords are not used (e.g. rfc8200) often result in multiple
    interpretations.  This document is inconsistent in the use of
    Normative Language: it is used sometimes, but not always.  Please be
    consistent!  Recommendation: use it!  I pointed out some cases, but
    the whole document would benefit from a re-read.
    
    
    (4) Use Cases
    
    rfc5512 gave an overview of the use case (mostly intra-AS), but this
    document doesn't mention use cases either as an introduction to the
    problem being solved or in the form of Operational Considerations.
    Sections 9 and 10 would be a good start for that type of section, but
    having a general description of the problem upfront (in the
    Introduction maybe) would be ideal.  Again, because this document
    Obsoletes rfc5512, then we can't really rely on the information there.
    I'm looking for a few paragraphs, mostly directed at readers that may
    not be familiar with rfc5512 and the potential applications.
    
    
    (5) When does an IP address "belong" to an AS?
    
    §3.1 uses this condition as a way to determine if the Tunnel Endpoint
    sub-TLV is malformed:
    
       o  It can be determined that the IP address in the sub-TLV's address
          subfield does not belong to the non-zero AS whose number is in the
          its Autonomous System subfield.  (See section Section 13 for
          discussion of one way to determine this.)
    
    IOW, for the Tunnel Endpoint sub-TLV to be formed correctly the IP
    address has to "belong" to the AS listed.
    
    
    §13 (Security Considerations) is very tentative about how to determine
    if an IP address "belongs" to the AS listed.  In fact, the text there
    starts by suggesting that "BGP Origin Validation [RFC6811] can be used
    to obtain assurance that the given IP address belongs to the given
    AS", but after pointing at some flaws concludes that:
    
       For these reasons, BGP Origin Validation should not be relied upon
       exclusively, and the filtering procedures of Section 10 should always
       be in place.
    
    
    §10 (Scoping) says that "any BGP speaker that understands the
    attribute MUST be able to filter the attribute from incoming/outgoing
    BGP UPDATE messages"...but it doesn't say anything about the topic of
    "belonging", or what these filters should consider when active.
    Presumably, the filters can be used to avoid receiving Tunnel Endpoint
    sub-TLV with IP addresses that don't belong to the AS listed...but
    that is not mentioned anywhere, much less which would be the criteria
    to make that filtering decision.
    
    
    The effect of the above is that the document doesn't really specify
    how to determine that the Tunnel Endpoint sub-TLV is not malformed.
    
    My suggestion is to not make "belonging" part of the criteria for a
    well-formed sub-TLV.  The validity of the address in terms of
    "belonging" is important and should be highlighted as it is in §13.
    Consider a stronger requirement for using Origin Validation (SHOULD
    ?).  Also, the deployment models (and use cases, as mentioned above),
    including the "well-defined scope" from §10 should be clear
    throughout.
    
    
    
    [Line numbers from idnits.]
    
    ...
    13	Abstract
    
    15	   RFC 5512 defines a BGP Path Attribute known as the "Tunnel
    16	   Encapsulation Attribute".  This attribute allows one to specify a set
    17	   of tunnels.  For each such tunnel, the attribute can provide the
    18	   information needed to create the tunnel and the corresponding
    19	   encapsulation header.  The attribute can also provide information
    20	   that aids in choosing whether a particular packet is to be sent
    21	   through a particular tunnel.  RFC 5512 states that the attribute is
    22	   only carried in BGP UPDATEs that have the "Encapsulation Subsequent
    23	   Address Family (Encapsulation SAFI)".  This document deprecates the
    24	   Encapsulation SAFI (which has never been used in production), and
    25	   specifies semantics for the attribute when it is carried in UPDATEs
    26	   of certain other SAFIs.  This document adds support for additional
    27	   tunnel types, and allows a remote tunnel endpoint address to be
    28	   specified for each tunnel.  This document also provides support for
    29	   specifying fields of any inner or outer encapsulations that may be
    30	   used by a particular tunnel.
    
    [nit] s/that have the "Encapsulation/that use the "Encapsulation
    
    
    ...
    142	1.1.  Brief Summary of RFC 5512
    ...
    161	   o  BGP speaker R1 has received and installed UPDATE U;
    
    [minor] To be consistent with rfc4271: s/installed UPDATE U/selected
    UPDATE U for local use
    
    ...
    171	   o  R1's best path to D is a BGP route that has R2 as its next hop;
    
    [minor] rfc4271 consistency: s/best path/best route/g
    
    
    ...
    179	1.2.  Deficiencies in RFC 5512
    ...
    191	   o  There is no way to use the Tunnel Encapsulation attribute to
    192	      specify the tunnel egress endpoint address of a given tunnel;
    193	      [RFC5512] assumes that the tunnel egress endpoint of each tunnel
    194	      is specified as the NLRI of an UPDATE of the Encapsulation-SAFI.
    
    [nit] s/Encapsulation-SAFI/Encapsulation SAFI
    
    
    ...
    209	1.3.  Brief Summary of Changes from RFC 5512
    
    [minor] It would be very nice if forward references were provided
    below -- to the section where the changes/enhancements are specified.
    
    
    211	   In this document we address these deficiencies by:
    
    [style nit] Please don't write in first person.  Instead: "This
    document addresses these deficiencies by:".  There is similar text in
    other places.
    
    
    ...
    233	   One of the sub-TLVs defined in [RFC5512] is the "Encapsulation sub-
    234	   TLV".  For a given tunnel, the encapsulation sub-TLV specifies some
    235	   of the information needed to construct the encapsulation header used
    236	   when sending packets through that tunnel.  This document defines
    237	   encapsulation sub-TLVs for a number of tunnel types not discussed in
    238	   [RFC5512]: VXLAN (Virtual Extensible Local Area Network, [RFC7348]),
    239	   VXLAN-GPE (Generic Protocol Extension for VXLAN,
    240	   [I-D.ietf-nvo3-vxlan-gpe]), NVGRE (Network Virtualization Using
    241	   Generic Routing Encapsulation [RFC7637]), and MPLS-in-GRE (MPLS in
    242	   Generic Routing Encapsulation [RFC2784], [RFC2890], [RFC4023]).
    243	   MPLS-in-UDP [RFC7510] is also supported, but an Encapsulation sub-TLV
    244	   for it is not needed.
    
    [nit] s/encapsulation sub-TLV/Encapsulation sub-TLV/g
    
    
    ...
    265	   [RFC5512] defines a Tunnel Encapsulation Extended Community, that can
    266	   be used instead of the Tunnel Encapsulation attribute under certain
    267	   circumstances.  This document addresses the issue of how to handle a
    268	   BGP UPDATE that carries both a Tunnel Encapsulation attribute and one
    269	   or more Tunnel Encapsulation Extended Communities.
    
    [nit] s/Community, that/Community that
    
    
    271	1.4.  Impact on RFC 5566
    
    273	   [RFC5566] uses the mechanisms defined in [RFC5512].  While this
    274	   document obsoletes [RFC5512], it does not address the issue of how to
    275	   use the mechanisms of [RFC5566] without also using the Encapsulation
    276	   SAFI.  Those issues are considered to be outside the scope of this
    277	   document.
    
    [major] See my related comments up front.  This section is not needed.
    
    
    279	2.  The Tunnel Encapsulation Attribute
    ...
    287	      0                   1                   2                   3
    288	      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
    289	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    290	     |    Tunnel Type (2 Octets)     |        Length (2 Octets)      |
    291	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    292	     |                                                               |
    293	     |                             Value                             |
    294	     |                                                               |
    295	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    
    297	              Figure 1: Tunnel Encapsulation TLV Value Field
    
    [minor] This TLV is called the Tunnel TLV all over the text...except
    here, where it is not given a name in the description text.  Note that
    besides the title of the figure above, "Tunnel Encapsulation TLV" is
    used in one other place too...
    
    
    299	   o  Tunnel Type (2 octets): identifies a type of tunnel.  The field
    300	      contains values from the IANA Registry "BGP Tunnel Encapsulation
    301	      Attribute Tunnel Types".
    
    303	      Note that for tunnel types whose names are of the form "X-in-Y",
    304	      e.g., "MPLS-in-GRE", only packets of the specified payload type
    305	      "X" are to be carried through the tunnel of type "Y".  This is the
    306	      equivalent of specifying a tunnel type "Y" and including in its
    307	      TLV a Protocol Type sub-TLV (see Section 3.4.1) specifying
    308	      protocol "X".  If the tunnel type is "X-in-Y", it is unnecessary,
    309	      though harmless, to include a Protocol Type sub-TLV specifying
    310	      "X".
    
    [minor] This is a good paragraph, but it may be more relevant in §3.4.1.
    
    [major] "...only packets of the specified payload type "X" are to be
    carried through the tunnel of type "Y"."  Is this intended to be a
    Normative statement?  Note that §3.4.1 says that the "protocol type
    sub-TLV...indicate the type of the payload packets that may be
    encapsulated with the tunnel parameters that are being signaled in the
    TLV."   In neither case are rfc2119 keywords used, but given that both
    mechanisms are intended to be equivalent, it would be great if the
    text was also equivalent.
    
    [major] rfc5512 said that "Unknown types are to be ignored and skipped
    upon receipt."  Is that still the case?
    
    
    ...
    320	                    +--------------------------------+
    321	                    | Sub-TLV Type (1 Octet)         |
    322	                    +--------------------------------+
    323	                    | Sub-TLV Length (1 or 2 Octets) |
    324	                    +--------------------------------+
    325	                    | Sub-TLV Value (Variable)       |
    326	                    +--------------------------------+
    
    328	               Table 1: Tunnel Encapsulation Sub-TLV Format
    
    [minor] s/Table 1/Figure 2   Please renumber the other Figures.
    
    
    330	   o  Sub-TLV Type (1 octet): each sub-TLV type defines a certain
    331	      property about the tunnel TLV that contains this sub-TLV.
    
    [major] Does this text from rfc5512 apply?
    
       When the TLV is being processed by a BGP speaker that will be
       performing encapsulation, any unknown sub-TLVs MUST be ignored and
       skipped.  However, if the TLV is understood, the entire TLV MUST
       NOT be ignored just because it contains an unknown sub-TLV.
    
    This text uses rfc2119 keywords, but the text about unknown TLVs
    (above, also from rfc5512) doesn't.  If adding the text back in,
    please be consistent on the wording.
    
    
    ...
    348	3.1.  The Tunnel Endpoint Sub-TLV
    
    [major] The Type of this sub-TLV is not specified anywhere.  Note that
    §12.4 talks about "Tunnel Egress Endpoint"...the names are obviously
    inconsistent. :-(
    
    [major] The length of a sub-TLV is specified above, but it would be
    ideal if a general statement for *this* sub-TLV was made; something
    along the lines of "the length MUST be 6 + length of the AF...".
    This would make the specification of the sub-TLV being malformed
    easier to do later (and for future extensions).
    
    
    350	   The Tunnel Endpoint sub-TLV specifies the address of the endpoint of
    351	   the tunnel, that is, the address of the router that will decapsulate
    352	   the payload.  It is a sub-TLV whose value field contains three sub-
    353	   fields:
    
    [nit] "sub-field" is used here (and in a couple of other places), but
    "subfield" is used the most.  Please be consistent.
    
    
    ...
    359	   3.  an address sub-field, whose length depends upon the Address
    360	       Family.
    
    [nit] s/address/Address/g   Capitalize to denote the name of the
    field, and not simply an address.
    
    362	      0                   1                   2                   3
    363	      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
    364	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    365	     |                  Autonomous System Number                     |
    366	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    367	     |      Address Family           |           Address             ~
    368	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               +
    369	     ~                                                               ~
    370	     |                                                               |
    371	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    
    373	               Figure 2: Tunnel Endpoint Sub-TLV Value Field
    
    [minor] Please reorder the text below to specify the fields in order.
    
    [suggestion] Some parts of the text talk about a "tunnel endpoint
    address", but it may not be obvious to everyone that the Address field
    of the Tunnel Endpoint sub-TLV is that "tunnel endpoint address".
    Suggestion: rename the field to "Tunnel Endpoint Address".
    
    
    375	   The Address Family subfield contains a value from IANA's "Address
    376	   Family Numbers" registry.  In this document, we assume that the
    377	   Address Family is either IPv4 or IPv6; use of other address families
    378	   is outside the scope of this document.
    
    [major] Given that EVPN (AFI/SAFI 25/70) is valid according to §5, and
    this document assumes only IPv4/IPv6, does that mean that the tunnel
    endpoint cannot be a MAC address, ESI, etc..?  That is not specified
    anywhere.   [Warning: my EVPN is rusty.]
    
    [major] Because there are special considerations depending on the AF,
    consider adding something along the lines of: "Documents specifying
    the use of other AFs are expected to..."
    
    [major] If only IPv4/IPv6 are in scope, what should a receiver do if
    another AF is received?  Should the sub-TLV be considered malformed?
    
    
    380	   If the Address Family subfield contains the value for IPv4, the
    381	   address subfield must contain an IPv4 address (a /32 IPv4 prefix).
    
    [major] s/must/MUST ??
    
    
    383	   In this case, the length field of Tunnel Endpoint sub-TLV must
    384	   contain the value 10 (0xa).
    
    [major] s/must/MUST ??
    
    
    386	   If the Address Family subfield contains the value for IPv6, the
    387	   address sub-field must contain an IPv6 address (a /128 IPv6 prefix).
    388	   In this case, the length field of Tunnel Endpoint sub-TLV must
    389	   contain the value 22 (0x16).  IPv6 link local addresses are not valid
    390	   values of the IP address field.
    
    [major] s/must/MUST ??
    
    [major] What if a link local address is received?
    
    
    392	   In a given BGP UPDATE, the address family (IPv4 or IPv6) of a Tunnel
    393	   Endpoint sub-TLV is independent of the address family of the UPDATE
    394	   itself.  For example, an UPDATE whose NLRI is an IPv4 address may
    395	   have a Tunnel Encapsulation attribute containing Tunnel Endpoint sub-
    396	   TLVs that contain IPv6 addresses.  Also, different tunnels
    397	   represented in the Tunnel Encapsulation attribute may have Tunnel
    398	   Endpoints of different address families.
    
    [nit] In this case "Tunnel Endpoints" is used to refer to the Address
    field in the sub-TLV, which is described as "the address of the
    endpoint of the tunnel" (in the first paragraph of this section).
    This section mostly used "address" to refer to the same field, which
    is called Address anyway.  Please be consistent.  I would prefer it if
    s/Tunnel Endpoint/address of the endpoint of the tunnel -- if you
    rather use "Tunnel Endpoint", then perhaps consider changing the name
    of the field.
    
    
    ...
    404	   The AS number in the sub-TLV MUST be the number of the AS to which
    405	   the IP address in the sub-TLV belongs.
    
    [major] What if it doesn't?  What does "belong" mean?  Please put a
    forward reference to §13.
    
    [major] Should the UPDATE with the Tunnel Encapsulation Attribute be
    originated by the same AS that originated the route to the destination
    reflected in the Address field?  Should the UPDATE with the Tunnel
    Encapsulation Attribute be originated by the same ASN in this sub-TLV?
     Are there valid cases where other ASes would/should originate a
    Tunnel Encapsulation Attribute used to send traffic somewhere else?
    
    
    407	   There is one special case: the Tunnel Endpoint sub-TLV MAY have a
    408	   value field whose Address Family subfield contains 0.  This means
    409	   that the tunnel's egress endpoint is the UPDATE's BGP next hop.  If
    410	   the Address Family subfield contains 0, the Address subfield is
    411	   omitted, and the Autonomous System number field is set to 0.
    
    [major] rfc4271/rfc4760 consistency: s/UPDATE's BGP next hop/address
    of the next hop
    
    [minor] To keep consistent with the text above, please specify the length (6).
    
    [major] Why isn't the ASN kept to indicate the AS to which the address
    in the NEXT_HOP belongs to?  I ask mostly because the specification
    above about the ASN says that it "MUST be the number of the AS...",
    which implies that there are no exceptions.  If at this point
    (assuming implementations) the ASN has to be 0, then please use
    "SHOULD" in the initial specification.
    
    
    413	   If any of the following conditions hold, the Tunnel Endpoint sub-TLV
    414	   is considered to be "malformed":
    
    416	   o  The sub-TLV contains the value for IPv4 in its Address Family
    417	      subfield, but the length of the sub-TLV's value field is other
    418	      than 10 (0xa).
    
    420	   o  The sub-TLV contains the value for IPv6 in its Address Family
    421	      subfield, but the length of the sub-TLV's value field is other
    422	      than 22 (0x16).
    
    424	   o  The sub-TLV contains the value zero in its Address Family field,
    425	      but the length of the sub-TLV's value field is other than 6, or
    426	      the Autonomous System subfield is not set to zero.
    
    [minor] Is there any way to generalize these conditions?  If general,
    then we don't need to worry about future AFs clearly specifying what a
    malformed sub-TLV is.
    
    Suggestion>
       ...the Length of the sub-TLV is not 6 + the length of the AF...as specified
       above.
    
    
    428	   o  The IP address in the sub-TLV's address subfield is not a valid IP
    429	      address (e.g., it's an IPv4 broadcast address).
    
    [major] What is a "valid IP address"?  The text above says that
    link-local are not allowed...and that a host address is expected...and
    that the address must "belong"...now you added broadcast addresses.
    Anything else?  Is a multicast destination "valid"?  Is there a
    reference that can be added here to avoid having to define it?
    
    
    431	   o  It can be determined that the IP address in the sub-TLV's address
    432	      subfield does not belong to the non-zero AS whose number is in the
    433	      its Autonomous System subfield.  (See section Section 13 for
    434	      discussion of one way to determine this.)
    
    [major] "one way"  I hope that it is the MTI way -- otherwise, the
    determination of the sub-TLV being malformed is not deterministic.
    
    
    436	   If the Tunnel Endpoint sub-TLV is malformed, the TLV containing it is
    437	   also considered to be malformed, and the entire TLV MUST be ignored.
    438	   However, the Tunnel Encapsulation attribute MUST NOT be considered to
    439	   be malformed in this case; other TLVs in the attribute MUST be
    440	   processed (if they can be parsed correctly).
    
    [major] "sub-TLV is malformed, the TLV containing it is also
    considered to be malformed, and the entire TLV MUST be ignored."  §11
    adds: "the entire TLV MUST be ignored, and MUST be removed from the
    Tunnel Encapsulation attribute".  Please specify things only once!
    Instead of specifying what to do in this section, put a reference to
    §11 and take care of it there.
    
    
    442	   When redistributing a route that is carrying a Tunnel Encapsulation
    443	   attribute containing a TLV that itself contains a malformed Tunnel
    444	   Endpoint sub-TLV, the TLV MUST be removed from the attribute before
    445	   redistribution.
    
    [minor] This paragraph seems to say the same thing as §11, but please
    take it out and specify things only once.
    
    [nit] rfc4271 consistency  s/redistribute/distribute/g
    
    447	   See Section 11 for further discussion of how to handle errors that
    448	   are encountered when parsing the Tunnel Encapsulation attribute