[Idr] Re: Intdir early review of draft-ietf-idr-sdwan-edge-discovery-17 - Updated to -21

[Susan Hares <[e-mail]>]

Antonine:

Thank you for doing a careful review 2nd review.

I've answered your 2 INT questions below.  I've also adjusted the text to address these INT issues.  I appreciate you reminding me regarding the private addresses.

I've published a -21 with textual fixes to the NITs and these 2 INT errors.  My apologies for so many NITs, I've reviewed all the NITs twice, but I may still miss something. Please note that 6 of NITs deserved an additional technical response.  These are listed as NIT-1 to NIT-6.

I've attached a diff between the two versions.

Sue Hares

From: Antoine FRESSANCOURT <[e-mail]>
Sent: Friday, January 17, 2025 9:07 AM
To: Antoine FRESSANCOURT <antoine.fressancourt=[e-mail]>; Susan Hares <[e-mail]>; Antoine Fressancourt <[e-mail]>; [e-mail]
Cc: [e-mail]; [e-mail]; John Scudder <[e-mail]>
Subject: RE: Intdir early review of draft-ietf-idr-sdwan-edge-discovery-17

Hello, I have carefully reviewed version 20 of draft-ietf-idr-sdwan-edge-discovery, and the restructured text greatly improves the readability and understandability of the document from my perspective
External ([e-mail]<mailto:[e-mail]>)
  Report This Email<https://protection.inkyphishfence.com/report?id=bmV0b3JnMTA1ODY5MTIvc2hhcmVzQG5kemguY29tL2E4Mjc5YzdjYjU1MDVlZWY5NTIxN2ZhZTU1ZTA5ZWI2LzE3MzcxMjI4NDYuMTEzODYxOA==#key=88b95b207938d9c438ff42898c6e91e4>  FAQ<https://www.godaddy.com/help/report-email-with-advanced-email-security-40813>  GoDaddy Advanced Email Security, Powered by INKY<https://www.inky.com/protection-by-inky>

Hello,

I have carefully reviewed version 20 of draft-ietf-idr-sdwan-edge-discovery, and the restructured text greatly improves the readability and understandability of the document from my perspective.

There are 2 INT-related issues remaining in the document from my perspective:

  *   In section 3.3.6, when reading the formatting of the sub-TLV, I was first wondering how you could determine whether the local and public IP addresses where IPv4 or IPv6 addresses. Then I began wondering what would be happening if the NAT were a 6to4 NAT. It might be anecdotic, but maybe it would be worth mentioning this case in the document.

The ports with either all be IPv6 or all be IPv4.  If IPv4 the length would be 32 (8+4+4+4+8).  If IPv6 the length would be 64.  I will note in the text that 6to4NAT would not be supported with this encoding.  I have annotated this.

The length (ExtPort Length) will be the indicator on whether the addresses are IPv4 or IPv6.   I've added text on the length.  I've also added a note that 6to4NAT cannot be supported.


  *   In section 4.2.2, the example given is using public IPv4 addresses to document the creation of IPSec Security association over SD-WAN tunnel. The INT area directorate strongly encourages the use of IPv6 addresses from the documentation space in the description of such examples. At least, if IPv4 is still used, it would be preferable to use a private addressing space.

I've adjusted the main addresses (192.10.0.10 and 192.0.0.1) to be private addresses.  Thank you for mentioning this fact.
The Node ID and Site ID are not IP addresses - but rather similar to BGP identifiers (RFC6286).

Potential Technical comment hiding in NITs below:

These nits are substantive, so

