Comments on draft-ietf-bess-mvpn-mib-02.txt ----------------------------------------------------- This is a preliminary review. Once the following points are taken care of, we can get down to a detailed in-depth review. 1. Abstract: 1.1 Please give the full expansion of MPLS/BGP 1.2 "In particular, it describes managed objects to configure and/or monitor Multicast in MPLS/BGP IP VPNs (MVPN) on a router." Is this for any router or, a "Provider Edge" router ? Please fix accordingly. 2. Introduction 2.1 PE - appears first time. Please give the full expansion. 2.2 Is the protocol for "exchanging VPN multicast" or "exchanging VPN multicast states"? Please fix appropriately. 2.3 The expression "standard MIB" in the following is confusing: "This document defines a standard MIB for MVPN-specific objects that are generic to both PIM-MVPN and BGP-MVPN." Is there any special significance in the "standard" above? If no, then please drop the word. Are the objects "generic" to PIM-MVPN and BGP-MVPN or "common" to PIM-MVPN and BGP-MVPN ? Please change accordingly. 2.4 Please give the full expansion of the abbreviations occuring for the first time in the document. (MPLS, L3VPN). 2.5 The terminology section is a bit terse. Explaining the terms that are used, with reference to the protocol documents will improve readability. e.g. - MVPN, PE, PMSI/tunnels, - C-multicast routing exchange protocol (PIM or BGP), C-multicast states - I-PMSI, S-PMSI, provider tunnels 3. MVPN MIB. This gives the overview of the MVPN MIB. The MIB module aims to provide "configuring and/or monitoring" 3.1 In "This MIB enables configuring and/or monitoring of MVPNs on PE devices: the whole multicast VPN machinery....." "the whole multicast VPN machinery" is very difficult to define. Please use precisely defined terms. 3.2 In "To represent them,...." "them" seems ambiguous, please clarify. 3.3 The diagram needs some explanation. What do the boxes represent? Tables ? The labels are meant to be table names ? The table names do not match the labels. What do the arrows signify? Please explain. 3.4 The short explanation of the tables could be augmented with some information on what they represent and an idea of how they will be used. ( RFC 4382 provides a good example). MIB definitions: 4. MIB syntax checking: smilint -s -e -l 5 mibs/MCAST-VPN-MIB 2>MCAST-VPN-MIB.txt mibs/MCAST-VPN-MIB:289: [4] {hyphen-in-label} warning: named number `highest-pe-address' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:290: [4] {hyphen-in-label} warning: named number `c-root-group-hashing' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:291: [4] {hyphen-in-label} warning: named number `ucast-umh-route' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:306: [4] {hyphen-in-label} warning: named number `sender-receiver' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:307: [4] {hyphen-in-label} warning: named number `receiver-only' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:308: [4] {hyphen-in-label} warning: named number `sender-only' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:369: [4] {hyphen-in-label} warning: named number `rpt-spt' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:370: [4] {hyphen-in-label} warning: named number `spt-only' must not include a hyphen in SMIv2 mibs/MCAST-VPN-MIB:412: [5] {index-exceeds-too-large} warning: index of row `mvpnPmsiConfigEntry' can exceed OID size limit by 398 subidentifier(s) mibs/MCAST-VPN-MIB:534: [5] {index-exceeds-too-large} warning: index of row `mvpnSpmsiConfigEntry' can exceed OID size limit by 430 subidentifier(s) mibs/MCAST-VPN-MIB:649: [5] {index-exceeds-too-large} warning: index of row `mvpnIpmsiEntry' can exceed OID size limit by 430 subidentifier(s) mibs/MCAST-VPN-MIB:741: [5] {index-exceeds-too-large} warning: index of row `mvpnInterAsIpmsiEntry' can exceed OID size limit by 174 subidentifier(s) mibs/MCAST-VPN-MIB:810: [5] {index-exceeds-too-large} warning: index of row `mvpnSpmsiEntry' can exceed OID size limit by 687 subidentifier(s) mibs/MCAST-VPN-MIB:1016: [4] {group-membership} warning: notification `mvpnMvrfChange' must be contained in at least one conformance group mibs/MCAST-VPN-MIB:1127: [5] {group-unref} warning: current group `mvpnPmsiConfigGroup' is not referenced in this module mibs/MCAST-VPN-MIB:1211: [5] {group-unref} warning: current group `mvpnOptionalGroup' is not referenced in this module mibs/MCAST-VPN-MIB:8: [5] {import-unused} warning: identifier `NOTIFICATION-GROUP' imported from module `SNMPv2-CONF' is never used mibs/MCAST-VPN-MIB:23: [5] {import-unused} warning: identifier `MplsLabel' imported from module `MPLS-TC-STD-MIB' is never used 5. IMPORTS clause MIB modules from which items are imported must be cited and included in the normative references. The conventional style is mplsStdMIB FROM MPLS-TC-STD-MIB -- [RFC3811] 6. Please update the MODULE-IDENTITY. (There are no syntantic errors.) 6.1 'ORGANIZATION "IETF Layer-3 Virtual Private Networks Working Group."' needs to be fixed to 'ORGANIZATION "IETF BESS Working Group."' or something more appropriate. 6.2 CONTACT-INFO Following the conventions (including indentation style) will improve the readability. (e.g. RFC4382, RFC5132). 6.2 REVISION clause: follow the convention of RFC4131 sec 4.5 REVISION "200212132358Z" -- December 13, 2002 DESCRIPTION "Initial version, published as RFC yyyy." -- RFC Ed.: replace yyyy with actual RFC number & remove this note: 6.3 OID assignment: follow the convention of RFC4131 sec 4.5 replace ::= { experimental 99 } -- number to be assigned by ::= { XXX } -- RFC Ed.: replace XXX with IANA-assigned number & remove this note will be the subtree under which the module will be registered. 7. Wherever possible, please provide references for objects used in the MIB. The references will point to specific sections/sub-sections of RFCs defining the protocol for which the MIB is being designed. 8. MOs. 8.1 Scalar Objects: The name mvpnMvrfNumber may be misleading. I would recommend the usage of the same naming style as RFC 4382 e.g. mvpnMvrfs, mvpnV4Mvrfs, mvpnV6Mvrfs (instead of mvpnMvrfNumber, mvpnMvrfNumberV4, mvpnMvrfNumberV6, ...) unless there is some good reason to not do it. 8.2 mvpnMvrfNumber OBJECT-TYPE SYNTAX Unsigned32 DESCRIPTION "The total number of MVRFs that are present on this device, whether for IPv4, IPv6, or mLDP C-Multicast." o Please make the description precise. E.g. if it is the sum of IPv4 MVRFs, IPv6 MVRFs and mLDP C-Multicast MVRFs state it explicitly. o The expression "present on this device" is used. Does "present" imply "configured" MVRFs or "active" MVRFs. If it is number of active MVRFs then one would expect that the number will vary (increase or decrease). If that is the case: replace SYNTAX Unsigned32 by SYNTAX Gauge32 8.3 For all the following scalars: mvpnMvrfNumber mvpnMvrfNumberV4 mvpnMvrfNumberV6 mvpnMvrfNumberPimV4 mvpnMvrfNumberPimV6 mvpnMvrfNumberBgpV4 mvpnMvrfNumberBgpV6 mvpnMvrfNumberMldp same comments as 8.2. 8.4 mvpnGenAddressFamily OBJECT-TYPE DESCRIPTION "The Address Fammily that this entry is for" s/Fammily/Family/ 8.5 mvpnGenOperStatusChange OBJECT-TYPE SYNTAX INTEGER { createdMvrf(1), deletedMvrf(2), modifiedMvrfIpmsiConfig(3), modifiedMvrfSpmsiConfig(4) } DESCRIPTION "This object describes the last operational change that o The name does not look right. From the SYNTAX and the DESCRIPTION it appears that this is about config or MVRF change rather than "OperStatus" change. Please check and fix. o Please confirm that the values in the row itself will not be changed after creation. ( you do not have a 'modifiedMvrfConfig') 8.6 mvpnGenCmcastRouteProtocol OBJECT-TYPE MAX-ACCESS read-write ::= { mvpnGeneralEntry 4 } o You cannot have MAX-ACCESS read-write for a row that may be dynamically created. Replace MAX-ACCESS read-write by MAX-ACCESS read-create if you want to dynamically change that value, otherwise, MAX-ACCESS read-only will suffice. 8.8 mvpnGenIpmsiConfig OBJECT-TYPE DESCRIPTION "This points to a row in mvpnPmsiConfigTable, for I-PMSI configuration." o Please specify the expected behaviour when it is not an I-PMSI 8.9 mvpnGenInterAsPmsiConfig o same comment as above 8.10 mvpnGenRowStatus mvpnGenRowStatus OBJECT-TYPE SYNTAX RowStatus DESCRIPTION "This is used to create or delete a row in this table." o The description is inadequate for an implementor (and others too). Detailed specification/instruction must be given indicating which columnar objects have to be set to valid values before the row can be activated; and whether columns in the row may be modified when the status value is active. Please do the needful. (Ref RFC 4181 Sec 4.6.4. Conceptual Table Definitions RFC 4382 MO mplsL3VpnIfConfRowStatus for one example. o You must have a mvpnGenRowStorageType or the DESCRIPTION of mvpnGenRowStatus must indicate what will happen to the row after an agent restart 9. Similar comments (8.1 ~ 8.10) for the remaining tables in the MIB Particularly 8.10 for the RowStatus type objects mvpnGenRowStatus mvpnPmsiConfigRowStatus mvpnSpmsiConfigRowStatus. Please check and fix. 10. mvpnMvrfChange NOTIFICATION-TYPE OBJECTS { mvpnGenOperStatusChange } ::= { mvpnNotifications 2 } o should be { mvpnNotifications 1 } o Include the MOs that the administrator/manager may want to see in OBJECTS. 11. The Security Considerations section does not follow the Security Guidelines for IETF MIB Modules http://trac.tools.ietf.org/area/ops/trac/wiki/mib-security. 12. COMPLIANCE. 12.1 You seem to mandate MAX-ACCESS read-write/read-create for compliance. Is this intended? Configuration capability MUST be supported? Please note that sec 2. MVPN MIB says "This MIB enables configuring and/or monitoring of MVPNs ..." The current compliance requirement contradicts the above claim. Please check and fix. It is general and sound practice to allow for MAX-ACCESS read-only compliance. Some implementations may support monitoring but not configuration. Please check and fix. General nits: 13.1 Page-1 s/an portion/a portion/ 13.2 Page-1 s/we'll/we will/ 13.3 Page-5 s/ mvpnSpmsiTable\/Etnry/mvpnSpmsiTable/ I think that the "/Entry" was removed from similar titles in the earlier draft as adivised by the document shepherd. This one should be removed too. 13.4 ID-nits: Checking nits according to http://www.ietf.org/id-info/checklist : ------------------------------------------------------------------------- ** There is 1 instance of too long lines in the document, the longest one being 4 characters in excess of 72. Checking references for intended status: Proposed Standard -------------------------------------------------------------------------- == Unused Reference: 'KEYWORDS' is defined on line 1381, but no explicit reference was found in the text '[KEYWORDS] Bradner, S., "Key words for use in RFCs to Indicate Requi...' 14. There is another WIP L2L3-VPN-MCAST-MIB in the WG. Is there a good reason for not merging the 2 documents? Some clarification or pointers will be helpful.