[mpls] MIB Dr. review of draft-ietf-mpls-tp-te-mib-05

["Joan Cucchiara" <[e-mail]>]

Sam,

I am sorry for such a long delay....here is the MIB Dr. Review.   Lots of 
progress on the draft!

Please see the comments below.

Thanks,
-Joan


smicng OUTPUT
--------------

MPLS-LSR-EXT-STD-MIB
======================
W: f(MPLS-LSR-EXT-STD-MIB.my), (216,18) For "mplsXCExtTunnelPointer", syntax 
is identical
W: f(MPLS-LSR-EXT-STD-MIB.my), (215,18) MIN-ACCESS value identical to access 
specified for "mplsXCExtTunnelPointer"
W: f(MPLS-LSR-EXT-STD-MIB.my), (223,18) For "mplsXCExtOppositeDirXCPtr", 
syntax is identical

*) The DESCRIPTION clauses related to these objects
should be updated to include the information in this
conformance section.




MPLS-TE-EXT-STD-MIB
=====================
E: f(MPLS-TE-EXT-STD-MIB.my), (284,17) Index item 
"mplsTunnelExtNodeIpMapNodeId" must be defined with syntax that includes a 
range

I see the following in MPLS-TC-EXT-STD-MIB
   MplsNodeId ::= TEXTUAL-CONVENTION
   ....
      SYNTAX  Unsigned32  -- the default range: (0..4294967295)

my suggestion is to include the range as part of the SYNTAX in the TC.


smilint
-----------
MIBs compile cleanly.


GENERAL COMMENTS
----------------
*) The terms augments/sparse augments/extensions are all used to
describe relationships with tables in this document and
tables in rfc3812 and rfc3813.  For example,
a table in this document says "sparse augments" but then has
a value of noSuchInstance returned if the counter is not applicable
for TP.

Another part of the document says that a table in this document
augments a table from elsewhere, but that does not seem to be the
case when looking at the MIB module.

The relationships beween tables in this document and
the tables in rfc3812 and rfc3813 need to be specified clearly.
I would ask that the authors please check these relationships between
the tables.   This may be (simply?) using the term
'sparse augments' consistently.



*) Appendix D of rfc4181 (MIB guidelines) suggests:


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

The MIB Modules in this doc:

   \-2 Conformance
     \-v-1 Groups

       \-2 Compliances

So Groups and Compliances are not in the suggested order.


Specific Comments
=====================

*) Introduction

"This MIB module should be used...."

Which MIB module are you referring to?   (Do you mean "These MIB modules"??)

*) Same Section (Intro)
"...for MPLS based traffic engineering configuration and management."

I don't understand completely, but seems like MPLS TP is probably what is 
meant.


*) 4. Motivations

s/used in non-IP environment./used in non-IP environments.

awkward sentence:  This MIB also defines three other MIB modules within this 
document.
Maybe:  This document defines 4 MIB modules: (and then list all 4)


5. Feature List
Could you refer to the MIB module by name?  So, instead of
"MPLS transport profile MIB module" use the MPLS-TE-EXT-STD-MIB
and so forth.


6. Brief description of MIB Objects
This section focuses on MPLS-TE-EXT-STD-MIB, so maybe rename the section
or also include the other MIB modules in this document.

6.5 mplsTunnelExtReversePerfTable

"This table augments the mplsTunnelTable..."

If it augments, then why is AUGMENTS not used in the
MPLS-TE-EXT-STD-MIB?   I think this relationship is a
sparce augments, not an actual augments?   Please
explain.


MPLS-TC-EXT-MIB
==================
*)  Why is the CC ID not represented in this MIB Module?

Basically, the question has to do with the following from
draft-ietf-mpls-tp-itu-t-identifiers, specifically:

  "Together, the CC and the ICC form the ICC_Operator_ID as:

      CC::ICC

   The ICC_Operator_ID is used as a replacement for the Global_ID as
   specified in [RFC6370], i.e. its purpose is to provide a globally
   unique context for other MPLS-TP identifiers."

