Subject: [mpls] AD review of draft-ietf-mpls-tp-cc-cv-rdi

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 29 Jun 2011 16:24:03 +0100

Hi authors,

I have done my usual AD review of your draft having received a
publication request from the working group. The purpose of the 
review is to ensure that the document has a smooth passage through
IETF last call and the IESG.

Listed below are a nuber of editorial issues that I would like you
to resolve before I advance the document. All points are open for
discussion.

I have put the document into "revised I-D needed" state, but as soon
as I see a revised version I will request IETF last call.

Thanks for the work.

Adrian

---

idnits shows that reference [12] is not used. Either insert this where
it is needed, or remove the reference.

---

BFD is not a "recognized" acronym. Please expand it on first use in the
Introduction. Please also include a reference to [4].

---

s/sub path/sub-path/

---

G-ACh, p2p and p2mp need to be expanded on first use.

---

Section 1.1 is unusual and seems unnecessary in view of the Authors'
Addresses section. The RFC Editor will ask you why you need this section
and I would encourage you to remove it.

---

Section 2.1

LSP is missing

MPLS-TP LSP s/Label Switch Path/Label Switched Path/

---

Section 3.1

   Implementations MAY also interoperate with existing equipment by 
   implementing [2], or [8] in addition to the procedures documented in 
   this memo.

s/MAY/may/
s/existing/legacy/                                              

But this is a strange sentence. It implies that there are two different
ways to interoperate with same legacy equipment. Is that right? 

It also implies that in order to interoperate with legacy equipment you
must implement *all* the procudures in this document.

I can't parse whether you mean...

   2 or (8 and this memo)
or
   (2 or 8) and this memo

---

Section 3.2

   RDI is communicated via the BFD diagnostic field in BFD CC messages. 
   It is not a distinct PDU. A sink MEP will encode a diagnostic code of 
   "1 - Control detection time expired" when the interval times detect 
   multiplier have been exceeded, and with "5 - Path Down" as a 
   consequence of the sink MEP receiving AIS with LDI set. A sink MEP 
   that has started sending diagnostic code 5 SHOULD NOT change it to 1 
   when the detection timer expires. 

I couldn't work out whether this is new behavior or existing behavion.
I think the latter. If so, you need a reference like "As defined in 
[foo] a sink MEP..."

Probably s/will/SHOULD/ according to the referenced text?

---

Section 3.3

This launches straight into the figure without saying what is going on.
Can you add a short paragraph to say "Figure 1 shows..."

   The first nibble (0001b) indicates the ACH. 
This is not a new definition, so can you add the reference?

   - BFD CC code point = 0xXX. [HH to be assigned by IANA from the PW 
   Associated Channel Type registry.] or, 

   - BFD proactive CV code point = 0xXX+1. [HH to be assigned by IANA 
   from the PW Associated Channel Type registry.] 
s/HH/XX/ twice

---

Section 3.4

I believe you will want the XX in the figure to be updated by the RFC
Editor. You need to add a note to ask the RFC Editor to do this.

---

Section 3.5

Similarly, you will want XX+1 replaced with an absolute number

   The length in the BFD control packet is as per [4], the MEP Source ID 
   TLV is not included.

Is that s/The length/If the length/ ?

   When GAL label is used, the time to live (TTL) field of the GAL MUST 
   be set to at least 1, and the GAL will be the end of stack label 
   (S=1). 

s/GAL label/GAL/
You say "will be" because this behavior is defined elsewhere. Please 
give a reference.

---

Section 3.5.1

I understand why this section is the way it is. However, you should
remove this section and add text to the Introduction such as...

   This document utilizes identifiers for MPLS-TP systems as defined in
   [9]. Work is on-going in the ITU-T to define a globally-unique
   semantic for ITU Carrier Codes (ICCs), and future work may extend
   this document to utilize ICCs as identifiers for MPLS-TP systems.

---

Section 3.5.2 - 3.5.4

There are stray "=" characters in the figures.

A bit odd that these figures are not numbered.

---

Section 3.5.3

   The format for the LSP MEP-ID is as defined in [9]

The referenced document does not define any format. This is good because
it means you are not duplicating a format definition, but is bad because
the text is wrong.

---

Section 3.5.4 

As 3.5.3

---
                                  
Section 3.7.3

   The blocking of traffic as a consequent action MUST be driven only by 
   a defect's consequent action as specified in draft-ietf-mpls-tp-oam-
   framework [11] section 5.1.1.2.

Please don't name the document in the text. Use the reference.

---

Section 3.7.5

It may be helpful to show the event "LDI set" in quotes in the text to
make things easier to parse.

   For independent mode, there are two state machines. One for the 
   source MEP (who requested bfd.MinRxInterval=0) and the sink MEP (who 
   agreed to bfd.MinRxInterval=0). 

s/who/which/  twice

---

Section 4

   The following is an exemplary set of configuration parameters for a 
   BFD session: 

Nothing perfect about that set :-)
s/exemplary/example/

---

Section 5

Dave Ward is an author. You can remove hi from this section.

---

Section 6

   This draft requires the creations of a source MEP-ID TLV 
   registry with initial values of: 

      Xx   - Section MEP-ID 

      Xx+1 - LSP MEP-ID 

      Xx+2 - PW MEP-ID 

You will need to say what the parent registry of this new subregistry 
is.

You need to say what information needs to be tracked (maybe just TLV
Type, Name, and reference.
                                             
You should reference RFC 5226 for the allocations policy.

In section 3.5 you use X1, X2 and X3. I think you should use that here.

---

Section 7 is very thin!

Anyway, in 3.5 you say...
   When digest based authentication is used, the Source ID TLV MUST NOT 
   be included in the digest 
But doesn't this give us a problem making sure that the TLV has not been
modified? It is clearly important that this is not modified because you
wrote:
   A node MUST NOT change the value in the MEP Source ID TLV. 
If this field cannot be protected, you need to note this in Section 7 
and observe the consequences if it is modified. Also helpful to state
the mitgation.