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

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Mon, 2 Jan 2006 18:12:10 +0100

Thanks Joan.

Let the authors and WG review and address your comments.

Bert

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]]
> Sent: Thursday, December 29, 2005 05:23
> To: [email-masked]; Wijnen, Bert (Bert); Mark Townsley
> Cc: [email-masked]
> Subject: MIB Dr. Review of draft-ietf-l3vpn-vr-mib-04.txt 
> 
> 
> 
> 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.
> 
> 
>