MPLS-TP Shared-Ring Protection (MSRP) Mechanism for Ring Topology
draft-ietf-mpls-tp-shared-ring-protection-06

Some nits and a question:

3.  MPLS-TP Ring Protection Criteria and Requirements
a.  The number of OAM entities...

"Each ring-node requires only one instance of the RPS protocol. " --- not super important, but is this "Each ring-node requires only one instance of the RPS protocol (regardless of the number of rings)" or "Each ring-node requires only one instance of the RPS protocol per ring"? -- if a node participates in multiple rings, does it need an instance for each ring? (I suspect that this is somewhat of an implementation choice, but am not sure).


4.  Shared Ring Protection Architecture
4.1.  Ring Tunnel
"... ring tunnels which provides a server layer
   for the LSPs traverse the ring."
I think "for the LSP's traversing the ring." (or perhaps "which traverse the ring.")

I'd like to see the discussion with gen-art reviewer conclude and the associated changes folded into the next version of the document.

This document describes 3 different protection mechanisms and it specifies that all nodes "MUST use the same protection mechanism".  When should these mechanisms be used?  What are the conditions that an operator should take into account when selecting between them?  I would like to see operational considerations explained.

Substantive:

- The abbreviation "MSRP" is already used by RFC 4975.  Please avoid overloading it if at all possible. (And you probably want to collide with "Manufacturer's Suggested Retail Price" even less.)

-4.4.2: "When the service LSP passes through the interconnected rings, the
   direction of the working ring tunnels used on both rings SHOULD be
   the same. "
Would it ever make sense for the directions to be different? (That is, why not MUST?) If so, a few words about that would be helpful.

-5.1, 3rd bullet: "Determination of the affected
      traffic SHOULD be performed by examining the RPS requests
      (indicating the nodes adjacent to the failure or failures) and the
      stored ring map (indicating the relative position of the failure
      and the added traffic destined towards that failure)."

Would it ever make sense to violate that SHOULD? (That is, why not MUST?)

-6.2: Why "standards action"? That's a high bar. Are there reasons why a lower bar like "specification required" would not be appropriate? For example, are we in danger of running out of code points? Is this registry at unusual risk for poor quality registrations?

Editorial:

-3: Is this section expected to be useful to implementors? It reads more like evidence to the WG that this meets the requirements. I suspect people won't much care about that once this is published as an RFC. Please consider moving it to an appendix, or even removing it entirely.

-4.4.2: "For example, if the service LSP uses the clockwise working
   ring tunnel on Ring1, when the service LSP leaves Ring1 and enters
   Ring2, the working ring tunnel used on Ring2 SHOULD also follow the
   clockwise direction."
Please avoid repeating the 2119 "SHOULD" in the example. 

- 5.1: "The MSRP protection operation MUST be controlled with the help of the
   Ring Protection Switch protocol (RPS)."
That seems like a statement of fact, rather than an implementation requirement.

Starting around 5.1, I notice several uses of the word "source" as a verb, where from context it seems like you mean "to send" or "to originate". Is that a term of art? I usually think of "source" as a verb to mind "acquire","find" or "find a source for"

-5.3: "... thus RPS SHOULD be capable of
   identifying and handling the different failures on the ring ..."
That seems like a statement of fact.

S 4.1.1.
   protect these LSPs that traverse the ring, a clockwise working ring
   tunnel (RcW_D) via E->F->A->B->C->D, and its anticlockwise protection
   ring tunnel (RaP_D) via D->C->B->A->F->E->D are established, Also, an
   anti-clockwise working ring tunnel (RaW_D) via C->B->A->F->E->D, and
   its clockwise protection ring tunnel (RcP_D) via D->E->F->A->B->C->D

Why does the protection tunnel include D on both ends whereas the working
tunnel does not?


S 4.2.
   packets are periodically exchanged between each pair of MEPs to
   monitor the link health.  Three consecutive lost CC packets will be
   interpreted as a link failure.

Is this a normative statement (i.e., does it need a MUST).


S 4.3.2.1.
Why do you ever not use short wrapping?


S 5.1.4.1
   A node MUST revert from pass-through state to the idle state when it
   detects NR codes incoming from both directions.  Both directions
   revert simultaneously from the pass-through state to the idle state.

incoming within what time frame?

Two technical comments that I think are important to address but do not warrant a discuss:

1) section 5.2: "As shown in Figure 14, when no protection switching is active on the
   ring, each node MUST send RPS requests with No Request (NR) to its
   two adjacent nodes periodically."
What does periodically mean here? Can you maybe give a number or even a normative statement like "and MUST NOT send more often than every X seconds" to avoid unnecessary congestion...?

2) section 5.1.1: "A ring node which is not the
   destination of the received RPS message MUST forward it to the next
   node along the ring immediately."
Why would you forward these? I thought you only send messages to your neighbors? Maybe I missed this but is there a use case for this scenario? Otherwise it might be safer to not forward to avoid that messages with a wrong destination node ID circle around forever. If you forward maybe you also need a hop-count to decrease or at least say that messages that are received and have the own node ID as source node ID MUST be dropped...?

Further, as mentioned by Ben for a couple of case, some of the uses of normative language in section 5 seems not to be appropriate as they don't specify a concrete implementation action. Please check carefully and change some to lower case instead, e.g.
"The MSRP protection operation MUST be controlled with the help of the
   Ring Protection Switch protocol (RPS). "
"The RPS protocol MUST carry the ring status information and RPS
   requests,.." (this sounds like a requirement on the protocol design but when you implement the protocol as specified there is no way to not do it, so this MUST is unnecessary)
"Each node on the ring MUST be uniquely identified by assigning it a
   node ID." (also requirement-like; the MUST in the next sentence is the important one)
"When a node detects a failure and determines
   that protection switching is required, it MUST send the appropriate
   RPS request in both directions to the destination node."
"MSRP mechanism SHOULD support multiple protection switches in the
   ring, resulting in the ring being segmented into two or more separate
   segments. "
"The first three RPS protocol messages carrying new RPS request SHOULD
   be transmitted as fast as possible." (Again the later SHOULD is the more important one)
There may be more…

Thanks for working through my Discuss.