Skip to main content

Last Call Review of draft-ietf-teas-rsvp-egress-protection-09
review-ietf-teas-rsvp-egress-protection-09-genart-lc-bryant-2018-02-19-00

Request Review of draft-ietf-teas-rsvp-egress-protection
Requested revision No specific revision (document currently at 16)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-02-27
Requested 2018-02-13
Authors Huaimo Chen , Autumn Liu , Tarek Saad , Fengman Xu , Lu Huang
I-D last updated 2018-02-19
Completed reviews Rtgdir Last Call review of -09 by Russ White (diff)
Genart Last Call review of -09 by Stewart Bryant (diff)
Secdir Last Call review of -09 by Rifaat Shekh-Yusef (diff)
Secdir Telechat review of -13 by Rifaat Shekh-Yusef (diff)
Genart Telechat review of -14 by Stewart Bryant (diff)
Assignment Reviewer Stewart Bryant
State Completed
Request Last Call review on draft-ietf-teas-rsvp-egress-protection by General Area Review Team (Gen-ART) Assigned
Reviewed revision 09 (document currently at 16)
Result On the Right Track
Completed 2018-02-19
review-ietf-teas-rsvp-egress-protection-09-genart-lc-bryant-2018-02-19-00
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

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

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.