Re: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt

[Glenn Mansfield Keeni <[e-mail]>]

Jeffrey,
　　Thanks for the update.
Will get back to you after a detailed review is
done.

Glenn
On 2016/04/16 21:47, Jeffrey (Zhaohui) Zhang wrote:
> Glenn,
> 
> Thanks for your comments. I've addressed most of your comments in the new revision:
> 
> URL:            https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-03.txt
> Status:         https://datatracker.ietf.org/doc/draft-ietf-bess-l2l3-vpn-mcast-mib/
> Htmlized:       https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-03
> Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-03
> 
> Please see below.
> 
>> 1.  Abstract:
>> 1.1 A sentence on how the managed objects will be used by
>>      applications for operations, monitoring and management
>>      would be good.
> 
> I had thought this would be standard/obvious for all MIB objects - the read-write ones are used to control how a device works, and the read-only ones are used for monitoring. Do I really need to say it explicitly?
> 
> I see RFC 4382 has the following:
> 
>     This memo defines a portion of the Management Information Base (MIB)
>     for use with network management protocols in the Internet community.
>     In particular, it describes managed objects to configure and/or
>     monitor Multiprotocol Label Switching Layer-3 Virtual Private
>     Networks on a Multiprotocol Label Switching (MPLS) Label Switching
>     Router (LSR) supporting this feature.
> 
> Is it enough to say something similar? For example:
> 
>          In particular, it describes common managed objects used to configure
>          and/or monitor both L2 and L3 VPN Multicast.
> 
>>
>> 2.  Introduction
>> 2.1 Please give the full expansion of the abbreviations
>>      appearing for the first time.  (PE, VPLS,..)
> 
> Fixed.
> 
>>
>> 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
> 
> As the paragraph alluded to, this MIB needs to be understood in the general context of L2/L3 multicast VPN and providing good explanation of the terms is not attempted. The references for the terms are the the RFCs for the relevant technologies.
> 
> Having said that, I'll explain PMSI a bit further.
> 
>> 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.
> 
> No difference. I was using "Layer 3" or "L3" but it was pointed out that the layer 3 VPN is often referred to IP VPN in other RFCs and I was advised to change it accordingly. Looks like I did not change all the cases.
> 
> On the other hand, I noticed that RFC 4382 does use "Layer 3 VPN" so I'll change it back.
> 
>>   
>>
>> 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).
> 
> I had that, but have added one sentence about the only table.
> 
>>
>> MIB definitions:
>> 4. MIB syntax checking:
>>     smilint -s -e -l 5 mibs/L2L3-VPN-MCAST-MIB 2>L2L3-VPN-MCAST-MIB.txt
> 
> I used simpleweb's validation tool but looks like I did not use the strictest level of validation. I've now fixed the following issues and verified.
> 
>>
>>     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
> 
> See later question/comments below.
> 
>>     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
> 
> Removed the above unused imports.
> 
>>
>> 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.
> 
> Added.
> 
>>
>> 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]
> 
> Added.
> 
>>
>> 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.
> 
> Fixed.
> 
>>      
>> 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:
> 
> Fixed.
> 
>> 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.
>>
> 
> I kept "experimental 99" so that I could continue to use mib tools to validate; but I added notes for the editor to replace them as you indicated.
> 
>>
>> 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 ).
> 
> Are spaces allowed? I don't know so I used hyphen. For now I replace with things like rsvpP2mp.
> Or could/should I just remove the definitions, so that if a new type is defined in the future there is no need to update the MIB?
> 
>>              
>> 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?
> 
> Fixed.
> 
>>
>> 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.
> 
> The intent is to simply return the octet value of the flags field, w/o listing individual bits like "Leaf Information Required". More bits could be defined in the future but the MIB would not change.
> 
> Is that OK?
> 
>>
>> 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.
> 
> Depending on the tunnel type, there could be different sizes. Future tunnel types could have other sizes that not specified today. I was thinking to just give a size range so that it is flexible. Is that ok?
> 
>>        
>>
>> 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.
> 
> I can give an example, like mplsTunnelTable [RFC 3812]. It could be whatever table that a tunnel may be put into.
> 
>>      o In what case will the tunnel exist and in what case will it not?
> 
> If a device supports mplsTunnelTable and the tunnel is represented there, then it exists.
> 
>>      o What will be the behaviour if the above condition is not satisfied?
> 
> A null pointer should be given.
> 
>>
>> 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?
> 
> Yes. Fixed.
> 
>>       o In what case does the TunnelIf exist and in what case will it not?
> 
> Some tunnels may not have a corresponding interface.
> 
>>       o What will be expected if the tunnel does not have a corresponding
>>         interface?
> 
> Null row pointer.
> 
>>
>> 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.
> 
> I was really hoping that it would not have to be that tedious. SNMP/MIB security should be no different from the CLI security - once you secure the infrastructure then what's more to do?
> 
> I'll need more time to work on this. Let me try to address the issues in the other mib first and come back to this.
> 
>>     
>>
>> 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.
> 
> I fixed some but there still three too long lines:
> 
>       l2L3VpnMcastPmsiTunnelAttributeType  L2L3VpnMcastProviderTunnelType,
> 
>    l2L3VpnMcastGroups      OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance 1}
>    l2L3VpnMcastCompliances OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance 2}
> 
> Should I break them into different lines or just keep them as is? Any example of expected indentation if I break the lines?
> 
>>     
>> 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...'
> 
> I hope I understood and fixed it (removing the space in "RFC 7117").
> 
>>
>> 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.
> 
> As mentioned in the introduction:
> 
>     this memo describes managed objects common to both VPLS
>     Multicast [RFC7117] and MVPN [RFC6513, RFC6514].
> 
> MVPN-MIB is for MVPN. There was another VPLS Multicast MIB in the work and both would reference common objects defined in this MIB.
> 
> Thanks!
> Jeffrey
> 
>> -----Original Message-----
>> From: BESS [mailto:[e-mail]] On Behalf Of Glenn Mansfield
>> Keeni
>> Sent: Tuesday, April 12, 2016 2:28 AM
>> To: Benoit Claise <[e-mail]>; EXT - [e-mail]
>> <[e-mail]>
>> Cc: Jeffrey (Zhaohui) Zhang <[e-mail]>; [e-mail]; Martin
>> Vigoureux <[e-mail]>; [e-mail]; Mach Chen
>> <[e-mail]>
>> Subject: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt
>>
>> 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
>>
> 
> _______________________________________________
> BESS mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/bess
>