Hi, John: I have updated draft according to your suggestions at https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip For the document track, I think it is OK to move it forward as the experimental track, we will try to update it later to the standard track after its experimental deployment. The detail responses are inline below【WAJ】
If there is more suggestions, please let us know. Or else, can we schedule the IESG Last Call then? Best Regards Aijun Wang China Telecom -----邮件原件----- 发件人: [email protected] [mailto:[email protected]] 代表 【外部账号】 John Scudder 发送时间: 2024年7月20日 4:35 收件人: [email protected]; [email protected] 抄送: [email protected]; [email protected]; [email protected]; Aijun Wang <[email protected]>; [email protected] 主题: AD review of draft-ietf-pce-pcep-extension-native-ip-30 Hi Authors, Working Group, Thanks for this document and your patience while I worked through it. I have several comments below, in line with the text according to my usual practice -- I’ve supplied my questions and comments in the form of an edited copy of the draft. Questions and comments are made in-line and prefixed with “jgs:”. You can use your favorite diff tool to review them; I’ve attached the iddiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case you want to use it for in-line reply. I also have one general concern to bring up here. My general concern is that it will be hard to move forward as a Standards Track document, without significant updates. I don't want to put you through that unless you are eager to put in the extra work. By contrast, I think the document is nearly ready for publication as an Experimental document. Reviewing the Shepherd write-up, it appears this proposal has already been discussed, and I want to suggest it again as a practical, lower-effort, way forward. Two examples of aspects of the document that would need work before it's ready for publication on the Standards Track are its security posture, and its precision of description of elements of procedure. Regarding the security posture, I've put specific suggestions in the Security Considerations that would bring it up to (what I consider) an acceptable level for an Experimental document. See those suggestions for an outline of work that would have to be completed before ready for Standards Track. Regarding elements of procedure, there are various points in the document that provide quick hints about how the protocol should be used. One example is in Section 5.1 (chosen only because I happened to have it open on my screen): "To cleanup the existing Native IP instructions, the SRP object MUST set the R (remove) bit." This is probably clear enough to experienced PCEP and BGP practitioners, and so I judge it adequate for an experimental document. For a Standards Track document, I would want you to provide more detail about exactly what you mean, that doesn't assume as much expertise. I’ve flagged a few others in the document as I've reviewed it, but not comprehensively. If you decide to stick with Standards Track I'd need to do a second pass to provide this level of critique. One sticking point about moving to Experimental would be the allocation you make from the PCECC-CAPABILITY sub-TLV, which has a Standards Action policy. I would propose to remedy this by asking the WG to write and progress a tiny draft to reclassify that sub-registry suitably, for example to IETF Review. Note that this wouldn't need to hold up progress, since there is already a temporary allocation, which can be extended if there is adopted work to reclassify the registry. (The WG might consider at the same time, whether to reclassify some of the other Standards Action sub-registries, and I’ve suggested you consider whether you want to make your own new sub-registries SA or if IETF Review might be better). Finally, I want to flag to the WG as a whole my suggestion to add a new boilerplate section to be used with all PCE docs that use RBNF, going forward (see first comment in my review). Regards, —John --- draft-ietf-pce-pcep-extension-native-ip-30.txt 2024-07-18 15:55:14 +++ draft-ietf-pce-pcep-extension-native-ip-30-jgs-comments.txt 2024-07-19 16:28:15 @@ -159,8 +159,25 @@ 14 [RFC2119] [RFC8174] when, and only when, they appear in all capitals, as shown here. ++--- +jgs: (For further context see my comment on Section 5.1.) +I propose that from now on, PCE documents that use RBNF should include +a statement to this effect: + +``` +2.1. Use of RBNF + +The message formats in this document are illustrated using Routing +Backus-Naur Form (RBNF) encoding, as specified in [RFC5511]. The use of +RBNF is illustrative only and may elide certain important details; the +normative specification of messages is found in the prose description. +If there is any divergence between the RBNF and the prose, the prose is +considered authoritative. +``` +Whether you adopt this or not, you need a normative reference to RFC 5511. ++--- 【WAJ】Have added the section about RBNF as you recommended. @@ -192,6 +209,12 @@ * PCECC: PCE as a Central Controller * RR: Route Reflector ++--- +jgs: please consider adding definitions of CCI and SRP here. I know you +define them elsewhere, but it's useful to have a consolidated quick +reference for first-time readers like myself. Also, please alphabetize +the terminology list. ++--- 【WAJ】Done 4. Capability Advertisement @@ -320,7 +343,7 @@ are as per [RFC8281]. The LSP and SRP objects are defined in [RFC8231]. - + When PCInitiate message is used for Native IP instructions, the SRP, LSP and CCI objects MUST be present. The error handling for missing SRP, LSP or CCI object is as per [RFC9050]. Further only one object @@ -330,9 +353,57 @@ by the PCE/PCC to uniquely identify the E2E native IP TE path. The related Native-IP instructions with BPI, EPR and PPA object are identified by the same Symbolic Path Name. + ++-- +jgs: The restrictions and requirements on the use of BPI, EPR, and PPA +aren't clearly stated. I assume what you intend is: +- One and only one of these objects MUST be present if accompanying CCI object type 2. +- All of these objects SHOULD be absent if accompanying any other CCI + object type. If present they MUST be ignored. This is very much a +guess, + but I couldn't see any reasonable use for a BGP Peer Info object in + conjunction with an MPLS Label CCI. +A suggested update to the paragraph is, + +NEW: + When PCInitiate message is used for Native IP instructions, + i.e. when CCI Object-Type is 2, + the SRP, + LSP and CCI objects MUST be present. The error handling for missing + SRP, LSP or CCI object is as per [RFC9050]. + Furthermore, one, and only one, object + among BPI, EPR, or PPA object MUST be present. The PLSP-ID and + Symbolic Path Name TLV are set as per the existing rules in + [RFC8231], [RFC8281], and [RFC9050]. The Symbolic Path Name is used + by the PCE/PCC to uniquely identify the E2E native IP TE path. The + related Native-IP instructions with BPI, EPR or PPA object are + identified by the same Symbolic Path Name. + +Note that in the second-to-last line, I changed "and" to "or" in +addition to my other changes. 【WAJ】Done. Adopt your suggestions. + +See also second suggested paragraph, following. + +(The following comment provides context for my earlier suggestion to +add a Section 2.1) + +Also, it makes me sad that the RBNF doesn't express any of these +restrictions. To me, this makes the RBNF almost worse than useless -- +the use of a formal language makes it seem as though the productions +will accurately capture what's legal and required in the protocol. But +it seems this is not so. In a side conversation with the shepherd +(Dhruv) he told me that this is known and expected for PCE documents, +and that the WG's expectation is that the RBNF is illustrative, not +normative. That being so, I don't insist that we change the practice +now, but I do think there should be a disclaimer to help others not +fall into the same error I did though (I had thought the RBNF was +wrong, because it didn't capture the restrictions faithfully). ++-- + + + Wang, et al. Expires 5 August 2024 [Page 6] Internet-Draft PCEP for Native IP February 2024 @@ -345,7 +416,20 @@ receiving PCC MUST send a PCErr message with Error-type=19 (Invalid Operation) and Error-value=22 (Only one BPI, EPR or PPA object can be included in this message). + ++--- +jgs: to complete the suggestion, +NEW: + When the PCInitiate message is not used for Native IP instructions, + i.e. when CCI Object-Type is not equal to 2, the SRP, LSP, and CCI + objects SHOULD NOT be present. If present, they MUST be ignored by + the receiver. + +Note this is just a straw man suggestion, I'm not at all sure the +suggested behavior is correct, I just wanted to provide a starting point. 【WAJ】No, The above suggestions are not necessary. The SRP, LSP, CCI Object may be also presented or not in situation than the CCI Object-Type is equal to 2. These objects are defined by other documents, and their usages are out of scope of this document. Actually, the proposed suggestion should be: When the PCInitiate message is not used for Native IP instructions, i.e. when CCI Object-Type is not equal to 2, the BPI,EPR, and PPA objects SHOULD NOT be present. If present, they MUST be ignored by the receiver. ++--- + To cleanup the existing Native IP instructions, the SRP object MUST set the R (remove) bit. @@ -384,6 +468,12 @@ The error handling for missing CCI object is as per [RFC9050]. Further only one object among BPI, EPR, or PPA object MUST be present. + ++--- +jgs: Similar comments to the ones I made for Section 5.1 apply here. +I'm not going to propose a rewrite here. Once we close on Section 5.1, +a similar rewrite should be done here. ++--- 【WAJ】Done @@ -435,6 +525,10 @@ SRP object) in PCInitiate message, the PCC should try to establish the BGP session with the indicated Peer as per AS and Local/Peer IP address. ++--- +jgs: Above, do you mean "... indicated Peer AS" (i.e., remove "as per")? +If that's not what you mean, please explain. ++--- 【WAJ】Here, "as per" just mean for different tuple(AS, Local IP, Peer IP), there will be different BGP session be established. During the establishment procedure, PCC should report to the PCE the status of the BGP session via the PCRpt message, with the status @@ -453,7 +547,16 @@ When PCC receives this message with the R bit set to 1 in SRP object in PCInitiate message, the PCC should clear the BGP session that is indicated by the BPI object. ++--- +jgs: The term "clear the BGP session" isn't standard (although it is +common colloquial usage). Looking at RFC 4271, in one place (Section 3) +it does talk about "resetting any BGP connections" so I think you would +be on firm ground if you want to say "reset". Or you might consider +referencing the AutomaticStop event (RFC 4271, Section 8.1.2, Event 8). 【WAJ】： Here, “clear the BGP session” is just want to express the deletion of the BGP configuration on PCC, not reset the BGP connection. Should it be more clearer, if we instead say "clear the BGP session configuration"?------Have updated the descriptions accordingly. +(Same point applies in the following line.) ++--- + When PCC clears successfully the specified BGP session, it should report the result via the PCRpt message, with the BPI object included, and the corresponding SRP and CCI objects. 【WAJ】Have updated the descriptions accordingly. @@ -574,6 +677,10 @@ The explicit route establishment procedures can be used to install a route via PCE on the PCC, using PCInitiate and PCRpt message pair. ++--- +jgs: Should the above be "... via PCEP"? Or, "... can be used by the +PCE to install a route on the PCC"? 【WAJ】"...can be used by the PCE to install a route on the PCC" is more accurate. Have updated the description accordingly. ++--- Such explicit routes operate the same as static routes installed by network management protocols (Network Configuration Protocol (NETCONF)/YANG). The procedures of such explicit route addition and @@ -582,6 +689,9 @@ The PCInitiate message should be sent to the on-path routers respectively. In the example, for explicit route from R1 to R7, the ++--- +jgs: What does “respectively” mean here? Can it be removed? 【WAJ】Here, we just want to describe such message should be sent every router on the path, each may has different content, for example, the different next hop information. ++--- PCInitiate message should be sent to R1, R2 and R4, as shown in Figure 3. For explicit route from R7 to R1, the PCInitiate message should be sent to R7, R4 and R2, as shown in Figure 5. @@ -589,6 +699,12 @@ When PCC receives the EPR and the CCI object (with the R bit set to 0 in SRP object) in PCInitiate message, the PCC should install the explicit route in the RIB/FIB to the peer. ++--- +jgs: What is “the RIB/FIB to the peer”? What does it mean for a RIB/FIB +to be “to the peer”? Should this be rewritten as “PCC should install +the explicit route to the peer in the RIB/FIB”, as hinted by the +following sentence? ++--- 【WAJ】Yes. Your suggestion is more clear. Have updated the content accordingly. When PCC install successfully the explicit route to the peer, it should report the result via the PCRpt messages, with EPR object and @@ -597,7 +713,15 @@ When PCC receives the EPR and the CCI object with the R bit set to 1 in SRP object in PCInitiate message, the PCC should clear the explicit route to the peer that is indicated by the EPR object. ++--- +jgs: A little bit like my earlier objection to the use of "clear" in +reference to a BGP session, I wonder if you can be more specific about +what you mean by the use of the word "clear" here. I think probably you +mean, remove that route from the RIB/FIB, right? 【WAJ】Yes. "Remove" is more clear. Have updated the contents accordingly. +Also applies to the following line. ++--- + When PCC has cleared the explicit route that is indicated by this object, it should report the result via the PCRpt message, with the EPR object included, and the corresponding SRP and CCI object. @@ -734,6 +858,15 @@ peer route, the EPR object should be sent to the PCCs in the reverse order of the E2E path. To remove the explicit peer route, the EPR object should be sent to the PCCs in the same order of the E2E path. ++--- 【WAJ】Done +jgs: If this remains targeted to the Standards Track, I would ask you +to add consideration of whether installation has to complete before the +next one is initiated. It depends on exactly how strong a guarantee of +"avoid the transient loop" you to provide. + +(This is just an example of the kind of detail I would ask for, I +haven't done a full review for this level of detail.) ++--- 【WAJ】The PCE can guarantee the installation should be complete before the next one is initiated., for example, after the PCE receives the successful report on one node, then it initiate the installation on the next node. To accomplish ECMP effects, the PCE can send multiple EPR/CCI objects to the same node, with the same route priority and peer address value @@ -743,6 +876,12 @@ case of failure, the PCC SHOULD send the corresponding error via PCErr message, with an error information: Error-type=33 (Native IP TE failure), Error-value=3 (Explicit Peer Route Error). ++--- +jgs: Can the SHOULD above be a MUST? This same comment applies to all +the other places you use SHOULD in connection with error reporting. If +not, can you give me some idea of what circumstances would be +reasonable for an implementor to ignore the SHOULD and not report an error? ++--- 【WAJ】 Replace "SHOULD" with "MUST" When the peer info is not the same as the peer info that is indicated in the BPI object in PCC for the same path that is identified by @@ -828,6 +967,10 @@ | | Figure 8: Message Information and Procedures ++--- +jgs: Figure 7 and Figure 8 have no corresponding body text describing +them. It would be nice to have at least a sentence or two. ++--- 【WAJ】 Done. Add the corresponding description part. The AFI/SAFI for the corresponding BGP session should match the Peer Prefix Advertisement Object-Type, AFI/SAFI should be 1/1 for IPv4 @@ -874,6 +1017,16 @@ The selection of Raw mode and Tunnel mode forwarding strategy are controlled via the "T" bit in BPI Object that is defined in Section 7.2 ++--- +jgs: This is another place where I have a hard time imagining the +Standards Track bar would be met. I think it's likely that questions +would be raised in later review, about why you specify IP-in-IP mode +only, without providing for any of the other popular tunnel types. + 【WAJ】：We just want to simplify the overall parameters negotiation between the PCCs(and PCE), and select the simplest and wide adoption tunnel mode. I have added one sentence, as "For simplicity, the IPinIP tunnel type is used between the BGP peer by default." +Given that you do only support IP-in-IP, it would be a good idea to say +a few words about that here, instead of leaving reviewers in suspense +until they reach Section 7.2. ++--- 6.5. Cleanup @@ -967,7 +1120,16 @@ By default, the Local/Peer IP address SHOULD be dedicated to the usage of native IP TE solution, and SHOULD NOT be used by other BGP sessions that established by manual or other configuration mechanism. ++--- +jgs: Section 6.1 says, "The Local/Peer IP address MUST be dedicated to +the usage of native IP TE solution, and MUST NOT be used by other BGP +sessions that established by manual or other ways." Note the MUST. +Please rewrite one or the other to be consistent -- either it's a MUST +and this section should be fixed, or it's a SHOULD and Section 6.1 (and +possibly elsewhere) needs to be fixed. ++--- 【WAJ】Update the "SHOULD" with "MUST" + BGP Peer Info Object-Class is 46 BGP Peer Info Object-Type is 1 for IPv4 and 2 for IPv6 @@ -1071,6 +1233,10 @@ - 2: Peer IP can't be reached, BGP Session Failure - 3-255: Reserved ++--- +jgs: Shouldn't you have a generic error code as well, e.g. "0: Generic +error", to catch cases other than the ones described by 1 and 2? ++--- 【WAJ】 What we thought is the following, that if there is some new error that lets to the BGP Session Failure, we should define exactly the code from the reserved range. The "generic" error gives no more detail indication of the error reason. Is it right? Flag: 1 Byte. @@ -1104,6 +1270,12 @@ establish the BGP session with the peer in AFI/SAFI=2/1. 7.3. Explicit Peer Route Object ++--- +jgs: you describe this object as "peer route", but isn't it just a +generic host route, in your architecture you happen to use for BGP +peers? It seems to me it would be a more accurate description if you +replaced the word "Peer" with "Host" throughout this section. ++--- 【WAJ】: Here, the "Peer Route" just wants to emphasize the route is to the peer, although actually is a host route( to the peer). Describe it solely only with "Host" can't have such refer and maybe mislead to the reader? We would like to keep it in this way? The Explicit Peer Route object is defined to specify the explicit peer route to the corresponding peer address on each device that is @@ -1189,7 +1361,18 @@ establishment. No TLVs are currently defined. 7.4. Peer Prefix Advertisement Object ++--- +jgs: It appears there is an assumption that IPv4 routes will be sent +over an IPv4 peering, and IPv6 routes will be sent over an IPv6 peering. +This might be problematic especially for IPv4 routes, if the provider +network doesn’t use IPv4 in the underlay and uses tunnel mode to carry +IPv4 traffic across an IPv6 backbone. +Well this restriction doesn't seem necessary to me, I think it would be +OK to flag it without correcting, if you switch to the Experimental +track. ++--- 【WAJ】It is possible to mix the carrier's transport address family with different address family of the actual traffic. And, again, we want to simplify the parameter negotiations between the PCCs(and PCE), then solidify the encoding as IPv4 traffic is carried by IPv4 transport, and the same as for IPv6 address family. Anyway, the devices within the operator network all support such behavior, but not all of them support the hybrid comination. + The Peer Prefix Advertisement object is defined to specify the IP prefixes that should be advertised to the corresponding peer. This object should only be included and sent to the source/destination @@ -1576,6 +1759,13 @@ BGP. Section 9 describes in detail the considerations regarding to the BGP. During BGP session establishment, implementation MUST NOT allow the use local/remote IP address already sent in the BPI object. ++--- +jgs: what does the final sentence above mean? It's completely unclear +to me, although I'm guessing your aim is to enforce the restriction +that, "The Local/Peer IP address MUST be dedicated to the usage of +native IP TE solution, and MUST NOT be used by other BGP sessions that +established by manual or other ways." ++--- 【WAJ】Yes. Have updated the contents as you suggested. 11.6. Impact on Network Operations @@ -1637,7 +1827,48 @@ validity of the PCE and ensure a secure communication channel between them. Thus, the mechanisms described in [RFC8253] and [RFC9050] should be used. ++--- +jgs: I appreciate that you are trying to provide bare-bones BGP session +establishment here, and I see the text in Section 9 that says, + This document defines the procedures and objects to create the BGP + sessions and advertise the associated prefixes dynamically. Only the + key information, for example peer IP addresses, peer AS number are + exchanged via the PCEP protocol. Other parameters that are needed + for the BGP session setup should be derived from their default + values. + +but your design makes it impossible to provide transport security, such +as TCP-AO, because although there is a way to tell the PCC what its BGP +peer is, there is no way to tell the PCC what key to use in +communicating with that peer. In the case of session keying, it's not +reasonable to suggest the key should be "derived from their default +values". Even if the practice of using a single default key for all +internal sessions is used (and I'm not saying that would be a best +practice!), this simply can't work for EBGP, and you do propose to +cover EBGP. + +If you do switch to the Experimental track, in my opinion, something +like the following would be adequate: + +NEW: + Because this specification does not provide a way to communicate + properties beyond peer address and AS number for the BGP sessions + that are established, it will not always be possible to follow best + practices as described in [BCP194], if suitable default values as + discussed in [Section 9] cannot be used. An example would be keying + for use with TCP-AO [RFC5925]. + + If such functionality is required in the future, it can be provided + through the addition of optional TLVs to the BGP Peer Info object, + that convey the necessary additional information (for example, a key + chain [RFC8177] name). + +Note, it's just my opinion that this would be good enough, other +reviewers might have their own thoughts (notably SECDIR and the SEC +ADs). ++--- 【WAJ】：Yes, we plan to add additional TLV to convey the information about the secure of the BGP session. I think your recommendation is appropriate for the security considerations. I have adopted part of your recommendation text as the followings: If suitable default values as discussed in section 9 isn't enough and securing the BGP transport is required(for example, the TCP-AO(RFC5925), it can be provided through the addition of optional TLV to the BGP Peer Info object that convey the necessary additional information(for example, a key chain(RFC8177) name + 14. IANA Considerations 14.1. Path Setup Type Registry @@ -1746,6 +1977,12 @@ IANA is requested to create a new subregistry to manage the Flag field of the new CCI Object called "CCI Object Flag Field for Native- IP". New values are to be assigned by Standards Action [RFC8126]. ++--- +jgs: in this and the following subsections, you might want to consider +whether you really want Standards Action (requires a Standards Track or +BCP RFC) or if IETF Review might be better for you (any IETF RFC is OK, +including Experimental, Informational, etc). ++--- 【WAJ】OK. It will be more relax for further proposal. Each bit should be tracked with the following qualities: bit number (counting from bit 0 as the most significant bit) _______________________________________________ Pce mailing list -- [email protected] To unsubscribe send an email to [email protected]