ICC_Operator_ID appears to be the counterpart to GLOBAL_ID.  In other
words, these both uniquely identify an operator. Whereas, ICC by itself
does not.


MPLS-ID-STD-MIB
=================

*)What is the relationship between mplsIdGlobalId and mplsIdNodeId?
Specifically, if mplsIdGlobalId has been set, can mplsIdNodeId be
set to a different value?


*) NIT:  Could the order reflect the same order as in MPLS-TC-EXT-STD-MIB?
The reason is that GLobal_ID and NODE_ID are related so think would be
logical to have them near each other.

*) Also, I'm not really sure why you want a MIB module with only 3 scalars
in it?  As far as I can see these scalars are only used in the
MPLS-TE-EXT-STD-MIB, so why separate them?

*) Error:  FullCompliance and ReadOnlyCompliance is EXACTLY the same.

MPLS-LSR-EXT-STD-MIB
=====================

Compliance:
     OBJECT      mplsXCExtTunnelPointer
     SYNTAX      RowPointer
     MIN-ACCESS  read-only
     DESCRIPTION
        "The only valid value for Tunnel Pointer is
         mplsTunnelTable entry."

*) This object is read-only, so no need for a read-only compliance
since object is already read-only.

*) The above DESCRIPTION needs to be included in the
object's DESCRIPTION.


     OBJECT      mplsXCExtOppositeDirXCPtr
     SYNTAX      RowPointer
     MIN-ACCESS  read-only
     DESCRIPTION
        "The only valid value for XC Pointer is
         mplsXCTable entry."

     ::= { mplsLsrExtCompliances 2 }

*) The above DESCRIPTION needs to be included in the
object's DESCRIPTION.

*) There is nothing here that specifies the readOnly compliance.
Would expect something like:

OBJECT nameOfObject
MIN-ACCESS  read-only
DESCRIPTION "Write access is not required."


MPLS-TE-EXT-STD-MIB
===================

*)          DESCRIPTION
           "This object indicates the Global Operator Identifier.
            This object value should be zero when
            mplsTunnelExtNodeConfigIccId is configured with non-null
            value."

DESCRIPTION should state that the object has no meaning when
mplsTunnelExtNodeConfigIccId is valid.  Same comment for
mplsTunnelExtNodeConfigId.

In other words, maybe an additional object is needed to indicate
Global or ICC?  What if a 3rd TP id is created, then what happens?

Better to be explicite I think, than to try and overload these
objects with values that they shouldn't have.

mplsTunnelExtNodeConfigNodeId  OBJECT-TYPE
         SYNTAX        MplsNodeId
         MAX-ACCESS    read-create
         STATUS        current
         DESCRIPTION
            "This object indicates the Node_ID within the operator.

*) Sentence is awkward.

mplsTunnelExtNodeConfigIccId
Similar comment to the above.  Additionally, how can an OCTET STRING
of size (1..6) have a zero value?

*) mplsTunnelExtNodeConfigStorageType

Not sure I see the advantage of having this as it is
already in the MplsTunnelTable.  Why have 2 StorageType objects
for the same row?

*) mplsTunnelExtIngressLSRLocalIdValid and
mplsTunnelExtEgressLSRLocalIdValid
Please fix the DESCRIPTION and REFERENCE.
(Looks like the DESCRIPTION clause has been continued after the
REF clause.)



*) mplsTunnelExtReversePerfTable

I would like to understand why noSuchInstance would be needed.
This table is a sparse augments,
so either the entry would be there or it wouldn't, right?

*) Probably could remove the following (comment applies to other
MIB Modules also.)

     -- Notifications
     mplsTeExtNotifications OBJECT IDENTIFIER
                                      ::= { mplsTeExtStdMIB 0 }

     -- Notifications.
     -- Notification objects need to be added here.
     -- End of notifications.


*) Compliance Section
ReadOnly compliance is the same as the Full Compliance.
In other words, readonly compliance does not really exist.
Please fix this.


14. Security Considerations
============================
Should specify which objects would jeopardize security.

15. IANA Considerations
========================

Where is it?  This section needs to be done.


-- the end --