Hi,
I have been asked to do a MIB Doctors review of
draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt.
My knowledge of L2L3VPN Multicast is limited to the reading
of this document and browsing through the documents referred
to in the draft and bess-wg mailing list archives.( read "shallow").
So some of the doubts and questions may sound trivial or
strange. Please bear with me and help me help you make
this into a better document :-)

The comments are attached.

Glenn


Comments on draft-ietf-bess-l2l3-vpn-mcast-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 A sentence on how the managed objects will be used by 
    applications for operations, monitoring and management 
    would be good.   

2.  Introduction
2.1 Please give the full expansion of the abbreviations 
    appearing for the first time.  (PE, VPLS,..) 

2.2 The terminology section is a bit terse. Explaining the 
    terms that are used, nicely with reference to the protocol 
    documents will improve readability.
    e.g.
     - PMSI, I-PMSI, S-PMSI, provider tunnels
2.3 Is there a difference between 
       "multicast in Layer 2 and Layer 3 VPNs , defined by
        RFC 7117 and RFC 6513/6514" 
    used in the DESCRIPTION in the MODULE-IDENTITY
    and 
       "multicast in BGP/MPLS L2 or IP VPN"
    used in the DESCRIPTION of L2L3VpnMcastProviderTunnelType ?
    If these are the same, it will be helpful to stick to the 
    same expression. If these are not the same, the dictinction 
    should be clarified.
 

3.  Summary of MIB Module.
    An overview of the L2L3-VPN-MCAST-MIB will be good- the 
    structure of the MIB, short descriptions of the table(s) 
    including usage of the table(s) for management and/or by 
    other MIB(s).

MIB definitions:
4. MIB syntax checking:
   smilint -s -e -l 5 mibs/L2L3-VPN-MCAST-MIB 2>L2L3-VPN-MCAST-MIB.txt

   mibs/L2L3-VPN-MCAST-MIB:63: [4] {hyphen-in-label} warning: named number 
`rsvp-p2mp' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:64: [4] {hyphen-in-label} warning: named number 
`ldp-p2mp' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:65: [4] {hyphen-in-label} warning: named number 
`pim-asm' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:66: [4] {hyphen-in-label} warning: named number 
`pim-ssm' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:67: [4] {hyphen-in-label} warning: named number 
`pim-bidir' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:68: [4] {hyphen-in-label} warning: named number 
`ingress-replication' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:69: [4] {hyphen-in-label} warning: named number 
`ldp-mp2mp' must not include a hyphen in SMIv2
   mibs/L2L3-VPN-MCAST-MIB:215: [5] {group-unref} warning: current group 
`l2L3VpnMcastOptionalGroup' is not referenced in this module
   mibs/L2L3-VPN-MCAST-MIB:4: [5] {import-unused} warning: identifier 
`NOTIFICATION-TYPE' imported from module `SNMPv2-SMI' is never used
   mibs/L2L3-VPN-MCAST-MIB:5: [5] {import-unused} warning: identifier 
`Unsigned32' imported from module `SNMPv2-SMI' is never used
   mibs/L2L3-VPN-MCAST-MIB:8: [5] {import-unused} warning: identifier 
`NOTIFICATION-GROUP' imported from module `SNMPv2-CONF' is never used
   mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier 
`TruthValue' imported from module `SNMPv2-TC' is never used
   mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier 
`RowStatus' imported from module `SNMPv2-TC' is never used
   mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier 
`TimeStamp' imported from module `SNMPv2-TC' is never used
   mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier 
`TimeInterval' imported from module `SNMPv2-TC' is never used
   mibs/L2L3-VPN-MCAST-MIB:15: [5] {import-unused} warning: identifier 
`SnmpAdminString' imported from module `SNMP-FRAMEWORK-MIB' is never used
   mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier 
`InetAddress' imported from module `INET-ADDRESS-MIB' is never used
   mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier 
`InetAddressType' imported from module `INET-ADDRESS-MIB' is never used

5. REFERENCE clauses: Please use REFERENCE clauses liberally.
   Wherever possible, provide references for objects used in 
   the MIB. The references will point to specific sections/
   sub-sections of the RFCs defining the protocol for which the 
   MIB is being designed. It will greatly improve the readability 
   of the document. 

6. 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]

7. Please update the MODULE-IDENTITY. (There are no syntantic errors.)
7.1 CONTACT-INFO
    Following the conventions (including indentation style) will 
    improve the readability. (e.g. RFC4382, RFC5132).
    Will be good if it does not overflow into the next page.
    
7.2 REVISION clause: follow the convention recommended in RFC4181 
    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:
7.3 OID assignment: follow the convention recommended in RFC4181 
    sec 4.5 i
    replace 
          ::= { experimental 99 } -- number to be assigned
    by
          ::= { <subtree> XXX }
   -- RFC Ed.: replace XXX with IANA-assigned number & remove this note 
   <subtree> will be the subtree under which the module will be 
   registered.