NIT-1: In page 14, section 2.4, The example is not clear, since some links in figure 2 are not present in figure 1 (like the link between CPE1 and CPE4, for instance).
[Sue]: This comment is close to a technical link, so I need clarification.


  *   Figure 1 is a physical diagram of all links, and Figure 2 is a diagram with SDWAN Hybrid Links. SD WAN provides a combining of the physical links in figure 1 to figure 2.
  *   Furthermore, the text in Figure 1 provide comments on which link goes over. The example provides you with 4 types of links: WAN NET1, WAN NET3, Direct L3 link, and Direct L2 link.  The SD WAN Hybrid links.  The WAN L3 NET1 and WAN L3 NET3 are linked (that's what WAN means).  The Direct links are just direct links.

So, what are you trying to ask the example to do?
.
NIT-2:  In page 24, section 3.1, "Encoding and mechanisms are defined 3.3.6.  Section 3.6" - > The sentence is not clear.
[Sue]: This is an editorial comment that may have technical importance.
The entire text is: "*4 - Encoding and mechanisms are defined in 3.3.6.  Section 3.6 provides content processing and validation procedures."
It is appropriate to note by content that Extended Port SubTLV and Underlay ISP SubTLV (acting as a SubSubTLV) are described in section 3.3.6, and that 3.6 provides error processing and validation procedures.  I've edited it as follows:

Old text:/*4 - Encoding and mechanisms are defined in 3.3.6.  Section 3.6 provides content processing and validation procedures." /
New text:/*4 - Encoding and mechanisms are defined 3.3.6. Section 3.6
provides error handling procedures in case of SubTLV being MALFORMED
or included with the wrong NLRI./

NIT-3: In page 24, section 3.2.1, Figure 8 is not using a consistent formatting with Figures 5 and 6
[Sue]: BGP documents typically use the format in diagram 8 for NLRIs, and the attribute forms in section 5 and 6.  The difference in format is by choice.
I will raise this issue with the document shepherd.

NIT-4: In page 25, section 3.2.1, Figure 9 is not using a consistent formatting with Figures 5 and 6. Besides fields of lengths 2 or 4 octets are represented in the same way, which looks odd.
[Sue]:  This format has been used by many IDR drafts  I will raise this issue with the document shepherd.

NIT-5: In page 31, section 3.3.2, "IPsec SA Identifier (IPSec-SA-ID):" - > It would be interesting to put the length in bits of the field.
[Sue]:   The field length is specified in ID-Length. The reason for this usage is allow for compatibility with proposed specifications (draft-ietf-bess-secure-evpn-02).

NIT-6: In page 33, section 3.3.4, is the Transform Attr Length necessary?
[Sue]: Yes, but the reason is outside the scope of this document.  The authors are attempting to keep compatibility with (draft-ietf-bess-secure-evpn-02) for
Transform structure.  You will note in section 10.3 of draft-ietf-bess-secure-evpn-02, there is a number of transforms in the first byte of the TLV after the header byte.
draft-ietf-bess-secure-evpn-02 is not passed WG LC so this document does not include it.





===================
Besides, the text has a number of nits that give the impression that it has not been written with care, and may harm its reception:

  *   In page 1, in the abstract, please introduce the acronyms SD-WAN and NLRI on the first time you are using them.

[Sue]:   NLRI and SD-WAN are in the RFC editor's abbreviation list. In the past, RFC editor and ADs have requested that items on the list not be defined.
               However, I have complied with your request.


  *   In page 3, section 1.1, "[SD-WAN-BGP-USAGE] the defines..." - > "[SD-WAN-BGP-USAGE] defines..."
[Sue]: fixed in -21, thank you

  *   In page 3, section 1.1, "Support for Client Services interfacs on edge..." - > "Support for Client Services interfaces on edge..."
[Sue]: fixed in -21,

  *   In page 4, section 1.1, "Homogeneous Encripted SD-WAN" - > "Homogeneous Encrypted SD-WAN"
[Sue]: Fixed in -21, thank you.

  *   In page 4, section 1.1, "[SD-WAN-BGP-USAGE] provides descriptions on the the use of" - > "[SD-WAN-BGP-USAGE] provides descriptions on the use of..."
[Sue]: Fixed in -21, thank you.

  *   In page 5, section 1.3, the text describes the acronym CPE, while, in many parts of the document the acronym C-PE is used. Please make sure to use a consistent acronym along the text (and my preference would go to CPE).
[Sue]:  I've augmented the discussion in 1.1 for CPE to include C-PE.  In the context of routing the C-PE emphasizes the PE functionality.

  *   In page 5, section 1.3, "RR:  Route Reflector [RFC4456" - > "RR:  Route Reflector [RFC4456]"
[Sue]: fixed in -21, thank you.

  *   In page 6, section 2, "..the overall solution parts based on the a reference diagram shown.." - > "..the overall solution parts based on the reference diagram shown.."
[Sue]: Fixed in -21

  *   In page 6, section 2, "...example Topologies..." - > "...example topologies..."
[Sue] : fixed in -21

  *   In page 4, section 2.1, "... The SD-WAN Secure L3VPN requires the L3VPN for IPv4 [RFC4364] and IPv6 [RFC4659] be expanded..." - > "... The SD-WAN Secure L3VPN requires the L3VPN for IPv4 [RFC4364] and IPv6 [RFC4659] to  be expanded..."
[Sue]:   Changed in -21.
                It is unclear why my sentence is not valid.   I'm curious so that I can prevent future errors in these reviews.
               The sentences structures is [noun clause] verb [complex direct object]  imperative verb [clause]
               You are recommending [noun clause] verb [complex direct object] preposition [Clause] ?


  *   In page 7, section 2.2.1, "Homogeneous Encripted SD-WAN" - > "Homogeneous Encrypted SD-WAN"
[Sue]: Fixed in 21

  *   In page 8, section 2.2.1, "sending the approprite traffic" - > "sending the appropriate traffic"
[Sue]: Fixed in 21

  *   In page 10, section 2.2.1, Figure 2 needs to be reworked so that the boxes and links are properly aligned.
[Sue]: Fixed in 21

  *   In page 13, section 2.3, "Like any VPN networks," - > "Like any VPN network," (2 times)
[Sue]: Fixed in 21


  *   In page 14, section 2.4, The example is not clear, since some links in figure 2 are not present in figure 1 (like the link between CPE1 and CPE4, for instance).
[Sue]: This is a technical point.  Figure 1 is a physical diagram of all links, and Figure 2 is a diagram with SDWAN Hybrid Links.
           I will address as a technical comment above.

  *   In page 14, section 2.4, "The Unsecure Link (P3" - > the sentence seems to finish abruptly.
[Sue]: Delete in -21

  *   In page 15, section 2.4, "This simple examples show the" - > "This simple example shows the"
[Sue]: Fixed in -21

  *   In page 16, section 2.5, "-  transform set." - > "-  Transform set."
[Sue]: Fixed in -21

  *   In page 17, section 2.6, "(bottomn)" - > "(bottom)"
[Sue]: fixed in -21

  *   In page 17, section 2.6, "same topology as a the virtual" - > "same topology as the virtual"
[Sue]: Fixed  in -21

  *   In page 17, section 2.6, "For example, in the picture below," - > Can you be more specific about the figure you refer to?
[Sue]: Fixed in -21,

  *   In page 20, section 3.1, "over a set links" - > "over a set of links"
[Sue]: Fixed in -21.

  *   In page 20, section 3.1, "links may or may not need encryptions." - > "links may or may not need encryption."
[Sue]: Fixed in -21

  *   In page 21, section 3.1, "the TEA form of the Hyrid SD-WAN Tunnel TLV depend whether the" - > "the TEA form of the Hybrid SD-WAN Tunnel TLV depends on whether the"
[Sue]: Fixed in -21

  *   In page 23, section 3.1, "Precdence between Color" - > "Precedence between Color"
[Sue]: Fixed in -21

  *   In page 24, section 3.1, "Encoding and mechanisms are defined 3.3.6.  Section 3.6" - > The sentence is not clear.
[Sue]: Moving to technical comment.
Change: *4 - Encoding and mechanisms are defined 3.3.6. Section 3.6 provides error handling procedures in case of SubTLV being MALFORMED
or included with the wrong NLRI.

In page 24, section 3.2, "with a the SD-WAN Underlay" - > "with a SD-WAN Underlay"
[Sue]: Fixed in -21

  *   In page 24, section 3.2, "The TEA can include with SubTLVs for" - > "The TEA can include SubTLVs for"
[Sue]: Fixed in -21

  *   In page 24, section 3.2.1, Figure 8 is not using a consistent formatting with Figures 5 and 6
[sue]: dealing with as technical issues (see NIT-3 above)

  *   In page 25, section 3.2.1, Figure 9 is not using a consistent formatting with Figures 5 and 6. Besides fields of lengths 2 or 4 octets are represented in the same way, which looks odd.
[sue]: dealing with as technical issues (see NIT-4 above)

  *   In page 28, section 3.3.1, the "IPsec SA Identifier #n" field in Figure 11 is not properly aligned (see last "|"). Besides, the length field displayed in the schema is not the same as the length field described in the text (6 bits vs. 8 bits)
[Sue]:Thank you (TY)

  *   In page 29, section 3.3.1, "the IPSec-SA provide the security" - > "the IPSec-SA provides the security"
[Sue]: Thank you. Fixed in -21

  *   In page 29, section 3.3.2, "The encoding iss shown in the figure below:" - > "The encoding is shown in the figure below:"
[Sue]: Thanks, fixed in -21

  *   In page 30, section 3.3.2, the length field displayed in the schema is not the same as the length field described in the text (6 bits vs. 8 bits).
[Sue]: Thank you, Fixed in -21

  *   In page 31, section 3.3.2, "IPsec SA Identifier (IPSec-SA-ID):" - > It would be interesting to put the length in bits of the field.
[Sue]:   I added text, but see the technical NIT above.

  *   In page 31, section 3.3.3, "The encoding iss shown in the figure below:" - > "The encoding is shown in the figure below:"
[Sue]: Fixed in -21

  *   In page 32, section 3.3.3, the length field displayed in the schema is not the same as the length field described in the text (6 bits vs. 8 bits).
[Sue]: Fixed in -21

  *   In page 32, section 3.3.3, "The IPsec SA Rekey SubTLV not help" - > "The IPsec SA Rekey SubTLV does not help"
[Sue]: Fixed in -21

  *   In page 33, section 3.3.4, "The encoding iss shown in the figure below:" - > "The encoding is shown in the figure below:"
[Sue]: Fixed in -21

  *   In page 33, section 3.3.4, is the Transform Attr Length necessary?
[Sue]: yes.   See technical NIT-6 above.

  *   In page 34, section 3.3.4, "The procedures for" - > "The procedure for"
Sue: Fixed in -21

  *   In page 34, section 3.3.5, in Figure 15, the second line of the header should not be in the figure.
Sue: Fixed in -21

  *   In page 36, section 3.3.5, "A Simplified IPsec SA Sub-TLVis a MALFORMED" - > "A Simplified IPsec SA Sub-TLV is a MALFORMED"
Sue: Fixed in -21

  *   In page 36, section 3.3.6, "Provides information related IPsec and NATs" - > "Provides information related to IPsec and NATs"
Sue: Fixed in -21

  *   In page 37, section 3.3.6, the descriptions of the header fields is not formatted in the same way as in the previous sections.
Sue: Could you be more specific? It has the same top level fields (SubTLV Name, subTLV code, SubTLV description, SubTLV encoding, SubTLV Error handling, Tunnel End Point validation).  I suspect you mean the "~" for continuation of bytes.  I've removed these.


  *   In page 42, section 3.4.1, "validation in section 6, 8, and 13 applied to the NextHop." - > "validation in sections 6, 8, and 13 applied to the NextHop."
Sue: Thank you.  Fixed.

  *   In page 45, section 3.5.2.2, "Subsequent subTLV ware ignored and not propagated." - > "Subsequent subTLV are ignored and not propagated."
Sue: Thank you fixed in -21

  *   In page 46, section 3.6.1, "The SD-WAN client routes associates some of" - > "The SD-WAN client routes associate some of"
Sue: Thank you. Fixed in -21

  *   In page 47, section 3.6.1, "Invalid tunnel type must be treat if the TLV was not present." - > "Invalid tunnel type must be treated if the TLV was not present."
Sue: Thank you, Fixed in -21.

  *   In page 47, section 3.6.1, ", but propogated." - > ", but propagated."
Sue: Thank you, Fixed in -21.

  *   In page 47, section 3.6.1, "For SD-WAN NLRI underlay routes, the the Extended Port subTLV" - > "For SD-WAN NLRI underlay routes, the Extended Port subTLV"
Sue: Thank you, Fixed in -21.

  *   In page 47, section 3.6.1, "If multiple instancs of the IPsec nonce," - > "If multiple instances of the IPsec nonce,"
Sue: Thank you, Fixed in -21.

  *   In page 48, section 3.6.2, "reception of an type value outside" - > "reception of a type value outside"
Sue:  Thank you, Fixed in -21

  *   In page 48, section 3.6.2, "Local configuration and policy must be carefully constrain" - > "Local configuration and policy must carefully constrain"
Sue: Thank you, Fixed in -21

  *   In page 48, section 3.6.2, "IPsec security associations in to create a" - > "IPsec security associations to create a"
Sue: Thank you, Fixed in -21

  *   In page 48, section 3.6.3, "The SD-WAN NLRI should not be paired with Encapsulation Extended" - > "The SD-WAN NLRI should not be paired with an Encapsulation Extended"
Sue: Thank you, Fixed in -21.

  *   In page 48, section 3.6.2, "If an SD-WAN NLRI is paried Encapsulation Extended" - > "If an SD-WAN NLRI is paired with and Encapsulation Extended"
Sue: Thank you, Fixed in -21.

  *   In page 49, section 4.1, "It is critical that the Hybrid SD-WAN Tunnel have correctly forward traffic" - > "It is critical that the Hybrid SD-WAN Tunnel correctly forwards traffic"
Sue: Thank you. Fixed in -21.

  *   In page 49, section 4.2, "IPsec mechansims require" - > "IPsec mechanisms require"
Sue: Thank you. Fixed in -21

  *   In page 50, section 4.2.1, "The BGP Peer's need to send the IPSec" - > "The BGP Peer needs to send the IPSec"
Sue: Fixed in -21 in slightly different way.
Best regards,

Antoine Fressancourt


From: Antoine FRESSANCOURT <antoine.fressancourt=[e-mail]<mailto:antoine.fressancourt=[e-mail]>>
Sent: jeudi 16 janvier 2025 09:15
To: Susan Hares <[e-mail]<mailto:[e-mail]>>; Antoine FRESSANCOURT <[e-mail]<mailto:[e-mail]>>; Antoine Fressancourt <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; John Scudder <[e-mail]<mailto:[e-mail]>>
Subject: RE: Intdir early review of draft-ietf-idr-sdwan-edge-discovery-17

Hello Susan,

Sorry for my delay in answering. I will review the changes you have made by the end of this week, and answer you about those changes.

Best regards,

Antoine Fressancourt

From: Susan Hares <[e-mail]<mailto:[e-mail]>>
Sent: mercredi 15 janvier 2025 20:36
To: Antoine FRESSANCOURT <antoine.fressancourt=[e-mail]<mailto:antoine.fressancourt=[e-mail]>>; Antoine Fressancourt <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; John Scudder <[e-mail]<mailto:[e-mail]>>
Subject: RE: Intdir early review of draft-ietf-idr-sdwan-edge-discovery-17

Antoine:

Thank you again for your excellent review of draft-ietf-idr-sdwan-edge-discovery-17.  I believe that the revisions in -20 of the draft address all your concerns.

Would you please review draft-ietf-idr-sdwan-edge-discovery-20.

Thank you,

Susan Hares

From: Antoine FRESSANCOURT <antoine.fressancourt=[e-mail]<mailto:antoine.fressancourt=[e-mail]>>
Sent: Wednesday, November 20, 2024 10:26 AM
To: Susan Hares <[e-mail]<mailto:[e-mail]>>; Antoine Fressancourt <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Cc: [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; John Scudder <[e-mail]<mailto:[e-mail]>>
Subject: RE: Intdir early review of draft-ietf-idr-sdwan-edge-discovery-17



Hello,



Thanks for your answers to the comments I made in the review of the document.



I will answer some requests for clarification and discuss some of your answers inline with the text of your previous message. My comments and answers are prefixed with [AFT]. I stripped some text that was not necessary to understand the discussion from this answer.



Best regards,

--

Antoine



[....]







[Discuss-1]



* In section 3.3.1, two types of encoding for the Client Route UPDATE messages



are given, "Barebones" and Tunnel Encaps Attribute, while they are described



only in section 5.







[Sue:]  RFC9012 defines two forms of a Tunnel Encapsulation attribute:



1: An Extended Community with just the Tunnel ID defined in RFC9012 as "Barebones",



2: a Tunnel Encapsulation Attribute.



So, are you asking for these definitions to be reiterated in the definitions?







[AFT]



In the text, I think that the one paragraph description that basically states what you write in your answer should be placed before the first use or presentation of BGP messages using either encoding. A more detailed description can be avoided with a reference to RFC 9012, but for the sake of clarity, I think positioning this simplified definition before section 3.3.1 is better.



----







[Discuss-2]



* In section 3.4, the behavior of the RR mentions authorized BGP peers, but the



document does not give an idea about where and how those authorized BGP peers



are set.







[Sue:]  Most BGP drafts concerning RR authorized peers assume local configuration specifies



  1.  peer address, and b) some secure link.  Are you asking that "authorized BGP peers" be



defined prior to the use of the term in section 3.4.







[AFT]



Providing the explanation you give as an answer to this point in the text would make the text clearer in my view. Besides, if this behavior is clearly explained in another document, I think it would be interesting to point to this document as an informative reference.



----







[Discuss-3]



* In section 4.2, the text mentions that the RR speaks to the BGP clients over



IPSec while section 3.4 mentions a secure transport session (e.g. TLS)







Sue: BGP works over Transport layer.  Contrary to the actual specification



many implementations use TLS or TCP over IP-SEC  or other means of securing the text.



Would adding this level of detail to section 3.4 resolve this "DISCUSS" level issue?



Or are you looking for something else?







[AFT]



My remark stems from an inconsistency between the text in sections 3.4 and 4.2. For the sake of clarifying, I think that you should clearly state the assumptions you are making about the security and integrity of the channel you are using to convey the BGP messages, and point to the documents describing the secure transport mechanisms you would use.



----







[Discuss-4]



* In section 8, the text refers to the IPSec-SA-ID, and assumes it has been set



but very little details are given in the document as to how this ID is set,



negotiated or provisioned in the IPSec SA endpoints. References are made to



other RFCs, but the text would benefit from a description of the mechanism the



authors are considering using.







[Sue-2]   First of all, BGP is not setting up the IPsec tunnel.  It is only passing parameters so



the tunnel can be set-up.   Second, if it is helpful to provide a description of the mechanism,



it can be added.  However, it seems you have something in mind.  Would you mind providing



4 bullet points on what you'd like added.







[AFT]



The only thing I have in mind is the objective to make the document's text clear enough to allow implementers to develop interoperable implementations