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

[Mach Chen <[e-mail]>]

Hi Glenn,

Thanks for your valuable review on the mibs.

Best regards,
Mach

> -----Original Message-----
> From: Glenn Mansfield Keeni [mailto:[e-mail]]
> Sent: Friday, May 13, 2016 4:48 PM
> To: Mach Chen; Jeffrey (Zhaohui) Zhang; Benoit Claise; EXT -
> [e-mail]
> Cc: [e-mail]; Martin Vigoureux; [e-mail]
> Subject: Re: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt
> 
> Hi Mach,
>     I have sent the review on  2016/05/09.
> 
> On 2016/05/09 0:01, Glenn Mansfield Keeni wrote:
>  > Jeffrey,
>  >> Thanks for your comments. I've addressed most of your comments  >> in
> the new revision:
>  > Thanks for your cooperation. I will need at least one more revision  > with
> the following comments/recommendations addressed before I will  > be able
> to complete the detailed review. In the following the numbers  > refer to the
> issue numbers in the initial review. The issues that are  > addressed and
> closed are not listed. For brevity, the issue  > descriptions have been trimmed.
> In case of doubts please look at the  > response mail appended below.
>  > Hope this helps.
> 
> Glenn
> 
> On 2016/05/13 17:22, Mach Chen wrote:
> > Hi Glenn,
> >
> > Based on your below response, we assume that you will do a detailed review
> on the mibs, can you estimate when the review will be finished.
> >
> > Thanks,
> > Mach
> >
> >> -----Original Message-----
> >> From: Glenn Mansfield Keeni [mailto:[e-mail]]
> >> Sent: Tuesday, April 19, 2016 6:24 AM
> >> To: Jeffrey (Zhaohui) Zhang; Benoit Claise; EXT -
> >> [e-mail]
> >> Cc: Mach Chen; [e-mail]; Martin Vigoureux; [e-mail]
> >> Subject: Re: [bess] MIBDoc review of
> >> draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt
> >>
> >> 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-m
> >> ib-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
> >>>
> >
> >