Re: [mpls] Tsvart last call review of draft-ietf-mpls-summary-frr-rsvpte-07

[Tarek Saad <[e-mail]>]

Hi Gorry,

Happy new year!
Thanks for your thorough review and comments. We have reviewed them, and provided responses. Please see inline for more details.

On 12/12/19, 3:47 PM, "Gorry Fairhurst via Datatracker" <[e-mail]> wrote:

    Reviewer: Gorry Fairhurst
    Review result: Ready with Nits
    
    This document has been reviewed as part of the transport area review team's
    ongoing effort to review key IETF documents. These comments were written
    primarily for the transport area directors, but are copied to the document's
    authors and WG to allow them to address any issues raised and also to the IETF
    discussion list for information.
    
    When done at the time of IETF Last Call, the authors should consider this
    review as part of the last-call comments they receive. Please always CC
    [e-mail] if you reply to or forward this review.
    
    I did not understand that there were transport-related issues, but I have one
    request, some comments on RFC2119 language and a set of NiTs that I think
    should be addressed.
    
    I could not see where it explained how this could be done:
    /   When using procedures defined in this document, the PLR MUST ensure
       bypass tunnel assignment can satisfy the protected LSP MTU
       requirements post FRR./
[TS]: the facility backup method introduced in RF4090 takes advantage of MPLS label stacking (PLR imposing additional MPLS label post FRR) to allow rerouting of protected traffic over backup path. The backup path may have stricter MTU requirement and due to label stacking at PLR, the protected traffic may exceed the backup path MTU. The operator is assumed to engineer their network to allow rerouting of protected traffic and the additional label stacking at PLR to not exceed the backup path MTU.



    RFC2119 Language:
    
    I think the following sentence is long and hard to parse. I think this needs to
    be reworded to clarify the meaning of the SHOULD: /The PLR SHOULD assign the
    same Bypass_Group_Identifier to all
       protected LSPs that share the egress link, and bypass tunnel as long
       as the protected LSP(s) have the common group attributes, including
       the modified tunnel sender address used for backup path
       identification as described in [RFC4090]./
[TS]:
NEW:
       The PLR SHOULD assign the same Bypass_Group_Identifier to all
       protected LSPs that egress the same protected interface, and are protected by the
       same bypass tunnel. This minimizes the number of SFRR groups and
       optimizes the amount of signaling needed between the PLR and MP after FRR.

       The PLR MUST ensure all protected LSP(s) that are assigned the
       same Bypass_Group_Identifier use the same modified tunnel sender address
       for the backup path identification after FRR as described in [RFC4090].



    It seems strange to have the RFC2119 requirements as parenthesis, please
    consider making these separate requirements: /The PLR also
       generates a MESSAGE_ID object (flags SHOULD be clear, Epoch and
       Message_Identifier MUST be set according to [RFC2961])./
[TS]:
NEW:    
     The PLR MUST generate a MESSAGE_ID object with Epoch and Message_Identifier
     set according to [RFC2961]. The MESSAGE_ID object flags SHOULD be cleared
     when transmitted by PLR and ignored when received at the MP.




    Is the following intended as an RFC2119 /MAY/? - mentioned more than once:
    /One or more Bypass_Group_Identifiers may be included./
[TS]: yes.
NEW:
    One or more Bypass_Group_Identifiers MAY be included.

    NiTs:
    
    The abbreviation MTU was not defined in the abbreviations list, although
    well-known this is probably helpful.
[TS]: OK, will be expanded in the abbreviations section.



    The following typos should be corrected (suggestions included):
    /MP node for similar/MP node for a similar/
    /to help with the scale. /to help with scaling/
    /one ore more groups/one or more groups/
    /are proposed in this draft to describe/are proposed in this document to
    describe/ /With exception of /With the exception of /
[TS]: OK for all.



    This phrase didn’t make sense to me:
    /that are being protected by the specified bypass tunnel are being rerouted./
[TS]:
NEW:
B-SFRR-Active: Bypass Summary FRR Active Extended ASSOCIATION
object.  Used to notify the MP node that one or more groups of
protected LSP(s) have been rerouted over the associated bypass tunnel.



    This sentence is long and hard to parse:
    /  The PLR node that supports Summary FRR procedures adds the Extended
       ASSOCIATION object with Type B-SFRR-Ready and respective Extended
       Association ID in the RSVP Path message of the protected LSP to
       inform the MP of the PLR's assigned bypass tunnel, Summary FRR
       Bypass_Group_Identifier, and the MESSAGE_ID that the PLR will use to
       refresh the protected LSP Path state after FRR occurs./
[TS]:
NEW:
       The PLR node that supports Summary FRR procedures adds an Extended
       ASSOCIATION object with B-SFRR-Ready Extended Association ID in the
       RSVP Path message of the protected LSP. The PLR adds the protected LSP
       Bypass_Group_Identifier, information from the assigned bypass tunnel, and
       MESSAGE_ID object into the B-SFRR-Ready Extended Association ID.
       The MP uses the information contained in the received B-SFRR-Ready
       Extended Association ID to refresh and merge the protected LSP Path state after
       FRR occurs.



    Is this:
    /The PLR considers the protected LSP as Summary FRR capable only if
       all the fields in the B-SFRR-Ready Extended ASSOCIATION ID that are
       sent in the RSVP Path message and the ones received in the RSVP Resv
       message (with exception of the MESSAGE_ID) match. /
    Better as:
    /The PLR considers the protected LSP as Summary FRR capable only if
       all the fields in the B-SFRR-Ready Extended ASSOCIATION ID that are
       sent in the RSVP Path message match the fields received in the RSVP Resv 
       message (with exception of the MESSAGE_ID). /
[TS]: accepted, we will update as per your suggestion.
    

    Is this correct English, or should it be:
    /If it does not match,/If the fields do not match,/
[TS]: suggestion is reasonable, we will adopt it.


Regards,
Tarek