Hi, Dhruv:
Thanks for your detail review！ We have updated the draft according to your suggestions, please check whether they address your concerns. The significant updates may be the followings: 1. Message flow procedures. I have updated the original table style to the flow chart style to reflect the time relationship between these messages. 2. BPI object encoding format to include the status field to reflect the BGP session status during the procedures. 3. Management Considerations. Detail responses are inline below. Best Regards Aijun Wang China Telecom 发件人: pce-boun...@ietf.org [mailto:pce-boun...@ietf.org] 代表 Dhruv Dhody 发送时间: 2023年8月31日 17:25 收件人: pce@ietf.org; draft-ietf-pce-pcep-extension-native...@ietf.org 主题: [Pce] Document Shepherd Review of draft-ietf-pce-pcep-extension-native-ip-25 # Document Shepherd Review of draft-ietf-pce-pcep-extension-native-ip-25 I have done a shepherd review of this I-D. I have some concerns that should be resolved before we send this to our AD. I continue to believe that this I-D is better suited as experimental; authors seem to disagree. 【WAJ】We would like to keep it in Standard track. ## Major - The use of the PCErr message to report an established BGP session as 'broken' is not right. The PCErr message is always sent in response to a message from a peer. We should use a PCRpt message with the status as 'down' in this case. Section 5.2 should also include the use of PCRpt messages during synchronization. 【WAJ】Have updated the BPI object encoding to include the “status” field, and uses it report the status of BGP session between BGP peers. - The I-D is silent on Native-IP path update procedures. I think this should be highlighted -- The EPR update is done as per the make-before-break procedures, i.e., the PCECC first updates new native-ip instructions based on the updated path and then informs the source node to switch traffic before cleaning up the former instructions as described in [RFC9050]. 【WAJ】 <https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip-25#section-6.2> https://datatracker.ietf.org/doc/html/draft-ietf-pce-pcep-extension-native-ip-25#section-6.2 has the corresponding descriptions: “In order to avoid the transient loop during the deploy of explicit 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 E2E path.” . And, based on your suggestions, add one more sentence in the end of this section：“If the PCE needs to update the path, it should first instruct new CCI with updated EPR corresponding to the new nexthop to use and then instruct the removal of older CCI” - Section 7.4, it is difficult to decode this object. All you have is the length of the full object from the object header and it is difficult to decode how many prefixes exist with additional TLV of variable length allowed. It is also unclear on the advantage of using RFC 3209 subobjects here since the mix of subobject types is anyway not allowed. I suggest changing this to - ~~~~ 0 1 2 3 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | Peer IPv4 Address | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | No. of Prefix | Reserved | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | IPv4 Prefix #1 | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |Prefix #1 Len | Reserved | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | : | | : | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | IPv4 Prefix #n | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |Prefix #n Len | Reserved | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // Additional TLVs // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ ~~~~ 【WAJ】Update the associated encoding according to your suggestions。 - Section 9, I am worried about this - ~~~~ When the PCE sends out the PCInitiate message with the BPI object embedded to establish the BGP session between the PCC peers, it should wait enough time to get the BGP session successful establishment report from the underlay PCCs. If the PCE can't get such report after the duration, then it should declare the failure of BGP session setup and try the establishment procedure again, or report the failure to the operator. ~~~~ I think you are doing this because you are expecting a PCRpt message to indicate BGP session establishment only in case of success and what error to use in other cases. This goes against the PCInitiate mechanism of RFC 8281. I suggest you add BGP session status in the BPI Object and allow a PCRpt message with status as in-progress, established, down, etc (or re-use LSP object status). Also, the BGP session must be explicitly cleared by the PCE. It is better if all CCIs (BPI/EPR/PPA) are explicitly cleared by the PCE as that would be aligned to the PCInitiate/PCECC way. 【WAJ】Add BGP session status in BPI Object, and change the description to the below: “When the PCE sends out the PCInitiate message with BPI object embedded to establish the BGP session between the PCC peers, PCC should report the BGP session status. For instance, the PCC could respond with "BGP Session Establishment In Progress" initially and on session establishment send another PCRpt message with state updated to "BGP Session Established". If there is any error during the BGP session establishment, the PCC should indicate the reason with the appropriate status value set in the BPI object.” ## Minor - I suggest enhancing the Introduction by summarizing <https://www.rfc-editor.org/rfc/rfc8821.html#section-3> https://www.rfc-editor.org/rfc/rfc8821.html#section-3 【WAJ】Add the following description in the end of Introduction part: “It requires only the PCE send the instructions to the PCCs, to build multiple BGP sessions, distribute different prefixes on the established BGP sessions and assign the different paths to the BGP nexthops.” - The role of the symbolic path name should be more clearly stated (currently it is mentioned in passing in 5.1). It looks like you expect the same symbolic path name (and PLSP-ID(?)) to be used in all related messages from the BGP session to the explicit route to prefix advertisement. This requirement needs to be made normative. 【WAJ】The symbolic path name should be used in all related messages. Add the following sentence in section 5.1: “It MUST be set the same value with the related” - In section 6.3, please state clearly that the prefixes encoded are advertised on the corresponding BGP session with reference to the relevant BGP RFC. 【WAJ】Changed the sentence to “…the PCC should send the prefixes indicated in this object to the appointed BGP peer via the corresponding BGP session[RFC4271]” - Section 7.2; - Please change the below text to MUST. Since you already have error codes associated with this. It makes complete sense for this to be a MUST. It would also be wise to move this to Section 6.1 ~~~~ OLD: 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 non PCE initiated configuration. NEW: 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. END ~~~~ 【WAJ】Done - Section 7.2; - Please expand ETTL 【WAJ】Expand. EBGP Time To Live, to indicate the multihop count for EBGP session. - Suggest to change Reserved to a Flag field and corresponding new IANA registry to allow easy extensibility 【WAJ】Done - When the T bit is cleared, what happens to the Tunnel source and destination IP? If they are not needed make them optional and let them be included only if T bit is set. Also, you should explain where IP in IP is used. 【WAJ】Remove the Tunnel source and destination IP fields, use the Local/Peer IP instead. Add the sentence: “When T bit is set, the traffic should be sent in IPinIP tunnel(Tunnel source is Local IP Address, tunnel destination is Peer IP Address). When T bit is cleared, the traffic is sent via its original source and destination address.”and add one more sentence along the definition of T bit: “The tunnel mode(T bit is set) is used when the operator want to assure only the traffic from the specified (source, destination) pair, the raw mode(T bit is clear) is used when the operator want to assure traffic from any source to the specified destination.” - Section 7.3; Should route priority be called route preference instead? Please check. 【WAJ】No. Route preference is used for different routing protocols. Here “route priority”is used to compare the priorities of explicit routes - Section 7.4; Please add an error code when the prefix received is already being advertised on some BGP session. 【WAJ】It is unnecessary. The PCC will select the optimize path that by the EPR object, and use other path as backup(secondary path). - Section 10; I think we can be explicit in saying that by using unique peer IP addresses that are not used for any other purpose we avoid making an impact on the other routes and routing system. 【WAJ】We have expressed such information in updated section 6.1 - I am not a fan of the message table (1-4) approach taken by this I-D. The better and more consistent way would be to use the same approach of event diagrams as RFC 9050 and past RFCs from this WG. It would show the timing relationship between these messages which is currently missing. But it is up to the authors to decide. If you keep the table, consider ordering tables 2 and 3 in the correct order. 【WAJ】Have updated the message table to the RFC 9050 style information flow chart, to reflect the timing relationship between these messages. - I suggest following RFC 6123 and including a crisp manageability consideration highlighting the key distinctions of Native IP operations. Currently, the I-D says "Manageability considerations that are described in [RFC9050] should be followed." 【WAJ】Done ## Nits - I have made edits directly in the XML, please review and accept if you agree with the changes - Expanded PCEP in the title - Various grammar errors (suggest using tools like Grammarly) - Replace head with source and end with destination. - Various others... 【WAJ】Thanks！ Regards, Dhruv XML: <https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.xml> https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.xml TXT: <https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.txt> https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.txt DIFF: <https://author-tools.ietf.org/diff?doc_1=draft-ietf-pce-pcep-extension-native-ip-25&url_2=https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.txt> https://author-tools.ietf.org/diff?doc_1=draft-ietf-pce-pcep-extension-native-ip-25&url_2=https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-native-ip-26.txt PS. In case of formatting issues, you can find this review at <https://notes.ietf.org/draft-ietf-pce-pcep-extension-native-ip-25?view> https://notes.ietf.org/draft-ietf-pce-pcep-extension-native-ip-25?view
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce
