Subject: [CCAMP] MIB Dr. review of draft-ietf-gmpls-ted-mib-5.txt

From: "Joan Cucchiara" <[email-masked]>
Date: Fri, 26 Jun 2009 09:33:09 -0400

Hello,

Very interesting MIB!  A good deal of thought went into this.
Thanks for that!

First, the MIB compiler output will be given.  Followed by a
general discussion and then specific comments given which
are in order of the document.

Thanks,
  -Joan

MIB Compiler output:
=======================

smicngPRO
----------

* (NIT) Unable to abstract MIB Module using mstrip tool.

E: f(TED-MIB.my), (460,4) Index item "teLocalIntAddrIndex" must be defined 
with syntax that includes a range
E: f(TED-MIB.my), (518,49) Index item "teRemoteIntAddrIndex" must be defined 
with syntax that includes a range
E: f(TED-MIB.my), (579,53) Index item "teSwCapIndex" must be defined with 
syntax that includes a range
E: f(TED-MIB.my), (765,53) Index item "teSrlgIndex" must be defined with 
syntax that includes a range
W: f(TED-MIB.my), (797,9) Variable "teAreaId" in notification 
"tedTeInfoStatusChange" is an index for a table
W: f(TED-MIB.my), (797,19) Variable "teRouterId" in notification 
"tedTeInfoStatusChange" is an index for a table
W: f(TED-MIB.my), (797,31) Variable "teLinkStateId" in notification 
"tedTeInfoStatusChange" is an index for a table
W: f(TED-MIB.my), (809,9) Variable "teAreaId" in notification 
"tedTeCreation" is an index for a table
W: f(TED-MIB.my), (809,19) Variable "teRouterId" in notification 
"tedTeCreation" is an index for a table
W: f(TED-MIB.my), (809,31) Variable "teLinkStateId" in notification 
"tedTeCreation" is an index for a table
W: f(TED-MIB.my), (819,9) Variable "teAreaId" in notification 
"tedTeDeletion" is an index for a table
W: f(TED-MIB.my), (819,19) Variable "teRouterId" in notification 
"tedTeDeletion" is an index for a table
W: f(TED-MIB.my), (819,31) Variable "teLinkStateId" in notification 
"tedTeDeletion" is an index for a table
W: f(TED-MIB.my), (926,7) NOTIFICATION-GROUP "tedNotificationGroup" is not 
used in a MODULE-COMPLIANCE in current module

smiLint
------------

Validation report
File: TED-MIB.my
Severity level requested: 6

Line  Severity  Problem
109 5 warning: index element `teAreaId' of row `tedEntry' should be 
not-accessible in SMIv2 MIB
        5 warning: index element `teRouterId' of row `tedEntry' should be 
not-accessible in SMIv2 MIB
        5 warning: index element `teLinkStateId' of row `tedEntry' should be 
