Re: [Gen-art] Hi, Gen-art Telechat review of draft-ietf-mpls-mldp-node-protection-06 - follow up

[IJsbrand Wijnands <[e-mail]>]

Thanks Elwyn!

Ice.

> On 16 Sep 2015, at 15:32, Elwyn Davies <[e-mail]> wrote:
> 
> Hi Ice.
> 
> I had a quick look through the updates in -07 and that has addressed all the points below.  Definitely good to go now.
> 
> Cheers,
> Elwyn
> 
>> 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.
>> 
>