8. Specific MO and TC related comments.
      L2L3VpnMcastProviderTunnelType ::= TEXTUAL-CONVENTION
        STATUS       current
        DESCRIPTION
            "Types of provider tunnels used for multicast in
             BGP/MPLS L2 or IP VPN."
        SYNTAX       INTEGER { unconfigured (0),
                               rsvp-p2mp (1),
                               ldp-p2mp (2),
                               pim-asm (3),
                               pim-ssm (4),
                               pim-bidir (5),
                               ingress-replication (6),
                               ldp-mp2mp (7)

    o Would be nice to align the enumeration labels with the 
      labels in the protocol document RFC 6514 unless there is 
      a good reason for not doing so. (You will have to take 
      care of the smi compilation errors too; '-' is not allowed ). 
            
8.1  l2L3VpnMcastPmsiTunnelAttributeEntry OBJECT-TYPE
         SYNTAX        L2L3VpnMcastPmsiTunnelAttributeEntry
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "An entry in this table corresponds to an PMSI attribute
              that is advertised/received on this router.
              For BGP-based signaling (for I-PMSI via auto-discovery
              procedure, or for S-PMSI via S-PMSI A-D routes),
              they are just as signaled by BGP (RFC 6514 section 5,
              'PMSI Tunnel attribute').
              For UDP-based S-PMSI signaling for PIM-MVPN,
              they're derived from S-PMSI Join Message
              (RFC 6513 section 7.4.2, 'UDP-based Protocol')..
    
              Note that BGP-based signaling may be used for
              PIM-MVPN as well."
    o Fix the ".." in "'UDP-based Protocol').." above. 
    o Please give the reference for this Table. 
      Is it-  "PMSI Tunnel attribute" in RFC 6513 Sec.4  ?   
              "PMSI Tunnel attribute" in RFC 6514 Sec.5  ?   
               both?
      Any other pointers?

8.2   l2L3VpnMcastPmsiTunnelAttributeFlags OBJECT-TYPE
         SYNTAX        OCTET STRING (SIZE (1))
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "For UDP-based S-PMSI signaling for PIM-MVPN, this is 0.
              For BGP-based I/S-PMSI signaling, this is the Flags
              field in PMSI Tunnel Attribute of the corresponding
              I/S-PMSI A-D route."
         ::= { l2L3VpnMcastPmsiTunnelAttributeEntry 1 }
    o  Please confirm that the above is a complete enumeration of the 
       types of signalling.  
    o  RFC 6514 Sec.5 says that the Flags field indicates
       "Leaf Information Required". That is useful information. 
       Please include in the description.  

8.3   l2L3VpnMcastPmsiTunnelAttributeId OBJECT-TYPE
         SYNTAX        OCTET STRING ( SIZE (0..37) )
         MAX-ACCESS    not-accessible
         STATUS        current
         DESCRIPTION
             "For UDP-based S-PMSI signaling for PIM-MVPN, the first
              four or sixteen octets of this attribute are filled with
              the provider tunnel group address (IPv4 or IPv6)..
              For BGP-based I/S-PMSI signaling, this is the Tunnel Identifier
              Field in PMSI Tunnel Attribute of the corresponding I/S-PMSI
              A-D route."
    o Check the size specifications. The specs above say it can be 
      all sizes 0..37. That is not clear from the DESCRIPTION clause. 
    o Fix the ".." in "(IPv4 or IPv6).." above. 
    o RFC 6514 Sec 5.  PMSI Tunnel Attribute gives the Tunnel Identifiers
      for mLDP, PIM-SM, PIM-SSM, BIDIR-PIM,Ingress Replication,MP2MP. 
      It appears that the sizes (range) for each case will be different. 
      Please clarify that, and if there are discrete sizes, specify 
      accordingly. 
      

8.3  l2L3VpnMcastPmsiTunnelPointer OBJECT-TYPE
        SYNTAX        RowPointer
        MAX-ACCESS    read-only
        STATUS        current
        DESCRIPTION
            "If the tunnel exists in some MIB table, this is the
             row pointer to it."
    o "some MIB table" : specify which MIB table. 
    o In what case will the tunnel exist and in what case will it not?
    o What will be the behaviour if the above condition is not satisfied?

8.4  l2L3VpnMcastPmsiTunnelIf OBJECT-TYPE
        SYNTAX        RowPointer
        MAX-ACCESS    read-only
        STATUS        current
        DESCRIPTION
            "If the tunnel has a corresponding interface, this is the
             row pointer to the ifName table."
     o DESCRIPTION looks incorrect. Please fix it. Do you want to say 
       this object points to the corresponding row in the ifTable? 
     o In what case does the TunnelIf exist and in what case will it not?
     o What will be expected if the tunnel does not have a corresponding 
       interface?

9. 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.
   Please fix. 
   

10.ID-nits
10.1 Checking nits according to http://www.ietf.org/id-info/checklist :
     ---------------------------------------------------------------------------
   
     ** There are 4 instances of too long lines in the document, the longest one
        being 3 characters in excess of 72.
   
10.2 Checking references for intended status: Proposed Standard
     ---------------------------------------------------------------------------
   
     == Missing Reference: 'RFC 7117' is mentioned on line 76, but not
        defined
        'described in [RFC6513, RFC6514, RFC 7117] and other documents tha...'

11.  There is another WIP MVPN-MIB in draft-ietf-bess-mvpn-mib-02.txt
     MVPN-MIB has objects that refer to L2L3-VPN-MCAST-MIB.   
     Is there a good reason for not merging the 2 documents? I have not seen 
     any discussion or explanation on this. I may have missed it. Please
     clarify or, give some pointers. 

_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess

Reply via email to