MIB Dr. Review of draft-ietf-l3vpn-vr-mib-04.txt

[[e-mail]]

Hi Folks,

Following is the MIB Dr. Review of draft-ietf-l3vpn-vr-mib-04.txt

Thanks,
  -Joan

-------------------

Compiler Issues
-------------------------

Please fix the following compiler 
issues:


Output from smilint
----------------------
Validation report
File: VIRTUAL-ROUTER-MIB
Severity level requested: 6

Line  Severity  Problem
72 5 warning: type `VrIdentifier' has no format specification


Output from smicng
-------------------
E: f(VIRTUAL-ROUTER-MIB), (321,18) Default value 
for "vrRpTrigger" must be named bits in "{ }"





GENERAL QUESTIONS OR OBSERVATIONS
---------------------------------

* Is this MIB supposed to be standards track?
  There was some discussion on making the
  IPSEC and VR VPN architectures informational.

* The abbreviation (VR) should be pluralized when
  referring to more than one Virtual Router.


* update to reference rfc4265.txt
  which is the VPN-TC-STD-MIB
  (both in MIB and in Normative Reference Section)


TABLE OF CONTENTS
---------------------
* Table of Contents needs to have
  page numbers added.

* Also, no need to use .0 as in 1.0, 2.0, etc. for
  sections.  Please just use numbers 1., 2., etc.


2.0 Introduction
-------------------

* The document refers to PPVPN but the working group is
  now L3VPN so should this be updated in the references
  and elsewhere.

* The second and third goal should be removed.  The MIB Module
  should be implementable and also provide functionality as
  described by the first goal, so no need to state the 2nd
  and 3rd goal.   Please combine the first goal with the
  previous paragraph.

* Suggestion only:  you may want to combine the Terminology
  Section with the Introduction.


4.0  Overview of the Virtual Router MIB Module
------------------------------------------------

4.1 SNMP Contexts for Management for Virtual Routers
-------------------------------------------------------

This section is somewhat troubling due to some statements
such as "There is a need for a single agent..." mixed in
with some insightful considerations that may be 
taken into account by a network management
architect of a Virtual Router device.

Could you list these considerations without mandating a 
specific architecture?

For example, instead of mandating the use of VR 0 as an
"Admin" domain which has certain properties, could you state
that one consideration is to have a SNMP entity
on the device that can view all of the data, including data
that resides in the alll Virtual Routers.  

Additionally, the use of the context as a unique index accross the
VPN seems wrong and not sure that this is actually
enforceable.

Last but not least, some of the language used is inconsistent
with that of RFC3411.  Since you reference RFC3411, please 
be consistent with the use of terminology. 


4.3  Creation and Deletion of VRs
---------------------------------

* Please rename this to "The Virtual Router Configuration Table"
and update the text as needed.

* The paragraph:
"VRID 0 is assigned to the Administrative VR, which exists by default,
and need not be created. Deletion of the Administrative VR will not be
permitted.  The VRID of the Administrative VR (VRID 0) should be a
reserved VRID number.  VRID 0 could be termed the "null VR" and it 
could be the context that manages the resource pool of unattached 
interfaces. Routing would then not exist in the context of 
Administrative VR."  

Should probably be removed (again, this is imposing an
architecture which is out of scope of a MIB document.)


4.4 Administrative and Operational Status of VRs
-------------------------------------------------
* could this be renamed "The Virtual Router Status Table"


4.4.1  VR Routing Protocol Trigger
------------------------------------
* could you please put "Object" on the end of this.


4.6 Setting per VR limits
---------------------------
* this should be a subsection under the vrConfigTable
section. 

4.8 Traps
-----------
* The current terminology is notification.  Please change
this in the title of this section and through the document.

4.9 Usability Considerations
-------------------------------
You may want to completely remove any architectual descriptions
from this MIB document.  That would be my preference, but if
you believe that it's best to include such  section than
think you should list the above single SNMP agent description in
this section, along with the multiple agent description.


4.9.2  Provisioning vs. Monitoring
----------------------------------

This section is not needed.  The MIB Module's conformance
statements should specify both a read-write/create (full conformance)
and a read-only conformance.

5.1  Creation of a VR
-------------------------
 
* Item #2 says "Using a context with a 'read-write' access
for system level entities.  This should probably be reworded.

6.0 Definition of the Virtual Router MIB Module
--------------------------------------------------

* Name of the Module:  VIRTUAL-ROUTER-MIB

- Could you include VPN some place in this title? 
  e.g. VIRTUAL-ROUTER-VPN-MIB.  This should be carried
  as the prefix throughout the MIB, so would expect
  to see vrVpnObjects, vrVpnConformance, etc.



- if this is a standard's track document, then also 
  STD should be included in the MIB title to be consistent
  with other MIB modules from this working group:
  e.g. VIRTUAL-ROUTER-VPN-STD-MIB

* The following note should be removed as this is now RFC4265.
-- RFC Ed.: VPN-TC-STD-MIB in Last Call in L3VPN WG

* DisplayString is no longer used in MIB Modules.  This is
 because it does not support internationalized text.  
see:  http://www.ops.ietf.org/mib-common-tcs.html
Better to use SnmpAdminString defined in
SNMP-FRAMEWORK-MIB [RFC3411], or some other TC which supports
internationalization.

* VrIdentifier, please remove the VRID of 0 is the
  Administrative VR.

* VrRpTriggerBitCode
  Please specify that the value of 1 indicates that the
  Routing Protocol will be or has been initiated on the
  Virtual Router, and 0 indicates that the Routing protocol
  is not running, or has been requested to be shutdown.
  (In other words, please describe what the BIT values of
   1 and 0 mean.)

* Please put top level OIDs together, for example:

    vrNotifications OBJECT IDENTIFIER ::= { virtualRouterMIB 0 }
    vrMIBObjects OBJECT IDENTIFIER ::= { virtualRouterMIB 1 }
    vrConformance OBJECT IDENTIFIER ::= { virtualRouterMIB 2 }

- Please rename: vrNotifications to vrVpnNotifications,
  vrMIBObjects to vrVpnObjects, and
 vrConformance to vrVpnConformance., 

* Please rename vrConfig to vrVpnConfigObjects



* vrConfigNextAvailableVrId could use the TC from
  the DIFFSERV-MIB: IndexIntegerNextFree.  This TC is
  in use in several MIBs and will provide the same 
  functionality as you intend. 


* VrConfigEntry
  Suggestion only:  could you put the
  object name and Type on the same line.

  Also, please put RowStatus as the last object in the
  entry.  Granted these are more stylistic suggestions
  but seem to be fairly common place in IETF MIB Modules.


* vrId
- as mentioned previously, please remove the discussion regarding
  VRID 0.
- Please include some discussion about how to use 
  vrConfigNextAvailableVrId to acquire a value for this index.
  (suggestion to look at DIFFSERV-MIB as an example.)

* vrTrapEnable
- Please rename to vrNotificationEnabled
- I *think* this object can be removed and the MIBs in RFC2573 be used
  instead.  Please comment on this.  In other words, this object
  might control whether or not 
  the instrumentation generates a notification, but
  the actual sending is controlled by the MIBs in RFC2573.
  For this object, it would seem that the sending is more
  the issue than the VR creation/deletion.


* vrAdminStatus
- Why is the DEFVAL { down } ?  

*vrMaxRoutesTrapEnable
- same comments as with vrTrapEnable


* vrStat
- Please rename to vrStatusObjects, vrStatusScalars, etc.

* vrConfiguredVRs and vrActiveVRs seem to be objects that
  could be figured out by the management station. Could you
  explain what the motivation is for having them in the MIB?

- Also, these should be type Gauge32.

* vrStatTable -- the description should be changed to status and
  not statistics.

* INDEX is the same as VrConfigTable, should this have an
  AUGMENTS relationship?

* vrStatEntry - same comments as with vrConfigEntry

* vrStatRouteEntries and vrStatFIBEntries should probably be
  Gauge32

*vrOperStatus - could unknown(3) be listed first?
  Also, wouldn't testing be a valid status for this object?


* vrIfConfig - please change to vrIfConfigObjects

* vrIfConfigTable's description is confusing.  Is this
  table used to add and delete Interfaces that exist in
  the ifTable to the Virtual Router?  If so then 
  could that be stated in the DESCRIPTION for the table?

* vrNotificationPrefix should be removed

* vrUp - could the description be a little more generic, 
  such as "operational status to up from any other state".
  In other words, must it really go from "down to up"
  or would it be valid to go from "testing to up"?

* vrDown - could you also include the vrAdminStatus for
  this object?

* Conformance/Compliance statements need to include 
  a statement for read-only compliance.  In other words,
  if an implementation only wants to use this MIB for
  monitoring, then they should also have a way to be
  compliant.

- Also, regarding InetAddressType and InetAddress, you
  may only want to require that an implementation support
  a subset (example from DIFFSERV-MIB):
    OBJECT diffServMultiFieldClfrAddrType
    SYNTAX  InetAddressType { unknown(0), ipv4(1), ipv6(2) }
    DESCRIPTION
       "An implementation is only required to support IPv4 and IPv6
       addresses."

    OBJECT diffServMultiFieldClfrDstAddr
    SYNTAX  InetAddress (SIZE(0|4|16))
    DESCRIPTION
       "An implementation is only required to support IPv4 and globally
       unique IPv6 addresses."

- also with RowStatus, you may want something like:
    OBJECT diffServAlgDropStatus
    SYNTAX RowStatus { active(1) }
    WRITE-SYNTAX RowStatus { createAndGo(4), destroy(6) }
    DESCRIPTION
       "Support for createAndWait and notInService is not required."


7. Acknowledgements

Please add:

This document is a product of the L3VPN Working Group.

Thanks for the acknowledgement.



8. Security Considerations

There are no objects listed but a couple of paragraphs say
something like:
" These are the tables and objects and their
sensitivity/vulnerability:"

but then there aren't any objects listed.  Please update
this section.  It was difficult to review as it seems incomplete.


9. References

Need to be updated.

[VPNTCMIB]  B. Schliesser, and T. Nadeau, "Definition of Textual
     Conventions for Provider Provisioned Virtual Private Network
     (PPVPN) Management.", Internet Draft
     <draft-ietf-l3vpn-tc-mib-03.txt>, May 2004.

As previously mentioned this is now rfc4265.

And there are other drafts which are now RFCs listed.


end.