Subject: [mpls] AD Review of draf-ietf-mpls-tp-fault-06

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 23 Aug 2011 17:22:25 +0100

Hi,

I have performed my usual AD review before passing your document on for IETF
last call and IESG review.

Unfortunately, I have found a remarkable number of nits and minor typos.
Fortunately, most of them should be very easy to fix.

Mixed in, I have found a number of small issues and concerns. Where possible, I
have supplied suggested resolution, and in all cases I do not believe that the
way forward requires substantial change to the document.

Nevertheless, a new revision of the draft is needed before we can take it
forward.

Thanks,
Adrian

===

Do you really need a normative reference to RFC 5920? Its use in
Section 7 does not appear normative to me.

---

Please replace /draft/document/ in the Abstract

---
    
The correct name for a MEP is
     MEP   Maintenance Entity Group End Point
as you capture in Section 1.1
So the Abstract is wrong.

Huub is in the process of fixing draft-ietf-mpls-tp-rosetta-stone to 
be consistent in these definitions.

---

Please use the RFC 6291 expansion of OAM
     OAM - Operations, Administration, and Maintenance

In the Abstract you are missing the plural from "Operations"
In Section 1.1 you are missing the comma.

---

Please try to expand acronyms on first use.
In the Abstract you expand OAM on second use.
In Section 1 you use OAM unexpanded.

---

Please avoid the passive voice. In the Abstract...

   An MPLS Operation, Administration, and Maintenance
   (OAM) channel is defined along with messages to communicate various
   types of service disruptive conditions.

"Is defined" by whom and where?

I think you need: "This document defines..."

In Section 1

   This capability is being defined to meet
   the MPLS-TP requirements as defined in RFC 5654 [1], and the MPLS-TP
   Operations, Administration and Maintenance Requirements as defined in
   RFC 5860 [2].

Again, "This document defines FM capabilities..."

---

Section 1

   When an event that
   causes disruption occurs on any link or node along the path of such a
   transport circuit, OAM indications are generated which may in turn
   suppress alarms and/or activate a backup circuit.

Pedantically, but I think with some importance, the OAM indication does 
not suppress the alarms and does not activate a backup circuit. The OAM
indication is an input to a control module and may act as a trigger for
these behaviors.

---

Fault and Defect

The definitions here are at odds with draft-ietf-mpls-tp-rosetta-stone-
04 and are also a little confusing.

It looks like the definition of Fault that you have is identical to the
definition of Defect in the Rosetta Stone.

For Defect you have:

   Within the Fault class, a further category, Defect is identified.  A
   defect is the inability of a function to perform a required action.
   A defect is a persistent fault.

I am not clear what it means for "a function to perform a required 
action" as distinct from "the ability to perform a required function
has been interrupted." Are functions performed, or do functions perform
actions?

Anyway, "A defect is a persistent fault" would be very clear if I didn't
wonder about the meaning of "persistent".

---
                                                                      
Section 1.1

LSR and S-PE are not used in the document

---

Section 2

   This document defines messages to indicate service disruptive
   conditions.  Two messages are defined, Alarm Indication Signal, and
   Lock Report.

A few superfluous words.

---

What purpose is served by the last paragraph of section 2? 
As far as I can see this text is an attempt to include a piece of ITU-T
architecture. Do we need it?
                                                                           
The question comes up because I am very uncomfortable with the 
architectural concept! It is a bit of squirming developed in order that
a MIP should not originate messages, but it is a layer violation in one
way or another. There are two possibilities: the adaptation function is 
part of the client layer or part of the server layer. If it is part of 
the server layer, then the server layer is aware of the client payload 
type and is not performing a transport function (i.e., it is not 
completely unaware of the payload). If the adaptation function is part
of the client layer then the MEP does not contain the adaptation
function since the MEP is in the server layer.
                                                                         
I think, in practice, the server layer MEP generates an indication to
the client layer, and the client layer is responsible for generating the
appropriate OAM message and inserting it into the stream.

Maybe this debate is best avoided in this document. Frankly, who cares
which piece of the architecture is responsible for this function? This
is a protocol specification. Let's just stick to stating which node 
builds what message.

Unfortunately, some of this issue percolates into other sections of the 
document where it is stated that the server MEP generates client layer
messages.

---

Section 2.1

