Re: [RTG-DIR] [mpls] Routing directorate review of draft-ietf-mpls-app-aware-tldp

[Santosh Esale <[e-mail]>]

Hi Bruno,
                Thanks for the detailed review and apologies for the delayed response. Please find answers inline.

On 10/10/16, 5:56 AM, "mpls on behalf of [e-mail]<mailto:[e-mail]>" <[e-mail]<mailto:[e-mail]> on behalf of [e-mail]<mailto:[e-mail]>> wrote:

Hello,

I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see ​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir<http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir>

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.

Document: draft-ietf-mpls-app-aware-tldp-05.txt<https://tools.ietf.org/id/draft-ietf-mpls-app-aware-tldp-05.txt>
Reviewer: Bruno Decraene
Review Date: 2016/10/10
IETF LC End Date: not started (AFAIK)
Intended Status: Proposed Standard

Summary:
I have significant concerns about this document and recommend that the Routing ADs discuss these issues further with the authors.
Comments:
Draft is well readable. But I feel that it could be even more precise on normative behaviors (cf minor comments).
Clarified normative behavior in most the cases alluded in this review.


Preliminary info:
- I'm not a LDP expert. Unfortunately, this may become apparent in the below comments. So you are welcome to disagree and provide the information and reasoning that I may be missing.
- Given that the required IANA code points have not been pre-allocated, I am assuming that no implementation of this document exists. Hence I'm not restricting my comments to minor comments.


Major Issue:
Abstract and Shepherd Write-Up defines the goal of the solution as advertising the purpose for sending this tLDP session request to the tLDP receiver, such that the receiver has enough information to decide to either accept or deny it.
I don't feel that the solution perfectly matches this purpose. First the solution requires that both ends of the tLDP session negotiate the _same_ set of applications. I don't see why this is needed to achieve the above goal.
 In LDP, negotiate is equivalent to both LSRs – initiating and receiving – sending their targeted application list to each other. And unless initiating LSR sends it's targeted application list to receiving LSR, the receiving LSR will be unaware of it.

Plus I find this problematic for some applications with asymmetric requirements (e.g. RLFA), in particular for incremental deployment of future asymmetric application. (more comments on this in the minor issues)
In addition, I don't see how the additional FEC filtering helps fulfilling this goal.
FEC filtering is used to advertise only necessary LDP FEC-label bindings over the session. This is additional goal, and not the same, of the document.

I also find it redundant with RFC 7473. I also don't think that this FEC filtering should be symmetrical (e.g. RLFA application).
In short, I would have made the TAI advertisement asymmetric, and removed the FEC filtering.
We have explained clearly how RFC 7473 is inadequate to address both these goals in section 4.


Finally, if the goal is to allow the receiver to limit incoming tLDP sessions, presumably for scalability purpose, if one application is allowed and hence the tLDP session is set up, what would be the reason to limit the applications exchanged over this session?
We limit the application exchanged to the negotiated application list. Use case 5.3 precisely answers this question.

IOW, why limiting the applications to the intersection of both advertised and received set of application?
This helps to advertise only the necessary fec-label bindings over the tLDP session, reducing the unnecessary fec-label binding advertisements.

Note that §5.3 lightly refers to this problem, but in a very application specific way (and not really normative), while the consequences could probably be made general. Especially given the pre-existence of RFC 7473 which can be used to limit the FEC advertisements.
Again, we have made it ample clear why RFC 7473 is not sufficient. First and foremost, with RFC 7473, receiving LSR is unaware of the targeted application. Thus, It has only two choices, either block or unblock all FECs that are advertised to it considering it is a passive LSR.

Most of these points are addressed in detail answering questions towards the end of this email.

Also, we have updated the draft addressing comments, which will be published in next few minutes.

Minor Issues:
§6 "Security considerations"
TAC negotiation seems to allow an entity to remotely discover the targeted LDP applications running on a remote node. This is a new security consideration which should probably be discussed.
No. Currently, the remote node implicitly knows anyway all the applications that are running on source node based on LDP FEC label bindings. Note – Currently, LDP advertise all FEC-label binding over a tLDP session.


Also, given that multiple applications maps to the same FEC, two peers in different ASes could try to cheat, by successively trying multiple applications mapping to the same FEC. (e.g. if I want IPv4 FEC mapping, I would try with "LDPv4 Tunneling", then "LDPv4 Remote LFA", then "IPv4 intra-area FECs", then possibly "LDP session Protection"). This is also not discussed.
The initiating LSR MUST only advertise what is supports – IOW, what it is configured for – over the tLDP session.  It is clearly stated in section 2.2 first paragraph.

