[Idr] Re: Revision for adding Ketan's comments (RE: AD evaluation review of draft-ietf-idr-sdwan-edge-discovery-29

[Ketan Talaulikar <[e-mail]>]

Hi Linda,

At a quick glance, I note that not all of my suggestions have been
considered even if several issues have been addressed. At this point, the
document is not with me; it is back with the WG. Therefore, the author team
and the WG should evaluate the document.

Thanks,
Ketan


On Tue, Sep 1, 2026 at 10:29 PM Linda Dunbar <[e-mail]>
wrote:

> Hi Ketan,
>
> I have now uploaded a revised version of
> draft-ietf-idr-sdwan-edge-discovery that I believe addresses all of your
> comments and suggestions:
> https://datatracker.ietf.org/doc/draft-ietf-idr-sdwan-edge-discovery/
>
> I would appreciate it if you could take a look at the updated version.
>
> The major changes include:
>
>    - Clarified the boundary between BGP signaling and IPsec processing.
>    BGP carries IPsec-related parameters but does not negotiate, derive,
>    validate compatibility, or establish IPsec SAs.
>    - Clarified the centrally controlled SD-WAN model and the role of the
>    RR/Controller, including authorization and policy enforcement.
>    - Updated the BGP transport security requirements so that
>    authentication, integrity protection, and confidentiality are required for
>    carrying the SD-WAN information defined in the document.
>    - Clarified SD-WAN Node ID, SD-WAN Color, BGP Next Hop, and their
>    respective roles in identifying nodes, grouping eligible tunnels, and
>    correlating Client Routes with SD-WAN Underlay Routes.
>    - Clarified that the SAFI-74 SD-WAN Underlay Route carries
>    control-plane tunnel information and is not installed as a forwarding route
>    for user traffic.
>    - Reworked the IPsec SA Rekey Counter, Public Key, SA Proposal, and
>    Simplified IPsec SA Sub-TLV descriptions, including length constraints,
>    validation, and the boundary between BGP and the IPsec implementation.
>    - Clarified that only one IPsec SA Proposal Sub-TLV is processed for a
>    given SD-WAN Hybrid Tunnel TLV, while multiple IPsec SA ID Sub-TLVs may be
>    used for distinct SAs.
>    - Corrected and clarified the Extended Port Attribute encoding,
>    including IPv4/IPv6 address lengths, NAT-related Local/Public IP and port
>    fields, and Sub-TLV length calculation.
>    - Clarified the handling of multiple WAN ports and Extended Port
>    Attribute Sub-TLVs.
>    - Improved validation and error-handling procedures for SD-WAN NLRIs
>    and Tunnel Encapsulation Sub-TLVs, including alignment with RFC 7606 and
>    RFC 9012.
>    - Reworked the IANA Considerations, including the existing Sub-TLV
>    assignments, the SAFI-74 Route Type range, the Extended Port registries,
>    and the overlapping/unassigned ranges.
>    - Updated the Security and Manageability Considerations and cleaned up
>    the normative/informative references.
>
> I also fixed the IDnits issues found during the revision.
>
> Please let me know if you see anything from your review that is still not
> adequately addressed.
>
> Thanks,
> Linda
>
>
>
>
>
> *From:* Linda Dunbar
> *Sent:* Saturday, August 29, 2026 8:02 PM
> *To:* Ketan Talaulikar <[e-mail]>; Susan Hares <
> [e-mail]>; Kausik Majumdar <[e-mail]>; Robert
> Raszuk <[e-mail]>; Venkit Kasiviswanathan <[e-mail]>
> *Cc:* idr@ietf. org <[e-mail]>
> *Subject:* RE: AD evaluation review of
> draft-ietf-idr-sdwan-edge-discovery-29
>
>
>
> Ketan,
>
>
>
> Thank you very much for the extremely detailed and thoughtful review. I am
> truly impressed by how thoroughly you went through the document and
> identified so many subtle technical, encoding, consistency, and
> interoperability issues.
>
>
>
> It took me many days to carefully work through all of your comments,
> understand the concerns, and make the corresponding changes. I really
> appreciate not only the issues you identified, but also the detailed
> explanations and concrete suggestions you provided for how they could be
> addressed. They were extremely helpful in working through some of the more
> complicated parts of the document.
>
>
>
> I have now gone through all of your comments and revised the document
> accordingly. I will prepare a summary of the major changes and post the
> revised version to the IDR WG for further review.
>
>
>
> I am very grateful for the significant time and effort you put into this
> review. Your comments and suggestions have substantially improved the
> document.
>
>
>
> Please see the resolutions below marked as [Linda] for your comments.
>
>
>
> Linda
>
>
>
> *From:* Ketan Talaulikar <[e-mail]>
> *Sent:* Wednesday, August 19, 2026 11:25 AM
> *To:* Linda Dunbar <[e-mail]>; Susan Hares <
> [e-mail]>; [e-mail]; Robert Raszuk <
> [e-mail]>; [e-mail]
> *Cc:* idr@ietf. org <[e-mail]>
> *Subject:* AD evaluation review of draft-ietf-idr-sdwan-edge-discovery-29
>
>
>
> Hello Linda, Sue, Kausik, Robert, Venkit, and IDR WG,
>
> I've done the review as part of AD evaluation of
> draft-ietf-idr-sdwan-edge-discovery-29 and would like to share the same
> with
> you.
>
> This has been thus far the most difficult and complex review that I've had
> to do as an AD (and even generally). The challenging part was not just the
> issues themselves but my attempt to find a way to progress this work. I
> hope
> this helps the authors and the WG.
>
>
> Summary: Not ready to progress to IESG evaluation. Given the nature of
> findings, I shall be returning this document back to the WG.
>
> Before getting into the substance of the review, I must clarify that the
> intent
> is to make the document correct and correctly bounded, not to block the
> approach it takes. Whether BGP is the right vehicle for SD-WAN secure
> tunnel
> establishment is a question for the WG to settle, not one an AD evaluation
> should relitigate, and none of the comments below asks the authors to
> defend
> that choice or to argue it against alternatives. What they do ask is that
> the specification be implementable, that it say which rules it follows and
> which it defines, and that it be candid about what it has not examined - so
> that a reader can make an informed decision that the document currently
> does
> not equip them to make.
>
> [Linda] For the detailed review and for clarifying the intent. We
> appreciate the comments and will address them individually below.
>
>
> Then, a declaration of prior involvement. I performed the RTGDIR early
> review of -18 (in 2024, "Not ready"). Some of those findings still remain
> and are covered in this review. The RTGDIR reviewer role was taken up by
> Alvaro after I stepped into the AD role. I will let Alvaro add/clarify on
> his review, but I believe some of his findings are also covered here.
>
> technical tldr:
>
> 1) Several of the new sub-TLVs cannot carry the values they are defined to
>    carry, because the one-octet length that RFC 9012 assigns to sub-TLV
> type
>    codes 64-70. E.g., it cannot hold a 2048-bit Diffie-Hellman public
> value —
>    the only group RFC 8247 makes mandatory to implement.
>
>
>
> [Linda] To stay within the RFC 9012 sub-TLV length limit, we can recommend
> using elliptic-curve DH groups, whose public values are much smaller, e.g.
> , 64 octets for P-256 (Group 19), 96 octets for P-384 (Group 20), and 132
> octets for P-521 (Group 21). For deployments using larger public values,
> such as 2048-bit MODP, the SD-WAN controller or management system can
> distribute the public keys for the nodes in the SD-WAN domain instead of
> carrying them in BGP. Here are the actual wording changes:
>
>
>
> Old:
>
> Key Exchange data: This refers to a copy of the sender's Diffie-Hellman
> public value. The length of the Diffie-Hellman public value is defined for
> MODP groups in [RFC7296] and for ECP groups in [RFC5903].
>
>
>
> New:
>
> Key Exchange data: This field contains a copy of the sender's
> Diffie-Hellman public value. Because the RFC 9012 sub-TLV Length field is
> one octet, the total value carried in this IPsec Public Key Sub-TLV is
> limited to 255 octets. Therefore, the encoded public value, together with
> the other fields in this Sub-TLV, MUST fit within this limit.
> Elliptic-curve Diffie-Hellman groups are RECOMMENDED because their public
> values are smaller; for example, Groups 19, 20, and 21 use public values of
> 64, 96, and 132 octets, respectively, as defined in [RFC5903]. If the
> selected Diffie-Hellman public value cannot fit within this Sub-TLV, the
> public key MUST be distributed by another mechanism, such as the SD-WAN
> controller or management system.
>
>
>
>
> 2) The document covers two different route types with one set of procedures
>    and never says which of them RFC 9012 governs, so the client routes
>    deviate from it where they may not, and the new SD-WAN SAFI is left
>    without a next-hop encoding, propagation rules or validation procedures
>    of its own.
>
>
>
> [Linda] We believe the document already separates the two cases: Section
> 2.2 defines SD-WAN underlay UPDATEs using SAFI 74, while Section 2.4
> defines client-route UPDATE procedures. Both use the Tunnel Encapsulation
> Attribute defined in RFC 9012. However, we agree that the applicability
> boundary should be stated more explicitly. RFC 9012 directly specifies use
> of the Tunnel Encapsulation Attribute for the client-route AFI/SAFIs, while
> this document extends its use to SAFI 74 and therefore needs to state the
> SAFI-74-specific next-hop, propagation, and validation procedures
> explicitly. We can make the following changes:
>
>
>
> Old:
>
> A new NLRI SAFI (SD-WAN SAFI=74) is introduced within the MP_REACH_NLRI
> Path Attribute of [RFC4760] for advertising the detailed properties of the
> SD-WAN tunnels terminated at the edge node.
>
>
>
> New:
>
> A new NLRI SAFI (SD-WAN SAFI=74) is introduced within the MP_REACH_NLRI
> Path Attribute of [RFC4760] for advertising the detailed properties of
> SD-WAN tunnels terminated at the WAN ports of the edge nodes. The SD-WAN
> SAFI uses the Tunnel Encapsulation Attribute and Tunnel TLV/sub-TLV
> encodings defined in [RFC9012]. Because SAFI 74 is outside the AFI/SAFIs
> for which [RFC9012] defines applicability procedures, this document defines
> the use of the Tunnel Encapsulation Attribute with SAFI 74, including the
> associated next-hop, propagation, and validation procedures.
>
>
> 3) There is no boundary between what BGP does and what IPsec and other
>    SD-WAN functional blocks do, so the document has absorbed IPsec and
> other
>    semantics that it then specifies incorrectly or incompletely.
>
>
>
> [Linda] BGP is used only to distribute the parameters needed by SD-WAN
> peers to establish or select secure tunnels. The actual IPsec SA creation,
> algorithm compatibility checks, key derivation, packet protection, rekeying
> behavior, and data-plane processing remain functions of the IPsec
> implementation. Section 3.2 already states that the compatibility check is
> performed at the IPsec layer, not by BGP.
>
>
>
> We can make the following wording changes to make the boundary clearer:
>
> At the beginning of Section 2.3:
>
> Old:
>
> The IPsec SA Property Sub-TLVs defined in this section are used to signal
> IPsec SA parameters for SD-WAN Hybrid Tunnels as defined in this document.
>
>
>
> New:
>
> The IPsec SA Property Sub-TLVs defined in this section are used by BGP
> only to advertise IPsec-related parameters associated with SD-WAN Hybrid
> Tunnels. BGP does not establish, negotiate, derive, or maintain IPsec SAs.
> The creation and operation of an IPsec SA, including algorithm validation,
> key derivation, SA state management, rekeying, and packet protection, are
> performed by the IPsec implementation according to [RFC4301] and, when
> IKEv2 is used, [RFC7296], together with local policy.
>
>
>
> Section 3.2:
>
> Old:
>
> When a new IPsec SA is to be established using parameters carried in
> Sub-TLVs ... the receiving SD-WAN node MUST validate that the proposed
> IPsec transforms and algorithms are compatible with its local configuration.
>
>
>
> New:
>
> When parameters carried in these Sub-TLVs are used to establish a new
> IPsec SA, the receiving SD-WAN node passes the advertised parameters to its
> IPsec function. The IPsec implementation determines whether the advertised
> transforms and algorithms are compatible with local configuration and
> performs SA establishment according to [RFC4301] and, when IKEv2 is used,
> [RFC7296]. These attributes, received via the Tunnel Encapsulation
> Attribute, provide the parameters associated with establishing the IPsec
> tunnel between local and remote WAN ports. BGP itself does not perform
> IPsec negotiation, compatibility checking, or SA establishment.
>
>
>
>
> 4) Section 2 interleaves the two route types so that neither can be read on
>    its own, and the document uses several different names for them making
>    it hard to follow.
>
>
>
> [Linda] The document distinguishes the two route types as: (1) Client
> Routes, using IPv4/IPv6 unicast or L3VPN AFI/SAFIs, and (2) SD-WAN Underlay
> Routes, using the new SD-WAN SAFI 74. We will use these two terms
> consistently throughout the document. And make the following wording
> changes:
>
>
>
> Section 2
>
> Old:
>
> The BGP mechanisms defined in this document serve two functions:
>
>
>
> New:
>
> The BGP mechanisms defined in this document support two types of
> advertisements:
>
>    - Advertise Client Routes with SD-WAN Hybrid Tunnel:
>    - Advertise SD-WAN Underlay Routes: A BGP speaker advertises SD-WAN
>    Underlay Routes using AFI/SAFI 1/74 or 2/74. The NEXT_HOP is set to a local
>    address of the advertising SD-WAN edge node, and the UPDATE includes a
>    Tunnel Encapsulation Attribute containing an SD-WAN Hybrid Tunnel TLV. The
>    SD-WAN NLRI identifies the WAN port or ports, SD-WAN Color, and SD-WAN Node
>    ID to which the advertised tunnel properties apply. The associated SD-WAN
>    Hybrid Tunnel TLV carries the corresponding tunnel-related information,
>    including IPsec SA properties and, optionally, NAT-related information.
>
>
>
> In this document, "Client Route" refers to the first type of
> advertisement, and "SD-WAN Underlay Route" refers to the second. The term
> "SD-WAN NLRI" refers specifically to the NLRI encoding used for an SD-WAN
> Underlay Route.
>
>
>
> Section 2.5 Title:
>
> Old: Procedure for Underlay Routes with SD-WAN Hybrid Tunnel TLV
>
>
>
> New: Procedure for SD-WAN Underlay Routes with SD-WAN Hybrid Tunnel TLV
>
>
>
> Old:
>
> The Edge BGP Peer using BGP SD-WAN discovery sends the SD-WAN NLRI...
>
> New:
>
> The Edge BGP Peer using BGP SD-WAN discovery advertises an SD-WAN Underlay
> Route whose NLRI is encoded as specified in Section 2.2.1...
>
>
>
> Old:
>
> SD-WAN Underlay UPDATE
>
> New:
>
> SD-WAN Underlay Route Advertisement
>
>
>
> Old:
>
> Procedure for Client Routes with SD-WAN Hybrid Tunnel
>
> New:
>
> Procedure for Client Route Advertisements with SD-WAN Hybrid Tunnel
>
>
>
> Section 2.6.1
>
> Old:
>
> For SD-WAN client routes with a Tunnel Encapsulation Attribute with a
> SD-WAN Hybrid Tunnel type TLV, the IPsec Sub-TLVs (IPsec SA ID, IPsec
> nonce, IPsec Public Key, IPsec Proposal, and Simplified IPsec SA) are
> meaningful, but MAY be rarely sent. Incorrect fields within any of these 5
> TLVs. Per [RFC9012], a malformed sub-TLV is treated as an unrecognized
> sub-TLV.
>
>
>
> New:
>
>
>
> For Client Routes carrying a Tunnel Encapsulation Attribute with an SD-WAN
> Hybrid Tunnel TLV, the IPsec Sub-TLVs (IPsec SA ID, IPsec SA Rekey Counter,
> IPsec Public Key, IPsec SA Proposal, and Simplified IPsec SA) MAY be
> included. Malformed Sub-TLVs are handled according to [RFC9012].
>
>
>
>
>
>
> 5) The AH and ESP algorithm fields cite documents that define no code
>    points, and the two keys in the Simplified IPsec SA sub-TLV have no
>    stated algorithm, format or role, so neither field can be implemented
>    interoperably.
>
> [Linda] The Simplified IPsec SA Sub-TLV is intended specifically for
> centrally controlled SD-WAN deployments where the SD-WAN Controller or
> management system pre-configures all participating nodes with the
> applicable IPsec algorithms, keying methods, and security parameters.
> Therefore, the Sub-TLV intentionally does not carry the full set of
> parameters needed for general IPsec negotiation. We can make the following
> wording changes to make the intention more clear.
>
>
>
> Section 2.3.5:
>
> Old:
>
> The Simplified IPsec SA Sub-TLV provides a compact way to signal
> pre-configured IPsec SA parameters for deployments where full transform
> negotiation (e.g., via IKEv2) is not supported or not necessary. In such
> deployments, SD-WAN edge nodes are provisioned (e.g., via SD-WAN controller
> or management system) with a common set of agreed security profiles,
> including allowed transforms and algorithms. This Sub-TLV signals which
> profile entry is to be used for a given SA instance.
>
>
>
> New:
>
> The Simplified IPsec SA Sub-TLV provides a compact way to signal IPsec SA
> parameters in a centrally controlled SD-WAN environment where the SD-WAN
> Controller or management system pre-configures the participating SD-WAN
> nodes with the required IPsec algorithms, keying methods, and other
> security parameters. Because these parameters are configured consistently
> across the SD-WAN domain, this Sub-TLV does not carry all of the
> information required for full IPsec negotiation. It carries only the
> parameters needed to identify and operate the applicable SA instance.
>
>
> 6) Error handling is contradictory — the same condition is given different
>    outcomes in different sections — and the RFC 7606 disposition that
>    applies to each failure mode is never stated.
>
>
>
> [Linda] We will consolidate the error handling in Section 2.6 and make the
> individual sections refer to it. The revised text will explicitly
> distinguish malformed Tunnel Encapsulation Sub-TLVs, unsupported SD-WAN
> NLRI Route Types, malformed SD-WAN NLRIs, missing required Tunnel
> Encapsulation Attributes, and malformed MP_REACH_NLRI next-hop encoding,
> and will state the applicable RFC 9012/RFC 7606 disposition for each case.
> Here are the detailed changes:
>
>
>
> Section 2.6.1:
>
>
>
> Old:
>
> The error handling for the tunnel encapsulation signaling (Encapsulation
> Extended Community and Tunnel Encapsulation Attribute) in this document
> follows the procedures specified in Section 13 of [RFC9012]. Unless
> otherwise stated, malformed or unrecognized Sub-TLVs MUST be handled as
> specified in [RFC9012]. This document defines new Sub-TLVs for Tunnel Type
> 25 (SD-WAN-Hybrid), but does not alter the validation behavior established
> in RFC 9012.
>
>
>
> New:
>
> The error handling for the tunnel encapsulation signaling (Encapsulation
> Extended Community and Tunnel Encapsulation Attribute) in this document
> follows the procedures specified in Section 13 of [RFC9012]. Unless
> otherwise stated, malformed or unrecognized Sub-TLVs MUST be handled as
> specified in [RFC9012]. This document defines new Sub-TLVs for Tunnel Type
> 25 (SD-WAN-Hybrid), but does not alter the validation behavior established
> in RFC 9012.
>
>
>
> A malformed Sub-TLV within an SD-WAN Hybrid Tunnel TLV MUST be handled
> according to Section 13 of [RFC9012]. Unless explicitly specified otherwise
> in this document, the malformed Sub-TLV is treated as an unrecognized
> Sub-TLV and ignored; it does not cause the entire SD-WAN Hybrid Tunnel TLV
> or the associated route to be withdrawn.
>
>
>
> Section 2.6.2:
>
> Old:
>
> The SD-WAN NLRI [AFI 1/SAFI = 74] utilizes a route type field to describe
> the format of the NLRI. This specification only allows an NLRI with a type
> value of 1. An NLRI with a type of field of another value is ignored, but
> distributed to other peers. The implementation MAY log an error upon the
> reception of a type value outside of Route Type 1. Error handling for the
> SD-WAN NLRI also adheres to the BGP UPDATE error handling specified in
> [RFC7606].
>
>
>
> New:
>
> The SD-WAN NLRI [AFI/SAFI = 1/74 or 2/74] utilizes a Route Type field to
> describe the format of the NLRI. This specification defines Route Type 1.
> An NLRI with an unsupported Route Type MUST be discarded and MUST NOT be
> propagated to other peers. The implementation MAY log an error upon
> reception of an unsupported Route Type.
>
> If a recognized SD-WAN NLRI Route Type is malformed but its NLRI boundary
> can still be determined, the affected NLRI MUST be handled as
> Treat-as-withdraw according to [RFC7606].
>
> Error handling for other malformed SD-WAN NLRIs follows the BGP UPDATE
> error-handling procedures specified in [RFC7606].
>
> If the MP_REACH_NLRI Next Hop field for an SD-WAN Underlay Route has an
> invalid encoding or length such that the NLRI field cannot be reliably
> located, the error MUST be handled according to the applicable
> session-reset or AFI/SAFI-disable procedures specified in [RFC7606].
>
>
>
> Section 2.6.3 :
>
> Old:
>
> The SD-WAN NLRI SHOULD NOT be paired with an Encapsulation Extended
> Community. If an SD-WAN NLRI is paired with an Encapsulation Extended
> Community rather than a Tunnel Encapsulation Attribute, the SD-WAN NLRI is
> considered malformed and the Treat-as-withdraw approach specified in
> [RFC7606] SHOULD be used.
>
>
>
> New:
>
> The SD-WAN NLRI MUST NOT be paired with an Encapsulation Extended
> Community. If an SD-WAN NLRI is paired with an Encapsulation Extended
> Community rather than a Tunnel Encapsulation Attribute, the SD-WAN NLRI is
> considered malformed and the Treat-as-withdraw approach specified in
> [RFC7606] MUST be used.
>
>
>
> Fix Section 2.5.2 step 3 to align with Section 2.6.1.
>
> Old:
>
> If any Extended Port Attribute Sub-TLV is determined to be invalid, the
> entire SD-WAN Hybrid Tunnel TLV MUST be considered invalid.
>
>
>
> New:
>
> If an Extended Port Attribute Sub-TLV is malformed, it MUST be handled
> according to Section 13 of [RFC9012] and Section 2.6.1 of this document.
>
>
>
> Section 2.2.2:
>
> Old:
>
> Any other value is not supported and the NLRI MUST be ignored, but
> distributed to other BGP peers.
>
>
>
> New:
>
> An SD-WAN NLRI with an unrecognized Route Type MUST be discarded and MUST
> NOT be propagated, as specified for typed NLRIs in Section 5.4 of [RFC7606].
>
>
>
> Length Field Validation:
>
> Old:
>
> Any other value renders the NLRI malformed, and the NLRI MUST be discarded.
>
>
>
> New:
>
> Any other value renders the SD-WAN NLRI malformed. Error handling is
> specified in Section 2.6.2.
>
>
>
>
>
>
> 7) Applying the mechanism to L3VPN routes is unimplementable as written
>    because label handling over the tunnel is never specified, and using the
>    Route Target as a VPN identifier on plain unicast cannot separate
>    overlapping address space.
>
>
>
> [Linda] We can make the following changes to clarify both points. For
> VPN-IPv4/VPN-IPv6 Client Routes, the VPN route label carried in the SAFI
> 128 NLRI is retained and used for forwarding at the egress SD-WAN edge.
> When the SD-WAN Hybrid Tunnel traverses an IPsec/GRE or IPsec/VXLAN
> segment, the labeled packet is carried through the secure tunnel and the
> egress node processes the VPN route label according to the applicable
> L3VPN/MPLS procedures. For plain IPv4/IPv6 unicast Client Routes, the Route
> Target is not part of the NLRI and therefore cannot by itself distinguish
> overlapping prefixes. We will clarify that plain unicast advertisements are
> only applicable where overlapping address space is not required, or where
> separate routing contexts are established by configuration. Overlapping VPN
> address spaces use the VPN-IPv4/VPN-IPv6 SAFI with an RD.
>
>
>
> Section 2.4.4 (SD-WAN VPN ID in Control Plane)change:
>
>
>
> Old:
>
> For IPv4/IPv6 Unicast (AFI/SAFI = 1/1 or 2/1), the Route Target Extended
> Community [RFC4360] SHOULD be included. The Route Target value is
> interpreted as the VPN ID. The Route Target is especially necessary when
> the SD-WAN edge node serves multiple VPNs on its client-facing interfaces.
> If all client routes belong to a single VPN and the association is
> unambiguous, the Route Target MAY be omitted.
>
>
>
> For VPN-IPv4/VPN-IPv6 (AFI/SAFI = 1/128 or 2/128), the RD in the NLRI
> serves as the VPN ID.
>
>
>
> New:
>
> For IPv4/IPv6 Unicast (AFI/SAFI = 1/1 or 2/1), client routes received from
> different client-facing ports may be configured to belong to different VPNs
> when traversing the SD-WAN network. The Route Target Extended Community
> [RFC4360] SHOULD be included in the BGP advertisement to identify the VPN
> associated with each client route. Remote SD-WAN edge nodes use the Route
> Target to associate the received client route with the corresponding VPN
> forwarding context. If all client routes belong to a single VPN, the Route
> Target MAY be omitted.
>
>
>
> For VPN-IPv4/VPN-IPv6 routes (AFI/SAFI = 1/128 or 2/128), the Route
> Distinguisher is part of the BGP NLRI and distinguishes otherwise identical
> prefixes belonging to different VPNs. The Route Target controls VPN
> membership and route import/export policy. Overlapping prefixes belonging
> to different VPNs therefore use VPN-IPv4/VPN-IPv6 routes with distinct
> Route Distinguishers.
>
>
>
> Section 2.4.5 (SD-WAN VPN ID in Data Plane)  change:
>
> Old:
>
> In the data plane, SD-WAN traffic can traverse either an MPLS or
>
> IPsec segment within a SD-WAN Hybrid Tunnel. The method for conveying
>
> the VPN ID depends on the encapsulation:
>
>    - MPLS Segments: When the SD-WAN Hybrid Tunnel uses MPLS transport,
>    the MPLS label stack is used to identify the VPN per [RFC8277]. Security is
>    assumed to be provided by the MPLS transport.
>    - IPsec Segments: When traversing a public network with IPsec
>    encryption: For GRE encapsulation within IPsec, the GRE Key field can carry
>    the SD-WAN VPN ID; For VXLAN network virtualization overlays within IPsec,
>    the VNI (Virtual Network Identifier) field is used to carry the VPN ID
>
> New:
>
> In the data plane, client traffic belonging to different VPNs MUST remain
> distinguishable while traversing the SD-WAN network. The method used to
> carry the VPN identifier depends on the encapsulation:
>
>
>
>    - For MPLS segments, an MPLS label is used to distinguish traffic
>    belonging to different VPNs according to the applicable MPLS VPN procedures.
>
>
>
>    - For IPsec segments, the VPN identifier is carried within the
>    encapsulation protected by IPsec. For GRE encapsulation, the GRE Key field
>    is used to identify the VPN. For VXLAN encapsulation, the VNI (Virtual
>    Network Identifier) is used to identify the VPN.
>
>
> 8) Every new value space is closed by normative text that makes any
>    unrecognised value a malformed sub-TLV, while IANA registries are
>    simultaneously requested for three of them, so nothing allocated in
>    future could ever be used.
>
>
>
> [Linda] Agree. These value spaces are intended to be extensible through
> the corresponding IANA registries. We can make the following changes of the
> validation text so that an unrecognized value does not by itself make the
> Sub-TLV malformed; malformed processing will be limited to invalid encoding
> or length.
>
>
>
> Section 2.3.6:
>
> Old:
>
> Those are the only two valid values for this field.
>
>
>
> New:
>
> This document defines Encap-Type values 1 (GRE) and 2 (VXLAN). Additional
> values may be assigned in the future through the IANA registry. An
> unrecognized Encap-Type value does not by itself make the Sub-TLV
> malformed; however, an implementation MUST NOT use an encapsulation type
> that it does not support.
>
>
>
> Section 2.3.6.1  (Extended Port Sub-Sub-TLV, Connection Type)
>
> Old:
>
> Values outside the range 1-4 are invalid and render the Sub-TLV malformed.
>
>
>
> New:
>
> This document defines Connection Type values 1 through 4. Additional
> values may be defined by future specifications and assigned through the
> IANA registry. Until then, values other than those defined by this document
> are unsupported. An implementation receiving an unsupported Connection Type
> value MUST NOT use that value for tunnel selection or establishment.
>
>
>
>
>
> Old:
>
> Underlay Network Type Sub-Sub-TLV is a MALFORMED Sub-Sub-TLV if the fields
> do not fit the limits specified above.
>
>
>
> New:
>
> The Underlay Network Type Sub-Sub-TLV is MALFORMED if its encoding or
> length is invalid. This document defines the valid Connection Type and Port
> Type values listed above. Additional values may be defined by future
> specifications and assigned through the corresponding IANA registries.
> Until such values are defined, other values are invalid.
>
>
> 9) The Security Considerations analyse neither the BGP layer nor the IPsec
>    layer, and nowhere state that the Route Reflector, being the SD-WAN
>    controller, is the trust anchor on which the whole solution rests.
>
>
>
> [Linda] The document already defines the RR as part of the SD-WAN
> Controller and states its authorization role. Section 1 says that the RR
> receives BGP UPDATEs from SD-WAN edges and propagates them only to peers
> authorized to communicate over the SD-WAN overlay. Section 3 further states
> that the RR maintains authorization policy and MUST verify that an
> advertising BGP speaker is permitted to originate SD-WAN Hybrid Tunnel
> information before reflecting it. Section 5 also requires strict filtering
> and policy controls for SD-WAN identifiers.
>
>
>
> If you think it is necessary, we can make the following changes to
> reiterate those points in Section 5 (Security Consideration):
> Old:
>
> This document defines BGP extensions for SD-WAN edge nodes to advertise
> their attributes for establishing IPsec SAs and underlay tunnel attributes,
> typically via a RR, which then propagates them to authorized SD-WAN peers.
> These BGP UPDATEs may contain sensitive information such as public keys,
> IPsec proposals, and nonces.
>
>
>
> New:
>
> This document defines BGP extensions for SD-WAN edge nodes to advertise
> their attributes for establishing IPsec SAs and underlay tunnel attributes,
> typically via a RR that is part of the SD-WAN Controller, which then
> propagates them to authorized SD-WAN peers. As described in Sections 1 and
> 3, the RR/Controller maintains the authorization policy for the SD-WAN
> domain and verifies that an advertising SD-WAN edge is authorized to
> originate the corresponding SD-WAN tunnel information before reflecting it
> to other peers. These BGP UPDATEs may contain sensitive information such as
> public keys, IPsec proposals, and nonces. In deployments where SD-WAN edge
> nodes communicate with the RR over public or untrusted networks, BGP SHOULD
> be run over TCP-AO secure transport that provides authentication and
> integrity for this data.
>
>
>
> Add the following paragraph after the 3rd one:
>
> As specified in Section 2.3, BGP only distributes IPsec-related
> parameters; IPsec SA establishment and operation are performed by the IPsec
> implementation according to [RFC4301] and, when IKEv2 is used, [RFC7296].
>
>
> 10) The IANA sections request assignments that were made years ago, six
>     of them under names that differ from the ones the document now uses.
>
>
>
> [Linda] For Section 6.2, Tunnel Type 25 has already been assigned as
> SD-WAN-Hybrid and the name used in the current document is consistent with
> the registry. No new assignment is needed; we will ask IANA to update the
> reference for the existing allocation to this document.
>
>
>
> Section 6.3:
>
> Old:
>
> IANA is requested to assign the following sub-Types in the BGP Tunnel
> Encapsulation Attribute Sub-TLVs registry...
>
>
>
> New:
>
> IANA has previously assigned the following Sub-TLV Types in the BGP Tunnel
> Encapsulation Attribute Sub-TLVs registry in the Border Gateway Protocol
> Tunnel Encapsulation Group. IANA is requested to update the descriptions
> and references for these existing assignments as follows:
>
>
> 11) The document asserts advantages over alternative mechanisms that it has
>     not established, and nowhere tells a reader what it has not analysed.
>
> [Linda] We can make the following wording changes to make it clearer:
>
> Section 2.3.6
>
> Old:
>
> Unlike pairwise NAT traversal mechanisms such as IKEv2 [RFC7296], which
> require peers to dynamically discover NAT properties during tunnel setup,
> the BGP-controlled SD-WAN architecture enables each SD-WAN edge node to
> proactively advertise its NAT properties to all peers through BGP
> signaling. This approach simplifies NAT traversal in large-scale SD-WAN
> deployments where each edge node may need to establish tunnels with many
> peers.
>
>
>
> New:
>
> Unlike pairwise NAT traversal mechanisms such as IKEv2 [RFC7296], in which
> NAT-related information is discovered between peers during tunnel
> establishment, the BGP-controlled SD-WAN architecture enables an SD-WAN
> edge node to advertise its NAT properties through the RR to authorized
> SD-WAN peers before individual tunnels are established. This avoids
> requiring the same NAT-related information to be discovered independently
> during the establishment of each SD-WAN tunnel and can reduce repeated
> parameter exchange in deployments where an SD-WAN edge establishes tunnels
> with many peers.
>
>
>
>
> The review has three parts: the tldr above; then the document-level issues
> and what I am asking for on each; then the detailed comments, inline in the
> idnits output of v29 of this document. The document-level issues are not
> repeated in the detailed part.
>
> All of this is an effort to convey the review findings appropriately to not
> just the authors but also the WG so they can do their part with reviewing
> the document as it progresses.
>
> Please look for the tag <EoRv29> at the very end of the review. If you
> don't
> see that, then likely the email has been truncated by your email client and
> you should look at the IDR WG email archive for the full version.
>
> Thanks,
> Ketan
>
> PS: This is not an editorial review and it does not list the many
> spelling,
> grammatical and typographical errors, and a few incomplete or garbled
> sentences. A proofreading pass is needed once the technical issues are
> sorted
> out.
>
>
> DOCUMENT-LEVEL ISSUES
>
> These are the whole-document matters. They are given in the order I would
> suggest that the authors work on them; it is just something that I felt
> may be helpful. These aspects are not repeated in the detailed part further
> below in the idnits output.
>
> (1) <major> Which of the document's two contexts does RFC 9012 govern?
>
> The document operates in two distinct contexts and treats them as one.
>
>    *  Client routes - AFI/SAFI 1/1, 2/1, 1/128 and 2/128. Every one of
> these
>       is covered by RFC 9012, and it's procedures bind in full. Here the
> TEA
>       does what RFC 9012 designed it to do: it tells the receiver which
> tunnel
>       to forward this route's traffic through.
>
>    *  Underlay routes - the new SD-WAN SAFI, 1/74 and 2/74. Here the TEA is
>       not selecting a tunnel for the NLRI's traffic; the NLRI is a
>       (Port-Local-ID, SD-WAN-Color, SD-WAN Node ID) tuple naming a WAN
> port,
>       and the TEA is the container for that port's properties. That is
>       structurally the same reuse RFC 9830 makes for the SR Policy SAFI.
>
> The document states neither the split nor its consequences. What it states
> instead, at Section 2.6.1, is a blanket claim of adherence to RFC 9012 that
> is wrong in different ways - see the comment at line 1555 below.
>
> The concrete gaps for the SD-WAN SAFI, each of which RFC 9830 answers
> explicitly for its own SAFI (73) and would be a good guideline to work
> from:
>
>    *  No applicability statement. Nothing says the SD-WAN Hybrid Tunnel
> Type
>       and sub-TLVs 64-70 are applicable only to particular AFI/SAFIs. Table
>       1 gestures at it, but a table introduced as a "Summary" (line 352)
> and
>       captioned as a figure (line 390) is not where normative applicability
>       belongs - and it is wrong on the Color entry (see the comment at line
>       362).
>
>    *  No disposition for the RFC 9012 sub-TLVs on the SD-WAN SAFI. Table 1
>       marks fourteen of them "not valid" without saying what that means
>       operationally - ignored, malformed, or stripped on propagation?
>       Section 2.1.1 step 2 says "ignore invalid Sub-TLVs (per [RFC9012])",
>       which for SAFI 74 cites a document that does not cover the case.
>
>    *  No statement of what the TEA means on the SD-WAN SAFI. RFC 9012's
>       whole model is route-resolution-over-tunnel. For an underlay route
>       there is no traffic to resolve, and the document never says so -
>       see the comment at line 1495, where that omission produces a defect.
>
>    *  No statement of the relationship to RFC 9012 generally, and no
>       acknowledgement that the SD-WAN SAFI sits outside its scope.
>
>    *  No best-path statement. Nothing says what BGP does when two paths for
>       the same SD-WAN NLRI are received, or whether the SD-WAN sub-TLVs
>       participate in best-path selection. See Sections 2.4 and 4.2 of RFC
>       9830 for the corresponding statements.
>
>    *  No statement of what happens when a TEA carrying only some other
>       tunnel type accompanies an SD-WAN NLRI. Section 2.6.3 covers the
>       absent case; the wrong-type case is left open. See Section 2.2 of
>       RFC 9830 for the corresponding rule.
>
>    *  No propagation rules. Nothing states how SD-WAN SAFI routes are
>       propagated: whether a Route Reflector reflects them under the
> ordinary
>       RFC 4456 rules; whether a receiving edge node re-advertises them
>       further, and to whom; whether propagation across an eBGP boundary is
>       permitted at all, given the single-administrative-domain assumption;
>       whether the best path only is propagated or all paths; and - since
> the
>       RR is by definition a component of the SD-WAN controller, and may
>       plausibly act on the information rather than merely reflect it -
>       whether it re-originates, modifies or filters the attributes it
>       propagates, or whether ordinary RR semantics apply unchanged. At
>       present a reader cannot tell whether the RR is a BGP transit function
>       or a SD-WAN control-plane participant. What propagation rules the
> document
>       does have are scattered into sections that are about something else:
>       Section 3's authorisation requirement on the RR before reflecting
> (see
>       the comment at line 1614) and Section 2.2.2's ignore-but-distribute
>       (see the comment at line 448). There is no propagation section. See
>       Section 4.2.3 of RFC 9830.
>
> Remedy. Add a short subsection - Section 2.1, before the encoding, is the
> right place - modelled on Sections 1, 2.2 and 2.3 of RFC 9830, stating: (i)
> that client routes use the TEA as specified in RFC 9012 and that this
> document does not alter those procedures; (ii) that AFI/SAFI 1/74 and 2/74
> are outside the scope of Section 6 of RFC 9012 and that this document
> defines the TEA's usage for them; (iii) that the SD-WAN Hybrid Tunnel Type
> and sub-TLVs 64-70 are applicable only to the AFI/SAFIs listed, in
> normative prose rather than only in Table 1; (iv) a default-deny rule
> for the RFC 9012 sub-TLV space on the SD-WAN SAFI, with the required and
> permitted ones named and everything else MUST-ignore / MAY-strip; (v) that
> the SD-WAN NLRI is not installed for forwarding; (vi) the propagation
> rules
> per the last gap above.
>
> I am referencing RFC 9830 (of which I am a co-author) since it is the
> closest
> existing precedent, it is an IDR product, and leveraging it might help the
> authors.
>
> I should also clarify, to avoid missteps, that this document does not
> update
> RFC 9012.
>
>
>
> [Linda] The current revision already distinguishes Client Routes from
> SD-WAN Underlay Routes and states that SAFI 74 is outside the AFI/SAFIs for
> which RFC 9012 defines applicability procedures. We can make the following
> wording change to further clarify that Client Routes follow RFC 9012
> unchanged, while this document defines the TEA semantics, applicable
> Sub-TLV processing, best-path behavior, error handling, and propagation
> rules for SAFI 74. We use RFC 9830 as a guideline for these clarifications
> where applicable. This document does not update RFC 9012.
>
>
>
> Section 2 (BGP SD-WAN Mechanisms)
>
> Old:
>
> In this document, "Client Route" refers to the first type of
> advertisement, and "SD-WAN Underlay Route" refers to the second. The term
> "SD-WAN NLRI" refers specifically to the NLRI encoding used for an SD-WAN
> Underlay Route.
>
>
>
> New:
>
> In this document, "Client Route" refers to the first type of
> advertisement, and "SD-WAN Underlay Route" refers to the second. The term
> "SD-WAN NLRI" refers specifically to the NLRI encoding used for an SD-WAN
> Underlay Route. For Client Routes, the Tunnel Encapsulation Attribute
> procedures of [RFC9012] apply. For SD-WAN Underlay Routes using SAFI 74,
> which is outside the applicability scope defined by [RFC9012], the use and
> processing of the Tunnel Encapsulation Attribute are defined by this
> document. This document does not update [RFC9012].
>
>
>
> Section 2.2  (SD-WAN Underlay Route Advertisement)
>
>
>
> Old:
>
> An Edge BGP Peer using BGP SD-WAN discovery advertises an SD-WAN Underlay
> Route whose NLRI is encoded as specified in Section 2.2.1. The route
> carries a Tunnel Encapsulation Attribute containing an SD-WAN Hybrid Tunnel
> TLV to advertise the detailed properties associated with the public-facing
> WAN port or ports and their associated IPsec tunnels.
>
>
>
> New:
>
> An Edge BGP Peer using BGP SD-WAN discovery advertises an SD-WAN Underlay
> Route whose NLRI is encoded as specified in Section 2.2.1. The route
> carries a Tunnel Encapsulation Attribute containing an SD-WAN Hybrid Tunnel
> TLV to advertise the detailed properties associated with the public-facing
> WAN port or ports and their associated IPsec tunnels. The SD-WAN Underlay
> Route carries control-plane information describing an SD-WAN WAN port and
> its tunnel properties; the SD-WAN NLRI is not itself installed as a route
> for forwarding user traffic.
>
>
>
> Section 2.6.1 ( Error Handling for Tunnel Encapsulation Signaling)
>
>
>
> Old:
>
> The error handling for the tunnel encapsulation signaling (Encapsulation
> Extended Community and Tunnel Encapsulation Attribute) in this document
> follows the procedures specified in Section 13 of [RFC9012]. Unless
> otherwise stated, malformed or unrecognized Sub-TLVs MUST be handled as
> specified in [RFC9012]. This document defines new Sub-TLVs for Tunnel Type
> 25 (SD-WAN-Hybrid), but does not alter the validation behavior established
> in RFC 9012.
>
>
>
> New:
>
> For Client Routes, error handling for the Tunnel Encapsulation Attribute
> follows Section 13 of [RFC9012]. For SD-WAN Underlay Routes using SAFI 74,
> error handling for the Tunnel Encapsulation Attribute and its Sub-TLVs is
> defined by this document. Sub-TLVs that are not applicable to SAFI 74 MUST
> be ignored and MAY be removed when the route is propagated.
>
>
>
> Section 2.5:   add the following sentence
>
>
>
> Normal BGP best-path selection applies when multiple paths for the same
> SD-WAN NLRI are received; the SD-WAN Hybrid Tunnel TLV and its Sub-TLVs do
> not participate in best-path selection. The RR reflects authorized SD-WAN
> Underlay Routes within the SD-WAN administrative domain according to
> [RFC4456].
>
>
>
> (2) <major> There is no boundary between BGP and IPsec, so the document
>     specifies IPsec incorrectly
>
> An IDR document should specify BGP: the encodings, the validation BGP can
> perform on them, the propagation rules. Everything else is domain-specific
> and declared out of scope. RFC 9830 did that by the handoff to RFC 9256.
> I am not sure if this document has equivalent to defer to - there is no
> specified SD-WAN tunnel manager, no SD-WAN-side equivalent of the SRPM,
> and no base specification for one. Having nowhere to put the IPsec
> semantics,
> the document has absorbed them, and it is in that absorbed material that
> the defects cluster: algorithm fields with no code point space (line 953),
> key fields with no stated algorithm, format or role (line 913), nonce
> rules
> misattributed to RFC 7296 (lines 685 and 981), and a rekey mechanism with
> no lifecycle (line 584).
>
> The document is already reaching for the boundary and does not know it.
> Section 3.2, at line 1674, says "This compatibility check is performed at
> the IPsec layer, not by BGP." That is exactly the right statement. It is
> made once, in an operational section, about one check, having already been
> contradicted by pages of BGP-side IPsec validation.
>
> I must also point out a charter scope issue here when IDR documents start
> to specify aspects beyond BGP for which we have other responsible WGs
> (which
> do not seem to have been consulted during this document's development).
>
> Remedy - draw the boundary in the abstract, without waiting for a companion
> specification. The document does not need RFC 9256's equivalent to exist;
> it
> needs to name a local functional component and state the interface to it.
> Concretely:
>
>    *  Name the component in Section 1.3 - perhaps "SD-WAN tunnel manager",
> the
>       direct analogue of the SR Policy Module that RFC 9830 hands off to -
>       defined as the local process that consumes the received
> underlay-route
>       information and establishes, rekeys and tears down the IPsec SAs. Its
>       internals are explicitly out of scope.
>
>    *  State that BGP performs syntactic validation only on the
> IPsec-bearing
>       sub-TLVs: field lengths consistent with the stated Length, values
>       present where required, sub-TLV boundaries aligned. Nothing else.
>
>    *  State explicitly that BGP MUST NOT perform semantic verification of
>       IPsec parameters - algorithm suitability, transform compatibility,
> key
>       validity, group support - and MUST NOT treat an advertisement as
>       malformed on those grounds. Section 5 of RFC 9830 has the sentence to
>       adapt.
>
>    *  State that the validated information is passed to the SD-WAN tunnel
>       manager, and that an advertisement whose parameters the component
>       cannot use is unusable rather than malformed, so it is still
>       propagated.
>
>    *  Then make the sub-TLV fields self-describing enough to survive the
>       handoff. This is where the boundary gets its support: if BGP is not
>       interpreting the AH/ESP algorithm values, the fields must carry
> values
>       from a namespace the receiving component can resolve - which is the
>       IANA IKEv2 transform registries, not RFC 8221 (line 953). If BGP is
>       not interpreting Key 1 and Key 2, the encoding must still say what
>       they are so the component knows what it has been handed (line 913).
> An
>       opaque field is fine; an ambiguous one is not.
>
> That last point is the important qualifier. The boundary reframes the
> processing concerns, not the encoding ones: the length and range arithmetic
> in the comments further below remains exactly as stated.
>
> One interaction to note. If the Section 3 policy problem (see the comment
> at
> line 1614) is resolved by assigning policy responsibility to a named
> management-plane component, then that component and this one are perhaps
> the same thing, and if so, the name could perhaps be - "SD-WAN Manager",
> covering both tunnel lifecycle and policy.
>
> I don't know if such an abstraction was considered by the authors despite
> some comments to that effect from Alvaro's RTGDIR review - that is, was
> there
> ever an effort to keep this document to BGP and push the IPsec processing
> to
> a separately specified component? My reading of the text is that it was
> not,
> and that the IPsec material accumulated into the document rather than
> having
> been placed there by a scoping decision; please correct me if I am wrong.
>
> That brings the question: Is the right long-term answer a short companion
> document specifying the SD-WAN tunnel manager interface, with this
> document
> reduced to the BGP encodings and procedures? That is a larger structural
> question than this review can settle, and it is the shape RFC 9830 and
> RFC 9256 have. Note that the interface can be described in this document
> without the companion existing - the five items above are the minimum, and
> they are worth doing even if no companion is ever written.
>
> [Linda]  In addressing your earlier comments, we have made changes to
> explicitly separates the BGP and IPsec responsibilities. Section 2.3 states
> that BGP only advertises IPsec-related parameters and does not establish,
> negotiate, derive, or maintain IPsec SAs; those functions, including
> algorithm validation, key derivation, SA state management, rekeying, and
> packet protection, are performed by the IPsec implementation. Section 3.2
> further states that the received parameters are passed to the IPsec
> function and that BGP does not perform IPsec negotiation, compatibility
> checking, or SA establishment.
>
>
>
> We agree that the BGP validation boundary can be stated more explicitly:
> BGP validates the encoding of the advertised Sub-TLVs, while semantic
> compatibility and usability of the IPsec parameters are determined by the
> IPsec implementation. We can make the following wording changes to clarify
> this point. The individual encoding issues identified below are addressed
> separately.
>
>
>
> Section 2.3 : clarify what BGP validates
>
>
>
> Old:
>
> The IPsec SA Property Sub-TLVs defined in this section are used by BGP
> only to advertise IPsec-related parameters associated with SD-WAN Hybrid
> Tunnels. BGP does not establish, negotiate, derive, or maintain IPsec SAs.
> The creation and operation of an IPsec SA, including algorithm validation,
> key derivation, SA state management, rekeying, and packet protection, are
> performed by the IPsec implementation according to [RFC4301] and, when
> IKEv2 is used, [RFC7296], together with local policy.
>
>
>
> New:
>
> The IPsec SA Property Sub-TLVs defined in this section are used by BGP
> only to advertise IPsec-related parameters associated with SD-WAN Hybrid
> Tunnels. BGP does not establish, negotiate, derive, or maintain IPsec SAs.
> BGP validation of these Sub-TLVs is limited to their encoding and syntactic
> validity. The creation and operation of an IPsec SA, including algorithm
> validation, key derivation, SA state management, rekeying, and packet
> protection, are performed by the IPsec implementation according to
> [RFC4301] and, when IKEv2 is used, [RFC7296], together with local policy.
>
>
>
> Section 3.2 :
>
>
>
> Old:
>
> BGP itself does not perform IPsec negotiation, compatibility checking, or
> SA establishment.
>
>
>
> New:
>
> BGP itself does not perform IPsec negotiation, compatibility checking, or
> SA establishment. If the IPsec implementation cannot use the advertised
> parameters, this does not by itself make the BGP advertisement malformed.
>
>
> (3) <major-editorial> Section 2 interleaves the two route types; split it
>     into three
>
> Section 2 runs into almost 2/3rds of this almost 50 page document and
> alternates between the two route types without a structural boundary:
> Section 2.1 is the SD-WAN Hybrid Tunnel TLV, covering both, with Table 1
> as the only place the applicability split is recorded; Section 2.2 is the
> new SAFI and its NLRI, underlay only; Section 2.3 is the IPsec and port
> sub-TLVs, mostly underlay and some valid for both; Section 2.4 is
> client-route procedures; Section 2.5 is underlay-route procedures; and
> Section 2.6 is error handling for both, restating parts of Sections 2.4
> and 2.5.
>
> This makes it really very hard for a reader who wants to implement the new
> SAFI - they must read Section 2.1, skip Section 2.4, read Sections 2.2,
> 2.3 and 2.5, then filter Section 2.6 - and must work out for themselves,
> at every step, which half of Sections 2.1 and 2.6 applies. That the two
> procedure sections have drifted apart, and that a client-route requirement
> has migrated into the underlay procedure, are both symptoms of this layout
> rather than independent mistakes. I found this extremely hard to work
> through
> and I believe so did some other directorate reviewers.
>
> Remedy - restructure Section 2 into three top-level sections.
>
>    *  The SD-WAN SAFI: encoding and procedures. Everything that concerns
>       underlay routes and nothing else - the applicability statement from
>       issue (1), the NLRI format and validation, the Next Hop encoding, the
>       SD-WAN Hybrid Tunnel TLV and sub-TLVs as they appear on this SAFI,
> the
>       underlay-route procedures, and this SAFI's error handling.
>       Self-contained: an implementer building only the underlay half should
>       not need to read further.
>
>    *  Applicability to the overlay SAFIs. How 1/1, 2/1, 1/128 and 2/128
>       signal that a route is to be carried over an SD-WAN Hybrid Tunnel -
>       the Encapsulation Extended Community option, the Tunnel Encapsulation
>       attribute option, and the trade-off between them (currently Sections
>       2.1.2 and 2.4). This section states explicitly that RFC 9012's
>       procedures apply unchanged here, which is the other half of issue (1)
>       and resolves two of the detailed comments below by construction: with
>       the client-route rules written in their own section, "treat an absent
>       Tunnel Egress Endpoint as AF 0" and "process only the first instance
>       of any sub-TLV" become visibly RFC 9012 violations rather than
>       plausible general statements.
>
>    *  Interaction between the two. The correlation procedures that are
>       currently scattered across Sections 2.1.2, 2.4.1, 2.4.3, 2.5 and 3.1:
>       how a receiver binds an overlay route to the underlay routes that can
>       carry it (Node ID / next-hop matching, SD-WAN-Color against the Color
>       Extended Community), what happens when no underlay route matches, and
>       the ordering and timing questions - an overlay route arriving before
>       its underlay routes, or its underlay routes being withdrawn while it
>       remains. Those last two are not covered anywhere at present, and this
>       section is where the gap becomes visible.
>
> Three consequential benefits beyond readability. It gives the colour
> correlation mechanism a single home, which is where the three-way muddle
> (see
> line 362) has to be resolved. It makes the terminology sweep in issue (4)
> mechanical rather than judgemental. And it directly answers the structural
> criticism running through the RTGDIR, OPSDIR and INTDIR early reviews.
> Those reviewers asked for restructuring twice and received edits instead;
> this is the restructuring. I am merely re-emphasizing what others have
> stated
> but also offering concrete suggestions on how to go about this.
>
> Within the first two of the three sections, please keep RFC 9012's own
> ordering discipline - encoding before procedures, so the reader has the
> construct before the behaviour.
>
> Sequencing: this split should be done before the trying to resolve most of
> the detailed comments related to section 2 below. Many of those is a
> contradiction between two sections that the split either eliminates or
> makes them easier to resolve.
>
>
>
> [Linda] This concern has been substantially addressed in the changes in
> addressing your earlier comments. Section 2 now explicitly identifies the
> two advertisement types and their terminology, Section 2.4 contains the
> Client Route procedures, Section 2.5 contains the SD-WAN Underlay Route
> procedures, and Section 2.6 distinguishes the corresponding error-handling
> rules. The revised text also now states explicitly that Client Routes
> follow RFC 9012, while SAFI 74 is outside the applicability scope defined
> by RFC 9012 and is specified by this document.
>
>
>
> With these changes we believe a full restructuring into three new
> top-level sections is no longer necessary.
>
>
>
> (4) <major> The two route types have no consistent names
>
> Designations used in the document for the routes on the new SAFI 74 -
> many of them: "SD-WAN NLRI", "SD-WAN Underlay NLRI", "SD-WAN Underlay
> UPDATE", "SD-WAN Underlay Tunnel Update", "underlay route(s)", "SD-WAN
> Hybrid NLRI" ... and I give up now. This continues even in the IANA
> considerations.
>
> For the routes on the existing AFI/SAFIs: "client route(s)" predominantly,
> plus "client route reachability" and "customer prefixes".
>
> The most consequential of these is the Section 6.1 heading, which names the
> SAFI carrying underlay routes the Overlay SAFI - see the comments at lines
> 1839 and 1842.
>
> Separately, "underlay" qualifies four different objects with no distinction
> drawn between them: "underlay route" (the NLRI), "underlay tunnel"
> (the tunnel between two WAN ports), "underlay link", "underlay network".
> Section 2.5.4 collides with itself: the heading reads "Multiple Tunnels
> attached to One Underlay Route" and the body three lines later reads
> "An underlay tunnel passes through only one SD-WAN Hybrid Tunnel".
>
> Remedy. Define exactly two terms in Section 1.3 and use them without
> variation throughout. I suggest:
>
>    *  client routes - routes on AFI/SAFI 1/1, 2/1, 1/128 or 2/128 that are
>       to be forwarded over an SD-WAN Hybrid Tunnel;
>
>    *  SD-WAN SAFI routes - the typed NLRI on AFI/SAFI 1/74 or 2/74
>       advertising the properties of a WAN port and its associated tunnel
> and
>       IPsec parameters.
>
> Then sweep the document: replace every one of those designations with
> "SD-WAN SAFI routes", retaining "SD-WAN NLRI" only where the encoding
> construct itself is meant (the figures and field definitions in Section
> 2.2.1); rename Section 6.1 to "SD-WAN SAFI" and ask IANA to correct the
> registry name; and reserve "underlay tunnel", "underlay link" and "underlay
> network" for the physical and logical objects, defining each in Section
> 1.3.
> While sweeping, also s/distributed to other BGP peers/propagated and
> s/distributed to other peers/propagated (lines 547-548, 1585) - see the
> comment at line 448.
>
> This sweep is best done after the restructuring in issue (3) rather than
> before, since the split puts the two route types in separate sections and
> makes a term used in the wrong one immediately visible.
>
> [Linda]  This concern has been substantially addressed by the changes in
> addressing your earlier comments. Section 2 now defines two consistent
> route terms: “Client Route” for AFI/SAFI 1/1, 2/1, 1/128, and 2/128, and
> “SD-WAN Underlay Route” for AFI/SAFI 1/74 and 2/74. The term “SD-WAN NLRI”
> is reserved for the NLRI encoding itself.
>
>
>
> We will make a final terminology sweep to remove remaining older
> designations, correct the Section 6.1 heading, and keep “underlay tunnel”,
> “underlay link”, and “underlay network” for the corresponding non-route
> objects.
>
>
>
> Section 1.3 Add the two route terms:
>
> Client Route: A route advertised using AFI/SAFI 1/1, 2/1, 1/128, or 2/128
> that is associated with an SD-WAN Hybrid Tunnel.
>
>
>
> SD-WAN Underlay Route: A route advertised using AFI/SAFI 1/74 or 2/74
> whose SD-WAN NLRI identifies a WAN port or ports and whose associated
> Tunnel Encapsulation Attribute advertises tunnel-related properties.
>
>
>
> Section 2.2.1 heading
>
>
>
> Old:
>
> 2.2.1. NLRI for SD-WAN Underlay Tunnel Update
>
>
>
> New:
>
> 2.2.1. NLRI for SD-WAN Underlay Route
>
>
>
> Section 2.5.1 heading
>
>
>
> Old:
>
> SD-WAN Hybrid NLRI without Encapsulation Extended Community
>
>
>
> New:
>
> SD-WAN Underlay Route without Encapsulation Extended Community
>
>
> Section 6.1 heading
>
>
>
> Old:
>
> 6.1. SD-WAN Overlay SAFI
>
>
>
> New:
>
> 6.1. SD-WAN SAFI
>
>
>
> Section 2.5.4
>
> Old:
>
> 2.5.4. Multiple Tunnels attached to One Underlay Route
>
>
>
> New:
>
> 2.5.4. Multiple Tunnels Associated with One SD-WAN Underlay Route
>
>
> (5) <major> No applicability or deployment considerations section: text
>     suggestions have been provided
>
> The document positions BGP as a mechanism for SD-WAN secure tunnel
> establishment without stating anywhere what that choice assumes, what was
> not analysed, or that alternatives exist. A reader encountering this
> specification has no way to learn any of it from the document or its
> related
> BESS document.
>
> This is not a request to argue against the approach, and it is explicitly
> not a request for a comparative analysis - writing one would be substantial
> work and is not what this document is for (neither is it in IDR WG
> charter).
> This is a request that the document be accurate about its own boundaries,
> prominently enough that the accuracy reaches a reader.
>
> Every statement in the suggested text is a statement about what this
> document
> does and does not contain. None of them is a claim about the technology, so
> none is arguable on technical grounds: the document either analysed RR
> compromise or it did not, either evaluated alternatives or did not. The
> deployment assumptions the mechanism depends on - a single administrative
> authority, a trusted RR holding every participant's security parameters, a
> policy-constrained walled garden - are currently stated nowhere a deployer
> would find them, and a deployment that violates them has a serious security
> exposure the document gave no warning of. Section 2.6.2, at line 1590,
> already requires local policy to create a "walled garden", in an
> error-handling subsection, with the term undefined.
>
> Let me share some specifics to help construct a 'Deployment Considerations
> and Applicability Statement' section in the document:
>
>    *  A new top-level section immediately following the Introduction.
>
>    *  A sentence in the first or second paragraph of Section 1 pointing to
>       it.
>
>    *  A pointer from Security Considerations (Section 5) to Section
>       [APPLICABILITY].2, so that security reviewers reach it.
>
>
>    Here is something to help get started on the suggested new top-level
> section:
>
>       [N]. Applicability and Deployment Considerations
>
>       This document specifies BGP signalling mechanisms for advertising
>       the properties of SD-WAN edge nodes and their underlay tunnels,
>       and for conveying the parameters used to establish IPsec security
>       associations between SD-WAN edge nodes. This section states the
>       deployment context that those mechanisms assume, and identifies
>       analysis that, being out of scope, this document does not perform.
>
>       [N].1. Deployment context
>
>       The mechanisms specified in this document assume all of the
>       following. They are not specified for use in deployments where any
>       of these does not hold.
>
>       *  All participating SD-WAN edge nodes and all participating BGP
>          Route Reflectors are under a single administrative authority,
>          and that authority controls the configuration and the policy of
>          every participant.
>
>       *  The BGP Route Reflector is a trusted component of that
>          authority's SD-WAN controller. Every participating edge node
>          relies on the Route Reflector to receive, and to correctly
>          redistribute without modification, the tunnel and security
>          parameters advertised by every other participating edge node.
>
>       *  BGP sessions between edge nodes and the Route Reflector are
>          constrained by local policy to a closed set of authorised
>          participants. This constraint is referred to elsewhere in this
>          document as a "walled garden" (see Section <add a new and short
>          subsection as N.1.1 to describe this>), and the properties
>          described in this document depend on it.
>
>       *  The cryptographic algorithms, transforms and security profiles
>          in use are provisioned consistently across participants by the
>          SD-WAN controller or management system. The mechanisms in this
>          document perform no negotiation of these parameters; a
>          participant whose provisioned parameters do not match those of
>          its peer will not establish a security association with that
>          peer.
>
>       These mechanisms are not specified for use between mutually
>       distrusting parties, across administrative boundaries, or in any
>       deployment in which the Route Reflector is not trusted with the
>       tunnel and security parameters of all participants.
>
>       [N].2. Analysis out of scope of this document
>
>       The following have not been analysed in this document as they are
>       outside its scope. Implementers and deployers are advised to
>       consider them before adopting these mechanisms.
>
>       *  Comparison with alternative mechanisms. Other mechanisms exist
>          for establishing IPsec security associations between SD-WAN
>          edge nodes, including the Internet Key Exchange Protocol
>          Version 2 (IKEv2) [RFC7296]. This document does not evaluate
>          those mechanisms, does not compare them with the mechanisms
>          specified here, and makes no claim that the mechanisms
>          specified here are preferable to them in any respect. The
>          selection of a mechanism for a given deployment is a matter for
>          the deployer.
>
>       *  Consequences of Route Reflector compromise. The tunnel and
>          security parameters of all participating edge nodes, including
>          any key material carried in the Sub-TLVs defined in Section
>          [SUBTLVS], are received and redistributed by the Route
>          Reflector. The consequences of a compromised, misconfigured or
>          coerced Route Reflector have not been analysed in this
>          document.
>
>       *  Forward secrecy. The forward secrecy properties of security
>          associations established using parameters distributed by BGP
>          have not been characterised in this document. These properties
>          differ from those of mechanisms in which the two endpoints of a
>          security association perform ephemeral key agreement directly
>          with one another.
>
>       *  Control plane load and convergence. The volume of BGP UPDATE
>          messages generated by security association rekeying at scale,
>          and its effect on BGP convergence and on Route Reflector
>          dimensioning, have not been analysed in this document. Section
>          [OPERATIONAL] provides operational guidance but no quantitative
>          analysis.
>
>       *  Behaviour under partial or reordered delivery. BGP does not
>          guarantee that successive advertisements for the same NLRI are
>          delivered to every recipient, or that they are delivered in the
>          order in which they were originated. The behaviour of the
>          mechanisms in this document when rekey advertisements are lost,
>          coalesced or reordered has not been analysed.
>
>       *  Interaction with other mechanisms operating on the same
>          security associations. The behaviour of these mechanisms in a
>          deployment where security associations between the same pair of
>          edge nodes may also be established or modified by another
>          mechanism has not been analysed.
>
> Now, this section can cover more aspects than the ones suggested above -
> this
> is something that I accumulated through the review. There are a few other
> comments/suggestions in the detailed review that probably end up adding
> text here: (a) the Section 3 policy problem (line 1614), if resolved
> by reframing it as deployment considerations, puts the policy expectations
> there in non-normative terms; (b) the transport-security requirement
> currently
> in the Introduction (line 148) moves there, normatively; and (c) the
> statement
> that the mechanisms are not RR-specific (line 179) moves there too. I let
> authors decide how to organize all these as subsections (similar to the
> walled-garden thing) under N.1 or N.2.
>
> This makes it possible to put cross-references from some challenging and
> problematic text in the document to the appropriate section/subsection
> without repeating or ignoring those aspects.
>
> And finally for this point, some text suggestions for referencing this new
> section:
>
>    From Section 1, first or second paragraph:
>
>       The deployment context that these mechanisms assume, and the
>       analysis that this document does not perform, are described in
>       Section [APPLICABILITY]. Readers considering these mechanisms for
>       deployment are advised to refer to that section.
>
>    From Section 5, Security Considerations, as a new paragraph:
>
>       Section [APPLICABILITY].2 enumerates security-relevant analysis
>       that this document does not perform, including the consequences of
>       compromise of the Route Reflector and the forward secrecy
>       properties of security associations established using parameters
>       distributed by BGP. Those items are not addressed in this section.
>
> I hope this sets up the narrow scope for this document and clarifies
> clearing
> things that are out of scope which others (especially from SEC area) may
> be looking for.
>
> [Linda] In addressing your earlier comments, the Introduction and Security
> Considerations now identify the RR as part of the SD-WAN Controller and
> describe its authorization role, Section 2.3 explicitly separates BGP
> signaling from IPsec processing, and Section 2.6.2 identifies the
> policy-constrained “walled garden” deployment assumption.
>
>
>
> We agree that the deployment assumptions and the analysis that is outside
> the scope of this document should be made more visible. Rather than adding
> a new top-level section, we can add concise applicability text in the
> Introduction, define the “walled garden” deployment assumption in Section
> 1.3, and add a short statement in the Security Considerations identifying
> the major security-related topics that this document does not analyze.
>
>
>
> Section 1 — Introduction
>
>
>
> Add after the paragraph that defines the RR as part of the SD-WAN
> Controller.
>
>
>
> Old:
>
> In the context of this document, BGP Route Reflector (RR) is the component
> of the SD-WAN Controller system that receives the BGP UPDATE from SD-WAN
> edges and in turn propagates the information to the intended peers that are
> authorized to communicate via the SD-WAN overlay network.
>
>
>
> New:
>
> In the context of this document, BGP Route Reflector (RR) is the component
> of the SD-WAN Controller system that receives the BGP UPDATE from SD-WAN
> edges and in turn propagates the information to the intended peers that are
> authorized to communicate via the SD-WAN overlay network.
>
>
>
> The mechanisms specified in this document are intended for a controlled
> SD-WAN environment in which the participating SD-WAN edge nodes and RRs
> operate under a common administrative authority and are constrained by
> policy to an authorized set of participants. They are not specified for use
> between mutually distrusting administrative domains.
>
>
>
> Add the definition of “Walled Garden” to the Section 1.3 (Terminology)
>
> Walled Garden: A controlled SD-WAN deployment in which participating
> SD-WAN edge nodes and RRs are authorized and managed under a common
> administrative authority, and BGP sessions and SD-WAN tunnel advertisements
> are constrained by local policy to those authorized participants.
>
>
>
> Last sentence of Security Considerations:
>
> Old:
>
> This specification does not define deployments across fully untrusted
> networks, but if such environments are used, strong transport security
> becomes a MUST, and additional validation mechanisms may be required to
> maintain SD-WAN tunnel and routing integrity.
>
>
>
> New:
>
> This specification does not define deployments across fully untrusted
> networks, but if such environments are used, strong transport security
> becomes a MUST, and additional validation mechanisms may be required to
> maintain SD-WAN tunnel and routing integrity. This document does not
> analyze the consequences of compromise or misconfiguration of the
> RR/Controller, the forward-secrecy properties of IPsec SAs established
> using parameters distributed by BGP, the control-plane impact of
> large-scale IPsec rekeying, or provide a general comparison with
> alternative mechanisms for SD-WAN discovery or IPsec SA establishment.
>
>
>
>
> (6) <major> Repetition and drift between the four validation sections
>
> Sections 2.1.1 (the summary), 2.4.2, 2.5.2 and 2.6 all state overlapping
> validation procedures, and they seem to have drifted apart. Several of the
> detailed comments below are largely the same procedure being stated in
> two or three places with different content. This is not an editorial thing
> as it is not just about duplication but also contradictions that make
> corrections/updates that much more harder.
>
> This should get largely resolved by the split suggested in (3) above, which
> puts each validation procedure in the section that owns it and removes the
> duplication. One statement of each validation procedure, in the section
> that
> owns the route type it applies to, with cross-references rather than
> restatements.
>
> [Linda] In addressing your earlier comments, Sections 2.4 and 2.5 now
> separately define the procedures for Client Routes and SD-WAN Underlay
> Routes, and Section 2.6 separately defines the corresponding error handling.
>
> We agree that Section 2.1.1 should not independently restate
> route-specific validation procedures. We will make it a short common
> validation overview and refer to Sections 2.4.2 and 2.5.2 for the
> route-specific procedures, with Section 2.6 remaining the authoritative
> error-handling section.
>
>
>
> Section 2.1.1 (Summary of Validation Procedure)
>
>
>
> Old:
>
> The validation procedure for the SD-WAN Hybrid tunnel TLV has the
> following steps
>
>    1. Validate tunnel TLV encoding and Sub-TLV encoding (per [RFC9012]
>    and sections 2.2 and 2.3 of this draft)
>    2. Check that Sub-TLVs are valid for NLRI type in route (see above
>    summarized in Table 1) and ignore invalid Sub-TLVs (per [RFC9012])
>    3. Validate that the tunnel egress endpoint is a reachable IP address
>    based on the BGP next-hop resolution rules.
>    4. Check if both the Color Sub-TLV and Extended Community for Color
>    exist, if so prefer the Extended Community for Color.
>
> After the SD-WAN Hybrid Tunnel TLV has been validated, the SD-WAN Peer
> processes based on the NLRI as (underlay route or client route) as
> described in sections 2.4 and 2.5. Prior to installing a route with a
> SD-WAN tunnel as an active route, the BGP peer installing the route MUST
> also validate that the SD-WAN tunnel and underlay links are active.
>
>
>
> New:
>
> The SD-WAN Hybrid Tunnel TLV and its Sub-TLVs MUST first be validated for
> correct encoding and applicability as specified in this section and Section
> 2.3. Route-specific validation procedures are specified in Section 2.4.2
> for Client Routes and Section 2.5.2 for SD-WAN Underlay Routes. Error
> handling is specified in Section 2.6.
>
>
>
> After the SD-WAN Hybrid Tunnel TLV has been validated, the receiving BGP
> speaker processes it according to the applicable route procedure.
>
>
>
> Section 2.6
>
>
>
> Old:
>
> The Error handling for SD-WAN VPN support has two components: error
> handling for Tunnel Encapsulation signaling (Encapsulation Extended
> Community and Tunnel Encapsulation Attribute) and error handling for SD-WAN
> Underlay Routes.
>
>
>
> New:
>
> This section specifies the error-handling procedures for validation
> failures identified by the procedures in Sections 2.4 and 2.5. It covers
> Tunnel Encapsulation signaling and SD-WAN Underlay Route errors.
>
>
> (7) <question> Was the discovery/keying split ever considered?
>
> The document's own stated motivation for the design is discovery scaling
> (lines 1028-1034). That argument supports carrying reachability, NAT
> properties, port characteristics and crypto capabilities in BGP. It does
> not
> by itself support carrying key material - those are separable, and a design
> that carried the first and left SA establishment to an existing
> key-management protocol would be a materially different document. RFC 7018,
> which this document already carries as a reference and does not cite,
> frames
> the auto-discovery-VPN problem that split addresses.
>
> So, a question for the WG: was that split - BGP for discovery, an existing
> protocol for keying - considered? If it was and the WG chose otherwise, a
> pointer to that discussion addresses the question and should go in the
> shepherd writeup. If it was not, that fact belongs in the applicability
> statement of issue (5), as part of what was not analysed. This is just
> something that I expect to come up during IESG Evaluation and it is better
> if it were addressed in the document (perhaps in the applicability
> analysis).
>
>
> [Linda] The intent of this document is not to define a key-management
> mechanism. The IPsec SA Property Sub-TLVs defined in this section specify
> encodings that allow BGP UPDATE messages to carry IPsec-related information
> to authorized peers.
>
> How the advertised IPsec parameters are generated, negotiated, or
> provisioned is outside the scope of this document. They may be provided by
> an SD-WAN controller or management system, IKEv2, or another mechanism.
> This document only defines how that information is carried in BGP UPDATE
> messages; BGP does not establish, negotiate, derive, or maintain IPsec SAs.
>
>
>
> Wording change to make it clear:
>
> Section 2.3:
>
>
>
> Old:
>
> The IPsec SA Property Sub-TLVs defined in this section are used by BGP
> only to advertise IPsec-related parameters associated with SD-WAN Hybrid
> Tunnels.
>
>
>
> New:
>
> The IPsec SA Property Sub-TLVs defined in this section specify encodings
> that allow BGP UPDATE messages to carry IPsec-related parameters associated
> with SD-WAN Hybrid Tunnels to authorized peers.
>
>
>
>
> DETAILED REVIEW
>
> The rest of the comments are provided inline in the idnits output:
>
>
> 14                  BGP UPDATE for SD-WAN Edge Discovery
> 15                 draft-ietf-idr-sdwan-edge-discovery-29
>
> <major> The title does not describe what the document specifies. I suggest
> "Advertising SD-WAN Edge Node and Underlay Tunnel Properties in BGP" -
> please let me know if you don't agree. My biggest concern is that this is
> not about BGP UPDATE messages!
>
> [Linda] How about “SD-WAN Edge and Underlay Tunnel Discovery Using BGP” as
> the title?
>
>
> 19   The document describes the BGP mechanisms for SD-WAN (Software
> 20   Defined Wide Area Network) edge node attribute discovery.  These
> 21   mechanisms include a new tunnel type and sub-TLVs for the BGP Tunnel-
> 22   Encapsulation Attribute (RFC9012) and set of NLRI (network layer
> 23   reachability information) for SD-WAN underlay information.
>
> <minor> How about the following suggestion to make the abstract clear?
>
>       CURRENT
>       The document describes the BGP mechanisms for SD-WAN (Software
>       Defined Wide Area Network) edge node attribute discovery. These
>       mechanisms include a new tunnel type and sub-TLVs for the BGP
>       Tunnel-Encapsulation Attribute (RFC9012) and set of NLRI (network
>       layer reachability information) for SD-WAN underlay information.
>
>       SUGGEST
>       This document specifies BGP mechanisms for advertising the
>       attributes of SD-WAN (Software-Defined Wide Area Network) edge
>       nodes. These mechanisms comprise a new tunnel type and associated
>       Sub-TLVs for the BGP Tunnel Encapsulation Attribute, and a new
>       Subsequent Address Family Identifier (SAFI) carrying a typed NLRI
>       for SD-WAN underlay tunnel information.
>
> [Linda] Thanks for the suggestion. We can take your advice and change the
> Abstract to the following:
>
> This document specifies BGP mechanisms for SD-WAN (Software-Defined Wide
> Area Network) edge node attribute discovery. These mechanisms comprise a
> new tunnel type and associated Sub-TLVs for the BGP Tunnel Encapsulation
> Attribute, and a new Subsequent Address Family Identifier (SAFI) carrying a
> typed NLRI for advertising SD-WAN underlay tunnel information.
>
>
>
> 27   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> 28   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
> 29   document are to be interpreted as described in [RFC2119] [RFC8174]
> 30   when, and only when, they appear in all capitals, as shown here.
>
> 271   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
> 272   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
> 273   "OPTIONAL" in this document are to be interpreted as described in BCP
> 274   14 [RFC8174] when, and only when, they appear in all capitals, as
> 275   shown here.
>
> <major> BCP 14 boilerplate is duplicated and neither of them is correct.
>
>
>
> [Linda] We will retain a single copy near the beginning of the document
> and replace it with the exact boilerplate specified by RFC 8174.
>
> Old:
>
> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>
> "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
>
> document are to be interpreted as described in [RFC2119] [RFC8174]
>
> when, and only when, they appear in all capitals, as shown here.
>
>
>
> New:
>
> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>
> "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
>
> "OPTIONAL" in this document are to be interpreted as described in
>
> BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
>
> capitals, as shown here.
>
>
>
> 148   BGP [RFC4271] serves as the control plane for these SD-WAN
> 149   deployments.  For SD-WAN implementations using BGP, the BGP Speaker
> 150   SHOULD establish a secure transport connection with each SD-WAN edge
> 151   operating under the same BGP control plane instance.  This secure
> 152   transport SHOULD support authentication and integrity, but MAY also
> 153   support confidentiality.  The BGP speaker can be an IBGP peer or a
> 154   RR.
> 156   SD-WAN deployments SHOULD use TCP-AO [RFC5925] to provide
> 157   authentication and integrity between the RR and each BGP Peer.  Other
> 158   mechanisms that can provide authentiation, integrity, and
> 159   confidentiality are in the process of being defined by the IETF.
> 160   Examples of these mechanisms are: BGP over QUIC [draft-retana-idr-
> 161   bgp-quic], BGP over TLS/TCP [draft-wirtgen-bgp-tls], or Securing
> 162   BGPv4 using IPsec [draft-ward-bgp-ipsec].  SD-WAN deployments MAY use
> 163   mechanisms that support authentication, integrity, and
> 164   confidentiality.
>
> <major> So many BCP 14 keywords in the Introduction, stating requirements
> that are repeated in different ways elsewhere at two further strengths -
> Section 4 at lines 1780-1782 makes secure transport RECOMMENDED, and
> Section
> 5 at lines 1815-1816 makes it optional. This is contradictions.
>
> As suggested, please move these out into the applicability and deployment
> considerations section. They are a basic requirement of the solution - the
> mechanism does not work safely without a secure transport - and a basic
> requirement belongs in the body of the document.
>
> Suggest: state the transport-security requirement once, normatively, in
> the
> new section; delete the Section 1 statements and replace them with a
>  non-normative sentence pointing at it; delete Section 4's RECOMMENDED
> restatement; and rewrite Section 5 to reference the requirement and
> analyse
> what happens when it is not met, rather than to restate it at a weaker
> strength.
>
> [Linda] We can make the following changes to consolidate the normative
> requirement in one place and remove the inconsistent restatements. The
> Introduction will state the requirement, Section 4 will refer to it rather
> than restate it, and Section 5 will describe the security consequences if
> the requirement is not met.
>
>
>
> Section 1 (Introduction)
>
> Old:
>
> BGP [RFC4271] serves as the control plane for these SD-WAN deployments.
> For SD-WAN implementations using BGP, the BGP Speaker SHOULD establish a
> secure transport connection with each SD-WAN edge operating under the same
> BGP control plane instance. This secure transport SHOULD support
> authentication and integrity, but MAY also support confidentiality. The BGP
> speaker can be an IBGP peer or a RR.
>
>
>
> SD-WAN deployments SHOULD use TCP-AO [RFC5925] to provide authentication
> and integrity between the RR and each BGP Peer. Other mechanisms that can
> provide authentiation, integrity, and confidentiality are in the process of
> being defined by the IETF. Examples of these mechanisms are: BGP over QUIC
> [draft-retana-idr-bgp-quic], BGP over TLS/TCP [draft-wirtgen-bgp-tls], or
> Securing BGPv4 using IPsec [draft-ward-bgp-ipsec]. SD-WAN deployments MAY
> use mechanisms that support authentication, integrity, and confidentiality.
>
>
>
> New:
>
> BGP [RFC4271] serves as the control plane for these SD-WAN deployments.
> BGP sessions used to exchange the SD-WAN information defined in this
> document MUST provide peer authentication and integrity protection. TCP-AO
> [RFC5925] is one mechanism that provides these properties. Other mechanisms
> that provide equivalent or stronger protection, including confidentiality,
> MAY also be used. The BGP speaker can be an IBGP peer or an RR.
>
>
>
> Section 4  (Manageability Considerations)
>
> Old:
>
> The use of secure transport connections (e.g., BGP over IPsec/TLS) is
> RECOMMENDED to ensure manageability in untrusted environments.
>
>
>
> New:
>
> The secure-transport requirements for BGP sessions carrying the SD-WAN
> information defined in this document are specified in Section 1.
>
>
>
> Section 5 (Security Considerations)
>
>
>
> Old:
>
> However, in closed or "walled garden" deployments, where SD-WAN edge nodes
> and the RR (SD-WAN controller) are within a trusted, secured environment
> (e.g., a private MPLS backbone or physically secured enterprise network),
> the risk of interception or tampering is significantly reduced. In such
> cases, the use of secure transport is optional, and operators may choose to
> run BGP over standard TCP, based on their internal risk assessment.
>
>
>
> New:
>
> In closed or "walled garden" deployments, where SD-WAN edge nodes and the
> RR are within a trusted and secured environment, the risk of interception
> or tampering may be reduced. However, the peer-authentication and integrity
> requirements specified in Section 1 still apply.
>
>
>
>
> 179   While the BGP RR functionin the SD-WAN controller scenario could be
> 180   implemented with targeted BGP sessions control by policy and secured
> 181   by security features that supply authentication, integrity, and
> 182   confidentiality, the authors chose to limit the descriptions to the
> 183   BGP RR.  This choice is due to the current status of BGP security
> 184   issues and not the BGP peer exchanges described in this text.  Due to
> 185   this decision, the BGP interactions are not strictly limited to BGP
> 186   RR features
>
> <major> This paragraph comes across as an authorial commentary about a
> drafting
> decision rather than specification. It is also the only place the document
> says the mechanisms are not RR-specific, which is information the reader
> needs - and it contradicts the RR-as-trust-anchor point by disclaiming the
> centring Section 1 has just established. If there is a deployment model
> without
> the controller as the RR (i.e.. iBGP full mesh), please specify how the
> procedures work, if not, then suggest to remove this.
>
>
>
> [Linda] Its intent is to state that the deployment model specified in this
> document is RR-based, with the RR acting as part of the SD-WAN Controller
> and providing the central point for authorization, policy, and controlled
> propagation of SD-WAN information. We can make the following changes to
> state this directly and remove the discussion of alternative BGP peering
> arrangements.
>
> Section 1
>
> Old:
>
> While the BGP RR functionin the SD-WAN controller scenario could be
> implemented with targeted BGP sessions control by policy and secured by
> security features that supply authentication, integrity, and
> confidentiality, the authors chose to limit the descriptions to the BGP RR.
> This choice is due to the current status of BGP security issues and not the
> BGP peer exchanges described in this text. Due to this decision, the BGP
> interactions are not strictly limited to BGP RR features
>
>
>
> New:
>
> The deployment model specified in this document is based on a BGP Route
> Reflector that is part of the SD-WAN Controller. The RR provides the
> central point for applying authorization and policy and for propagating
> SD-WAN route and tunnel information to authorized peers.
>
>
>
> 156   SD-WAN deployments SHOULD use TCP-AO [RFC5925] to provide
> 157   authentication and integrity between the RR and each BGP Peer.  Other
> 158   mechanisms that can provide authentiation, integrity, and
> 159   confidentiality are in the process of being defined by the IETF.
>
> <major> This makes [RFC5925] a normative reference.
>
>
>
> [Linda] Changed [RFC5925] a normative reference
>
>
>
> 160   Examples of these mechanisms are: BGP over QUIC [draft-retana-idr-
> 161   bgp-quic], BGP over TLS/TCP [draft-wirtgen-bgp-tls], or Securing
> 162   BGPv4 using IPsec [draft-ward-bgp-ipsec].
>
> 1798   over new IETF technologies (e.g. BGP over QUIC
> [draft-retana-idr-bgp-
> 1799   quic], BGP over TLS/TCP [draft-wirtgen-bgp-tls], or Securing BGPv4
> 1800   using IPsec [draft-ward-bgp-ipsec]) which provide authentication,
> 1801   confidentiality, and Integrity.
>
> 730         Contextual Note 1: The ID Length is set to 4 octets, but ID in
> 731         [draft-ietf-bess-secure-evpn-02] under development in the IETF
>
> <major> Three individual (non-WG) drafts, cited twice each, with no
> references!
> Citing them as "in the process of being defined by the IETF" is problematic
> since none of them can be relied on by this specification and hence quite
> meaningless. For confidentiality, the only available mechanism is to use
> IPSec
> as a tunnel. This topic was raised, discussed, and addressed during the
> review
> of the BESS SD-WAN document. Those changes need to propagate into this
> document
> where they are absolutely a MUST have for this solution. Please correct me
> if
> I am wrong.
>
>
>
> [Linda] This concern has already been addressed in the changes made in
> addressing your earlier comments. The Introduction no longer cites the
> three individual BGP transport-security drafts. It now states a single
> requirement that BGP sessions carrying the SD-WAN information defined in
> this document provide peer authentication and integrity protection, with
> TCP-AO [RFC5925] given as one mechanism. Other mechanisms providing
> equivalent or stronger protection may also be used.
>
> We will also remove the remaining references to those individual drafts
> from the Security Considerations.
>
>
>
> Here are the wording changes:
>
>
>
> Section 5 (Security Considerations):  Remove only the obsolete
> individual-draft examples.
>
>
>
> Old:
>
> Some network operators running SD-WAN edge over public or untrusted
> networks desire confidentiality as well as authentication and integrity. In
> this case, network operators can consider running BGP over new IETF
> technologies (e.g. BGP over QUIC [draft-retana-idr-bgp-quic], BGP over
> TLS/TCP [draft-wirtgen-bgp-tls], or Securing BGPv4 using IPsec
> [draft-ward-bgp-ipsec]) which provide authentication, confidentiality, and
> Integrity.
>
>
>
> New:
>
> Some network operators running SD-WAN edge over public or untrusted
> networks may require confidentiality in addition to authentication and
> integrity. In such deployments, a mechanism providing confidentiality, such
> as IPsec [RFC4301], can be used to protect the path between the SD-WAN edge
> and the RR.
>
>
>
> 291   Advertise Underlay Routes (SD-WAN NLRIs) with SD-WAN Hybrid Tunnel
> 292   Encapsulation Attribute:  A BGP speaker advertises SD-WAN NLRI for
> 293      IPv4/IPv6 (AFI/SAFI 1/74 or 2/74) with the NEXT_HOP attribute set
> 294      to the local address of the advertising speaker, and includes a
>
> 421   describe the processing.  The SD-WAN NLRI is originated using the
> 422   loopback address of the C-PE.
>
> 332         (IANA assigned).  The NextHop Field in the BGP update indicates
> 333         the tunnel egress endpoint and this field SHOULD be set to the
> 334         BGP Peer Address for the SD-WAN Peer.
>
> <major> A few problems with the Next Hop, and the first is the more
> serious.
>
> (a) The encoding is genuinely absent. RFC 4760 requires each AFI/SAFI
> specification to define the format of the Next Hop Network Address field in
> MP_REACH_NLRI. For 1/74 and 2/74 this document does not, and because the
> SD-WAN SAFI is outside RFC 9012's scope (issue (1) above) there is no other
> document that could supply it. Unstated: the permitted lengths of the field
> for each AFI; whether an IPv6 next hop may be carried for AFI 1/74, in the
> manner of RFC 8950; whether a link-local address may accompany a global
> IPv6
> next hop for 2/74, and if so in what order; and what a receiver does with a
> length it does not recognise. This was pointed out in my RTGDIR early
> review,
> still open.
>
> (b) The semantics are stated in at least four places, with three different
> answers. "The local address of the advertising speaker", "the loopback
> address of the C-PE" and "the BGP Peer Address for the SD-WAN Peer" are
> three different specifications. The first is indeterminate - an edge node
> with several WAN ports and a loopback has several local addresses. The
> third
> is at SHOULD strength, which conflicts (c) below.
>
>
>
> [Linda] The changes in addressing your earlier comments now defines the
> SAFI 74 next-hop procedures, but the MP_REACH_NLRI Next Hop encoding still
> needs to be explicit and the existing descriptions should be made
> consistent. We can make the following changes to specify the Next Hop
> length for AFI/SAFI 1/74 and 2/74 and consistently identify the Next Hop
> with the advertising SD-WAN edge, normally its loopback/SD-WAN Node ID.
>
>
>
> Section 2.2.1: Add the following paragraph after the first paragraph that
> introduces SAFI 74:
>
> For AFI/SAFI 1/74, the MP_REACH_NLRI Next Hop Network Address field MUST
> contain a 4-octet IPv4 address. For AFI/SAFI 2/74, it MUST contain a
> 16-octet IPv6 address. The Next Hop identifies the advertising SD-WAN edge
> and SHOULD be the same address as the SD-WAN Node ID, normally the loopback
> address.
>
>
> Section 2 (Advertise SD-WAN Underlay Routes)
>
> Old:
>
> Advertise SD-WAN Underlay Routes: A BGP speaker advertises SD-WAN Underlay
> Routes using AFI/SAFI 1/74 or 2/74. The NEXT_HOP is set to a local address
> of the advertising SD-WAN edge node, and the UPDATE includes a Tunnel
> Encapsulation Attribute containing an SD-WAN Hybrid Tunnel TLV.
>
> New:
>
> Advertise SD-WAN Underlay Routes: A BGP speaker advertises SD-WAN Underlay
> Routes using AFI/SAFI 1/74 or 2/74. The NEXT_HOP is set to a reachable
> address of the advertising SD-WAN edge node, typically its loopback
> address, and the UPDATE includes a Tunnel Encapsulation Attribute
> containing an SD-WAN Hybrid Tunnel TLV.
>
>
>
> Section 2.1.2:
>
> Old:
>
> The SD-WAN NLRI is originated using the loopback address of the C-PE.
>
>
>
> New:
>
> The SD-WAN NLRI is originated by the C-PE, and the BGP Next Hop is set to
> a reachable address of the C-PE, typically its loopback address.
>
>
>
>
> 1442   Remote nodes use the SD-WAN Node ID carried in the SD-WAN SAFI to
> 1443   correlate client routes whose NextHop address matches the SD-WAN
> Node
> 1444   ID.  This allows the receiving node to associate each client route
> 1445   with the appropriate set of tunnel attributes advertised by the
> 1446   corresponding SD-WAN edge node.
>
> 1633   *  The NextHop in the client route update matches a known SD-WAN
> Node
> 1634      ID.
>
> (c) The correlation rule depends on the Next Hop and is stated only
> in procedure and advisory sections. Binding a client route to its tunnel
> attributes turns entirely on the client route's Next Hop being equal to the
> SD-WAN Node ID. That is the load-bearing rule of the whole mechanism, and
> it
> appears once inside a procedure section and once as a SHOULD-level bullet
> in
> Section 3.1, which is advisory (see the comment at line 1614). It is
> nowhere
> stated as a requirement on the originator to set the Next Hop accordingly,
> so an implementer reading Section 2.4 alone will not know to do it - and
> Section 2.1's SHOULD at line 333 points at a different value.
>
> Two consequences the document does not address. First, it constrains
> next-hop handling on the client routes: any deployment step that rewrites
> the next hop (next-hop-self at an ASBR, an RR configured to change it, a
> route server) breaks the correlation silently, and nothing says the
> mechanism assumes the next hop is preserved end to end. Second, the
> document's only worked example contradicts the rule - Section 3.2.1
> sets NextHop to 198.51.100.10 and the SD-WAN Node ID to 1.1.1.1 (see the
> comment at line 1690).
>
> The fix has three parts, and the split in issue (3) determines where each
> lands. First, specify the encoding in the SD-WAN SAFI section: permitted
> lengths per AFI, next-hop address family decoupled from the NLRI AFI,
> link-local handling, and the malformed condition - Section 2.1 of RFC 9830
> does this in one paragraph and is worth copying almost directly. Second,
> state the Next Hop semantics once, per route type, in the section that owns
> that route type; reconcile the three current answers into one for the
> SD-WAN
> SAFI and delete the other two occurrences rather than leaving them as
> restatements. Third, promote the correlation rule to a normative
> requirement
> on both ends - the originator MUST set the client route Next Hop to its
> SD-WAN Node ID, and the receiver correlates on that equality - placed in
> the
> interaction section that issue (3) creates, and state the assumption that
> the Next Hop is preserved between originator and receiver, and what happens
> when it is not. While in the same text: state that the Tunnel Encapsulation
> attribute is carried with SAFI 74 and that this document defines that
> usage,
> and state the capability requirement for AFI/SAFI 1/74 and 2/74.
>
>
>
> [Linda] This issue has been addressed in the changes for resolving your
> earlier comments. The document now distinguishes the BGP Next Hop from the
> SD-WAN Node ID and states that the Next Hop is a reachable address of the
> advertising SD-WAN edge, typically its loopback address. Section 2.2.1 also
> explicitly defines the MP_REACH_NLRI Next Hop encoding for AFI/SAFI 1/74
> and 2/74.
>
>
>
> 346         Public Key, IPsec SA Proposal, and Simplified IPsec SA.  If a
> 347         Tunnel Egress End Point is not included in SD-WAN Hybrid Tunnel
> 348         TLV, it is treated as if a Tunnel Egress End Point SubTLV is
> 349         included with an address family identifer of 0.
>
> 386      *2 - if a Tunnel Egress End Point (EP) Sub-TLV is not included,
> 387      the SD-WAN Hybrid tunnel treats as if Tunnel egress End Point
> 388      exists with AFI of 0.
>
> 1341   1.  Check for Well-formed SD-WAN Hybrid Tunnel TLV:  If the SD-WAN
> 1342      Hybrid Tunnel TLV does not have a Tunnel Egress Endpoint SubTLV,
> 1343      the tunnel validation process treats it as if a Tunnel Egress
> 1344      SubTLV exists with an AFI of 0(Per [RFC9012], a Tunnel Egress
> 1345      SubTLV exists with an AFI of 0 uses the packet Next Hop as the
> 1346      tunnel endpoint).
>
> <major> This construct - treating an absent Tunnel Egress Endpoint sub-TLV
> as Address Family 0 - appears multiple times times, and for client routes
> it
> contradicts Section 13 of RFC 9012. For client routes the document uses
> AFI/SAFI 1/1, 2/1, 1/128 and 2/128 (line 355), every one of which is in the
> first-paragraph list of Section 6 of RFC 9012. For those, Section 13 of RFC
> 9012 says: "a TLV that does not contain exactly one Tunnel Egress Endpoint
> sub-TLV MUST be treated as if it contained a malformed Tunnel Egress
> Endpoint sub-TLV" - and a malformed Tunnel Egress Endpoint sub-TLV means
> "the entire TLV MUST be ignored and MUST be removed from the Tunnel
> Encapsulation attribute before the route carrying that attribute is
> distributed." The document's rule produces the opposite outcome from the
> same input: an RFC 9012 implementation drops the TLV, an implementation of
> this document builds a tunnel to the next hop. That is a straightforward
> contradiction and affects interoperability failure. Why not follow RFC
> 9012?
>
>
>
> [Linda]  Agree. For Client Routes, the Tunnel Encapsulation Attribute
> procedures of RFC 9012 apply unchanged, including the requirement for the
> Tunnel Egress Endpoint Sub-TLV. The “assume AFI=0 when the Tunnel Egress
> Endpoint Sub-TLV is absent” behavior is intended only for SD-WAN Underlay
> Routes using SAFI 74. We can make the following changes to scope that rule
> accordingly and remove the conflicting Client Route text.
>
>
>
> Section 2.1 (SD-WAN Hybrid Tunnel TLV Encoding)
>
> Old:
>
> If a Tunnel Egress End Point is not included in SD-WAN Hybrid Tunnel TLV,
> it is treated as if a Tunnel Egress End Point SubTLV is included with an
> address family identifier of 0.
>
>
>
> New:
>
> For SD-WAN Underlay Routes using SAFI 74, if a Tunnel Egress Endpoint
> Sub-TLV is not included in the SD-WAN Hybrid Tunnel TLV, it is treated as
> if a Tunnel Egress Endpoint Sub-TLV with an Address Family value of 0 were
> included. For Client Routes, Tunnel Egress Endpoint processing follows
> [RFC9012].
>
>
>
> Table 1 footnote *2
>
> Old:
>
> *2 - if a Tunnel Egress End Point (EP) Sub-TLV is not included, the SD-WAN
> Hybrid tunnel treats as if Tunnel egress End Point exists with AFI of 0.
>
>
>
> New:
>
> *2 - For SD-WAN Underlay Routes, if a Tunnel Egress Endpoint Sub-TLV is
> not included, it is treated as if a Tunnel Egress Endpoint Sub-TLV with AFI
> 0 were included. For Client Routes, [RFC9012] applies.
>
>
>
> Section 2.4.2 : simplify the Step 1 so the Client Route processing clearly
> follows RFC 9012
>
> Old:
>
> Check for Well-formed SD-WAN Hybrid Tunnel TLV: If the SD-WAN Hybrid
> Tunnel TLV does not have a Tunnel Egress Endpoint SubTLV, the tunnel
> validation process treats it as if a Tunnel Egress SubTLV exists with an
> AFI of 0 (Per [RFC9012], a Tunnel Egress SubTLV exists with an AFI of 0
> uses the packet Next Hop as the tunnel endpoint). Also, per [RFC9012],
> MALFORMED Sub-TLVs are ignored and a Sub-TLV with an unknown type is
> ignored. The SD-WAN Hybrid Tunnel TLV only processes the first instance of
> a Sub-TLV except for the IPsec SA ID Sub-TLV. For the IPsec SA ID Sub-TLV,
> if the IPsec SA Identifiers are unique are multiple IPsec SA ID sub-TLVs
> are processed. If all the IPsec SA Identifies are not unique, the second
> IPsec SA ID Sub-TLV is ignored and not propagated.
>
>
>
> New:
>
> Check for Well-formed SD-WAN Hybrid Tunnel TLV: For Client Routes, the
> SD-WAN Hybrid Tunnel TLV and its Tunnel Egress Endpoint Sub-TLV are
> validated and handled according to [RFC9012]. MALFORMED and unrecognized
> Sub-TLVs are handled according to [RFC9012]. The SD-WAN Hybrid Tunnel TLV
> processes only the first instance of each Sub-TLV, except for the IPsec SA
> ID Sub-TLV. Multiple IPsec SA ID Sub-TLVs MAY be processed if their IPsec
> SA Identifiers are unique. If a subsequent IPsec SA ID Sub-TLV contains an
> identifier already advertised in an earlier instance, that Sub-TLV is
> ignored and not propagated.
>
>
>
> 1559   TLVs MUST be handled as specified in [RFC9012].  This document
> 1560   defines new Sub-TLVs for Tunnel Type 25 (SD-WAN-Hybrid), but does
> not
> 1561   alter the validation behavior established in RFC 9012.
>
> <major> And it is stated in the same document that no such change is being
> made. For client routes, please require the Tunnel Egress Endpoint sub-TLV
> to be present, with Address Family 0 where the next hop is the endpoint -
> which is what RFC 9012 already provides and costs almost nothing. Change
> Table
> 1's client-route column to "required" and delete footnote *2 as it applies
> to that column. If the WG genuinely wants the omission to be legal for
> client routes, that is an update to RFC 9012 and must be declared as one in
> the header, the Abstract and the Introduction, with the interoperability
> consequence spelled out - but I would not recommend that path for the
> saving
> involved.
>
> Two residual points on the underlay side, both belonging to issue (1)
> rather
> than here. First, the document should say it is defining its own rule for a
> SAFI outside RFC 9012's scope, rather than presenting it as RFC 9012
> behaviour - lines 1344-1346 read "(Per [RFC9012], a Tunnel Egress SubTLV
> exists with an AFI of 0 uses the packet Next Hop as the tunnel endpoint)",
> which cites RFC 9012 for a case RFC 9012 excludes. Second, and worth
> putting
> to the authors: Section 2.3 of RFC 9830 answered the analogous question by
> declaring the Tunnel Egress Endpoint sub-TLV simply not applicable to its
> SAFI. Is it actually meaningful for an SD-WAN underlay route? Section 2.5.2
> step 2 says it "identifies a specific WAN interface or ... the remote
> SD-WAN
> edge node at which the outer SD-WAN Hybrid Tunnel terminates" - but the
> SD-WAN Node ID in the NLRI already identifies the node and the Extended
> Port
> Attribute already carries the port addresses, so it is not obvious what the
> sub-TLV adds. If it is genuinely redundant, marking it not-utilized for
> SAFI
> 74 would be cleaner than defaulting it.
>
>
>
> [Linda] The changes that were made to address your earlier comments
> already clarify that Client Routes follow RFC 9012 unchanged, while SAFI 74
> is outside the RFC 9012 applicability scope and has procedures defined by
> this document. Section 2.4.2 has also been revised accordingly. We can make
> the following changes to clean up the remaining:
>
>
>
> Table 1: remove the footnote *2 from the Client Route column for the
> “Tunnel Egress EP”
>
>
>
> Footnote *2
>
> Old:
>
> *2 - For SD-WAN Underlay Routes, if a Tunnel Egress Endpoint Sub-TLV is
> not included, it is treated as if a Tunnel Egress Endpoint Sub-TLV with AFI
> 0 were included. For Client Routes, [RFC9012] applies.
>
>
>
> New:
>
> *2 - For SD-WAN Underlay Routes, if a Tunnel Egress Endpoint Sub-TLV is
> not included, it is treated as if a Tunnel Egress Endpoint Sub-TLV with AFI
> 0 were included.
>
>
>
> Section 2.4 Second bullet:
>
>
>
> Old:
>
> This outer tunnel is identified by the Tunnel Egress Endpoint Sub-TLV.
> Recall as stated earlier, if a Tunnel Egress Endpoint Sub-TLV does not
> exist, the SD-WAN Hybrid Tunnel processing treats it as a Tunnel Egress
> Endpoint Sub-TLV with AFI equal to zero.
>
>
>
> New:
>
> This outer tunnel is identified by the Tunnel Egress Endpoint Sub-TLV.
> When the BGP Next Hop is used as the tunnel endpoint, the Tunnel Egress
> Endpoint Sub-TLV is encoded with Address Family 0 as specified in [RFC9012].
>
>
>
> 352   Sub-TLV Support per NLRI type:  Summary table
> 353                     Table 1
>
> 390                            Figure 1: sub-TLV list
>
> <nit> The block is introduced as "Sub-TLV Support per NLRI type: Summary
> table / Table 1" (lines 352-353), referred to as "Table 1" throughout the
> document (lines 350, 400), and captioned "Figure 1: sub-TLV list" (line
> 390). Please make it a numbered table with a table caption.
>
> [Linda] changed the Table title to “Sub-TLV Support for Client and
> Underlay Routes”
>
>
>
>
> 362      Color                3   valid *1      not valid
>
> 384      *1 - validation per [RFC9012]. Color Extended Community
> 385      takes precedence over Color Sub-TLV.
>
> 405      4) Check if both the Color Sub-TLV and Extended Community for
> 406      Color exist, if so prefer the Extended Community for Color.
>
> <major> Two distinct issues here:
>
> (a) Wrong code point. Section 3.4.2 of RFC 9012 defines the Color sub-TLV
> as
> Type Code 4. Code 3 is "IPsec Tunnel Authenticator (DEPRECATED)" in the
> IANA
> BGP TEA Sub-TLVs registry.
>
> (b) Wrong model. Section 8 of RFC 9012 does not put the Color sub-TLV and
> the Color Extended Community in competition on the same route. The Color
> sub-TLV appears on the tunnel route and the Color Extended Community on the
> client route, and they are matched: a packet must not be sent through the
> tunnel unless the client route carries a Color Extended Community matching
> one of the tunnel route's Color sub-TLVs. There is no precedence rule in
> RFC
> 9012, and inventing one in a table footnote and a numbered validation step
> is a third, incompatible mechanism alongside the document's own
> SD-WAN-Color
> field (Section 2.2.1) and the Color Extended Community matching described
> in
> Sections 2.4.1, 2.4.3 and 3.1.
>
> Please correct the code to 4. Then decide on one colour mechanism and
> describe it
> once: my reading of the intent is that the SD-WAN-Color in the underlay
> NLRI
> is matched against the Color Extended Community on the client route, in
> which case the RFC 9012 Color sub-TLV is not used at all and should be
> marked "not valid" for both columns, with footnote *1 and Section 2.1.1
> step
> 4 deleted.
>
>
>
> [Linda] The intended color mechanism in this document is not the RFC 9012
> Color Sub-TLV. Client Routes carry the Color Extended Community, while
> SD-WAN Underlay Routes carry the SD-WAN-Color field in the SAFI-74 NLRI,
> and the receiving node correlates those values as described in Sections
> 2.4.1, 2.4.3, and 3.1.
>
> Section 2.1.1 has already been revised and no longer contains the
> procedure that preferred the Color Extended Community over the Color
> Sub-TLV. We will correct the Color Sub-TLV code point to 4, mark it as not
> valid for both Client Routes and SD-WAN Underlay Routes, delete the
> obsolete Table 1 footnote, and add the following short clarification of the
> color mechanism used by this document.:
>
>
>
> The Color Sub-TLV defined in [RFC9012] is not used by the SD-WAN
> procedures defined in this document. Client Routes carry the Color Extended
> Community, while SD-WAN Underlay Routes carry the SD-WAN-Color field in the
> SAFI-74 NLRI. The receiving BGP speaker correlates these values as
> described in Sections 2.4.1, 2.4.3, and 3.1.
>
>
>
> 378      Underlay Type       66   not valid     valid
>
> 1194 2.3.6.1.  Extended Port Sub-Sub-TLV
> 1196   One Extended Sub-Sub-TLVs is specified in this document: Underlay
> 1197   Network Type Sub-Sub-TLV.
> 1204   multiple WAN ports are available.  The Underlay Network Type
> Sub-Sub-
> 1205   TLV is only valid for the Tunnel SD-WAN Hybrid Tunnel TLV within the
> 1206   Extended Port Attribute Sub-TLV.
> 1210   66 (IANA Assigned).
>
> 1864       66  Underlay Type                  This document  2.3.6.1
>
> <major> Table 1 lists 66 as a TEA sub-TLV valid
> for underlay routes. Section 6.3 registers 66 in the BGP TEA Sub-TLVs
> registry.
> Section 2.3.6.1 defines it as a sub-sub-TLV that "is only valid ... within
> the
> Extended Port Attribute Sub-TLV". These cannot all be true. As it stands,
> an
> implementation following Table 1 will accept type 66 at sub-TLV level,
> where
> the document gives it no meaning.
>
> Consequently, the document defines a new sub-sub-TLV namespace - the
> Extended Sub-Sub-TLV field of sub-TLV 65 - and requests no registry for it,
> so no second sub-sub-TLV can ever be defined. Section 2.3.6.1's own heading
> ("Extended Port Sub-Sub-TLV") and its "One Extended Sub-Sub-TLVs is
> specified in this document" both signal that more are expected. Note this
> is
> also how the value got its name in IANA: the registry currently reads "66 |
> Underlay ISP Properties | draft-ietf-idr-sdwan-edge-discovery-04" in the
> sub-TLV registry - see the comment at line 1842.
>
> Please decide which level it lives at. If sub-sub-TLV, which the encoding
> and Section 2.3.6.1 both indicate: remove 66 from Table 1 and from Section
> 6.3, ask IANA to return code point 66 in the sub-TLV registry, and create a
> new "SD-WAN Extended Port Sub-Sub-TLV Types" registry with a stated policy
> and 1 = Underlay Network Type as the initial entry. I think this is the
> intent?
>
> But if not and, if sub-TLV: move Section 2.3.6.1 out to its own Section
> 2.3.7,
> give it its own encoding independent of the Extended Port Attribute, and
> correct Section 2.3.6's length formula (see the comment at line 1065)
> since
> the sub-sub-TLV would no longer be inside it.
>
>
>
> [Linda] The intended encoding is for Underlay Network Type to be a
> Sub-Sub-TLV carried within the Extended Port Attribute Sub-TLV, not a
> top-level Tunnel Encapsulation Attribute Sub-TLV.
>
> We will remove value 66 from Table 1 and from the Tunnel Encapsulation
> Attribute Sub-TLV registry request in Section 6.3, request that the
> existing value 66 assignment in that registry be returned to the unassigned
> pool, and create a separate registry for Extended Port Sub-Sub-TLV Types.
> The initial assignment in that registry will be value 1 for Underlay
> Network Type.
>
>
>
> 448   connection.  Each BGP UPDATE message with a SD-WAN Underlay NLRI MUST
> 449   contains a Tunnel Encapsulation Attribute with a SD-WAN Hybrid Tunnel
> 450   TLV.  If an SD-WAN Underlay NLRI is received without a Tunnel
> 451   Encapsulation Attribute with a SD-WAN Hybrid tunnel TLV, the NLRI is
> 452   treated as "Treat-as-withdraw".
>
> 1596   The SD-WAN NLRI (AFI=1/SAFI=74) MUST be paired with Tunnel
> 1597   Encapsulation attribute with a SD-WAN Hybrid tunnel TLV, If the SD-
> 1598   WAN NLRI exist in an BGP UPDATE without a Tunnel Encapsulation
> 1599   Attribute with a SD-WAN Hybrid tunnel TLV, the NLRI is considered
> 1600   malformed and Treat-as-withdraw approach (per RFC7606).
>
> <major> Issues with error handing treatment
>
> (a) The granularity of treat-as-withdraw is incorrect. The TEA is a
> path attribute, so it applies to every NLRI carried in the MP_REACH_NLRI
> of
> that UPDATE. Its absence is therefore a property of the whole message, not
> of any one NLRI, and every SD-WAN SAFI NLRI in that UPDATE is affected. As
> written - "the NLRI is treated as" - the text reads as though one NLRI
> could
> be withdrawn while others in the same message survive, which cannot arise
> from this condition. The same wording problem appears at Section 2.6.3.
> Please state the scope explicitly and give the reason: where the required
> TEA with an SD-WAN Hybrid Tunnel TLV is absent, all SD-WAN SAFI NLRIs in
> that UPDATE are treated-as-withdraw, because the attribute applies to all
> of them. One statement, in one place, with Sections 2.2 and 2.6.3 not both
> carrying it.
>
> [Linda] Agree. The Tunnel Encapsulation Attribute is a path attribute and
> therefore applies to all SD-WAN SAFI NLRIs carried in the UPDATE. The
> current wording incorrectly refers to a single NLRI and repeats the same
> rule in Sections 2.2 and 2.6.3.
>
> We will keep the normative error-handling rule in Section 2.6.3,
> explicitly state that all SD-WAN SAFI NLRIs in the affected UPDATE are
> treated-as-withdraw when the required Tunnel Encapsulation Attribute
> containing an SD-WAN Hybrid Tunnel TLV is absent, and change Section 2.2 to
> reference Section 2.6.3.
>
>
>
> Here are the wording changes:
>
> Section 2.2
>
> Old:
>
> Each BGP UPDATE containing an SD-WAN Underlay Route MUST include a Tunnel
> Encapsulation Attribute with an SD-WAN Hybrid Tunnel TLV. If an SD-WAN
> Underlay Route is received without the required Tunnel Encapsulation
> Attribute and SD-WAN Hybrid Tunnel TLV, the route MUST be handled as
> Treat-as-withdraw as specified in Section 2.6.3.
>
>
>
> New:
>
> Each BGP UPDATE containing SD-WAN Underlay Routes MUST include a Tunnel
> Encapsulation Attribute containing an SD-WAN Hybrid Tunnel TLV. Error
> handling when this required attribute is absent is specified in Section
> 2.6.3.
>
>
>
> Section 2.6.3
>
> Old:
>
> The SD-WAN NLRI (AFI/SAFI=1/74 or 2/74) MUST be paired with a Tunnel
> Encapsulation Attribute containing an SD-WAN Hybrid Tunnel TLV. If the
> SD-WAN NLRI exists in a BGP UPDATE without a Tunnel Encapsulation Attribute
> containing an SD-WAN Hybrid Tunnel TLV, the NLRI is considered malformed
> and the Treat-as-withdraw approach specified in [RFC7606] MUST be used.
>
>
>
> New:
>
> A BGP UPDATE carrying SD-WAN SAFI NLRIs (AFI/SAFI 1/74 or 2/74) MUST
> include a Tunnel Encapsulation Attribute containing an SD-WAN Hybrid Tunnel
> TLV. Because the Tunnel Encapsulation Attribute applies to all NLRIs
> carried in the UPDATE, if the required attribute or SD-WAN Hybrid Tunnel
> TLV is absent, all SD-WAN SAFI NLRIs in that UPDATE MUST be
> treated-as-withdraw according to [RFC7606].
>
>
>
> 546   - Route Type Validation:  The Route Type field MUST be equal to 1.
> 547      Any other value is not supported and the NLRI MUST be ignored, but
> 548      distributed to other BGP peers.
>
> (b) Ignore-but-propagate for unrecognised route types, and the wrong
> term for it. Section 5.4 of RFC 7606 sets the default for typed NLRI: "A
> BGP
> speaker advertising support for such a typed address family MUST handle
> routes with unrecognized NLRI types within that address family by
> discarding
> them, unless the relevant specification for that address family specifies
> otherwise." Overriding it is permitted, but the document must say it is
> doing so, and must then answer what the override raises: on what basis a
> speaker re-advertises an NLRI it cannot parse; how the Route Reflector
> applies the Section 3 authorisation check to it; whether it occupies an
> Adj-RIB-In entry; and what withdraws it. Note also that this rule and
> Section 2.5.2 step 4's "A MALFORMED NLRI MUST be discarded" sit in
> different
> sections with no cross-reference and no stated ordering against the Length
> check at lines 550-552, which cannot apply to an unknown route type.
>
> On terminology, the correct BGP term is propagation, not distribution:
> s/distributed to other BGP peers/propagated to other BGP peers here, and
> the
> same at line 1585. This is not merely stylistic - "distribution" invites
> the
> reading that some component other than ordinary BGP propagation is
> involved,
> which is precisely the ambiguity the SD-WAN SAFI does not need more of.
>
>
>
> [Linda] This issue has already been addressed in the changes addressing
> your earlier comments. An SD-WAN NLRI with an unrecognized Route Type is
> now discarded and MUST NOT be propagated, following Section 5.4 of RFC
> 7606. The earlier “ignore but distribute” behavior has been removed, and
> “propagated” is now used consistently.
>
> We can add the following clarification that the Length and subsequent
> field validation procedures apply only after the Route Type has been
> recognized.
>
> Section 2.2.2
>
> Old:
>
> Route Type Validation: The Route Type field MUST be equal to 1. An SD-WAN
> NLRI with an unrecognized Route Type MUST be discarded and MUST NOT be
> propagated, as specified for typed NLRIs in Section 5.4 of [RFC7606].
>
> Length Field Validation: The Length field MUST contain a value of either
> 12 or 24 octets, as defined in Section 2.2.1.
>
>
>
> New:
>
> Route Type Validation: The Route Type field MUST be equal to 1. An SD-WAN
> NLRI with an unrecognized Route Type MUST be discarded and MUST NOT be
> propagated, as specified for typed NLRIs in Section 5.4 of [RFC7606]. The
> remaining validation steps in this section apply only to recognized Route
> Types.
>
> Length Field Validation: For Route Type 1, the Length field MUST contain a
> value of either 12 or 24 octets, as defined in Section 2.2.1.
>
>
>
> 509   Length:  The value of the Length field for Route-Type 1 MUST be
> 510      either 12 octets (when the SD-WAN Node ID is an IPv4 address) or
> 511      24 octets (when the SD-WAN Node ID is an IPv6 address).  Any other
> 512      value is invalid, and the NLRI MUST be treated as malformed and
> 513      discarded.
>
> (c) Per-NLRI discard is workable here because the Length field lets
> a parser skip the malformed NLRI and continue - but the document should say
> that is why it is workable, and say what happens when the Length field
> itself runs past the end of the MP_REACH_NLRI attribute, at which point
> parsing genuinely fails and RFC 7606's AFI/SAFI-disable or session-reset
> applies.
>
> Please consolidate all three of the above into one error-handling section,
> state explicitly which RFC 7606 disposition applies to each failure mode
> and
> at what scope, and state that (b)'s ignore-but-propagate is a deliberate
> override of Section 5.4 of RFC 7606, with the reasoning. Section 5 of RFC
> 9830 is the model and is short enough to adapt almost directly: it defines
> the syntactic validation required, then an escalation ladder from per-NLRI
> treat-as-withdraw (where the malformed NLRI can be skipped) to AFI/SAFI
> disable to session reset (where the UPDATE cannot be parsed at all).
> Section
> 5 of RFC 9830 also separates BGP-level validation from validation that is
> not BGP's business, which is the distinction this document most needs for
> the IPsec parameters - see issue (2).
>
>
>
> [Linda] Agree. The changes in resolving your earlier comments already
> removed the earlier ignore-but-propagate behavior for unrecognized Route
> Types and now follows Section 5.4 of RFC 7606: an NLRI with an unrecognized
> Route Type is discarded and MUST NOT be propagated.
>
> The current revision also consolidates the SD-WAN NLRI error handling in
> Section 2.6.2. We will further clarify the error-handling scope for Length
> failures. If the NLRI Length value is invalid but still allows the
> malformed NLRI boundary to be determined, the affected NLRI is handled as
> treat-as-withdraw. If the Length encoding extends beyond the remaining
> MP_REACH_NLRI data so that the NLRI boundary cannot be reliably determined,
> AFI/SAFI disable or session reset is applied according to RFC 7606. This
> follows the error-handling model used in RFC 9830.
>
>
> Section 2.2.1 — Length definition
>
> Old:
>
> Length: The value of the Length field for Route-Type 1 MUST be either 12
> octets (when the SD-WAN Node ID is an IPv4 address) or 24 octets (when the
> SD-WAN Node ID is an IPv6 address). Any other value is invalid, and the
> NLRI MUST be treated as malformed and discarded.
>
>
>
> New:
>
> Length: The value of the Length field for Route-Type 1 MUST be either 12
> octets (when the SD-WAN Node ID is an IPv4 address) or 24 octets (when the
> SD-WAN Node ID is an IPv6 address). Any other value is invalid. Error
> handling for an invalid Length field is specified in Section 2.6.2.
>
>
>
>
> 456   Section 2.3.6) or IPsec information (see Section 2.3).  The IPsec
> 457   information sub-TLVs include: IPsec SA ID, IPsec SA Nonce, IPsec
> 458   Public Key, IPsec SA Proposal, and Simplified IPsec SA.
>
> 1119      identifies the encapsulation protocol used within the IPsec
> 1120      payload when IPsec SA Sub-TLVs (IPsec SA ID, IPsec SA Nonce,
> IPsec
> 1121      Public Key, IPsec SA Proposal, or Simplified IPsec SA) are
> present
>
> 1563   For SD-WAN client routes with a Tunnel Encapsulation Attribute with
> a
> 1564   SD-WAN Hybrid Tunnel type TLV, the IPsec Sub-TLVs (IPsec SA ID,
> IPsec
> 1565   nonce, IPsec Public Key, IPsec Proposal, and Simplified IPsec SA)
> are
> 1566   meaningful, but MAY be rarely sent.
>
> <major> Also at lines 1134-1135 and 1570. No sub-TLV named "IPsec SA Nonce"
> is defined; sub-TLV 67 is the IPsec SA Rekey Counter. This seems a leftover
> from an earlier revision, and it is the name still sitting in the IANA
> registry for value 67 - so the two need fixing together. Section 2.6.1
> also
> drops the "SA" from "IPsec SA Proposal" and Section 6.3 drops it from
> "Simplified
> IPsec SA". Please use the defined names - IPsec SA ID, IPsec SA Rekey
> Counter,
> IPsec Public Key, IPsec SA Proposal, Simplified IPsec SA - in all five
> places
> and in Section 6.3, and ask IANA to rename value 67 accordingly (see the
> comment
> at line 1842).
>
> [Linda] The changes made in addressing your earlier comments already
> identify Sub-TLV 67 as “IPsec SA Rekey Counter” in Section 6.3, but several
> occurrences of the old “IPsec SA Nonce” name and shortened Sub-TLV names
> remain.
>
>
>
> 584   To support key rotation (e.g., updating IPsec keys or parameters),
> 585   the SD-WAN NLRI (identified by Port-Local-ID, SD-WAN-Color, and SD-
> 586   WAN Node ID) can be re-advertised via a BGP UPDATE message containing
> 587   updated IPsec SA information.  This mechanism enables rapid
> 588   distribution of new keys without requiring separate key negotiation
> 589   protocols.
>
> <major> The claim of "rapid distribution" is not supported, and the
> mechanism has challenges that need stating.
>
> Implicit withdraw destroys the old SA before the new one is usable.
> Re-advertising the same NLRI replaces the previous path attributes. IPsec
> rekeying requires an overlap window where both the old and the new SA are
> installed, or traffic is dropped for the duration. Nothing in the document
> creates that overlap - the IPsec SA ID Sub-TLV can carry multiple SPIs
> (lines 601-607), which is the natural place to build make-before-break, but
> no procedure uses it that way.
>
> BGP gives no delivery or ordering guarantee for successive UPDATEs. An RR
> applying MRAI, or an implementation coalescing updates, may collapse a
> rapid
> sequence of rekeys and deliver only the last - or deliver them out of order
> to different peers, so different edges hold different generations of the
> same SA simultaneously. The Rekey Counter exists to detect this and no
> procedure consumes it - see issue (7)(d).
>
> The churn is proportional to (edges x ports x peers x rekey rate) and lands
> on the control plane. A 5000-site SD-WAN with 3 WAN ports rekeying hourly
> generates 15000 UPDATEs per hour through the RR at steady state, each
> carrying key material, each reflected to every authorised peer. There is no
> guidance on a minimum rekey interval, no MRAI or damping recommendation,
> and
> no statement of the relationship between rekey rate and BGP convergence. I
> am not asserting the design is unworkable, but I would want this addressed
> before IETF Last Call.
>
> Framed by the boundary in issue (2), the fix splits at the BGP/IPsec line
> rather than being specified end to end. On BGP's side, which this document
> must specify: what the Rekey Counter means on the wire and who consumes it.
> It is a field in a BGP sub-TLV, so the document must either state a
> BGP-visible comparison rule - an advertisement whose Rekey Counter is not
> greater than the stored value for that SA is stale and is not passed on to
> the SD-WAN tunnel manager - or state that the field is opaque to BGP and
> handed over intact, in which case say so and delete the anti-replay claim
> at
> line 717. It cannot be silent, which is the current state. Also on BGP's
> side: the churn. An Operational Considerations subsection should give a
> recommended minimum rekey interval as a function of deployment scale and
> state the relationship to MRAI and RR update generation. On the SD-WAN
> tunnel manager's side, which this document should hand over and not
> specify:
> the SA lifecycle - the make-before-break overlap, when the old SA is torn
> down, what happens if the new parameters do not work. The encoding already
> supports overlap, since the IPsec SA ID Sub-TLV can carry multiple SPIs
> (lines 601-607); the document should say that carrying both the old and the
> new SPI is how an overlap is signalled, and leave the management of the
> window to the SD-WAN tunnel manager.
>
> <major> Separately, and per issue (6): "rapid" is unquantified and is
> contradicted by the document's own delivery characteristics, as above.
> "Without requiring separate key negotiation protocols" asserts an advantage
> over IKEv2 as though it were a bare fact. Are you sure to project BGP as
> an alternative to IKE to the SEC folks?
>
>
>
> [Linda] The BGP/IPsec responsibility boundary has already been clarified
> in the changes addressing your earlier comments: BGP carries the
> IPsec-related parameters and validates their encoding and syntax, while
> IPsec SA state management, rekeying, and packet protection are performed by
> the IPsec implementation.
>
>
>
> 603   This enables advertisement of SA updates, key rotations, or
> 604   operational state changes without resending full SA parameter sets,
> 605   thereby significantly reducing the size of BGP UPDATE messages and
> 606   allowing pairwise IPsec rekeying to proceed independently for each
> 607   SA.
>
> <major> Per issue (6): "significantly" is unquantified and measured against
> nothing. The mechanical part of the sentence is accurate and should stay.
> Perhaps:
>
>       This enables advertisement of SA updates, key rotations, or
>       operational state changes without resending full SA parameter
>       sets, and allows pairwise IPsec rekeying to proceed independently
>       for each SA.
> [Linda] We will remove the unquantified “significantly reducing” claim and
> keep the mechanical description of the behavior.
>
>
>
>
> 617      |IPsec SA ID Sub|   Length      |  Reserved                     |
> 667       |SA-RekeyCounter|   Length      |  Reserved                     |
> 746       |IPsec-PublicKey|   Length      |  Reserved                     |
> 815   |  SA Proposal  |   Length      |       Reserved (16 bits)      |
> 907       |Sub-TLV type   |    Length     |             Reserved          |
> 1044      |Type (65)      | Length        |Flags          |I|O|R|R|R|R|R|R|
> 1216       | UnderlayType  |      Length   |      Reserved
> |
>
> 1208   Underlay Network Type.
> 1210   66 (IANA Assigned).
>
> <minor> The way this document presents its TLV and sub-TLV encodings - in
> both the figures and the accompanying field descriptions - departs from the
> convention used across IDR documents. RFC 9830 is a good reference for the
> house style, and following it would make the encodings easier to review and
> easier to implement. Please take care of the following to get some
> consistency
> and to improve clarity and readability.
>
> (a) Bit rulers are inconsistent and often incomplete. Please use the full
> two-line ruler in every figure.
>
> (b) The Type field is labelled with the sub-TLV's name instead of "Type" -
> seven different labels appear in the type-field position across sub-TLVs.
>
> (c) The field descriptions have no common template. Each sub-TLV describes
> its own type field differently
>
> (d) Field widths are given in bits where octets are conventional, and the
> two units are mixed within single descriptions - for instance the Rekey
> Counter at line 709 is "64 bits" while the field beside it at line 693 is
> described in octets. TLV and sub-TLV field widths are normally given in
> octets, with bits reserved for flags and sub-octet fields.
>
> Please adopt one template for all seven sub-TLVs and the sub-sub-TLV,
> suggest
> following RFC 9830's presentation: and a "where:" list describing each
> field in the order it appears, with widths in octets.
>
>
>
> [Linda] We will make the encoding figures consistent by using the same
> two-line bit ruler, label the first octet uniformly as “Type”, and
> normalize the field descriptions to follow the same “where:” template.
> Field widths will be expressed in octets for whole-octet fields and in bits
> only for flags or sub-octet fields.
>
>
>
> 635      *  Length (8 bits): Specifies the total length in octets of the
> 636         value field (not including the Type and Length fields).  For
> 637         the IPsec SA ID Sub-Type, the Length field SHOULD be equal to 2
> 638         + 4 *(number of IPsec SA Identifier fields).
>
> 648   Sub-TLV Error Handling:  If the length value is invalid, (that is 2+
> 649      4*number of IPsec SA IDs), then the IPsec SA ID SubTLV is
> 650      Malformed.
>
> <major> A length field cannot be a SHOULD - a receiver has no way to parse
> a
> sub-TLV whose length is optionally correct, and there is no defined
> behaviour for the case where it is not. The same applies to ID Length at
> line 694. The error-handling sentence then says the opposite of what it
> means: as written, "if the length value is invalid, (that is 2 + 4*number
> of
> IPsec SA IDs)" defines the correct value as the invalid one. The
> parenthetical needs to be the valid value with the test inverted. How
> about:
>
>       The Length field MUST be equal to 2 + 4 x (number of IPsec SA
>       Identifier fields). A sub-TLV whose Length field has any other
>       value is malformed and is handled per [RFC9012].
>
> Please apply the same treatment to ID Length in Section 2.3.2.
>
> [Linda] Agree. The Length field and the ID Length field are part of the
> encoding and therefore need exact values, not SHOULD-level guidance. We
> will make both normative and simplify the malformed-length text so that the
> valid value is stated directly and any other value is treated as malformed
> according to RFC 9012.
>
>
>
>
> 640      *  Reserved: The Reserved field SHOULD be set to zero and ignored
> 641         upon receipt.  Received values MUST be propagated without
> 642         change.
>
> <major> Two problems, raised here on the first Reserved field the document
> defines and applying to every Reserved field and Reserved bit in the
> document. There are several different formulations across them, varying
> between SHOULD and MUST on setting to zero, between SHOULD, MUST and
> unmarked on ignoring, and between carrying a propagation sentence and not.
>
> (a) The normative keyword strength varies for no reason.
>
> (b) "Received values MUST be propagated without change" contradicts "set to
> zero on transmission". A speaker re-originating the sub-TLV cannot do both.
> The sentence is also redundant: Section 13 of RFC 9012 already requires
> unrecognised and disregarded sub-TLVs to be propagated intact, and the
> TEA is transitive. Note also that the Reserved field in Section 2.3.1 is
> given no explicit width, though Figure 4 and the length formula imply two
> octets.
>
> Please adopt one formulation and use it verbatim everywhere - "Reserved for
> future use. MUST be set to zero on transmission and MUST be ignored on
> receipt." - drop the propagation sentences, and state the width of every
> Reserved field. This applies throughout the document: please check all
> Reserved fields and Reserved bits for consistency rather than only the
> instance quoted above.
>
>
>
> [Linda] Agree. The Reserved fields and bits should use one consistent
> formulation throughout the document. We will use: “Reserved for future use.
> MUST be set to zero on transmission and MUST be ignored on receipt.”
>
> We will remove the separate “propagated without change” sentences and
> explicitly state the width of each Reserved field or bit field. This is an
> editorial consistency change and does not alter the encoding semantics.
>
>
>
> 685      *  length (8 bits): Specifies the total length in octets of the
> 686         value field of the SubTLV.  The total length is variable with
> 687         the value equal to 18 plus Nonce Length.
>
> 693      *  ID Length (8 bits): indicates the length in octets of IPsec SA
> 694         Identifer.  This length SHOULD be 4 octets.
> 696      *  Nonce Length (16 bits): indicates the length, in octets, of the
> 697         Nonce Data.  The value MUST be a non-zero multiple of 4 (i.e.,
> 698         the Nonce Data length MUST be a multiple of 32 bits)[RFC7296].
>
> 709      *  Rekey Counter (64 bits): the number of key updates or rekeys
> 710         that have occurred.
>
> 720   Sub-TLV Error Handling:  The IPsec SA Rekey Counter Sub-TLV is
> 721      considered malformed under any of the following conditions:
> 723         The total Sub-TLV Length is less than the sum of ID Length,
> 724         Nonce Length, and 4 octets for the Rekey Counter.
> 726         The Nonce Length field is zero or not a multiple of 4.
>
> <major> Four defects in this sub-TLV.
>
> The length formula holds only for ID Length = 4. Counting from Figure 5:
> Reserved 2 + ID Length 1 + Nonce Length 2 + I/Flags 1 + Rekey Counter 8 +
> SA
> Identifier (ID Length) + Nonce Data (Nonce Length) = 14 + ID Length + Nonce
> Length. That equals "18 plus Nonce Length" only when ID Length is 4 - which
> the document explicitly declines to require (line 694, "SHOULD be 4
> octets",
> with Contextual Note 1 at lines 730-735 explaining that the SHOULD exists
> precisely so Secure EVPN can use a longer identifier).
>
> The malformed test uses 4 octets for a 64-bit field. The Rekey Counter is 8
> octets, not 4. The test also omits Reserved, ID Length, Nonce Length and
> the
> flags octet - 6 further octets - so a sub-TLV 10 octets shorter than it
> should be passes the test.
>
> The multiple-of-4 nonce rule is attributed to RFC 7296, which does not
> contain it. Section 3.9 of RFC 7296 says: "The size of the Nonce Data MUST
> be between 16 and 256 octets, inclusive." There is no multiple-of-4 or
> multiple-of-32-bits requirement in Section 3.9 or in Section 2.10 ("Nonces
> used in IKEv2 MUST be randomly chosen, MUST be at least 128 bits in size,
> and MUST be at least half the key size of the negotiated pseudorandom
> function (PRF)"). The document's own rule also fails to enforce the
> 16-octet
> minimum that RFC 7296 does impose: a 4-octet nonce passes both stated
> tests.
>
> Nonce Length is 16 bits inside a sub-TLV whose Length field is 8 bits, so a
> conforming Nonce Length value can exceed what the sub-TLV can hold - see
> the
> comment at line 929.
>
> Please state the length as 14 + ID Length + Nonce Length; make ID Length a
> MUST-be-4 unless the Secure EVPN case is actually in scope, in which case
> allow a range and validate it (a SHOULD on a length field is not a workable
> extensibility mechanism - the ID Length field itself already provides the
> extensibility); correct the malformed test to Length != 14 + ID Length +
> Nonce Length; replace the nonce rule with Section 3.9 of RFC 7296's actual
> constraint (16-256 octets inclusive) further capped by the sub-TLV length;
> and reduce Nonce Length to 8 bits, or move the sub-TLV to a
> two-octet-length
> type per the comment at line 929.
>
> [Linda]  Agree. We will correct the length calculation and nonce
> validation. As addressed in the preceding comment, ID Length will be
> required to be 4 octets.
>
> The Sub-TLV Length is 14 + ID Length + Nonce Length, which with the
> required 4-octet ID is 18 + Nonce Length. The malformed-length check will
> use this exact relationship, and the Rekey Counter will be correctly
> accounted for as 8 octets.
>
> We will also replace the incorrect multiple-of-4 nonce requirement with
> the RFC 7296 requirement. Because the Sub-TLV itself has an 8-bit Length
> field, this encoding further limits Nonce Length to 237 octets. The
> existing 16-bit Nonce Length field can therefore remain; values outside the
> permitted range are malformed.
>
>
> 913       | key1 length   |         Key 1                                 ~
> 915       | key2 length   |         Key 2                                 ~
>
> 971   key1 length (8 bits):  indicates the IPsec public key 1 length.
> 974   Public Key 1:  IPsec public key 1
> 976   key2 length (8 bits):  indicates the IPsec public key 2 length.
> 979   Public Key 2:  IPsec public key 2
>
> <major> The figure calls them "Key 1"/"Key 2"; the field descriptions call
> them "Public Key 1"/"Public Key 2"; the field titles call them "key1
> length"/"key2 length" and "IPsec public key 1 length". Nothing states what
> algorithm or curve/group the key belongs to, or how a receiver determines
> that - the sub-TLV has no Diffie-Hellman Group field, unlike sub-TLV 68,
> and
> the Transform/AH/ESP fields describe the SA's algorithms, not the key's.
> Nothing states what the encoding is (raw point, DER SubjectPublicKeyInfo,
> the RFC 7296 Key Exchange Data format). Nothing states why there are two,
> or
> which is which - inbound/outbound, encryption/integrity,
> initiator/responder
> are all plausible readings and produce different SAs.
>
> Nor is it clear that they are public keys at all. Section 2.3.5 introduces
> this sub-TLV for "deployments where full transform negotiation (e.g., via
> IKEv2) is not supported or not necessary" with nodes "provisioned ... with
> a
> common set of agreed security profiles". A pre-provisioned profile plus two
> keys and a nonce, with no Diffie-Hellman group and no key-agreement step,
> reads much more like symmetric keying material than like public keys - and
> Section 3.2 confirms there is no negotiation: "The C-PE devices do not
> attempt to negotiate IPsec SA parameters or transform sets with remote
> peers. Instead, the configurations must match as advertised."
>
> If they are symmetric keys, this sub-TLV distributes SA keys in a
> transitive
> BGP path attribute, through an RR, on a session that Section 5 says may run
> over plain TCP in a walled garden. That is a serious exposure and it is not
> analysed anywhere - see issue (7). If they are public keys, the document
> still never states how a receiver binds a key to an identity: anyone who
> can
> inject a route with the right Node ID substitutes their own key.
>
> Please state plainly what Key 1 and Key 2 are, with an algorithm identifier
> field, an encoding reference, and an explicit statement of each key's role.
> If they are symmetric keys, say so and make transport confidentiality a
> MUST
> wherever this sub-TLV is used, with the reasoning in Section 5. If they are
> public keys, add the group/algorithm identifier and specify the identity
> binding.
>
> [Linda] The intent is for these fields to carry the multiple pieces of
> keying material needed by the IPsec SA, not to define a separate public-key
> exchange mechanism.
>
> We will clarify the role of each key, align the field names in the figure
> and text, and specify that their interpretation is determined by the IPsec
> transform(s) selected for the SA. We will also remove the current “Public
> Key” terminology where it is misleading.
>
> The Simplified IPsec SA Sub-TLV is intended for centrally controlled
> deployments in which the participating nodes are provisioned with the same
> security profile, including the algorithms and keying method. It therefore
> carries the keying material needed for the configured SA rather than
> negotiating a key-exchange algorithm in BGP. The exact encoding and role of
> each key will be stated explicitly so that the receiver can unambiguously
> associate the advertised keying material with the configured IPsec
> transforms.
>
>
>
>
> 929   Length (8 bits):  the length of Sub-TLV in octets (based on key
> 930      length).  Length is variable, and calculated by 17 + key1 length +
> 931      key2 length + nonce length.
>
> 971   key1 length (8 bits):  indicates the IPsec public key 1 length.  The
> 972      length has a valid range of 32 octets to 512 octets [RFC8247]
> 974   Public Key 1:  IPsec public key 1
> 976   key2 length (8 bits):  indicates the IPsec public key 2 length.  The
> 977      length has a valid range of 32 octets to 512 octets [RFC8247]
>
> <major> All seven sub-TLVs defined in this document use type codes 64-70,
> so
> per RFC 9012 each has a one-octet Length field and a maximum value field of
> 255 octets. Three of them are specified with value ranges that do not fit,
> and this one is the clearest case: the key length fields are 8 bits but are
> given a range up to 512 octets, which an 8-bit field cannot express; and
> the
> sub-TLV Length is 8 bits, so 17 + key1 + key2 + nonce <= 255 caps the two
> keys plus nonce at 238 octets combined.
>
> [Linda] The preceding comment is being addressed by clarifying the
> semantics and roles of Key 1 and Key 2, but the current key-length ranges
> also exceed what the encoding can represent.
>
> Because the Simplified IPsec SA Sub-TLV uses the one-octet Length field
> defined by RFC 9012, its value cannot exceed 255 octets. We will remove the
> current 32–512-octet key-length range and explicitly require that the
> encoded Key 1, Key 2, and Nonce lengths fit within the Sub-TLV Length. The
> actual key lengths are determined by the configured IPsec
> algorithms/security profile.
>
>
>
>
>
>
> 764      length (8 bits): Specifies the total length in octets of SubTLV
> 765      value field.  The total length is variable with the length being
> 766      10 + the Key Exchange Data length.
>
> <major> Same class of problem in the IPsec Public Key Sub-TLV, where Length
> = 10 + Key Exchange Data. Section 3.4 of RFC 7296: "The length of the
> Diffie-Hellman public value for MODP groups MUST be equal to the length of
> the prime modulus over which the exponentiation was performed." Group 14
> (2048-bit MODP) therefore has a 256-octet public value; 256 + 10 = 266 >
> 255. Group 14 is the only DH group RFC 8247 marks MUST for IKEv2 (Section
> 2.4 of RFC 8247). So the sub-TLV cannot carry the mandatory-to-implement
> group. Group 5 (192 octets) fits at 202; ECP groups 19, 20 and 21 (64, 96
> and 132 octets) fit. The document does not restrict the usable groups, so
> an
> implementation cannot know which the peer will send.
>
> The third case is that the Length field is 8 bits while Nonce Length is 16
> bits in the IPsec SA Rekey Counter Sub-TLV (see the comment at line 685),
> so
> the nonce length field can express values the containing sub-TLV cannot
> hold. This 'plain as daylight' inconsistency is what go me going down into
> the IPsec and IKE rabbit hole to find these whole set of issues!
>
> My preferred fix is to move these sub-TLVs into the 128-255 type range,
> which gives a two-octet Length. That requires returning 64-70 and
> requesting
> new codepoints (128-191 is Standards Action, 192-252 is FCFS) - worth doing
> immediately. The alternative, if the codepoints are to be kept:
> restrict the permitted DH groups to those whose public value fits in 245
> octets, state that restriction normatively, correct the key-length fields
> to
> 16 bits, and state the real ceiling on key material - I don't think this
> option will survive SEC reviews. Either way, please state explicitly for
> each sub-TLV what happens when the material does not fit.
>
> [Linda] Agree that the one-octet Sub-TLV Length limits the key-exchange
> data that can be carried. This has been partially addressed in the current
> revision: Section 2.3.3 now states the 255-octet Sub-TLV limit, recommends
> smaller ECP groups, and requires public values that do not fit to be
> distributed by another mechanism. The Rekey Counter nonce-length issue is
> also being addressed separately by bounding the permitted Nonce Length to
> the enclosing Sub-TLV size.
>
> We will make the Public Key Sub-TLV restriction explicit: only
> key-exchange groups whose encoded public value fits within the available
> 245 octets may be carried in this Sub-TLV. A sender MUST NOT encode a
> larger public value in this Sub-TLV; another key-distribution mechanism
> must be used instead.
>
>
>
>
>
>
> 953   AH algorithms (8 bits):  Specifies the AH authentication algorithm to
> 954      be used.  The values are defined in [RFC4835] and its updates
> 955      (e.g., [RFC8221]).
>
> 963   ESP algorithms (8 bits):  Specifies the ESP encryption algorithm, as
> 964      defined in [RFC4835], [RFC8221], and their updates.
>
> <major> RFC 4835, RFC 7321 and RFC 8221 do not define numeric values for
> these algorithms. They are implementation-requirements documents: they list
> algorithms by name with a requirement level (MUST / SHOULD / SHOULD NOT /
> MUST NOT) and nothing else. Section 9 of RFC 8221 states "This document
> does
> not require any IANA actions."; Section 1.3 of RFC 8221 states "The
> algorithms considered here are listed by IANA as part of the IKEv2
> parameters." The numeric code points live in the IANA IKEv2 "Transform Type
> 1 - Encryption Algorithm Transform IDs" and "Transform Type 3 - Integrity
> Algorithm Transform IDs" registries - which are 16-bit spaces, not 8-bit.
> So
> as written there is no way for two implementations to agree on what value
> 12
> in the ESP algorithms field means. Separately, RFC 4835 is obsolete
> (obsoleted by RFC 7321, itself obsoleted by RFC 8221) and should not be
> cited at all.
>
> Please redefine both fields as 16-bit values from the IANA IKEv2 Transform
> Type 3 and Transform Type 1 registries respectively, cite those registries,
> and make RFC 7296 (already normative) the reference. Drop RFC 4835 entirely
> and reduce RFC 8221 to an informative pointer for algorithm selection
> guidance. Widening these fields also forces a re-count of the Simplified
> IPsec SA length formula at line 929 - please do both in one pass. Note that
> this field pair also duplicates what the IPsec SA Proposal Sub-TLV (69)
> already carries properly via Transform Type / Transform ID; consider
> whether
> the Simplified sub-TLV needs them at all (see the comment at line 1117).
> [Linda] We will change the AH and ESP algorithm fields to 16-bit values
> and define them using the IANA IKEv2 Transform Type 3 (Integrity Algorithm)
> and Transform Type 1 (Encryption Algorithm) registries, respectively, with
> RFC 7296 as the protocol reference.
>
> RFC 4835 will be removed. RFC 8221 may remain as an informative reference
> for algorithm-selection guidance.
>
> The widening of these two fields will also be reflected in the Simplified
> IPsec SA Sub-TLV encoding and Length calculation. The fields are retained
> because this compact Sub-TLV explicitly identifies the configured
> algorithms used by the SA, whereas the more general IPsec SA Proposal
> Sub-TLV provides the flexible transform representation.
>
>
>
>
> 981   nonce-length (8 bits):  indicates the Nonce key length.  Per
> 982      [RFC7296], the minimum length for nonce is 16 octets.  [RFC7296]
> 983      doesn't specify the maximum length for nonce.  Therefore, the
> 984      maxmimum length for nonce MUST be small enough so that the entire
> 985      length of the BGP UPDATE is less than the BGP UPDATE maximum.
>
> <major> Section 3.9 of RFC 7296 specifies both bounds: "The size of the
> Nonce Data MUST be between 16 and 256 octets, inclusive." The minimum
> stated
> here is right; the maximum claim is wrong; and the substitute rule that
> follows from it is not implementable as a validation test - a receiver
> parsing one sub-TLV cannot evaluate "the entire length of the BGP UPDATE is
> less than the BGP UPDATE maximum" as a property of this field, and a sender
> that has already built the UPDATE has a much tighter constraint anyway (the
> sub-TLV's own 8-bit length, per the comment at line 929). Note the
> inconsistency with the Rekey Counter sub-TLV at line 685: two sub-TLVs in
> the same document carry a nonce under two different, mutually incompatible
> length rules (multiple of 4, no maximum; versus 16 octets minimum, no
> maximum), neither of which matches RFC 7296. In both places, please state:
> "The Nonce Data length MUST be between 16 and 256 octets inclusive
> [RFC7296], and MUST NOT exceed the space available in the enclosing
> sub-TLV." Delete the BGP-UPDATE-maximum sentence.
>
> [Linda] Agree. The current text incorrectly states that RFC 7296 does not
> define a maximum nonce length. Section 3.9 of RFC 7296 defines the Nonce
> Data length as 16 through 256 octets inclusive.
>
> We will use the same rule consistently in both nonce-carrying Sub-TLVs and
> also require that the nonce fit within the enclosing Sub-TLV. The current
> BGP UPDATE maximum-size sentence will be removed.
>
>
>
> 1028   Unlike pairwise NAT traversal mechanisms such as IKEv2 [RFC7296],
> 1029   which require peers to dynamically discover NAT properties during
> 1030   tunnel setup, the BGP-controlled SD-WAN architecture enables each
> SD-
> 1031   WAN edge node to proactively advertise its NAT properties to all
> 1032   peers through BGP signaling.  This approach simplifies NAT traversal
> 1033   in large-scale SD-WAN deployments where each edge node may need to
> 1034   establish tunnels with many peers.
>
> <major> Per issue (6): this is the document's only direct comparison with
> an
> alternative and it carries no analysis - no scaling figures, no statement
> of
> what "simplifies" means or what it costs, and no acknowledgement of what
> IKEv2 provides that this mechanism does not. Please delete both sentences
> and substitute:
>
>       Each SD-WAN edge node advertises the NAT properties of its WAN
>       ports to its peers through BGP signalling, independently of and in
>       advance of the establishment of any tunnel. Section
>       [APPLICABILITY].2 describes the relationship between the
>       mechanisms in this document and alternative mechanisms for
>       establishing IPsec security associations.
>
> This passage seems to be the document's stated motivation for the design,
> and is
> the text issue (10)'s question is about.
>
> [Linda] The change made in addressing your earlier comments now state that
> an SD-WAN edge can advertise its NAT properties through the RR to
> authorized peers before individual tunnels are established, avoiding
> repeated discovery of the same NAT-related information during establishment
> of each SD-WAN tunnel. We will keep the comparison limited to that concrete
> behavior and avoid broader claims that the mechanism “simplifies” NAT
> traversal.
>
>
>
> 1065   *  Length: the length of the value field in octets excluding the
> Type
> 1066      and the Length fields.  If IPv4, the length is 32 (8 header, 32
> 1067      address, 8 for 1 Sub-Sub-TLV).  If IPv6, the length is 64 (8
> 1068      header, 48 addresses, 8 for 1 subSubTLV).
>
> <major> The IPv4 sum does not add up on its own terms: 8 + 32 + 8 = 48, not
> 32. The units are also inconsistent - "32 address" is bits where the
> surrounding numbers are octets. Counting the fields from Figure 8 (lines
> 1042-1059), the value field is Flags 2 + NAT Type 1 + Encap-Type 1 + Trans
> networkID 1 + RD ID 1 + Local IP 4 + Local Port 4 + Public IP 4 + Public
> Port 4 + Sub-Sub-TLV 8 = 30 octets for IPv4, and 2+4+16+4+16+4+8 = 54
> octets
> for IPv6. Neither 32 nor 64 is reachable from the figure.
>
> [Linda] We will recalculate the value-field length directly from the
> fields in Figure 8 and express all whole-field sizes consistently in
> octets. For the current encoding, the fixed value length is 22 octets for
> IPv4 and 46 octets for IPv6, plus the length of any included Sub-Sub-TLVs.
>
>
>
> 1159   *  Local IP: The local (or private) IP address of the WAN port.  If
> 1160      the Sub-TLV Length field is 32, the Local IP is a 32 bit IPv4
> 1161      address.  If the Sub-TLV Length field is 64, the Local IP is a 64
> 1162      bit IPv6 address.  This MUST be a valid IP address.
>
> 1167   *  Public IP: The IP address after the NAT.  If NAT is not used,
> this
> 1168      field is set to all-zeros.  If not, this is a valid IP addres.
> If
> 1169      the Sub-TLV Length field is 32, the Local IP is a 32 bit IPv4
> 1170      address.  If the Sub-TLV Length field is 64, the Local IP is a 64
> 1171      bit IPv6 address.
>
> <major> An IPv6 address is 128 bits, not 64, in both places; and the Public
> IP bullet describes the Local IP.
>
> Two further structural problems in the same field. First, the length is
> derived from a fixed assumption of exactly one 8-octet sub-sub-TLV, while
> Section 2.3.6.1 says the sub-sub-TLV is optional and the field is named
> "Extended Sub-Sub-TLV" (singular in the figure, plural in the prose at line
> 1196) - so a sub-TLV with no sub-sub-TLV, or two, has no defined length.
> Second, the address family is signalled twice and inconsistently: by the I
> and O flag bits (lines 1076-1085) and by the Length field, with no
> statement
> of what happens when they disagree, and with the flags describing an
> inner/outer split while the Length rule assumes both addresses are the same
> family.
>
> [Linda] We will correct the IPv6 address width to 128 bits and fix the
> Public IP description. We will also make the Extended Port Attribute length
> depend on the actual address families and the actual number/length of
> included Sub-Sub-TLVs, rather than fixed 32/64 values. The I and O bits
> will be the authoritative indicators of the Local and Public IP address
> families; the Length field will only describe the encoded size, and
> inconsistent encodings will be treated as malformed.
>
>
>
> 1082      -  O bit (Outer address scheme):
> 1084         o  If set to 0, indicate the inner (private) address is IPv4.
> 1086         o  If set to 1, indicates the inner address is IPv6.
>
> <major> The O bit description is a copy of the I bit description - see the
> next comment, where this and the undefined "inner"/"outer" terminology are
> treated in full.
>
> Please recompute both lengths from the figure and state them as a formula
> over the present fields rather than two constants: Length = 14 + (4 or 16
> for Local IP) + (4 or 16 for Public IP) + sum of sub-sub-TLV lengths,
> adjusting for the port field width per the comment at line 1164. Make the I
> and O flags the sole address-family indicators, delete the
> length-implies-family rules, and specify the malformed condition when the
> length is inconsistent with the flags. And fix the O bit text to say "outer
> (public) address".
>
> [Linda] We will correct the O-bit text to refer to the outer (public)
> address, make the I and O bits the sole indicators of the Local and Public
> IP address families, and remove the current Length-based address-family
> inference. The Length will be defined from the actual encoded fields and
> included Sub-Sub-TLVs, and any inconsistency between the flags and encoded
> length will make the Sub-TLV malformed.
>
>
>
> 1070   *  Flags (16 bits):
> 1072      -  Flags field starts with 8 bits which are reserved for future
> 1073         use.  MUST be set to 0, and ignored upon reception.
> 1075      -  I bit (C-PE port address or Inner address scheme):
> 1077         o  If set to 0, indicate the inner (private) address is IPv4.
> 1079         o  If set to 1, indicates the inner address is IPv6.
> 1081      -  O bit (Outer address scheme):
> 1083         o  If set to 0, indicate the inner (private) address is IPv4.
> 1085         o  If set to 1, indicates the inner address is IPv6.
>
> <major> A few related defects here, and together they make the flags field
> unimplementable from its own text.
>
> (a) Neither term is defined, and there is no reference. "Inner" and "outer"
> appear in the document only in these lines. Section 1.3 Conventions
> does not define them, no section explains what they denote, and no
> reference
> is cited that would.
>
> (b) The intended meaning appears to be pre-NAT versus post-NAT, but that is
> inference. The parenthetical "(private)" at lines 1077 and 1083, read
> against the Extended Port Attribute's actual fields, suggests inner = the
> Local IP (the WAN port's own address) and outer = the Public IP (the
> address
> after NAT). But the fields are named Local IP and Public IP, never "inner"
> and "outer", and the mapping between the two flags and the two address
> fields is stated nowhere. A reader has to guess which flag governs which
> field.
>
> (c) "Outer" already means something else in this document. The word is used
> in other places, in an unrelated and conventional sense - the outer
> tunnel of the SD-WAN Hybrid Tunnel:
>
>
>
> [Linda] We will remove the undefined “inner/outer” terminology and use the
> field names consistently: the I bit indicates the address family of the
> Local IP field, and the O bit indicates the address family of the Public IP
> field. This also avoids confusion with the unrelated use of “outer tunnel”
> elsewhere in the document.
>
>
>
> 1294      Hybrid Tunnel TLV specifies the outer tunnel through which the
> 1295      underlay tunnel identified by SD-WAN Node ID passes.  This outer
>
> 1357      Endpoint represents the outer tunnel through which the underlay
>
> <major> So one word carries two unrelated meanings in one specification,
> neither of them defined.
>
> (d) This is a tunnel encapsulation document. It defines a tunnel type, it
> carries IPsec parameters, and it explicitly supports IPsec tunnel mode
> (Section 2.3.5, line 947: "Mode = 1 indicates that the Tunnel mode is
> used").
> In that context "inner address" and "outer address" have a
> well-established
> meaning - the addresses of the encapsulated header and of the
> encapsulating
> delivery header respectively - and an implementer is more likely arrive at
> that
> reading than for pre-NAT/post-NAT. Under the wrong reading, "the inner
> address is IPv6" says something about the traffic carried over the tunnel
> rather than about the WAN port's own address, which is a different field, a
> different question, and a different bit setting. The two readings conflict.
>
> (e) If the terms are defined and the mapping is stated, a reader gets the
> intent from context. Here there is no context to figure it from - an
> undefined term, no mapping to the fields, a competing meaning elsewhere
> in the document, and the one place the outer flag is described says
> "inner" twice.
>
> Simplest and best (assuming that I've understand all of this correctly?):
> drop "inner" and "outer" entirely and name the flags after the fields they
> govern. Rename the I bit to the L bit (Local IP address family) and the
> O bit to the P bit (Public IP address family), and describe each as
> governing the address family of the correspondingly-named field:
>
>       - L bit (Local IP address family): if 0, the Local IP field
>       contains a 32-bit IPv4 address; if 1, it contains a 128-bit IPv6
>       address.
>
>       - P bit (Public IP address family): if 0, the Public IP field
>       contains a 32-bit IPv4 address; if 1, it contains a 128-bit IPv6
>       address.
>
> That resolves (a) through (e) at once, removes the collision with the
> tunnel
> sense of "outer", and makes the flags self-describing. It also fixes the
> "128 bits, not 64" error in the previous comment, so both should be done in
> the same edit. If you prefer to keep "inner" and "outer", then both
> terms must be defined in Section 1.3, the definition must state explicitly
> that they denote the pre-NAT and post-NAT addresses and not the
> encapsulated
> and encapsulating headers, the mapping to the Local IP and Public IP fields
> must be stated, and the tunnel-sense uses at lines 1294, 1295, 1357, 1474
> and 1477 need a different word. That is more work. Note also the third
> naming
> in the same line: the I bit heading offers "C-PE port address or Inner
> address scheme", two names for one bit. Whichever fix is taken, one name!
> [Linda]  Agree. Renaming the flags to match the fields they govern is
> clearer and avoids the overloaded “inner/outer” terminology. We will rename
> the I bit to L for the Local IP address family and the O bit to P for the
> Public IP address family, with 0 indicating IPv4 and 1 indicating IPv6.
> This also aligns with the corrected 128-bit IPv6 field size.
>
> 1090   *  NAT Type (8 bits): an unsigned integer indicating the NAT
> behavior
> 1091      observed for this WAN port.  The values are derived from the
> 1092      legacy NAT classification model described in RFC 8489 Section 5.
>
> 1109      The NAT Type value is determined by the sender using NAT
> discovery
> 1110      procedures (e.g., STUN [RFC8489] with legacy tests [RFC8489]) or
> 1111      local administrative configuration.
>
> <major> Section 5 of RFC 8489 is titled "STUN Message Structure" and
> contains no NAT classification. RFC 8489 does not define Full Cone,
> Restricted Cone, Port Restricted Cone or Symmetric NAT anywhere; that
> taxonomy is from RFC 3489, and RFC 5389 removed it deliberately - it was
> found not to predict NAT behaviour reliably, which is why RFC 4787 and RFC
> 5382 replaced it with a mapping-behaviour / filtering-behaviour
> decomposition. "STUN [RFC8489] with legacy tests [RFC8489]" cites the same
> RFC twice, and the "legacy tests" are in RFC 3489.
>
> Beyond the citation, the substantive question is whether a single 8-bit
> legacy NAT class is the right thing to signal at all. The behaviour a peer
> needs to know in order to decide whether direct tunnel establishment can
> work is the mapping behaviour and the filtering behaviour, which RFC 4787
> defines separately and which do not collapse cleanly onto the cone
> taxonomy.
>
> At minimum, correct the citation: reference RFC 3489 for the legacy
> classification and say plainly that it is retained for compatibility with
> existing SD-WAN implementations. Better, and worth putting for your
> thinking:
> replace the single class with the RFC 4787 mapping-behaviour and
> filtering-behaviour pair (two 4-bit or two 8-bit fields), referencing RFC
> 4787 normatively, and keep the legacy classes only if deployed
> implementations require them. Either way, create the IANA registry (see the
> next comment and the comment at line 1885) so the value set can evolve.
>
> [Linda] Agree. The current RFC 8489 citation is incorrect for the legacy
> cone-based NAT taxonomy. We will either reference RFC 3489 explicitly if
> retaining the legacy classification for compatibility, or revise the
> encoding toward the RFC 4787 mapping/filtering behavior model. In either
> case, we will remove the incorrect “legacy tests [RFC8489]” text and define
> the corresponding IANA registry so the value space can evolve.
>
>
>
>
> 1112      to verify NAT behavior but MUST validate that the received NAT
> 1113      Type field is within the range 1-7.  Values outside this range
> are
> 1114      considered invalid and result in the Sub-TLV being treated as
>
> 1132      -  Those are the only two valid values for this field.
>
> 1244      *  Values outside the range 1-4 are invalid and render the
> Sub-TLV
> 1245         malformed.
>
> 1259      *  Values outside the range 1-4 are invalid and render the
> Sub-TLV
> 1260         malformed.
>
> <major> Four fields define a small set of values and then make every other
> value a malformed-sub-TLV condition, while Sections 6.5, 6.6 and 6.7 ask
> IANA to create registries for three of those spaces (Encapsulation Types,
> Connection Types, Physical Port Types). The two are incompatible: the point
> of a registry is that new values get assigned, and the normative text
> guarantees that any newly assigned value will be treated as malformed by
> every already-deployed implementation. Because the failure is at the
> sub-TLV
> level, a single unknown Connection Type - say a future "6 = satellite" or
> "5
> = Wi-Fi 7" - invalidates the Extended Port Attribute sub-TLV and, per
> Section 2.5.2 step 3, the entire tunnel TLV. That is a
> forward-compatibility
> trap that will bite the first extension. The NAT Type space has the
> opposite
> problem: it is closed by normative text and has no registry requested at
> all, so it cannot be extended even by a future Standards Track document
> without an document update.
>
> The real distinction to introduce is malformed versus unusable, not lenient
> versus strict. An unrecognised Connection Type, Port Type, Encap-Type or
> NAT
> Type is not a syntax error - the sub-TLV parses perfectly - so it should
> not
> be a malformed condition. Whether the advertisement is then usable is a
> separate, local question. RFC 9830 keeps these strictly apart and is worth
> following, not least because it shows that the strict answer is legitimate
> once it is put in the right box: malformation in its Section 5,
> unrecognised-but-well-formed content in its Section 4.2.2 as a usability
> question. The consequence of that placement is the point - an unusable
> update is still propagated, whereas a malformed one is withdrawn. This
> document currently routes both to the same destructive outcome.
>
> So: redefine unknown values in these four fields as "the receiver does not
> know this property", state what a receiver does with a port whose
> properties
> it cannot fully interpret (my suggestion: use it, but exclude it from
> property-based tunnel selection), and reserve malformed for genuine
> encoding
> errors. Add a NAT Type registry to Section 6. The pattern is worth applying
> across the document, not only in these four places.
>
>
>
> [Linda]  Agree. The current text incorrectly treats
> unrecognized-but-well-formed values as malformed, which prevents future
> registry extensions from being usable.
>
> We will distinguish syntax errors from unsupported content. Unknown NAT
> Type, Encap-Type, Connection Type, and Port Type values will not by
> themselves make the Sub-TLV malformed. The receiver will treat the
> corresponding property as unsupported and MUST NOT use that property for
> property-based tunnel selection or establishment, while the advertisement
> remains otherwise valid and propagatable.
>
> We will also add an IANA registry for NAT Type and apply the same
> malformed-versus-unsupported distinction consistently across these fields.
>
>
>
> 1117   *  Encap-Type (8 bits): An unsigned integer indicating the
> 1118      encapsulation type supported for this WAN port.  The Encap-Type
> 1119      identifies the encapsulation protocol used within the IPsec
> 1120      payload when IPsec SA Sub-TLVs (IPsec SA ID, IPsec SA Nonce,
> IPsec
> 1121      Public Key, IPsec SA Proposal, or Simplified IPsec SA) are
> present
> 1122      in the SD-WAN Hybrid Tunnel.  This field is distinct from the
> 1123      Tunnel Type field in the BGP Tunnel Encapsulation Attribute
> 1124      [RFC9012].
>
> <question> RFC 9012 already has an Encapsulation sub-TLV (type 1) and a
> Protocol Type sub-TLV (type 2) for exactly this - identifying the
> encapsulation and the protocol carried within it - and Table 1 marks both
> "not valid". The document then defines a new 8-bit field with two values
> and
> requests a new registry for it (Section 6.5), duplicating registry space
> that already exists. The note at line 1122 acknowledges the overlap and
> dismisses it without giving a reason. So: why do the RFC 9012 Encapsulation
> and Protocol Type sub-TLVs not serve here, and what would break if they
> were
> used instead of a new field and a new registry? A sentence of rationale in
> the document would settle it. I am not asserting the reuse is better, only
> that the choice is unexplained.
>
>
>
> [Linda] Good question. The intent is that Encap-Type is a property of an
> individual WAN port carried inside the Extended Port Attribute, whereas the
> RFC 9012 Encapsulation and Protocol Type Sub-TLVs describe the tunnel
> associated with the route/Tunnel TLV as a whole. We will add a sentence
> making this scope distinction explicit and explaining why the RFC 9012
> Sub-TLVs are not reused here.
>
>
>
> 1164   *  Local Port: used by Remote SD-WAN edge node for establishing
> IPsec
> 1165      to this specific port.  Valid values: 0x00 - 0xFFFFFFFF.
>
> 1173   *  Public Port: The Port after the NAT.  If NAT is not used, this
> 1174      field is set to all-zeros.  Otherwise, the value can be 0x01 to
> 1175      0xFFFFFFFF.
>
> <major> Figure 8 gives both fields a full 32-bit row. A UDP or TCP port
> number is 16 bits. Assuming these are transport ports, in which case the
> fields are twice the necessary width and the stated ranges admit values
> that
> cannot be put on the wire - and it is undefined whether a receiver rejects
> a
> value above 65535 or truncates it, which is an interop question - or they
> are not transport ports, in which case the document needs to say what they
> are. The name ("used by Remote SD-WAN edge node for establishing IPsec to
> this specific port") reads as a transport port for IKE or ESP-in-UDP. Two
> consequential points: the two ranges differ from each other for no stated
> reason (0x00 vs 0x01 as the low bound), and the all-zeros convention at
> lines 1177-1179 collides with 0 being a legal Local Port value. Please make
> both fields 16 bits with valid range 0-65535, reclaim the remaining 32 bits
> as Reserved (or drop them and adjust the length formula at line 1065), and
> state that Local Port 0 means unspecified if that is intended. If the
> intent
> was to leave room for a non-port identifier, say what it is.
>
> [Linda] Agree. These fields are intended to carry transport-layer port
> numbers. We will change both Local Port and Public Port to 16 bits, use the
> valid range 0–65535, and define value 0 as “unspecified/not applicable.”
> The Extended Port Attribute length calculation will be updated accordingly.
>
>
> 1184   Sub-TLV Error Handling:  If the Extended Port Attribute Sub-TLV is
> 1185      malformed (e.g., incorrect length, invalid address format, or
> 1186      unrecognized NAT type), it MUST be ignored per the procedures
> 1187      described in [RFC9012].
>
> 1268   Underlay Network Type Sub-Sub-TLV is a MALFORMED Sub-Sub-TLV if the
> 1269   fields do not fit the limits specified above.  If a MALFORMED Sub-
> 1270   Sub-TLV is contained in the Extended Port Attribute Sub-TLV, then
> the
> 1271   Extended Port Attribute Sub-TLV is MALFORMED.  Per [RFC9012], a
> 1272   MALFORMED Sub-TLV is ignored.
>
> 1479   3.  Validate Extended Port Attribute Sub-TLV(s):  As described in
> 1480      Section 2.3.6, each Extended Port Attribute sub-TLV describes the
> 1481      properties of a single WAN port.  Therefore, multiple Extended
> 1482      Port sub-TLVs may be present when the SD-WAN edge node has
> 1483      multiple WAN ports.  Each sub-TLV MUST be validated to ensure
> that
> 1484      the port information it contains is sufficient to support the
> 1485      establishment of a tunnel to the remote peer.  If any Extended
> 1486      Port Attribute Sub-TLV is determined to be invalid, the entire
> SD-
> 1487      WAN Hybrid Tunnel TLV MUST be considered invalid.
>
> <major> Section 2.3.6 says ignore the sub-TLV; Section 2.3.6.1 says ignore
> the sub-TLV; Section 2.5.2 step 3 says invalidate the whole TLV. Three
> outcomes for one condition, inside one document - that is the core of this
> comment. What it does contradict is the claim at line 1560 that the
> document
> "does not alter the validation behavior established in RFC 9012". It does
> alter it, legitimately, for the context where it may - and says it does
> not.
> See issue (1).
>
> There is a second, independent problem in the same step. "validated to
> ensure that the port information it contains is sufficient to support the
> establishment of a tunnel to the remote peer" is not a testable condition -
> "sufficient" is not defined anywhere, so two implementations will draw the
> line differently, and the consequence (whole TLV invalid) is severe.
>
> Please pick one outcome and state it once, in Section 2.6.1, with Sections
> 2.3.6 and 2.3.6.1 cross-referencing rather than restating. My
> recommendation
> is to keep the strict outcome for underlay routes and declare it, following
> Section 5 of RFC 9830: a malformed Extended Port Attribute sub-TLV renders
> the SD-WAN Hybrid Tunnel TLV unusable, with the RFC 7606 disposition named
> explicitly. Then separate the two questions the current text conflates:
> malformed (a syntactic property - length, NAT Type in range, address-family
> consistency, the Public IP/Public Port both-or-neither rule at lines
> 1177-1179) versus unusable (a semantic judgment - the port cannot be used
> to
> build a tunnel). Replace "sufficient to support the establishment of a
> tunnel" with the syntactic list, and handle the semantic case as a local
> matter that does not affect the validity of the advertisement. RFC 9830
> keeps these apart - malformed in its Section 5, unusable in its Section
> 4.2.2 - and that separation is what this document is missing.
>
>
>
> [Linda] Agree. We will make the handling consistent and distinguish
> malformed encoding from a semantically unusable port. Section 2.6.1 will
> define the single normative outcome for a malformed Extended Port
> Attribute, and Sections 2.3.6/2.3.6.1 will simply reference it. The
> “sufficient to support tunnel establishment” test will be removed; semantic
> usability will remain a local decision and will not make the advertisement
> malformed.
>
>
>
> 1262   Port Speed:  An unsigned 16-bit integer representing the port speed
> 1263      in megabits per second (Mbps).  For example, a value of 1000
> 1264      represents a port speed of 1000 Mbps (1 Gbps).  The valid range
> is
> 1265      1-65535.  A Port Speed value of 0 is invalid and renders the Sub-
> 1266      TLV malformed.
>
> <minor> 65535 Mbps is roughly 65 Gbps, which is below current WAN port
> speeds - 100G and 400G interfaces are possible at SD-WAN aggregation sites
> and in the cloud-DC scenario the document itself cites, and cannot be
> represented at all. Please widen the field to 32 bits in Mbps; the
> sub-sub-TLV has two octets of Reserved available, so the cost is nil.
> Separately, redefine a Port Speed of 0 as "unknown or not advertised"
> rather
> than malformed, consistent with the comment at line 1112.
>
>
> [Linda] Agree. We will widen Port Speed to 32 bits in Mbps, using the
> existing reserved space, so higher-speed WAN interfaces can be represented.
> We will also define a value of 0 as “unknown or not advertised” rather than
> malformed, consistent with the treatment of other well-formed but
> unavailable properties.
>
> 1276   Client routes with NLRI of AFI/SAFI IPv4 Unicast (1/1), IPv6 (2/1),
> 1277   L3VPN v4 Unicast (1/128), and IPv6 L3VPN (2/128) that use the SD-WAN
> 1278   Hybrid Tunnel Type can be advertised using one of two mechanisms:
>
> <major> Label handling for the L3VPN AFI/SAFIs is not specified. Section
> 2.4
> (line 1276) applies the mechanism to AFI/SAFI 1/128 and 2/128, which carry
> an MPLS label in the NLRI. When such a route is resolved over an SD-WAN
> Hybrid Tunnel whose underlay segment is IPsec with GRE or VXLAN inside, the
> document never says what happens to that label - whether it is imposed and
> carried inside the tunnel, whether it is discarded, or how the receiver
> knows which. Section 2.4.5 (lines 1424-1432) answers a different question
> with a third mechanism: for a 1/128 route over the IPsec segment of a
> hybrid
> tunnel there are then two VPN demultiplexers - the VPN label from the NLRI,
> and the GRE Key or the VNI - with no statement of which is authoritative,
> whether both are present, or how the GRE Key or VNI value is derived from
> the control plane (from the RD? the RT? which octets of an 8-octet RT map
> into a 24-bit VNI?). RFC 9012 already models this with the Embedded Label
> Handling sub-TLV (type 9) and the MPLS Label Stack sub-TLV (type 10), both
> of which Table 1 marks "not valid". The hybrid case makes it contradict:
> the
> same client route may traverse an MPLS underlay segment on one path and an
> IPsec segment on another within the same hybrid tunnel, so the
> demultiplexer
> changes per underlay link for the same route. This was flagged in my RTGDIR
> early review and is still open.
>
> Please specify label handling explicitly for 1/128 and 2/128: state whether
> the Embedded Label Handling sub-TLV applies (and if so mark it valid in
> Table 1), state the mapping from the control-plane VPN identity to the GRE
> Key and to the VNI with field widths and truncation rules, and state which
> identifier is authoritative when both are present. If interoperable label
> handling over the IPsec segment is not actually specified in this document,
> the honest fix is to restrict Section 2.4 to 1/1 and 2/1 and remove 1/128
> and 2/128 from scope until a document specifies it - that is better than
> shipping an unimplementable case.
>
> [Linda]  Agree. The current text does not define interoperable MPLS label
> handling for L3VPN routes over the IPsec portion of the hybrid tunnel.
> Rather than introduce a new label-to-GRE/VNI mapping mechanism here, we
> will restrict the Client Route procedures in this document to IPv4 and IPv6
> unicast (AFI/SAFI 1/1 and 2/1) and remove AFI/SAFI 1/128 and 2/128 from
> scope.
>
>
> 1347      ignored and a Sub-TLV with an unknown type is ignored.  The
> SD-WAN
> 1348      Hybrid Tunnel TLV only processes the first instance of a Sub-TLV
> 1349      except for the IPsec SA ID Sub-TLV.
>
> <major> This is Section 2.4.2 step 1 - the client-route procedure, where
> RFC
> 9012 binds. Section 13 of RFC 9012: "The following sub-TLVs defined in this
> document may appear zero or more times in a given Tunnel TLV: Protocol Type
> and Color. Each occurrence of such sub-TLVs is meaningful. For example, the
> Color sub-TLV may appear multiple times to assign multiple colors to a
> tunnel." Table 1 marks the Color sub-TLV valid for client routes, so a
> blanket first-instance-only rule silently drops colours an RFC 9012 speaker
> would have honoured. Note that this compounds the comment at line 362: the
> document already gives Color the wrong code point, and now also processes
> only its first instance.
>
>
>
> [Linda] Agree. For Client Routes, RFC 9012 occurrence rules apply, so the
> blanket “first instance only” rule is incorrect. We will remove that rule
> from the Client Route procedure and rely on RFC 9012 for handling repeated
> Sub-TLVs, including multiple Color and Protocol Type Sub-TLVs.
>
>
>
> 1465      If multiple Extended Port Sub-TLVs exist, the TLVs must be
> 1466      validated in step 4.  For all other valid Sub-TLVs (see section
> 1467      2.1), only the first instance of a Sub-TLV is processed;
> 1468      subsequent ones are ignored.
>
> <major> This is Section 2.5.2 step 1, the underlay-route procedure. There
> the rule is OK in itself - no RFC 9012 sub-TLV except Tunnel Egress
> Endpoint
> is valid on SAFI 74, and this document may set its own multiplicity rules.
> The problem there is different: Section 2.5.2 step 1, Section 2.4.2 step
> 1,
> Section 2.6.1 line 1577 ("local policy MUST determine which is to be
> used")
> and Section 2.3.6 line 1189 (first-one-wins-if-identical) give different
> answers for the Extended Port Attribute alone.
>
> Please split the rule by context. For client routes, scope the
> first-instance rule to the sub-TLVs this document defines and leave RFC
> 9012's own sub-TLVs to RFC 9012 - which for Color means honouring all
> occurrences. For underlay routes, state multiplicity per sub-TLV in one
> place; Section 2.4 of RFC 9830 shows the pattern, a general
> first-instance-wins rule that explicitly does not treat the duplicates as
> malformed, plus a one-line multiplicity statement in each individual
> sub-TLV
> definition. Adding that one line to each of Sections 2.3.1 to 2.3.6 would
> remove this whole class of contradiction.
>
> [Linda] Agree. We will separate the multiplicity rules by context. Client
> Routes will follow RFC 9012 for RFC 9012-defined Sub-TLVs. For SAFI-74
> Underlay Routes, this document will define the multiplicity of each
> SD-WAN-specific Sub-TLV consistently in its individual definition, with
> duplicate well-formed instances not treated as malformed.
>
>
>
> 1361   3.  Check for Multiple SD-WAN Hybrid Tunnel TLVs  Multiple unique
> SD-
> 1362      WAN Hybrid Tunnel TLVs MAY be included in the Tunnel
> Encapsulation
> 1363      Attribute, but duplicate SD-WAN Hybrid Tunnel TLVs should be
> 1364      ignored silently.  Optionally, duplicate SD-WAN Hybrid Tunnel
> TLVs
> 1365      MAY be logged.
>
> 1537   An underlay tunnel passes through only one SD-WAN Hybrid Tunnel.
> 1538   Therefore, if there are more than one SD-WAN Hybrid Tunnel TLV
> within
> 1539   a single Tunnel Encapsulation Attribute, the first is processed and
> 1540   the subsequent SD-WAN Hybrid Tunnel TLVs are ignored.
>
> <major> For client routes, multiple TLVs are allowed if "unique" and
> duplicates are ignored; for underlay routes, only the first is ever
> processed. "Unique" and "duplicate" are not defined - identical
> byte-for-byte, same Tunnel Egress Endpoint, same colour set? Two TLVs
> differing only in an IPsec SA ID are presumably not duplicates, but two
> differing only in a Reserved field presumably are.
>
> Two options. If multiple SD-WAN Hybrid Tunnel TLVs are genuinely useful on
> a
> client route, define "duplicate" concretely - I suggest two are duplicates
> if their Tunnel Egress Endpoint sub-TLVs resolve to the same endpoint - and
> state the rule once for both contexts with a normative keyword. If they are
> not, the cleaner answer is Section 2.2 of RFC 9830's: forbid more than one
> tunnel TLV of the type outright and treat a violation as malformed. That
> removes the need to define "duplicate" at all, and Section 2.5.4's own
> reasoning - "An underlay tunnel passes through only one SD-WAN Hybrid
> Tunnel" - points that way for the underlay case at least. Whichever is
> chosen, the two sections must agree.
>
> <major> Separately, on the lowercase "should" at line 1363: this is a
> lowercase keyword in a sentence that is normatively load-bearing. Whether a
> receiver ignores a duplicate tunnel TLV or acts on it determines which
> tunnel gets built, so two implementations reading it differently will not
> interwork. It needs an uppercase BCP14 keyword, alongside the definition of
> "duplicate" above.
>
>
>
> [Linda] Agree. The Client Route and Underlay Route procedures should use
> one consistent rule. We will allow only one SD-WAN Hybrid Tunnel TLV per
> Tunnel Encapsulation Attribute; if more than one is present, the attribute
> is malformed and handled according to the error procedures in Section 2.6.
> This removes the undefined “unique/duplicate” distinction and the
> non-normative lowercase “should”.
>
>
>
> 1407   *  For IPv4/IPv6 Unicast (AFI/SAFI = 1/1 or 2/1), the Route Target
> 1408      Extended Community [RFC4360] SHOULD be included.  The Route
> Target
> 1409      value is interpreted as the VPN ID.  The Route Target is
> 1410      especially necessary when the SD-WAN edge node serves multiple
> 1411      VPNs on its client-facing interfaces.  If all client routes
> belong
> 1412      to a single VPN and the association is unambiguous, the Route
> 1413      Target MAY be omitted.
> 1415   *  For VPN-IPv4/VPN-IPv6 (AFI/SAFI = 1/128 or 2/128), the RD in the
> 1416      NLRI serves as the VPN ID.
>
> <major> The second bullet is right and the first does not work. Under 1/1
> and 2/1 the NLRI is the bare prefix, so two routes for 10.1.1.0/24 from
> two
> different VPNs are the same route in the Adj-RIB-In; the Route Target is a
> path attribute and cannot separate them. The receiving speaker runs
> best-path selection between them and keeps one. This is exactly the problem
> RFC 4364's Route Distinguisher exists to solve, and the reason L3VPN uses
> 1/128 rather than 1/1 with an RT.
>
> The mechanism does work in the restricted case where prefixes are globally
> unique across all VPNs served by the SD-WAN - which is the case the second
> half of the bullet describes ("If all client routes belong to a single
> VPN"). But it is stated as a general mechanism for the multi-VPN case
> ("especially necessary when the SD-WAN edge node serves multiple VPNs"),
> which is precisely where it fails. There is a second, smaller point that
> Alvaro also raised and that is still open: "When is it ok to not include
> the Route Target Extended Community?" - "the association is unambiguous"
> is not a testable condition for a normative SHOULD.
>
> Please state the applicability limit explicitly: RT-as-VPN-ID over 1/1 and
> 2/1 is usable only where the address space is non-overlapping across the
> VPNs carried, and multi-VPN deployments with overlapping address space MUST
> use 1/128 or 2/128. Then either delete "especially necessary when the
> SD-WAN
> edge node serves multiple VPNs" or rewrite it to say that the RT identifies
> the VPN for policy purposes but does not provide address separation. And
> replace the "unambiguous" condition with a concrete one.
>
>
>
> [Linda] Agree. Since we are removing AFI/SAFI 1/128 and 2/128 from the
> Client Route scope, we will also remove this VPN-ID mechanism. For AFI/SAFI
> 1/1 and 2/1, a Route Target may still be used for policy, but it does not
> provide address-space separation; overlapping VPN prefixes are therefore
> outside the scope of this mechanism.
>
>
>
> 1420   In the data plane, SD-WAN traffic can traverse either an MPLS or
> 1421   IPsec segment within a SD-WAN Hybrid Tunnel.  The method for
> 1422   conveying the VPN ID depends on the encapsulation:
> 1424   *  MPLS Segments: When the SD-WAN Hybrid Tunnel uses MPLS transport,
> 1425      the MPLS label stack is used to identify the VPN per [RFC8277].
> 1426      Security is assumed to be provided by the MPLS transport.
> 1428   *  IPsec Segments: When traversing a public network with IPsec
> 1429      encryption: For GRE encapsulation within IPsec, the GRE Key field
> 1430      can carry the SD-WAN VPN ID; For VXLAN network virtualization
> 1431      overlays within IPsec, the VNI (Virtual Network Identifier) field
> 1432      is used to carry the VPN ID.
>
> <major> Independently of the label question raised earlier in this
> review, this text specifies data-plane encapsulation semantics - what the
> GRE Key field and the VXLAN VNI carry - in an IDR control-plane document,
> with no normative language at all in a section whose content determines
> interoperability; no reference to GRE (RFC 2890, which defines the Key
> field) or VXLAN (RFC 7348, which defines the VNI), neither of which is
> referenced anywhere in the document although Encap-Type values 1 and 2 are
> GRE and VXLAN; no derivation rule from the control plane; and "Security is
> assumed to be provided by the MPLS transport" stated as a bare assumption
> in
> a specification section.
>
> My recommendation is to keep the text here and specify it properly. What
> it needs is to be specified rather than described:
>
>    1) Normative language. This text determines whether two implementations
>       interwork; it currently contains no BCP 14 keywords at all.
>
>    2) References. Cite RFC 2890 for the GRE Key field and RFC 7348 for the
>       VXLAN VNI, both as normative references.
>
>    3) Derivation. State how the SD-WAN VPN ID value is obtained from the
>       control plane and how it maps into each field - an 8-octet Route
>       Target does not fit a 32-bit GRE Key or a 24-bit VNI, so the
>       truncation or allocation rule has to be stated.
>
>    4) Precedence. State which identifier is authoritative when a route
>       carries both an MPLS VPN label and a GRE Key or VNI, and whether both
>       are present.
>
>    5) Per-segment behaviour. A hybrid tunnel may carry the same client
> route
>       over an MPLS underlay segment on one path and an IPsec segment on
>       another, so the demultiplexer changes per underlay link for one
> route.
>       Say what an implementation does at that boundary.
>
>    6) Delete "Security is assumed to be provided by the MPLS transport", or
>       replace it with a statement of what is actually assumed and by whom.
> A
>       bare assumption of that kind in a specification section is not
>       something the Security Considerations can build on.
>
> [Linda] This is largely addressed by the earlier change removing AFI/SAFI
> 1/128 and 2/128 and the associated MPLS-label/VPN-ID handling from scope.
> We will also remove this GRE Key/VXLAN VNI VPN-ID mapping text, since the
> document no longer defines a VPN demultiplexing mechanism for Client
> Routes. That avoids introducing additional data-plane mapping and
> precedence rules in this control-plane document.
>
>
>
>
> 1495   5.  Validate Next Hop:  The IP address specified in the Next Hop
> 1496      field MUST be reachable by the tunnels.  Traffic from the
> 1497      validated NLRIs must be forwarded to the next hop router over the
> 1498      tunnel.
>
> <major> This is step 5 of the underlay route procedure (Section 2.5.2), and
> the second sentence is incoherent for that NLRI. An SD-WAN Route Type 1
> NLRI
> is a (Port-Local-ID, SD-WAN-Color, SD-WAN Node ID) tuple identifying a WAN
> port. It is not a prefix and it has no traffic; there is nothing that could
> be "traffic from the validated NLRIs". Nor is there a reason for the
> underlay route's next hop to be "reachable by the tunnels" - the tunnels
> are
> what this NLRI is describing, so the requirement is circular.
>
> The document comes close to saying the right thing in Section 2.2.3 (lines
> 561-570: the attributes "apply to the WAN-facing tunnel endpoints being
> advertised, not to client routes") but never draws the consequence: the
> SD-WAN NLRI is not installed in the forwarding table, and its next hop is a
> BGP-protocol next hop for propagation purposes, not a forwarding next hop.
> This is a direct symptom of issue (1) - the client-route procedure (Section
> 2.4.2) and the underlay-route procedure (Section 2.5.2) were written in
> parallel with the same five step headings, and step 5 was carried across
> from a context where it makes sense to one where it does not.
>
> Please delete the second sentence of Section 2.5.2 step 5, and replace the
> first with the actual requirement - that the Next Hop is validated per the
> encoding rules asked for at line 291 and used for propagation, and that the
> SD-WAN NLRI is not installed for forwarding. Section 4.2 of RFC 9830 gives
> the sentence to model: the best route on that SAFI is not downloaded to
> forwarding but passed to a separate local component. State the analogous
> thing here, naming the SD-WAN tunnel manager per issue (2).
>
>
>
> [Linda] Agree. This is not addressed by the earlier changes. For an SD-WAN
> Underlay Route, the NLRI describes WAN/tunnel properties and is not itself
> a forwarding prefix. We will remove the traffic-forwarding sentence and
> clarify that the Next Hop is validated as a reachable BGP Next Hop for the
> advertising SD-WAN edge, while the SAFI-74 route is used for SD-WAN tunnel
> information rather than installed as a forwarding route.
>
>
>
> 1520      Port-Local-ID (0), SD-WAN-Color (10), SD-WAN Node ID (2.2.2.2),
> 1522      Port-Local-ID (0), SD-WAN-Color (20), SD-WAN Node ID (2.2.2.2),
> 1525      Port-Local-ID (0), SD-WAN-Color (30), SD-WAN Node ID (2.2.2.2).
>
> 1695   *  SD-WAN Node ID: 1.1.1.1
>
> <nit> idnits reports these as instances of IP addresses that are not from
> the
> documentation prefixes.
>
> [Linda] Agree. We will replace these example addresses with addresses from
> the IPv4 documentation prefixes defined in RFC 5737 so that idnits no
> longer flags them.
>
>
>
>
> 1555   The error handling for the tunnel encapsulation signaling
> 1556   (Encapsulation Extended Community and Tunnel Encapsulation
> Attribute)
> 1557   in this document follows the procedures specified in Section 13 of
> 1558   [RFC9012].  Unless otherwise stated, malformed or unrecognized Sub-
> 1559   TLVs MUST be handled as specified in [RFC9012].  This document
> 1560   defines new Sub-TLVs for Tunnel Type 25 (SD-WAN-Hybrid), but does
> not
> 1561   alter the validation behavior established in RFC 9012.
>
> <major> The final sentence is incorrect, and it is the single passage most
> responsible for the confusion described in issue (1). For client routes,
> where RFC 9012 does govern, the document does alter its validation
> behaviour
> - in at least two places where it is not free to: treating an absent
> Tunnel
> Egress Endpoint sub-TLV as Address Family 0 (line 346), and processing only
> the first instance of any sub-TLV, which contradicts RFC 9012's rule for
> the Color sub-TLV (line 1347). For underlay routes on AFI/SAFI 1/74 and
> 2/74,
> RFC 9012 established no validation behaviour, because Section 6 of RFC
> 9012
> puts other AFI/SAFIs outside its scope. So the claim is not false there so
> much as empty - and its effect is to conceal that the document is making
> its
> own validation choices for that SAFI, some of which are stricter than
> RFC 9012's (line 1184). Those choices are defensible; presenting them as
> RFC 9012 conformance is not.
>
> Please replace the final sentence with a statement of which RFC 9012
> procedures are inherited in which context. Something of the form: "For the
> AFI/SAFIs listed in Section 6 of [RFC9012], the validation and error
> handling procedures of Section 13 of [RFC9012] apply unchanged. For the
> SD-WAN SAFI, which is outside the scope of [RFC9012], the procedures in
> Section [X] of this document apply." Then correct the two client-route
> deviations per the comments at lines 346 and 1347, and declare the
> underlay-route choices per the comment at line 1184. The whole passage
> should move into whichever of the three new sections it belongs to once the
> split in issue (3) is done.
>
> [Linda] This is addressed by the earlier changes: Client Routes now follow
> RFC 9012 validation and multiplicity rules, including requiring the Tunnel
> Egress Endpoint Sub-TLV as specified there, while SAFI-74 Underlay Routes
> use the validation and error-handling procedures defined by this document.
> We will replace the current final sentence with that explicit distinction.
>
>
>
>
> 1582   The SD-WAN NLRI [AFI 1/SAFI = 74] utilizes a route type field to
> 1583   describe the format of the NLRI.  This specification only allows an
> 1584   NLRI with a type value of 1.
>
> 1596   The SD-WAN NLRI (AFI=1/SAFI=74) MUST be paired with Tunnel
> 1597   Encapsulation attribute with a SD-WAN Hybrid tunnel TLV, If the SD-
> 1598   WAN NLRI exist in an BGP UPDATE without a Tunnel Encapsulation
> 1599   Attribute with a SD-WAN Hybrid tunnel TLV, the NLRI is considered
> 1600   malformed and Treat-as-withdraw approach (per RFC7606).
>
> <major> Both passages scope themselves to AFI 1 only, while Sections 2.2.1
> and 2.4 define both 1/74 and 2/74. Taken literally, Section 2.6.2's
> route-type validation and Section 2.6.3's pairing requirement do not apply
> to IPv6 SD-WAN SAFI routes at all - so an implementation could accept an
> IPv6 SD-WAN SAFI route with no TEA and remain conformant. That is a
> substantive gap in the error handling rather than a citation slip. Please
> write both as "SD-WAN SAFI routes (AFI/SAFI 1/74 and 2/74)" per the
> terminology in issue (4), repair the second sentence, and check the rest of
> Section 2.6 for the same AFI-1-only scoping.
>
>
>
> [Linda] Agree. We will scope both sections consistently to SD-WAN SAFI
> routes using AFI/SAFI 1/74 and 2/74, and correct the pairing requirement
> accordingly. We will also check the rest of Section 2.6 for any remaining
> IPv4-only references.
>
>
>
> 1602   The SD-WAN NLRI SHOULD not be paired with an Encapsulation Extended
> 1603   Community.  If an SD-WAN NLRI is paried with an Encapsulation
> 1604   Extended Community rather than a Tunnel Encapsulation Attribute, the
> 1605   SD-WAN NLRI is considered malformed and the Treat-as-withdraw
> 1606   approach (per [RFC7606]) SHOULD be used.
>
> <major> Against Section 2.5.1 (lines 1450-1452), which says "MUST NOT"
> where
> this says "SHOULD not", for the same condition. So, we have: a direct
> normative contradiction between two sections on the same requirement;
> "SHOULD not" is not a BCP 14 construction, which idnits flags and which is
> real, so whichever way the requirement resolves the keyword must be "SHOULD
> NOT" or "MUST NOT"; and the receiver action is a SHOULD, when whether a
> route is withdrawn or accepted cannot be a local choice - one speaker
> withdraws and its neighbour does not, and the RIBs diverge. Please delete
> Section 2.6.3's second paragraph and keep Section 2.5.1's MUST NOT, adding
> the receiver behaviour there as a MUST (treat-as-withdraw per RFC 7606).
> Reconcile the first paragraph of Section 2.6.3 with Section 2.2 at the same
> time - the two say the same thing in different words, which is how they
> drifted apart.
>
>
>
> [Linda] This has been addressed by the earlier Section 2.6.3 cleanup. We
> will also remove this remaining duplicate/inconsistent paragraph, retain
> the MUST NOT requirement in Section 2.5.1, and make the receiver action a
> normative MUST treat-as-withdraw per RFC 7606.
>
>
>
> 1614   It is essential to ensure that advertisements from an SD-WAN edge
> 1615   node are legitimate.  The RR, which maintains policy information
> 1616   about which SD-WAN nodes are authorized to communicate, MUST verify
> 1617   that the advertising BGP speaker is permitted to originate SD-WAN
> 1618   Hybrid Tunnel information before reflecting such routes to other
> 1619   peers.
>
> 1628   To maintain correctness and security, both the RR and BGP speakers
> 1629   SHOULD validate that the client routes and associated tunnel
> 1630   information are consistent with expected configurations.  This
> 1631   includes verifying that:
> 1633   *  The NextHop in the client route update matches a known SD-WAN
> Node
> 1634      ID.
> 1636   *  The tunnel's egress endpoints are reachable and authorized.
> 1638   *  The advertised SD-WAN Color in the underlay NLRI matches the
> Color
> 1639      Extended Community attached to the client route.
> 1640
>
> <major> Both passages carry BCP 14 keywords placed on BGP - a MUST on the
> RR
> and a SHOULD on "both the RR and BGP speakers" - and so read as protocol
> requirements. They are not implementable as such. BGP has no notion of
> which
> node is "permitted to originate SD-WAN Hybrid Tunnel information", no
> notion
> of "expected configurations", and no way to determine that a tunnel egress
> endpoint is "authorized". The document supplies no procedures for any of
> it:
> no source of the policy information, no encoding for it, no distribution
> mechanism, no matching algorithm, no behaviour on failure beyond the bare
> verb "verify". A normative keyword on an operation with no specified
> procedure is not a requirement; it is an aspiration in requirement's
> clothing, and an implementer cannot conform to it or fail to conform to it.
>
> This is the only origin-authorisation statement in the document
> (issue (7)(e)), and the third bullet is the only normative statement of
> the colour-correlation rule that Sections 2.4 and 2.5 depend on (see the
> comments at lines 291 and 362). So the load-bearing security and
> correlation
> requirements of the whole mechanism rest on text that specifies nothing.
>
> Please consider one of the two options; either is acceptable and the choice
> is yours.
> (a) Reframe as applicability or deployment considerations and drop
> the BCP 14 keywords: state the same content as what a deployment is
> expected
> to arrange, in the applicability and deployment considerations section
> created by issue (5), in non-normative language - that the SD-WAN
> controller
> is expected to hold policy governing which edge nodes may originate which
> Node IDs and Colors, and that the mechanisms in this document assume such
> policy is in place. This is honest about what the document actually
> delivers,
> and it is consistent with Section [N].1 of the supplied text, which
> already
> states the closed-set assumption.
> (b) Assign the onus to a named component: introduce, or extend, a
> management-plane component that owns policy - either something new, or by
> broadening the SD-WAN tunnel manager of issue (2) and renaming it
> accordingly
> ("SD-WAN Manager" would cover both tunnel lifecycle and policy). The
> document
> would then state that policy definition, distribution and enforcement are
> the
> responsibility of that component and out of scope here, with BGP's role
> limited to carrying the information. If option (b) is taken, the component
> name and scope in issue (2) change throughout and both should be resolved
> together.
>
> In either case, this material does not belong in Section 2. Section 2 is
> the
> BGP protocol specification, and none of this is BGP protocol. One
> consequential point: issue (7)(e) asks for the origin-authorisation
> constraint - which Node IDs, Colors and Port-Local-IDs an authorised edge
> may claim - to be stated as normative text in the specification sections,
> and that placement follows whichever of (a) or (b) is chosen here.
> Separately, the colour-correlation rule in the third bullet is genuinely a
> protocol matter and does belong in the specification, but as part of the
> interaction procedures that issue (3) creates, expressed as an
> encoding-level comparison the receiver performs - not as a policy check.
>
> [Linda]  Agree. We will treat origin authorization and expected
> configuration as deployment/management policy rather than BGP protocol
> requirements, and remove the BCP 14 keywords from those statements. The
> color-correlation check will remain in the protocol procedures as an
> explicit comparison between the Client Route Color Extended Community and
> the SD-WAN-Color carried in the underlay NLRI.
>
>
>
>
> 1683 3.2.1.  Example creation of IPsec SA over SD-WAN Hybrid Tunnel
>
> <minor> No IPv6 examples. The document defines AFI 2/74, a 16-octet SD-WAN
> Node ID, and IPv6 handling in the Extended Port Attribute, but every
> example
> is IPv4. This will likely run into problem from INT ADs during IESG review.
> Please add one, using 2001:db8::/32.
>
> [Linda] Agree. We will add an IPv6 example to Section 3.2.1 using an
> address from the 2001:db8::/32 documentation prefix, covering AFI/SAFI 2/74
> and the corresponding IPv6 SD-WAN Node ID/port information.
>
>
>
>
> 1690   To establish this tunnel, C-PE1 must advertise the following
> 1691   attributes required for setting up the IPsec SA:
> 1693   *  NextHop: 198.51.100.10
> 1695   *  SD-WAN Node ID: 1.1.1.1
> 1697   *  SD-WAN-Site-ID: 1502
>
> <major> Three problems in the document's only end-to-end example. The
> reason
> this is not merely an example-quality nit is that the example and the
> specification contradict, and it is at least as likely that the example
> reflects what implementers actually do and the specification is the part
> that is wrong!
>
> (a) It contradicts Section 2.5, and the contradiction points at the
> specification rather than at the example. Lines 1442-1444: "Remote nodes
> use
> the SD-WAN Node ID carried in the SD-WAN SAFI to correlate client routes
> whose NextHop address matches the SD-WAN Node ID." And Section 2.1.2, lines
> 1421-1422: "The SD-WAN NLRI is originated using the loopback address of the
> C-PE." In the example the NextHop is 198.51.100.10 - a WAN port address,
> and
> the address of the very port the example is building a tunnel to - while
> the
> Node ID is 1.1.1.1. They do not match, so the correlation the specification
> depends on would fail. Setting the next hop to the WAN port address is the
> natural thing to do when the advertisement is about that port, and it is
> evidently what the example's authors expected. That suggests the
> specification's correlation rule is the part that is wrong, or that it is
> stated for the client-route case and has been wrongly generalised to the
> SD-WAN SAFI case. Please resolve it as a specification question first - the
> comment at line 291 is where the rule itself is examined - and correct the
> example to match whatever comes out of that, rather than the other way
> round.
>
> (b) SD-WAN-Site-ID is used and never defined. It appears here and at line
> 1734 and nowhere else in the document - not in Section 1.3 Conventions, not
> in any encoding, not in IANA. A reader cannot tell whether it is the
> SD-WAN-Color, a field that was removed, or something from a companion
> document.
>
> (c) The example demonstrates only failure, using undefined placeholders.
> Lines 1761-1762: "As there is no matching transform between the WAN ports
> P2
> and P2 in C-PE1 and C-PE2, respectively, no IPsec Tunnel will be
> established." But both sides advertise Transform Type ENCR with Transform
> ID
> = 1 - the same algorithm. The only difference is the Transform Attributes,
> given as "trans 1" and "trans 2" (lines 1718, 1752), which are not values
> defined anywhere. So the example asserts a mismatch that its own data does
> not show, and the document contains no example of a successful SA
> establishment - which is the case a reader actually needs.
>
> A minor consistency point in the same example: the tunnel type is written
> "Type = SD-WAN Hybrid Tunnel" for C-PE1 (line 1699) and "Type=SD-WAN" for
> C-PE2 (line 1736). And line 1705, "IPsec information for detailed
> information about the ISP", has no discernible meaning.
>
> Please rework the example to show a successful establishment with real
> values from the IANA IKEv2 registries, correct the NextHop / Node ID
> relationship to match Section 2.5 (or fix Section 2.5), define or delete
> SD-WAN-Site-ID, and keep the failure case as a second, shorter example if
> it
> is wanted. Use RFC 5737 documentation addresses throughout, per the comment
> at line 1520.
>
> [Linda] Agree. We will rework the example to be consistent with the
> revised specification: the BGP Next Hop will be a reachable address of the
> advertising SD-WAN edge and will not be required to equal the SD-WAN Node
> ID; the undefined SD-WAN-Site-ID will be removed; and the example will show
> a successful IPsec SA establishment using defined IKEv2 transform values
> and RFC 5737 documentation addresses. We will also fix the tunnel-type
> naming and remove the unclear ISP sentence.
>
>
>
> 1780   interfaces.  The use of secure transport connections (e.g., BGP over
> 1781   IPsec/TLS) is RECOMMENDED to ensure manageability in untrusted
> 1782   environments.
>
> <major> The connection between secure transport and manageability is not
> apparent, and this is the third statement of the transport-security
> requirement at a third strength. Please delete the sentence; the comment at
> line 148 places the single normative statement that replaces all three.
> And BGP over TLS is not an RFC - please refer to discussions in the IESG
> during the review of the accompanying BESS SD-WAN document.
> [Linda] Agree. We will delete this duplicate transport-security sentence.
> The single normative transport-security requirement will remain in Section
> 1, and we will not cite BGP over TLS as an established RFC mechanism.
>
> 1784 5.  Security Considerations
>
> 1786   This document defines BGP extensions for SD-WAN edge nodes to
> 1787   advertise their attributes for establishing IPsec SAs and underlay
> 1788   tunnel attributes, typically via a RR, which then propagates them to
> 1789   authorized SD-WAN peers.  These BGP UPDATEs may contain sensitive
> 1790   information such as public keys, IPsec proposals, and nonces.  In
> 1791   deployments where SD-WAN edge nodes communicate with the RR over
> 1792   public or untrusted networks, BGP SHOULD be run over TCP-AO secure
> 1793   transport that provides authentication and integrity for this data.
>
> 1795   Some network operators running SD-WAN edge over public or untrusted
> 1796   networks desire confidentiality as well as authentication and
> 1797   integrity.  In this case, network operators can consider running BGP
> 1798   over new IETF technologies (e.g. BGP over QUIC
> [draft-retana-idr-bgp-
> 1799   quic], BGP over TLS/TCP [draft-wirtgen-bgp-tls], or Securing BGPv4
> 1800   using IPsec [draft-ward-bgp-ipsec]) which provide authentication,
> 1801   confidentiality, and Integrity.
>
> 1803   These two sets of secure transport technologies for BGP provide
> 1804   different levels of protection against tampering or interception.
> 1805   These secure transport connections are needed to protect all fields,
> 1806   including cryptographic attributes, from tampering or interception.
> 1807   Without such protection, the system maybe vulnerable to spoofed
> 1808   tunnel attributes, unauthorized route injections, or replayed IPsec
> 1809   setup information.
>
> 1811   However, in closed or "walled garden" deployments, where SD-WAN edge
> 1812   nodes and the RR (SD-WAN controller) are within a trusted, secured
> 1813   environment (e.g., a private MPLS backbone or physically secured
> 1814   enterprise network), the risk of interception or tampering is
> 1815   significantly reduced.  In such cases, the use of secure transport
> is
> 1816   optional, and operators may choose to run BGP over standard TCP,
> 1817   based on their internal risk assessment.
>
> 1819   Regardless of the transport used, BGP policy enforcement remains
> 1820   critical.  The RR SHOULD apply strict filtering and policy controls
> 1821   to validate that only authorized SD-WAN edge nodes advertise
> specific
> 1822   Node IDs, Route Targets, or VPN identifiers.  While route origin
> 1823   validation via RPKI helps, it does not cover SD-WAN-specific fields
> 1824   like Tunnel attributes or SA proposals.  Local policies, when
> 1825   misconfigured, may introduce vulnerabilities; therefore, policy
> 1826   application points SHOULD be carefully audited.
>
> 1828   Many of the general BGP security risks discussed here are also
> 1829   covered in [RFC4271], [RFC4272], and [RFC9012].  This document
> 1830   inherits those considerations and introduces no new cryptographic
> 1831   requirements beyond what is described for securing BGP transport and
> 1832   validating the correctness of SD-WAN tunnel attribute exchanges.
>
> 1834   This specification does not define deployments across fully
> untrusted
> 1835   networks, but if such environments are used, strong transport
> 1836   security is critical and additional validation mechanisms may be
> 1837   required to maintain SD-WAN tunnel and routing integrity.
>
>
> <major> The Security Considerations analyse neither layer, and never state
> the
> trust model
>
> (a) The RR is the trust anchor of the entire solution, and the document
> never says so. Section 1, at lines 173-177, does establish what the RR is:
> "In the context of this document, BGP Route Reflector (RR) is the component
> of the SD-WAN Controller system that receives the BGP UPDATE from SD-WAN
> edges and in turn propagates the information to the intended peers that are
> authorized to communicate via the SD-WAN overlay network." By that
> definition the RR is the SD-WAN controller, and it is therefore inherently
> the trusted nerve centre of the whole solution: every edge node's tunnel
> and
> security parameters pass through it, it is the only entity with a complete
> view of them, and the mechanism works only insofar as it redistributes them
> faithfully and to the correct peers. That is the architecture, and it is
> not
> a criticism of it. The defect is that this consequence is drawn nowhere -
> the reader is left to infer the trust model from a definitional sentence in
> the Introduction.
>
> Two places actively obscure it. First, Section 5 mentions the relationship
> only inside the walled-garden paragraph at lines 1811-1815, where it reads
> as a property of particular deployments. By Section 1's definition the RR
> is
> the controller in every deployment, so the trust relationship is not
> deployment-dependent; only the environment it sits in is. As written, a
> reader could conclude that the RR is trusted only in walled-garden
> deployments, which is not the model. Second, Section 1's own framing
> disclaims the centring it has just established, at lines 179-186 - see also
> the comment at line 179, where that passage is separately flagged as
> unfinished.
>
> State the trust model plainly and early - in Section 1 where the RR is
> defined, and again at the top of Section 5. The supplied text for Section
> [N].1 in issue (5) already carries the wording; please use it rather than
> composing a second version. Then rewrite the walled-garden paragraph so
> that
> it is about the environment the RR sits in, not about whether the RR is
> trusted.
>
> (b) Confidentiality is made optional exactly where it cannot be. Continuing
> the same passage, lines 1815-1817: "In such cases, the use of secure
> transport is optional, and operators may choose to run BGP over standard
> TCP, based on their internal risk assessment." This is stated
> unconditionally, with no carve-out for UPDATEs carrying the Simplified
> IPsec
> SA (sub-TLV 70) or IPsec Public Key (sub-TLV 68) sub-TLVs. If the two keys
> at line 913 turn out to be symmetric keys, running that over plain TCP
> defeats the purpose of the IPsec tunnels the document exists to establish.
> Note also that TCP-AO, which the document recommends as the primary
> mechanism, provides authentication and integrity but not confidentiality,
> so
> its own primary recommendation does not protect the key material it
> carries.
> Make transport confidentiality a MUST for any UPDATE carrying sub-TLV 68 or
> 70, and restrict the walled-garden paragraph to deployments not using those
> sub-TLVs. The single normative statement of the transport requirement
> itself
> is placed by the comment at line 148.
>
> (c) <question> Have the forward secrecy properties of security associations
> established from BGP-distributed parameters been considered, and is there a
> WG discussion to point at? The document says nothing about them either way.
> No assertion and no comparison is intended here. Whatever the answer, the
> actionable requirement is issue (5)'s: the applicability section must
> record
> that these properties have not been characterised. If they have been
> considered, then the analysis should appear in Section 5 and the
> corresponding item in the supplied text can be withdrawn.
>
> (d) The Rekey Counter has no anti-replay procedure. The field is defined at
> lines 709-711 and the nonce is described as "a random or pseudo-random
> number for preventing replay attacks" at line 717, but no section says what
> a receiver does with either: whether an advertisement carrying a Rekey
> Counter not greater than the stored value is rejected, what happens on
> counter wrap, or how the counter survives an edge node restart. The
> anti-replay machinery is present in the encoding and absent from the
> procedures. This is the clearest instance of the root cause in issue (2):
> the question "who consumes the Rekey Counter" has no answer in the document
> because there is no stated division of labour between BGP and anything
> else.
> Once that boundary exists the question resolves one way or the other -
> BGP-visible with a stated comparison rule, or opaque and handed over - and
> either answer is fine. The fix is carried by the comment at line 584.
>
> (e) The authorised-but-malicious edge node is not considered. Section 3
> requires the RR to check that a speaker is permitted to originate SD-WAN
> Hybrid Tunnel information (lines 1614-1619), but nothing constrains which
> Node IDs, SD-WAN-Colors or Port-Local-IDs an authorised edge may claim. An
> authorised edge can advertise another edge's Node ID, or claim membership
> of
> a Color it should not be in, and attract that traffic. State the constraint
> and the receiver's validation of it: an SD-WAN edge node MUST NOT originate
> an SD-WAN SAFI route whose SD-WAN Node ID is not its own, and a receiver
> MUST validate the SD-WAN Node ID against the identity of the advertising
> speaker before using the advertisement. Section 5 then references this as
> the mitigation for the exposure.
>
> This one is checkable from the advertisement itself - the Node ID is on the
> wire and the advertising speaker's identity is known from the session - so
> it is a protocol requirement and belongs in Section 2 with the rest of the
> SD-WAN SAFI procedures, subject to the destination chosen for the comment
> at
> line 1614. What is not protocol is the out-of-band policy governing which
> edge nodes may communicate with which, and which Colors an edge is entitled
> to use; those cannot be validated from the advertisement and have no
> procedures, and they go wherever the comment at line 1614 is resolved to.
> Keeping the two apart is the point: the wire-checkable constraint should be
> normative here, and the policy should stop pretending to be.
>
> Overall: rewrite Section 5 around the exposures the mechanisms create at
> the
> BGP layer - which is all of (a), (b), (d) and (e), and none of which
> requires straying into IPsec internals or into comparison with other
> mechanisms. What cannot be analysed at that layer is declared in the
> applicability section of issue (5) rather than attempted in Section 5. RFC
> 4301 and RFC 7296 are already referenced and carry the mitigations to point
> at.
>
> [Linda] Those issues have been largely addressed by the earlier changes.
> We have already made the RR/controller trust model explicit, required
> authenticated and integrity-protected BGP sessions, separated deployment
> policy from BGP protocol validation, and clarified that the Rekey Counter
> is opaque to BGP and handled by the IPsec implementation. We will revise
> Section 5 to reflect those decisions consistently, add the confidentiality
> requirement for UPDATEs carrying sensitive keying material, and state the
> wire-checkable Node ID origin constraint separately from out-of-band
> authorization policy.
>
> 1839 6.  IANA Considerations
> 1840 6.1.  SD-WAN Overlay SAFI
>
> 1842   IANA has assigned SAFI = 74 as the SD-WAN SAFI.
>
> 1846   IANA is requested to assign a type from the BGP Tunnel Encapsulation
> 1847   Attribute Tunnel Types registry
>
> 1856   IANA is requested to assign the following sub-Types in the BGP
> Tunnel
> 1857   Encapsulation Attribute Sub-TLVs registry
>
> <major> The IANA sections do not describe the registries as they actually
> stand. All the code points this document "requests" were registered years
> ago
> against previous draft versions, under different names. Many names differ,
> and
> references are outdated.
>
> These are FCFS registrations, not RFC 7120 early allocations - the
> tunnel-type registry is FCFS throughout and the sub-TLV range 64-125 is
> FCFS
> - so they carry no TEMPORARY marker and will not expire. There is nothing
> to
> renew, but there is a great deal to correct. Section 6.1 reports a
> completed
> action but not the registered name and does not ask for it to be corrected;
> Sections 6.2 and 6.3 request assignments that already exist.
>
> Please rewrite Sections 6.1 to 6.3 as update requests, one sub-section per
> registry, each stating explicitly the current registry contents, the new
> Description string, and the new Reference (this RFC, with the correct
> section). Note that Section 6.1's own heading calls SAFI 74 the "SD-WAN
> Overlay SAFI" while the registry calls it "SD-WAN Capabilities" and the
> body
> calls it "the SD-WAN SAFI" - three names in one sub-section, and the
> heading
> names the wrong one of the two route types (issue (4)). Settle the term
> first, then write the request. For Section 6.3, for example: "IANA has
> previously assigned the following values in the 'BGP Tunnel Encapsulation
> Attribute Sub-TLVs' registry with a reference to
> draft-ietf-idr-sdwan-edge-discovery-04. IANA is requested to update the
> Description and Reference for these values as follows: ..." and give the
> table. Do the same for SAFI 74 and tunnel type 25. If the sub-sub-TLV
> question at line 378 is resolved by making 66 a sub-sub-TLV, this is also
> where the request to return code point 66 goes. And if the sub-TLV range
> question at line 929 is resolved by moving to 128-255, Section 6.3 changes
> shape entirely - so please settle those two before rewriting Section 6.
>
>
>
> [Linda] We have already settled two related points: code point 66 will be
> returned and redefined as an Extended Port Sub-Sub-TLV, and we are keeping
> the existing 64–70 Sub-TLV assignments rather than moving them to the
> extended-length range. We will now rewrite Sections 6.1–6.3 as IANA update
> requests, using one consistent name for SAFI 74 and updating the existing
> Description and Reference fields instead of requesting new assignments.
>
>
>
> 1885   IANA is requested to create a new registry titled "SD-WAN Extended
> 1886   Port Encapsulation Types" under the BGP Tunnel Encapsulation Group.
> 1888      Value   Type Description            Reference
> 1889      -----   -----------------------     -------------
> 1890       0      Reserved                    This document
> 1891       1      GRE                         This document
> 1892       2      VXLAN                       This document
> 1893       3~255  Reserved for future
>
> 1902       0      Reserved                    This document
> 1903       1      Wired                       This document
> 1904       2      WIFI                        This document
> 1905       3      LTE                         This document
> 1906       4      5G                          This document
> 1907       5~255  Unassigned
> 1908           255    Reserved for Experimental Use
>
> <major> The issues here apply for other similar sections as well:
>
>    1) No registration policy for Section 6.5, 6.6 or 6.7. RFC 8126 requires
>       one. Without it IANA cannot operate the registry.
>
>    2) Sections 6.6 and 6.7 define 5-255 as Unassigned and then 255 as
>       Reserved for Experimental Use - overlapping and contradictory, and
> the
>       indentation suggests the second line was meant to be a sub-range of
>       the first. A single value for Experimental Use is also an odd choice;
>       if experimental values are wanted, a small range is conventional. I
>       would ask whether they are needed at all here: these are three small,
>       slow-moving enumerations with no plausible pressure on the space, and
>       reserving the top of each range for Standards Action would be the
>       better hedge.
>
>    3) Section 6.5 says "Reserved for future" where it means Unassigned - a
>       Reserved range cannot be allocated at all, which is presumably not
> the
>       intent.
>
>    4) Section 6.4's "Values 2-255 are reserved for future assignments" is
>       wrong for a 2-octet field. The Route Type field is 2 octets (lines
> 469
>       and 482), so the range is 2-65535.
>
>    5) No reference column pointing at the defining section in any of the
>       four new registries, and no change controller stated. Registry
> entries
>       should point at the section that defines the item.
>
>    6) Two registries are missing entirely: the NAT Type values (Section
>       2.3.6, values 1-7, closed by normative text with no registry - see
> the
>       comments at lines 1090 and 1112), and the Extended Port Sub-Sub-TLV
>       Types namespace (see the comment at line 378).
>
> [Linda] Agree. We will clean up all of the new IANA registries
> consistently: add an RFC 8126 registration policy and section references,
> correct Reserved versus Unassigned ranges and the Route Type range, and
> remove the overlapping value 255 definitions. We will also add the missing
> NAT Type and Extended Port Sub-Sub-TLV Type registries, consistent with the
> earlier changes.
>
>
>
>
> 1925 7.  References
>
> 1927 7.1.  Normative References
>
> <major> Some updates are required for the normative references.
>
> Currently normative, but should be perhaps informative? [MEF70.1] and
> [MEF70.2], used only to say what an SD-WAN is with nothing in the
> encodings,
> procedures or anything in the specification depending on them.
>
> [RFC4456], which is normative and but unused - please add it as a reference
> to where RR procedures are specified.
>
> Currently informative, but should be normative: [RFC8221] and [RFC4835].
> But then we have a chain of obsoletion to deal with RFC 4835 -> RFC 7321 ->
> RFC 8221 - this needs to be aligned. And check if the IANA IKEv2 transform
> registries and RFC 7296 instead should be normative.
>
> Also [RFC5903] and [RFC5114], seem normative. However, RFC 5903 and RFC
> 5114
> are both Informational, so making them normative creates downrefs. There
> may
> be better references but you will need to check with security folks.
>
> RFC 5925 is missing and normative.
>
> draft-ietf-bess-bgp-sdwan-usage is also normative and since it is in RFC Ed
> Q, it is not an issue - it just needs to be called out as a downref.
>
> [Linda] Agree. We will update the reference classifications accordingly:
> move MEF70.1/70.2 to Informative, add RFC 4456 where RR procedures are
> referenced, make RFC 5925 normative, and use RFC 7296 plus the IANA IKEv2
> transform registries as the normative basis for algorithm identifiers. RFC
> 4835 will be removed, RFC 8221 retained as informative algorithm-selection
> guidance, and the RFC 5903/RFC 5114 downref question will be checked with
> the Security review. The BESS SD-WAN document will also be identified
> appropriately as a normative downref.
>
>
>
>
> 1998 7.2.  Informative References
>
> <minor> Unused references, RFC 1997, RFC 4659, RFC 4761, RFC 4762, RFC
> 5701,
> RFC 6793, RFC 7018 and RFC 8092 - check if you really forgot to use them or
> delete them. These were all warnings from idnits that seem to have been
> ignored.
>
> And then [Secure-EVPN], is not used but instead
> [draft-ietf-bess-secure-evpn-02]
> is - please fix this. I don't think this reference is really needed - it
> can
> use the extension provided by it but please avoid adding "hooks" to
> unnecessary
> documents.
>
> [Linda] Agree. We will remove the unused references unless they are
> actually cited by the revised text, and correct the Secure-EVPN citation
> inconsistency. If the Secure-EVPN reference is not needed by the final
> procedures, we will remove it rather than retain an unnecessary dependency.
>
>
> <EoRv29>
>
>
>