I quote the whole section 

   The MPLS Alarm Indication Signal (AIS) message is generated in
   response to detecting faults in the server (sub-)layer.  The AIS
   message SHOULD be sent as soon as the condition is detected.  For
   example, an AIS message may be sent during a protection switching
   event and would cease being sent (or cease being forwarded by the
   protection switch selector) if the protection switch was successful
   in restoring the link.

   The primary purpose of the AIS message is to suppress alarms in the
   layer network above the level at which the fault occurs.  When the
   Link Down Indication is set, the AIS message MAY be used to trigger
   recovery mechanisms.

1. "SHOULD be sent as soon as..."
  Under what circumstances MAY the message not be sent as soon as...

2. "an AIS message may be sent during"
  Is that "MAY"?

3. "an AIS message may be sent during a protection switching
   event and would cease being sent (or cease being forwarded by the
   protection switch selector) if the protection switch was successful

   in restoring the link."
  What do you mean "during a protection switching event"? Are you trying
  to say that if server layer protection switching is in place it is
  possible that a server layer fault will be detected and AIS will be
  sent, but that the server layer may repair the server layer connection
  (perhaps through protection switching) causing the defect to clear and
  AIS messages to cease being sent? If this is what you are trying to 
  say, you should probably explain why it is interesting (because the 
  client layer might want to soak associated events such as CC failures,
  before undertaking its own recovery actions).

4. "The primary purpose of the AIS message is to suppress alarms in the
   layer network above the level at which the fault occurs."
  This statement (and others about alarm suppression in the document) is
  entirely without context. Probably you need to include a brief 
  statement somewhere in Section 1 about what is going on, viz.
  The server layer detects a fault and reports it as an alarm to the 
  management system and as a fault to the client layer. Each client
  layer LSP will also detect a fault at the client layer and would
  normally raise an alarm leading to a storm of client layer alarms.
  The AIS message, sent as a result of the server layer fault report, is
  delivered to the client layer MEPs on each client layer LSP before or
  within a short period after the client layer LSPs each detect their
  own faults, and so the client layer alarms can be suppressed. In this
  model, a storm of client layer alarms is traded for a storm of client
  layer AIS messages.

---

Section 2.1.1 has some icky passive voice.
L flag is set by whom?

   For example during a
   protection switching event the L-flag is not set

Can you clarify that as a server layer protection switching event?
                                                                           
---

Section 2.1.1

   Note again
   that the L-flag is not until a defect has been declared.

s/is not/is not set/

---
                                                                     
What can you tell me about a manual switch-over or a protected server
layer connection? Does this use AIS with L-bit clear, or does it use
LKR? In either case, the fault would be cleared pretty fast.

---

2.3.  Propagation of MPLS Fault Messages

   If the CC function is disabled, a MEP SHOULD generate AIS messages
   toward any client when either the AIS or LKR indication is raised.
   Note that the L-flag is not automatically propagated.  The rules of
   Section 2.1.1 apply.  In particular, the L-flag is not set until a
   defect has been declared.

Is this talking about CC in the client or server layer?
How would the MEP (in the server layer) or a MIP in the client layer
know the status of CC in the client layer?
I think this text is ambiguous (and possibly unhelpful). 
Can you rewrite or delete?

---

Section 3

  code point = 0xHH

You use 0xHH notation several times, but Section 8.1 uses 0xHHHH

----

Section 3

   Note: An early codepoint
   allocation has made: 0x0058 Fault OAM (TEMPORARY - expires
   2012-07-20)]

Can you move this to Section 8.1
s/has/was/

---

Section 4

It would be nice to order the text in the same way as the fields in the
message.

---

Section 4

It seems to me really odd to be having a version within a version.
Couldn't a new version simply burn a new ACH Type?

---

Section 4

   Total TLV Length

      The total TLV length is the total of all included TLVs.

In bytes?                                  

---

Section 4

      R-flag

         The R-flag is normally set to zero.  A setting of one indicates
         the removal of a previously sent FM condition.

Normal depends on your frame of reference. Some people come from rural
West Virginia!

Can you say:

      R-flag

         The R-flag is clear t indicate the presence of an FM condition 
         and is to one to indicate the removal of a previously sent FM 
         condition.

---

Section 5.2

   Ceasing to send FM messages will clear the indication after 3.5 times
   the refresh timer.  To clear an indication more quickly, the
   following procedure is used.  The R-flag of the FM message is set to
   one.  Other fields of the FM message SHOULD NOT be modified.  The
   message is sent immediately and then retransmitted two more times at
   an interval of one second.

Ouch!

Suppose the fault recurs before the last retransmission has been sent?
Since you are not using fault sequence numbers or timestamps, you MUST
stop retransmitting the fault clear indication if the fault reappears.

Similarly, when clearing a fault, you MUST stop sending any fault
indication retransmissions.

