Subject: Last Call comments on draft-ietf-ccamp-gmpls-alarm-spec-01.txt

From: "Adrian Farrel" <[email-masked]>
Date: Tue, 21 Sep 2004 18:44:55 +0100

Hi,

A few comments on this draft.
Boilerplate and nits issues may have been caught when you went to rev01. I haven't
checked.

Thanks,
Adrian
======
Editorial:

Please make sure you have new boilerplate. Run the draft through the script at
http://ietf.levkowetz.com/tools/idnits/

Abstract has "Generalized MPLS" with an expansion of MPLS, but then uses GMPLS without
explanation.
Suggest you change it to "GMPLS (Generalized Multi-Protocol Label Switching)"

Section 3.1.2
   record the information contained in a received ALARM_SPECs for later
Either "in received ALARM_SPECs" or "in a received ALARM_SPEC"

Section 3.1.2
   for the affected LSP.  The IPv4 or IPv6 IF_ID ALARM_SPEC object
"effected"!

Section 3.1.2
   Note, ALARM_SPEC objects SHOULD NOT be added to LSPs that are in
   "alarm communication inhibited."  ALARM_SPEC objects MAY be added to
   LSPs that are "administratively down".  These states are indicated by
   the I and A bits of the Admin_Status object, see Section 3.2.
Two points:
a. Alarm_Spec objects are added to messages not to LSPs (twice)
b. "alarm communication inhibited." add "state"

Section 3.1.2
   message that contain ALARM_SPEC objects.  Note that changes in
"message" --> "messages"

Section 3.2.2
   (0), it should add any local alarm information to the matching LSP's
read "SHOULD"

Section 3.5
   interface, see [GMPLS-ASON].  At this interface, restrictions may be
"ASON" --> "ENNI"

5. IANA Considerations
   of [RFC3473].  We recommend that IANA being managing assignment of
"being" --> "begin"

6.2. Informative References
Since the Bellcore document is not an IETF document, it would be useful to give a
reference that indicates where the document can be located.

References.
Some of the references are out of date.

Alarm MIB is now RFC3877

==========
Technical:

1. Introduction
I think it is necessary to add a final paragraph to make it abundantly clear that these
extensions are NOT intended to offer alarm signaling for fault propagation. It is very
important to make sure that this mechanism is not seen as a replacement for existing
(faster) fault propagation techniques. It may even be worth adding this to the Abstract.

3.1.1
   No rules apply to the relative ordering of these TLVs.  These TLVs
   MUST be listed after any interface identifying TLVs.
This is ambiguous. Does this mean that if an interface identifying TLV is present *all* of
the other TLVs MUST be present?
I don't think so. Perhaps you mean "Where an interface identifying TLV is used, it MUST
appear before any of the TLVs defined here."

3.1.1
Reference Count TLV
      Reference Count: 32 bits

         The number of times this alarm has been repeated.  This field
         MUST NOT be set to zero.
This is ambiguous. Does "repeated" mean "over all time", "since service establishment" or
"concurrently".
In fact, there is no description of the use of the Reference Count TLV anywhere in the
document. Please add something to 3.1.2.

3.1.1
Severity TLV
      Reserved: 24 bits

         This field is reserved.  It MUST be set to zero on generation
         and MUST be ignored on receipt.
I believe we have fallen foul of this ambiguity before. Some transit nodes that forward an
object believe that they are "generating" it (or at least re-generating it) and therefore
must zero out such fields. Clearly this breaks future extensions. So please add "and MUST
be forwarded unchanged and unexamined" or somesuch.

3.1.1
Severity TLV
      Impact: 4 bits

         Indicates the impact of the alarm indicated in the TLV.  The
         following values are defined:

          Value       Definition
          -----       ---------------------
            0         Unspecified impact
            1         Non-Service Affecting
            2         Service Affecting
Although it may seem obvious, you need to state the meaning of service affecting or give a
reference to something that defines it. For example, if the service is degraded but not
failed, is that service affecting or not?

3.1.1
Severity TLV
      Severity: 8 bits

         Indicates the impact of the alarm indicated in the TLV.  The
         following values are defined:
I believe you need to describe how a value is selected. For this field it is probably best
to state that the value set is dependent on the alarm raised and is a local matter which
may be governed by network policy. The alternative is to refer to some external
documentation of the correlation between alarms and severity.

3.1.1
Error String TLV
Please clarify whether the length field of the TLV includes or excludes the padding.

3.1.2
   The IPv4 and
   IPv6 formats of the ALARM_SPEC object, C-Type 1 and 2, SHOULD NOT be
   used as they do not support the inclusion of the TLVs defined above.
It still baffles me why this document defines two object C-Types in order to specify that
they SHOULD NOT be used. IMHO, they SHOULD NOT be defined.

3.1.2
   on Error Code and Error Values.  The InPlace and NotGuilty flags
   SHOULD NOT be set.
And what does it mean when they are set? or MUST they be ignored?

3.1.2
   any, the severity, the time and a brief string associated with the
Please quantify "brief" or delete the word.

3.1.2
There is no description of the inclusion of timestamps. In particular, whether both Global
and Local Timestamps may be included, and why an implementation would include one rather
than the other.

3.1.2
I didn't find any comment about the order of Alarm_Spec objects. I presume that "no rules
apply to the relative ordering of Alarm_Spec objects within a Path or Resv message".

3.2.2. Procedures
It is not clear to me whether any node can decide to toggle the I-bit. If so, might we not
have a "toggle war" between two nodes? For example, suppose an LSP is originated by an LSR
that does not support these extensions. By default it will set the I-bit to zero
indicating that alarm information is required. An LSR several hops downstream may
recognise that inhibition is required and set the bit. Does RFC3473 cover this situation?
There are two alternatives:
a. only the ingress can set the I-bit
b. transit nodes may modify the I-bit with the intent of it applying only on the next
segment of the path of the message
Further, are the I-bits on Path and Resv independent?

3.2.2.
The text describes what to do when the A and I bits are set, and when the A and I bits are
clear.
It does not describe what to do when only one of the two bits is set.

3.2.2
   The processing of non-locally
   generated ALARM_SPEC objects MUST NOT be impacted by the contents of
   the Admin_Status object.
For clarity, please add "that is, received ALARM_SPEC objects MUST be forwarded unchanged
regardless of the received or transmitted settings of the I and A-bits."

3.4. Relationship to GMPLS UNI
In the bullets, the text refers to "sending to the overlay UNI LSP". I think you need to
discuss this in terms of "sending to the overlay network edge node".

3.4
The first and fourth bullets appear to say the same thing. Should one of them describe
alarms from the overlay network and one of them alarms from the core network?

4. Security Considerations
   Some operators may consider alarm information as sensitive.  To
   support environments where this is the case, implementations SHOULD
   allow the user to disable the generation of ALARM_SPEC objects.
Please add "or to filter or correlate them at domain boundaries"