----

§7 "IANA Considerations"
"0x0001 - 0x1FFF  Available for assignment by IETF consensus"
According to https://tools.ietf.org/html/rfc5226#section-4.1 "IETF Review" is the new name for "IETF consensus"
Updated.


"0x7FFF - 0xFFFE  Available for vendor specific private use"

- This look like a very large range for a private use (half of the registry)
- "vendor specific private use" is not a Well-Known IANA Policy as defined in  https://tools.ietf.org/html/rfc5226#section-4.1 . Is there any definition of this Policy or should you describe it? How is code-point collision supposed to be avoided in deployments? (I'm guessing that this is coming from RFC 5036 IANA section, but RFC5036 seems to have a "vendor-ID" field to differentiate vendors, which is not the case of this document.)
On my side, I'd rather provision 2 very small pools for experimental and private-use. (i.e. private to the user/ the network provider.)
Thanks for the suggestion. Updated the draft to allocate two small ranges of 1k each for experimental and private use.


----

§1 "Introduction"
"Applications such as Remote LFA and BGP auto discovered pseudowire automatically initiate asymmetric extended discovery to any LSR in a network based on local state only."
I agree that Remote LFA is asymmetric.
I've not been following the work on BGP auto-discovery, but I would have a priori assumed that both tLDP end points are running BGP autodiscovery and hence the discovery is symmetric.
 BGP auto discovery is not always asymmetric. For example – BGP auto discovered multisegment pseudo wire can have different forward and reverse auto-discovered path to signal a tLDP session.

----

§2.1 "Encoding"

"Targeted Application Identifier (TA-Id): a 16 bit Targeted Application Identifier value."
According to the figure just above, the field seems to only have 15 bits. (As bit 0 is used for the E-bit). If so the IANA registry would also need to be modified. Otherwise, the figure needs to be fixed.
Correct. There was a problem with the figure. Updated.


