Re: [Gen-art] Genart last call review of draft-ietf-teas-rsvp-egress-protection-09

[Huaimo Chen <[e-mail]>]

Hi Stewart,

    Thank you very much for your comments.
    Answers to your comments are inline below with prefix [HC].
    Would you mind reviewing them to see if they address the issues?

Best Regards,
Huaimo
-----Original Message-----
From: Stewart Bryant [mailto:[e-mail]] 
Sent: Monday, February 19, 2018 7:06 AM
To: [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Genart last call review of draft-ietf-teas-rsvp-egress-protection-09

Reviewer: Stewart Bryant
Review result: On the Right Track

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.

[HC] We will add more details about the example in section 6 and check the 
rest with it.


There are a lot of cases where terms and abbreviations are used before definition.

[HC] We will define the terms and abbreviations before they are used.


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.

[HC] We will revise/re-write it with more details.


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.

[HC] We will update the document to make it clear that we are protecting the 
egress node. 
If egress node failure is determined, then the procedure for protecting the
egress node is triggered. If link failure and egress node alive are determined,
then link protection procedure is triggered. How to determine these is out of
scope for this document.


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?

[HC] Some cost management is needed. 
The cost to L1 need to be less than the cost to La
in order for the traffic to be sent to L1. In this context, L1 is a primary 
egress node of an LSP and La is the backup egress node which is used to
protect the primary egress node L1. Thus L1 and La (i.e., the primary egress
node and the backup egress node protecting the primary egress node) are 
different nodes and the VPN connected to L1 is not the same VPN to La 
(i.e., the same VPN is not connected to both L1 and La). 
In case that we want to the VPN traffic to be sent to both L1 and La, 
they (i.e., L1 and La) are for the same VPN, but different sites. 
If L1 and La are for the same VPN, 
we want to the VPN traffic is sent to L1 and La individually, and 
La is used as the backup egress to protect the primary egress L1, 
then the VPN traffic is sent to L1 and La individually, which send the
traffic to their sites respectively in normal operations
since L1 and La have different IP addresses (e.g., L1-VpnAddress 
and La-VpnAddress). When La is used as the backup egress to protect the 
primary egress L1, L1's address (e.g., L1-VpnAddress) may be used/
configured as a local address on La and as BGP next hop for the VPN traffic.
Thus, in normal operation, the VPN traffic is sent to both L1 and La for 
two VPN sites. For the traffic to L1, it will be sent to L1 normally
since the cost to L1-VpnAddress on L1 is less than the cost to 
L1-VpnAddress on La. 


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?

[HC] In this context, an egress is a destination (or egress) of the 
P2MP LSP having R1 as its source/ingress, L1 and L2 as its 
destinations/egresses. We will update the text as below:
   Figure 1 shows an example of using backup LSPs to locally protect
   egresses of a primary P2MP LSP from ingress R1 to two egresses L1 and L2.


=============

   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 set 
SB> 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.

[HC] We will updated the text accordingly as below:
   Bit 31 (Egress local protection flag):  It is the least significant
      bit of the 32-bit word and is set to 1 indicating an egress local 
      protection.

   Bit 30 (S2L sub LSP backup desired flag): It is the second least
      significant bit of the 32-bit word and is set to 1 indicating S2L      
      sub LSP (ref to RFC 4875) is desired for protecting an egress of a
      P2MP LSP.

    The Reserved parts MUST set to zero.

S2L will be defined in Terminologies.


==============

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 and 
SB> vice versa

[HC] We will re-order the paragraph according to the order of 
the first, second and third item.


============

   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 Same with the 
SB> similar IPv6 object, and the other two objects you register.

[HC] We will make them consistent (i.e., tied in with the IANA).


============

     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 
SB> the document.

[HC] We will update the text as below:
     o Tunnel ID (ref to RFC 4875 and RFC 3209): 
         A 16-bit identifier being constant over the life of the tunnel,
         occupies the least significant 16 bits of the 32 bit word.
     o Extended Tunnel ID (ref to RFC 4875 and RFC 3209):
         A 4-byte identifier being constant over the life of the tunnel


===========

 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.

[HC] We will define these and others (found through a thorough scrub) 
in Terminologies section.