Re: [mpls] Ben Campbell's No Objection on draft-ietf-mpls-mldp-node-protection-05: (with COMMENT)

[IJsbrand Wijnands <[e-mail]>]

Hi Ben,

> -- 2.3, last paragraph: "Along with the PLR MP Status a MP FEC TLV MUST
> be included"
> Does that mean that both MUST be included, or if the first is included,
> the second MUST also be?

Both MUST be included.

> 
> -- 4.1.3, last paragraph:
> Just “recommended”? Is link flapping a minor enough that it doesn't
> justify a MUST?

Well, its really an implementation choice to save resources being using in deleting and re-creating the tLDP session. If the implementation has means to deal with this or has other mechanism to solve this problem, that is fine too. 

> 
> -- 6:
> It would be nice to show your work a bit more in the security
> considerations. This draft adds new protocol elements and procedures. If
> the working group has determined that those new bits add no new security
> concerns, it would be good to say why.

Yep, that is done.

> 
> Editorial and Nits:
> 
> A (probably first) paragraph in the intro that defined exactly what the
> draft means by "protection" would be helpful. (The existing first
> paragraph talks about how you provide protection, but one must infer what
> this protects _against_.
> 
> -- 1, 2nd paragraph: Lots of the terms here could use (informative)
> citations.
> 
> -- 2.1, first paragraph: Consider s/"we are describing"/"we describe"
> -- 2.1, last paragraph, 2nd to last sentence, "See section 5":
> unbalanced parentheses.
> 
> -- 2.2, 1st paragraph:
> s/"we are describing"/"we describe"
> 
> -- 2.2, last paragraph: 
> "protection mechanism don’t" -- Noun/verb disagreement (singular/plural)
> s/ help restoring/help restore
> 
> -- 2.3, 2nd to last paragraph:
> I suggest the “A node N” phrase be moved to the first mention of N in
> this paragraph.
> 
> "Removing a PLR address is likely due to a link failure, see the
> procedures as documented in Section 4.1. ":
> Comma splice. Consider a semicolon.

Ok, I think I fixed the above.

> 
> "MUST encode PLR Status Value Element": Missing article.

What do you mean here?

> 
> -- 3, 2nd paragraph:
> "Ln that was assigned to N via the normal mLDP procedures, and Label Lpx
> that was assigned for PLR (LSR1) for the purpose of node protecting MP
> LSP via node N."
> I can’t parse this sentence. (Incomplete sentence?)

Ok, updated it, hope its better now.

> 
> -- 3, 3rd paragraph: "For this reason, the FEC Label Mapping (FEC <R,X> :
> Lpx) sent by the MPT over the tLDP session to the PLR MUST include a
> Status TLV with
>   MP Status including a new LDP MP status Value Element called the
> "Protected Node Status Value Element"."
> Convoluted sentence. Consider breaking into multiple, simpler sentences.
> 
> -- 5, 1st sentence:
> s/we are organizing/we organize/
> 
> 

Ok,

Thx!

Ice.