This is "obvious to one skilled in the art" but your spec actually 
appears to say otherwise.

---

Section 5.3

Unless you retire the Ver field (see above) its section needs to say what
to do if the version is not supported.

---

Section 5.3

   If the R-flag is set to zero, the MEP checks to see if a condition
   matching the message type and IF_ID exists.  If it does not, the
   condition to the message type is entered.  An expiration-timer is set
   to 3.5 times the refresh timer.  If the message type and IF_ID match
   an existing condition, message is considered a refresh and the
   expiration-timer is reset.

s/condition to the message/condition specific to the message/

s/message is/the message is/

Note that you said that IF_ID is an optional TLV if the R-flag 
procedures are not going to be used. Should you describe what happens if
the IF_ID is not included? Or maybe you should cut out one of the 
options such that either R-flag is always used or (more likely) IF_ID is
always mandatory.

Or, perhaps the check is actually on the tuple
{source node, LSP, message type, [IF_ID]}

---                                     

Section 6

   The following items are optional to implement.

s/optional/OPTIONAL/

---

Section 6

   2.  Support of setting the L-flag to indicated a defect.

s/indicated/indicate/

---

Section 6

OLD
   1.  Sending AIS and LKR message with other values of the Refresh
       Timer other than 1 second.
NEW
   1.  Sending AIS and LKR message with values of the Refresh Timer
       other than 1 second.

---

Section 7

OLD
   Thus, there is no simple way of
   preventing any traffic being carried between in the G-ACh consenting
   nodes.
NEW
   Thus, there is no simple way of
   preventing any traffic being carried in the G-ACh between consenting
   nodes.

---

Section 7

   It should be noted that the G-ACh is essentially connection-oriented
   so injection or modification of control messages specified in this
   document require the subversion of a transit node.

s/require/requires/

---

Section 7

   However, since these messages are carried in a control
   channel, except of one case discussed below

s/of/for/

---

Section 7

   If external MPLS traffic is mapped to an LSP via a PHP forwarding
   operation, it is possible to insert a GAL label followed by a fault
   OAM message.  In such a situation an operator SHOULD filter any fault
   OAM messages with the GAL label at the top of the label stack.

s/GAL label/GAL/  (twice)
s/SHOULD filter/SHOULD protect against this attack by filtering/

---

Section 8

Shouldn't you have a registry for MPLS Fault Management Message flags?
        
---

Section 8.2

   This sections details the MPLS Fault OAM TLV Registry, a new name
   spaces to be managed by IANA. 

s/sections/section/
s/spaces/space/

---

Section 8.2                                                                
                                                                          
   MPLS Fault OAM Message Types take values in the range 0-255.
   Assignments in the range 0-251 are via Standards Action; values in
   the range 251-255 are for Private Use, and MUST NOT be allocated.

   Message Types defined in this document are:

      Msg Type           Description
      --------           -----------------------------
        0x0              Reserved

1. What does "Reserved" mean?
   IANA will need to know whether this is available for allocation or 
   not. If it is not available I suggest you write...
   "Reserved (not available for allocation)"
   But, if this is the case, you need to explain to me why the range is
   0-255 and why zero is not available.

2. I am *extremely" wary of "private use" in this context. This is an
   end-to-end (or at least middle-to-end) protocol, yet you are
   suggesting site-specific usage. There is no way for the receiver of
   one of these messages to easily work out which site has transmitted.

   Can you explain what you have in mind?

---

Section 8.3

   This sections details the MPLS Fault OAM TLV Registry, a new name
   spaces to be managed by IANA.

s/sections/section/
s/spaces/space/

---

Section 8.3

   MPLS Fault OAM TLVs which take values in the range 0-255.
   Assignments in the range 0-191 are via Standards Action; assignments
   in the range 192-248 are made via "Specification Required"; values in
   the range 248-255 are for Private Use, and MUST NOT be allocated.

   TLVs defined in this document are:

      Value    TLV Name
      -----    -------
          0    Reserved
          1    Interface Identifier TLV
          2    Global Identifier

Same questions as for the message types with the additional question of
why you felt the need for Specification Required code points.

---

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 23 Aug 2011 17:25:59 +0100

Re-send with correct tools address.

Please reply on this email.

Adrian

> -----Original Message-----
> From: Adrian Farrel [mailto:[email-masked]]
> Sent: 23 August 2011 17:22
> To: [email-masked]'
> Cc: [email-masked]; [email-masked]
> Subject: AD Review of draf-ietf-mpls-tp-fault-06
> 
> Hi,
> 
way
> forward requires substantial change to the document.
>