I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at . Document: draft-ietf-teas-rsvp-egress-protection-09 Reviewer: Stewart Bryant Review Date: 2018-02-19 IETF LC End Date: 2018-02-27 IESG Telechat date: 2018-03-08 Summary: I think that there are a number of issues that need to be looked at in detail before this is ready. It could be that a number of these are assumed knowledge of the subject area, but that is not the case for all of them. Section 6, the example, explains how this works and the motivation. It is very abbreviated and I am unclear about how the solution fits together. This needs clarification and then the rest of the text needs checking against that. There are a lot of cases where terms and abbreviations are used before definition. Major issues: Section 6 needs a re-write for clarity. As written it looks like a number of slides were just transcribed int the text rather than taking care to write for clarity. The section makes a number of assumptions that really should be spelled out. I think that it needs to be clear that you are protecting the egress nodes, but that as far as I can see you are vulnerable to egress link failure. Certainly the example in section 6 does a BFD test between R3 and L1 which only tests L1 reachability. Related to which are you constraining BFD to run over the LSP? If it is running as regular IP I don't think that validates the LSP. In this text: This can be achieved by configuring a same local address on L1 and La, using the address as a destination of the LSP and BGP next hop for VPN traffic. SB> Don't you have to do cost management to get the right behaviour? SB> what if you really want to send to them individually, does this SB> confuse things? Minor issues: 1.1. Egress Local Protection Figure 1 shows an example of using backup LSPs to locally protect egresses of a primary P2MP LSP from ingress R1 to two egresses, SB> What is an egress in this context? ============= x01 (Egress local protection bit): It is set (1) to indicate an egress local protection. x02 (S2L sub LSP backup desired bit): It is set (1) to indicate S2L Sub LSP (ref to RFC 4875) is desired for protecting an egress of a P2MP LSP. SB> Maybe this is standard RSVP notation, but I find it confusing. SB> I assume that x01 means bit one of the E-Flags field rather than SB> set the flags field to be 0x01, but it is a strange notation. SB> Same for x02. SB> is bit 1 bit 28 or bit 31 of the longword? SB> S2L is not expanded in this document. The Reserved parts MUST set to zero. SB> It is more conventional to call the parts fields as you do below. ============== Nits/editorial comments: The final (third) subobject in the SERO contains the egress node of the backup LSP, i.e., the address of the backup egress node. SB> It is odd that the second item in the list is called the third SB> and vice versa ============ The Type of an IPv4 primary egress subobject is 1, and the body of the subobject is given below: SB> This really ought to be tied in with the IANA registry SB> Same with the similar IPv6 object, and the other SB> two objects you register. ============ o Tunnel ID: A 16-bit identifier being constant over the life of the tunnel o Extended Tunnel ID: A 4-byte identifier being constant over the life of the tunnel SB> Tunnel ID really needs a reference. SB> Also there ought to be MBZ consistency in the descriptions used in the SB> document. =========== If S2L Sub LSP backup is desired to protect a primary egress of a SB> I am not sure S2L is defined before use. SB> Indeed the whole document could use a thorough scrub for used before defined terms. SB> UA, PLR and Resv are another examples, but these are only the ones I noticed. The SB> draft needs a systematic review of this to save the RFC Editor doing it and asking about them.