# 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. ## 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. - 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]. - 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 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ ~~~~ - 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. ## Minor - I suggest enhancing the Introduction by summarizing https://www.rfc-editor.org/rfc/rfc8821.html#section-3 - 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. - 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. - 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 ~~~~ - Section 7.2; - Please expand ETTL - Suggest to change Reserved to a Flag field and corresponding new IANA registry to allow easy extensibility - 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. - Section 7.3; Should route priority be called route preference instead? Please check. - Section 7.4; Please add an error code when the prefix received is already being advertised on some BGP session. - 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. - 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. - 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." ## 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... Regards, Dhruv 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 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 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
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce