[CCAMP] AD review of draft-ietf-ccamp-gmpls-ted-mib

"Adrian Farrel" <adrian@olddog.co.uk> Tue, 29 May 2012 22:22 UTC

Return-Path: <adrian@olddog.co.uk>
X-Original-To: ccamp@ietfa.amsl.com
Delivered-To: ccamp@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5754E11E8125 for <ccamp@ietfa.amsl.com>; Tue, 29 May 2012 15:22:39 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.989
X-Spam-Level:
X-Spam-Status: No, score=-1.989 tagged_above=-999 required=5 tests=[AWL=0.010, BAYES_00=-2.599, J_CHICKENPOX_21=0.6]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TsNdWsrXKbcU for <ccamp@ietfa.amsl.com>; Tue, 29 May 2012 15:22:38 -0700 (PDT)
Received: from asmtp4.iomartmail.com (asmtp4.iomartmail.com [62.128.201.175]) by ietfa.amsl.com (Postfix) with ESMTP id 1415811E815C for <ccamp@ietf.org>; Tue, 29 May 2012 15:22:37 -0700 (PDT)
Received: from asmtp4.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp4.iomartmail.com (8.13.8/8.13.8) with ESMTP id q4TMMals010332; Tue, 29 May 2012 23:22:36 +0100
Received: from 950129200 (dsl-sp-81-140-15-32.in-addr.broadbandscope.com [81.140.15.32]) (authenticated bits=0) by asmtp4.iomartmail.com (8.13.8/8.13.8) with ESMTP id q4TMMZfB010314 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Tue, 29 May 2012 23:22:36 +0100
From: Adrian Farrel <adrian@olddog.co.uk>
To: draft-ietf-ccamp-gmpls-ted-mib.all@tools.ietf.org
Date: Tue, 29 May 2012 23:22:33 +0100
Message-ID: <0b7101cd3de9$8bfadbc0$a3f09340$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Thread-Index: Ac096YYfEpv6SLpwSQaFEN34z8kq/Q==
Content-Language: en-gb
Cc: ccamp@ietf.org
Subject: [CCAMP] AD review of draft-ietf-ccamp-gmpls-ted-mib
X-BeenThere: ccamp@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
Reply-To: adrian@olddog.co.uk
List-Id: Discussion list for the CCAMP working group <ccamp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ccamp>, <mailto:ccamp-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/ccamp>
List-Post: <mailto:ccamp@ietf.org>
List-Help: <mailto:ccamp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ccamp>, <mailto:ccamp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 29 May 2012 22:22:39 -0000

Hi,

I have carried out my normal AD review of your draft. There is no cause
for alarm!

This looks to be a sound document, but as always with MIB modules it is
possible to find a number of nits and questions. I would appreciate a 
revision of the document before we move it forward so that it will get a
smoother ride through the rest of the process. I will put the document
into "Revised ID needed" state in the tracker.

As always, all issues and questions are open for discussion.

Thanks,
Adrian

---

Per idnits, please change the header 
OLD
Intended status: Standards Truck
NEW
Intended status: Proposed Standard
END

---

I am not comfortable with the two definitions
   OAM:   Operations and Management 
   OA&M:  Operations, Administration and Maintenance. 
This is counter to RFC 6291which, although only guidelines, should be
followed unless there is a good reason. That RFC has...
   o  O&M - OAM and Management
   o  OAM - Operations, Administration, and Maintenance
Fortunately neither term is used in your document, and so they can be
deleted.

---

There are some acronyms in 3.3 that are not used in the rest of the
document: FSC, LDP, LSC, LSR, OAM, OA&M, RSVP, 

---

Could you please sort out "MIB" versus "MIB module". There is only one
MIB. There are many MIB modules - this document defines a MIB module.

---

Section 6 is missing a close brace "}" for tedTable

---

The copyright date in the MODULE-IDENTITY is 2011.

---

You need to avoid using citations (e.g. [RFC3630]) within the MIB module
itself. MIB modules are extracted from their documents and stand alone.

---

The DESCRIPTION for the tedTable is
  "This table indicates multiple TED information which has been 
   supported by [RFC3630]." 

This seems to imply this doesn't apply to ISIS. Maybe update the text?

Similarly, a number of objects only give references to OSPF or OSPF-TE.
Not referencing the corresponding IS-IS specs looks like the object only
has meaning for OSPF.

---

   tedEntry OBJECT-TYPE 
       SYNTAX       TedEntry 
       MAX-ACCESS   not-accessible 
       STATUS       current 
       DESCRIPTION 
       "This entry contains TED information commonly utilized in both 
   MPLS and GMPLS." 
      INDEX { tedLocalRouterId, tedRemoteRouterId, 
   tedLinkInformationSource, tedLinkIndex } 
   ::= { tedTable 1 } 
    
   TedEntry ::= SEQUENCE { 
       tedLinkInformationSource     INTEGER, 
       tedLocalRouterId             TedRouterIdTC, 
       tedRemoteRouterId            TedRouterIdTC, 
       tedLinkIndex                 TedLinkIndexTC, 

It is unusual to list the index fields in a different order from that
which they appear in the table entry. Is this intended?

---

Tom's surname is misspelled in several of the references!

---                                                      

I'm having some trouble with the use of 2119 language in the Description
for tedLinkInformationData.

Two examples...

          If tedLinkInformationSource has the value unknown(0), this 
       object SHOULD contain a value of zeroDotZero.  

Under what circumstances can this use of "SHOULD" be varied?

          If tedLinkInformationSource has the value 
       locallyConfigured(1), this object MAY contain the identifier of 
       the corresponding row entry in the teLinkTable of TE-LINK-STD-
       MIB[RFC4220], the identifier of the corresponding row in a local 
       proprietary TE link MIB module, or the value of zeroDotZero 
       otherwise. 

The use of "MAY" here implies that an implementation can choose to fill
in the row pointer if it likes, but this is entirely an implementation 
choice. Is this what you are saying, or do you want to constrain the
behavior more forcefully if TE-LINK-STD-MIB is implemented?

---

   tedRouterIdAddrType OBJECT-TYPE 
       SYNTAX       InetAddressType 
       MAX-ACCESS   read-only 
       STATUS       current 
       DESCRIPTION 
         " This object indicates the TE-Router ID address type. Only 
   values unknown(0), ipv4(1) or ipv6(2) must be supported. " 

Do you mean s/must be/are/  ?

Similarly for tedLinkIdAddrType

---

It's a small point, but I would have preferred you to rename
tedRouterIdAddrType and tedRouterIdAddr as tedTeRouterIdAddrType and
tedTeRouterIdAddr to distinguish them from the normal router ID.

---

I don't understand what values should be returned for objects that 
only apply in some circumstances. For example, in a non-GMPLS system,
what would be returned for tedLinkProtectionType? In either a non-GMPLS
system or one with numbered links, what values would be returned for
tedLocalId and tedRemoteId? What about tedSwCapIndication for non-TDM
links?

Section 6 says in these cases the objects are not supported, but I don't
think it is that simple. Or is it?

Even if it is that simple, I think the Description clauses should not 
the circumstances under which the object is not supported.

(And yes, I do see the compliance statements which are very 
comprehensive, but I think they say what you have to support in 
specific circumstances, not how to behave if you support an object but
have no information to return.)

---    
    
   tedLocalIfAddrEntry OBJECT-TYPE 
       SYNTAX       TedLocalIfAddrEntry 
       MAX-ACCESS   not-accessible 
       STATUS       current 
       DESCRIPTION 
         "This entry contains the IP address information of the local TE 
   link." 
       INDEX { tedLinkIndex, tedLocalIfAddr } 
   ::= { tedLocalIfAddrTable 1 } 
    
   TedLocalIfAddrEntry ::= SEQUENCE { 
       tedLocalIfAddrType    InetAddressType, 
       tedLocalIfAddr        InetAddress 
    
       } 

What would it look like to walk this table by increasing index value?
Would all the shorter v4 addresses show first, or would the numerically
smaller v6 addresses get mixed in with the v4 addresses?

If the latter is possible, you should use the address type as an index 
as well.

---

A broader point about the use of addresses as indexes is found in
RFC 4001 in the Description of the InetAddress TC...

         When this textual convention is used as the syntax of an
         index object, there may be issues with the limit of 128
         sub-identifiers specified in SMIv2, STD 58.  In this case,
         the object definition MUST include a 'SIZE' clause to
         limit the number of potential instance sub-identifiers;
         otherwise the applicable constraints MUST be stated in
         the appropriate conceptual row DESCRIPTION clauses, or
         in the surrounding documentation if there is no single
         DESCRIPTION clause that is appropriate."

---

tedStatusChangeNotificationMaxRate and
tedCreatedDeletedNotificationMaxRate

I am surprised that the Defval for these is 0 (no throttling) since 
the text recognises that this is a risky situation that may cause the
box to blow up. Surely the default should be some safe setting.

But then I wondered what a suitable safe setting would be and realised
it would be "No Notifications". Except there is no way to say this with
these objects.

What do you think?                                 

---

Would the Notifications be more helpful if they indicated the index of
the entry in the tedTable to which they applied?

---

For completeness, we usually get asked to state in the Security section
what the risks are with write access objects. In this case it is really
easy...

   There are only two write-access objects in this MIB module:
   tedStatusChangeNotificationMaxRate and 
   tedCreatedDeletedNotificationMaxRate.  Malicious modification of 
   these objects could cause the management agent, the network, or the
   router to become overloaded with Notifications in cases of high
   churn within the network.