----
§2.2 "Procedures"
"The TAC TLV's Capability data MUST consists of none, one or more TAE"
:s/MUST/MAY   ? (otherwise, It's not clear to me what is the mandated behavior since all possible behaviors seems allowed)

Updated.

----
§1

"For an application such as FEC 128
   pseudowire, the remote LSR is configured with the source LSR address
   so that it can use that information to accept or ignore given tLDP
   Hello.

   Applications such as Remote LFA and BGP auto discovered pseudowire
   automatically initiate asymmetric extended discovery to any LSR in a
   network based on local state only. With these applications, the
   remote LSR is not explicitly configured with the source LSR address.
   So the remote LSR either responds or ignores all tLDP Hellos."

 The introduction seems to imply that this document would give a remote peer additional information in order to accept or ignore tLDP hello.
My limited understanding of LDP capability is that they are exchanged  in Initialization and Capability messages, i.e. not in Hello message.
Hence it's not clear to me how this document helps the remote LDP speaker in deciding to accept or ignore tLDP hello.
Procedure section 2.2 explains how to achieve it as follows - "Also, currently the remote LSR accepts asymmetric extended Hellos by default or by appropriate configuration. With this document, the LSR MUST accept tLDP hellos in order to then accept or reject the tLDP session based on the application information.”

----
§2.2
"   If there is at least one TAE common between the TAC TLV it has
   received and its own, the session MUST proceed to establishment as
   per [RFC5036]."

I'm not sure this is “as per [RFC5036]” since this document defines additional rules to define which FEC mapping needs to be advertised, and whether or not to accept the session.
It is in the reverse order. From RFC 5036 section 6 “The document specifying procedures for the capability MUST describe the behavior in this situation. If the specified procedure is to terminate the session,then the LDP speaker SHOULD send a Notification message to the peer before terminating the session.”


---
§2.2
"The TAC TLV's Capability data MUST consists of none, one or more TAE"
It's not clear to me what is the use case to advertise none TAE, given that in this case, the intersection of the received and sent TA-Id will be null and hence the tLDP session will be closed by any of the tLDP speakers.
The use-case is for receiving LSR playing the active role in tLDP session establishment. If the receiving LSR does not have any configured tLDP application and do not want to support any tLDP session establishment, it will send TA-Id as null. The initialing LSR after receiving the TA-Id as null and playing the passive role in tLDP session establishment will then tear down the tLDP adjacency, eventually leading to the destruction of tLDP session.


---
§2.2
"If the receiver LSR receives an unknown TA-Id in the TAE, it MUST silently ignore such a TAE and continue processing the rest of the TLV."
Assuming the receiver (node A) supports Non Stop Routing and is upgraded to support a new TA-Id should it check for the previously received TAE that it has silently ignored or does the speaker (node B) supposed to re-send all it's TA-ID if it receive new TA-Id from node A? My reading of the end of section 2.2 is that in this case the receiver must checked from previously received TAE.
May be :s/receives/received  would be enough to address this case. Or preferably adding a sentence.
Updated.


---
§2.2
"In the last instance, suppose the
   initiating LSR advertises A, B and C as a TA-Ids and the responding
   LSR advertises D and E as TA-Ids, than the negotiated targeted
   applciations as per both the LSRs are none. The Responding LSR sends
   'Session Rejected/Targeted Application Capability Mis-Match'
   Notification message to the initiating LSR and may close the
   session."

I'd prefer having normative text stating the required behavior rather than having an example.
i.e. I'm looking for a text similar to "If the intersection of the sets of received and sent TA-Id is null, then LSR MUST sends 'Session Rejected/Targeted Application Capability Mis-Match'
   Notification message to the initiating LSR and close the session."

   or :s/MUST/SHOULD or :s/MUST/MAY   as you wish
Updated.


---
§2.2
"If it sets the session setup retry interval to maximum, the session MAY stay in a non-existent state."

If this refers to the LDP FSM, RFC 5036 used the term "NON EXISTENT" rather than "non-existent"
Updated.

---
§2.1
This section mixes copy of previous specifications (e.g. RFC 5561) with the new specification. Personally, I'd prefer that the document be clear on the part which are reused unchanged and the part which are new specification. In general, I personally don't think that  this is a good practice, as it becomes unclear which document is the normative definition. And in particular in this document, there is a copy/paste error during the copy from RFC 5561, so it my be unclear if the goal is to redefine RFC 5561 or not. ("Typ" field has been increased by 1 bit and the "Length" has been decreased by 1 bit)

I would propose the following change:

OLD:

   An LSR MAY advertise that it is capable to negotiate a targeted LDP
   application list over a tLDP session by using the Capability
   Advertisement as defined in [RFC5561].

   A new optional capability TLV is defined, 'Targeted Application
   Capability (TAC)'. Its encoding is as follows:

      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
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |U|F| Targeted App. Cap.(IANA)|             Length              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |S|  Reserved   |                                               |
     +-+-+-+-+-+-+-+-+                                               |
     |                                                               |
     ~                 Targeted App. Cap. data                       ~
     |                                                               |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

     As described in [RFC5561]
     U: set to 1. Ignore, if not known.
     F: Set to 0. Do not forward.
     S: MUST be set to 1 or 0 to advertise or withdraw the TAC TLV
        respectively.

     Targeted Application Capability data:
       A Targeted Applications Capability data consists of none, one
       or more 32 bit Targeted Application Elements. Its encoding is
       as follows:

       Targeted Application Element(TAE)

        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

       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |E|    Targ. Appl. Id           |       Reserved                |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

       Targeted Application Identifier (TA-Id):
       a 16 bit Targeted Application Identifier value.

       E-bit: The enable bit indicates whether the sender is
       advertising or withdrawing the TAE. The E-bit value is used as
       follows:

         1 - The TAE is advertising the targeted application.
         0 - The TAE is withdrawing the targeted application.

     The length of TAC depends on the number of TAEs. For instance,
     if two TAEs are added, the length is set to 9.

NEW

   An LSR MAY advertise that it is capable to negotiate a targeted LDP
   application list over a tLDP session by using the Capability
   Advertisement as defined in [RFC5561] and encoded as follows:

         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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |U|F| TLV Code Point            |            Length             |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |S| Reserved    |                                               |
      +-+-+-+-+-+-+-+-+       Capability Data                         |
      |                                               +-+-+-+-+-+-+-+-+
      |                                               |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


                This document defines a new optional capability TLV of type TBD1 called 'Targeted Application
   Capability (TAC)'.
   Flag "U" MUST be set to 1 to indicate that this capability must be silently ignored if unknown.

   It's encoded as follows:

       Targeted Application Element(TAE)

        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
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |E|    Targ. Appl. Id           |       Reserved                |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


          Targeted Application Identifier (TA-Id):
       a 16 bit Targeted Application Identifier value.

       E-bit: The enable bit indicates whether the sender is
       advertising or withdrawing the TAE. The E-bit value is used as
       follows:

         1 - The TAE is advertising the targeted application.
         0 - The TAE is withdrawing the targeted application.


Updated.

---
§2.2
"If the tLDP session changes to link session, a LSR should withdraw it"...
:s/should/SHOULD
Updated.


..."with S bit set to 0, which indicates wildcard withdrawal of all TAE elements."
Where is this behavior defined?
My reading of RFC5036 is that sending the capability with the S bit set to 0 means withdrawing the capability. In which case this documents states that "If the receiver LSR does not receive the TAC TLV in the
   Initialization message or it does not understand the TAC TLV, the TAC negotiation MUST be considered unsuccessful and the session establishment MUST proceed as per [RFC5036]." i.e. the tLDP sessions stays up.
This is not the same as "wildcard withdrawal of all TAE elements" which means that the TAC capability is advertised with no TA-Id, it the tLDP sessions will be closed.
Correct. Removed this part of the text "which indicates wildcard withdrawal of all TAE elements"


--
§2.2

OLD:
   "Also, currently the remote LSR accepts asymmetric extended Hellos by
   default or by appropriate configuration. With this document, the LSR
   MUST accept tLDP hellos in order to then accept or reject the tLDP
   session based on the application information."

What is the goal of this paragraph? I'm reading that a LSR may not be configured anymore to reject tLDP hellos. Why not?
I would propose
NEW
"By default, LSR SHOULD accept tLDP hellos in order to then accept or reject the tLDP
   session based on the application information."

Updated.

---
   §2.3.1
"    1. The S-bit of the Targeted Application Capability TLV MUST be
     set to 1 to advertise Targeted Application Capability and
     SHOULD be ignored on the receipt."


This behavior is defined in RFC 5561. I don't think that it's good practice to redefine it.  I'd rather have this sentence deleted. Alternatively, it should be made non normative and refer to RFC 5561.
Updated.

---
§2.3.1
" 2. The E-bit of the Targeted Application Element MUST be set to 1 to
     enable Targeted application and SHOULD be ignored on the receipt.               "

I understand that you are mimicking the behavior of the S-bit. It looks debatable to ignore this direct protocol violation and accept the TAE while the speaker expressed its williness to withdraw it. I would personnaly be enclined to ignore the TAE advertise with the E-bit cleared.
This is the first time the peer is sending TAE in the initialization message. If it is not advertising it, there is no point in sending it.
Thus, Its a fair assumption that the peer wants to advertise this TAE.


---
§2.3.2
"     If the S-bit is set to 0, the TAC is disabled for the session.
     After that, the session may remain in established state or
     torn down based on [RFC5036] rules."

IMO, if TAC is disabled, the session MUST behave as per RFC5036 rules. (in particular, I don't see a reason to tear down the session, but I do see a reason to advertise all FEc mappings which where previously filtered based on TA-Id negociation.)
This could be automatic session. Before this draft, LSR use to either accept or deny such session based on local configuration.
Hence, if the TAC is not negotiated for a session, the LSR may decide to bring down the session.

Updated the text.
---
§2.3.2
  "5. If the S-bit is set to 1, a LSR process a list of TAEs from
     TACs capability data with E-bit set to 1 or 0 to update the
     peers TAE. Also, it updates the negotiated TAE list over the
     tLDP session."

What's new compared to the already defined points 1, 2 and 3?
If this point 5 is kept:
  :s/peers/peer's
  What does the second sentence adds to the first one?
Updated. Removed the second sentence.


§3
Personally, I would prefer that the array also include the normative code points (rather than requiring an indirection in the IANA section)
I will keep it that way for simplicity. It also avoids updating two places when the code point changes.

§2.3
Why does the document mandates that TAE be symmetrically negotiated?  Here we are not negotiated capabilities which requires both ends to support the capability before its usage. We are mostly advertising the willingness of one LSR to receive FEC mappings from its peer.
There are two goals. (1) Both initiating and receiving LSRs know about the supported targeted application over the session. (2) Advertise FEC-label bindings only for those targeted applications.

e.g. let's take the RLFA application which has asymmetric needs. The PLR is willing to get the IP prefixes mapping from the merge point (PQ node). But its peer (the merge point) is not willing to get any mapping. So why would it need to receive mappings of no interest, and why would it need to advertise TAE which does not itself?
Let me answer your question in two parts - (1) The merge point needs to know that the PLR is establishing the tLDP session for R-LFA. Based on that it may allow the merge point to establish the tLDP session or it may not allow if it has reached the maximum limit for the automatic tLDP session for remote LFA. (2) Once when they establish the tLDP session, we only advertise IPv4 or IPv6 FEC-label bindings over the session. Given the merge point do not need any FEC-label bindings, it may then use the RFC 7473 to request PLR to not to send any FEC-label bindings to it. Therefore, this draft helps to advertise what is necessary over the tLDP session, avoiding the advertisement of other FEC label bindings such as FEC128 and FEC129.

The document could have chosen an asymmetric model, where the advertisement of a TAE means "I'd like to receive the corresponding FEC mappings"
Explained above.

Coming back to the RLFA use case, this requires configuring the Merge Point with the willingness to advertise RLFA application, including when RLFA in not configured on this node. So this is additional configuration requirement. In addition, RLFA is designed to be asymmetric in nature, with feature requirement only on the local node. Such document would now require some support for RLFA awareness on the MP node which is undesirable.
I don’t think so. It actually the apposite. As the merge point do not know about why the PLR is establishing the tLDP session to it, it has no control over these sessions. This is practical deployment problem of a remote LFA today.

I guess that this is not too bad to RLFA which is a pre-existing application, but what about future applications which would also be asymetric?
The very purpose of this document is to make the receiving LSR aware of targeted LDP application. Given that, it has more control over these tLDP sessions and corresponding fec-label bindings advertised over them.

§4
"With the SAC, the responding LSR is not aware of
   targeted applications. Thus it may be unable to communicate its
   interest or disinterest to receive state information from the peer."

Sorry but I fail to see the logic or the use case. One is a priori capable of communicate its (_own_) interest, independently of the ones of its peer. e.g., If I'd like a beer in a restaurant, I ask for a beer. (I don't need to wait for the menu, so see whether I wan't a beer)
I wish everything is as simple as ordering a beer that we love :)
For instance – Lets take a example of BGP auto discovered multisegment pseudo wire with different forward and reverse auto-discovered tldp path through the network.
 R1---------R2---------R3
|                |               |
R4---------R5----------R6
Forward path - R1 to R2 to R3. Reverse path - R3 to R6 to R5 to R4 to R1
In such a case, R1 will establish a tLDP session to R2 for forward path. Here R2 can’t decide what it wants. R2 simply accepts all the fec-label bindings that R1 advertise including ipv4 fec-label bindings. With this draft, R1 and R2 knows that the tLDP application is BGP auto discovered pseudowire. So they exchange only FEC 129 label binding over the session avoiding the unnecessary IPv4 fec-label bindings.


"  Therefore, when the responding LSR is not aware of targeted
   applications such a remote LFA and BGP auto discovered pseudowires,
   TAC mechanism should be used and when the responding LSR is aware
   (with appropriate configuration) of targeted applications such as FEC
   128 pseudowire, SAC mechanism should be used."

Well, as already expressed, I don't feel that TAC is well suited for the RLFA application as it adds the requirement to configure both ends. (more specifically the Merge Point). In which case, as per your above text, SAC should be used instead of TAC.
If LDP has already a way to control FEC/state advertisement (as per RFC 7473), I feel that adding the same functionality in TAC is redundant.
Already explained. In summary, this document makes receiving LSR aware of targeted LDP applications and only advertise FEC-label bindings for those negotiated tLDP applications.

---
§4
"The set of
   applications negotiated by the TAC mechanism is symmetric between the
   two LDP peers."

ok. for the _negociated_ applications.

"In the absence of further mechanisms, two LDP peers
   will both advertise state information for the same set of
   applications."

What do you mean by "state information"? At first reading, I understood "TAE" but this would be incorrect. So now I guess that you mean "FEC mappings"

Yes, FEC mappings.

---
Usage (new section)
To allow for incremental deployment of new TAI as well as private usage by a network operator, I think the configuration SHOULD allow the configuration of any TAI (including the ones unknown from the implementation) on both the sending side and the receiving side.
Added a text to procedural section 2.2 last paragraph.

---
§4
"The TAC mechanism MUST
   take precedence over the SAC mechanism with respect to enabling
   applications for which state information will be advertised."

Is this not a case where the document meta data should indicate that this document updates RFC 7473?
---
Updated.

§3
"IPv4 intra-area FECs"
Is this just a name of application, or is there a specific behavior associated with it? (e.g. only advertising the IPv4 FEC from my an area.) If so, the behavior should be specified. And please clarify whether this is the area of the sender or of the receiver, as in the general case, they may be in a different area. And clarify the behavior for IS-IS level 2 nodes.
We would add some text to address it.

Thanks
Santosh