not-accessible in SMIv2 MIB
926 5 warning: current group `tedNotificationGroup' is not referenced in 
this module

General Discussion
------------------

* As an fyi, the OSPFv3 MIB (for IPv6) is "IESG Evaluation".

* In what GMPLS WG document is this functionality defined?

I see a lot of MIBs referenced in MPLS, OSPF, ISIS and GMPLS and
also a lot of documents being referenced, but not a specific TED one.

* The MIB objects themselves do not have any REFERENCE clauses and most of 
these
probably should given the nature of this MIB (i.e. that it is pulling from
so many different documents.)  Please be specific in your REFERENCE clauses
such as document, section, and sub-section, if appropriate.

* The names in this MIB are not prefixed consistently.
Please see Appendix C of RFC4181.

For example, TED-MIB is the MIB's name, and then
TedEntry, but the objects in this table are prefixed by "te" (but "ted" 
would
be more consistent.)

- The abbreviation for interface is "int" but would prefer either "if" or 
"intf".

* With the exception of 2 objects, the MIB consists of read-only objects.
While this is fine in and of itself, there are several places in the text 
that the
authors warn that if local configuration is done then routing needs to be
triggered so the new configuration gets dispersed.  When looking at these
MIB tables, is there any way to tell if the information is currently in use 
or not?

Specific Comments (in order of document)
------------------------------------------

* Table of Contents

s/Author's Address/Authors' Addresses/

* TOC is missing "Informative References" entry.

* 2. Introduction

NIT:
s/related with/related to/

* In what GMPLS WG document is the functionality of TED defined?

4. Motivations

* Why is ISIS not mentioned here?

6. Example of TED MIB module usage
s/module usage/Module Usage/

* Second paragraph "TE MIB"
s/TE MIB/TED MIB/

The second paragraph is confusing.  The Tables reflect information as
read only, yet, this paragraph discusses changing this information
locally (e.g. on a router) and then triggering routing to update
this information.

The example given is confusing. What are the index values supposed to be?

MIB MODULE
-------------

* Please differentiate values as XXX or YYY for the rfc editor.

* as suggested by Appendix D of RFC4181 (Guidelines for MIB Documents)
the oid layout should be:

         xxxMIB
         |
         +-- xxxNotifications(0)
         +-- xxxObjects(1)
         +-- xxxConformance(2)
             |
             +-- xxxCompliances(1)
             +-- xxxGroups(2)

This OID layout is:
- tedMIB
 \-v-0 tedNotifications
   |-1 tedObjects
   \-2 tedConformance
     \-v-1 tedGroups
       \-2 tedCompliances

Groups and Compliances should be reversed.

* Additionally there are extraneous sub-ids
of tedScalars and tedTables under the tedObjects branch.
While many older MIBs separated scalars and tables this way,
this is not needed and while fine for the MIB as it exists
currently, if this MIB is added to in the future (e.g. if an
enterprise MIB developer wants to add objects via his/her own
enterprise MIB Module), then keeping the separation of scalars
and tables may be a hinderance.   I would ask that you remove
these sub-ids (tedScalars and tedTables) so that the oidtree
will look like:

   |-1 tedObjects
   |   \-1 tedNotificationEnabled, TruthValue[SNMPv2-TC], rw, current
   |   \-2 tedNotificationMaxRate, Unsigned32, rw, current
       \-3 tedTable
     | \---1 tedEntry, INDEX{ teAreaId,teRouterId,teLinkStateId }
     |     \-v-1 teAreaId, Unsigned32(1..4294967295), ro, current

etc.

* tedNotificationEnabled

Why is this object needed?  Please answer the question wrt rfc3413.

* tedNotificationMaxRate

This one throttling mechanism is used to control 3 different notifications.
This might be okay, but that depends on the notifications and as they are
written now, they seem to be overlapping.  For example, if a TED is created
then are 2 notifications sent (one ofr StatusChange and one for Creation)
or just one (for Creation)?

* teLinkInformationData

The DESCRIPTION mentions the "teLinkTable of TE-LINK-STD-MIB", could you 
please
tell me what draft or rfc this is?

What about OSPFv3 and other protocols that may be related to this MIB (at 
some point?)
Are you sure that this object is going to be useful (even in the 
short-term?)

* Notifications

The descriptions are very unclear.  Under what circumstances are these
generated?  Please be specific.

* Conformance statements are missing the restrictions for
IpAddress and IpAddressType objects in this MIB.
Please correct this.

* ReadOnly Compliance is incorrect.  There are 2 writable objects
which need to be given read-only conformance.

* 8. Security consideration

s/consideration/Considerations/

- The first paragraph states there aren't any read-write objects but
this is not accurate.  Please update.

---

From: Lou Berger <[email-masked]>
Date: Fri, 26 Jun 2009 13:57:37 -0400

Joan,
	Thank you for the thorough review!

Authors,
	Please update the document accordingly and let the WG know once your
changes have been submitted.

Lou

On 6/26/2009 9:33 AM, Joan Cucchiara wrote:
> E: f(TED-MIB.my), (460,4) Index item "teLocalIntAddrIndex" must be
> defined with syntax that includes a range
> E: f(TED-MIB.my), (518,49) Index item "teRemoteIntAddrIndex" must be
> defined with syntax that includes a range
> E: f(TED-MIB.my), (579,53) Index item "teSwCapIndex" must be defined
> W: f(TED-MIB.my), (926,7) NOTIFICATION-GROUP "tedNotificationGroup" is
> not used in a MODULE-COMPLIANCE in current module
> 
> 
> 
> 
> 
> smiLint
> ------------
> 
> Validation report
> File: TED-MIB.my
>        5 warning: index element `teLinkStateId' of row `tedEntry' should
> be not-accessible in SMIv2 MIB
> * The MIB objects themselves do not have any REFERENCE clauses and most
> of these
> - The abbreviation for interface is "int" but would prefer either "if"
> or "intf".
> 
> 
> While this is fine in and of itself, there are several places in the
> text that the
> MIB tables, is there any way to tell if the information is currently in
> use or not?
> 
> 
> 
> What about OSPFv3 and other protocols that may be related to this MIB
> (at some point?)