[Gen-art] Gen-art Telechat review of draft-ietf-mpls-mldp-node-protection-06

[Elwyn Davies <[e-mail]>]

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-mpls-mldp-node-protection-06.txt
Reviewer: Elwyn Davies
Review Date:  2015/09/15
IETF LC End Date: 2015/09/08
IESG Telechat date: 2015/09/17

Summary: Ready for publication on standards track.  Thanks for your 
generous comments on my review and the updated version -06 which fixes 
almost all of the issues.  The nits below are mostly suggestions related 
to the updated text apart from the last one on s2.3 which got missed.

Major issues:
None

Minor issues:
None

Nits/editorial comments:
s1:  The new text referring to the need for capability negotiation is 
not easy to parse.   Suggested alternative:
OLD:
    In order for a node to be protected, the protecterd node, the PLR and
    the MPT MUST support the procedures as described in this draft.
    Detecting the protected node, PLR and MPT support these procedures is
    done using [RFC5561].
NEW:
    In order to allow a node to be protected against failure, the LSRs 
providing
    the PLR and the MPT functionality as well as the protected node MUST
    support the functionality described in this document.  RSVP capability
    negotiation [RFC5561] is used to signal the availability of the 
functionality
    between the participating nodes; these nodes MUST support capability
    negotiation.
END

s2, last para: s/This because/This is because/

s2.1, last para; s2.2, last para: s/Procedures how to setup/The 
procedures for setting up/

s2, s2.2 and s3: s/this draft/this document/ (3 places) [A 4th instance 
is replaced in the suggested text for s1 above.]

>> s2.2:  If I understand correctly, the bypass LSPs have to be bidirectional (or they could be two unidirectional ones) unlike those in s2.1 which will be unidirectional.  I think this ought to be mentioned, assuming I am right - and presumably one could do a bit of optimisation in setup.  This has some knock-on effects as regards what happens when the node fails.  I wonder if there should be some explanation of what happens in an extra sub-section in s4 - just that the various LSRs need to think about what role they are playing depending on where the incoming packets are coming from, I guess.
> Ice: Yes, that is a good observation about unidirectional and bidirectional LSPs. I’ll add a node to make that clear.
The fixes for that are fine and helpful IMO.
> Since the MPT will receive packets with the MPLS label it originally expected, it does not really care where the packets are coming from. So I’m not sure anything else needs to be added here.
>
Probably right.  Actually the fact that the bypass LSPs are 
bidirectional does sort out the differentiation of roles anyway. 
Incoming = MPT, Outgoing = PLR.  The note could be extended to mention 
this.

s2.3:
>        Num PLR entry: Element as an unsigned, ***non-zero*** integer followed
>        by that number of "PLR entry" fields in the format specified
>        below.
Per the discussion of my last call comments, the Num PLR entry can be zero.