Re: [MIB-DOCTORS] Update to draft-ietf-bess-mvpn-mib-06

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

Hi Tsunoda,
    I have completed a pass of the document. The document
is shaping up well.Once again thanks for the good work.
The comments follow. I would also recommend a closer check
for editorial nits in the next draft.

Glenn
----------------------------------------------------------

draft: draft-ietf-bess-mvpn-mib-06.txt

1. Please make the usages uniform. e.g.
    "Multicast VPN (MVPN)" vs "Multicast VPN" (MVPN)
    MVPN vs Multicast VPN
    "Inclusive PMSI (I-PMSI)" vs "Inclusive PMSI" (I-PMSI)

2. Page 3, Sec 1, para 2
    This document describes managed objects to configure and/or monitor
    MVPN.
    s/MVPN/MVPNs/

3. Page 3, Sec 1.1, para 3
    s/A PE uses to/A PE uses a P-tunnel to/
4. Page 3, Sec 1.1, para 5
    What is the minimum number of VRFs on a PE? 0? 1? Please update
    the paragraph accordingly.

5. Page 4, Sec 3.1 Summary of MIB Module

    s/attribute informations of MVRFs of MVPNs/attributes of MVPNs/
    s/Configuring some timers/Monitoring and configuring some timers/
    s/Monitoring attribute informations of PMSIs/
      Monitoring PMSI attribute information/
    s/Monitoring advertisement exchanged/Monitoring statistics on 
advertisements
exchanged/

6. Table naming.
     mvpnIpmsiAdvtTable, mvpnInterAsIpmsiAdvtTable, mvpnSpmsiAdvtTable
    These are tables containing counters about advertisements received.
    But the names seem to imply that they contain advertisements. Please
    rename appropriately.


7. Page 5, Sec 3.1 Summary of MIB Module (contd)
    s/contain information of MVRFs of MVPNs/contain information of MVPNs/
    s/represetns/represents/
    s/Selective PMSI (S-PMSI)/S-PMSI/
    s/mvpnGeneralTable/mvpnGenericTable/
    s/mvpnSpmsiConfigTable/mvpnSpmsiTable/
    s/that is advertised/that are advertised/

8. This MIB, particularly mvpnGenericTable, uses MPLS-L3VPN-STD-MIB. Please
    mention this explicitly.

9. Page 62 Sec 4.
    s/read- write/read-write/
    s/mvpnGenCmcastRouteProtocol, mvpnGenIpmsiInfo,
      mvpnGenInterAsPmsiInfo, mvpnGenUmhSelection,
      mvpnGenCustomerSiteType, mvpnGenSPTunnelLimit, mvpnBgpGenMode,
      mvpnBgpGenVrfRtImport, mvpnPmsiEncapsulationType,
      mvpnSpmsiThreshold, mvpnSpmsiPmsiPointer/
      mvpnBgpGenCmcastRouteWithdrawalTimer,
      mvpnBgpGenSrcSharedTreeJoinTimer,
      mvpnBgpGenMsgRateLimit,
      mvpnBgpGenMaxSpmsiAdRoutes,
      mvpnBgpGenMaxSpmsiAdRouteFreq,
      mvpnBgpGenMaxSrcActiveAdRoutes,
      mvpnBgpGenMaxSrcActiveAdRouteFreq,
      mvpnSpmsiThreshold/
10. Please do the '[TBD]'



On 2018/05/13 18:56, Glenn Mansfield Keeni wrote:
> Hi Tsunoda,
>     I have done a review of the MIB. The comments follow.
> Glenn
> 
> draft: draft-ietf-bess-mvpn-mib-06.txt
>         Only the MIB has been reviewed.
> 
> COMMENTS:
> 
> 1. Naming: Please review.
>          MO: mvpnBgpGenVrfRouteImport:
>                if it is a condition for import name accordingly
> 
> 2. Usage:  Please make the usages of the following uniform in the document
>                        MVPN/mvpn/MVRF
>                        tunnel type/Tunnel type
>                        Source AS/source-as
> 
> 
> 3. Units:  The units for the following MOs are unspecified.
>          Please check.
>          MO: mvpnBgpGenCmcastRouteWithdrawalTimer
>          MO: mvpnBgpGenSrcSharedTreeJoinTimer
>          MO: mvpnBgpGenMsgRateLimit
>          MO: mvpnBgpGenMaxSpmsiAdRoutes
>          MO: mvpnBgpGenMaxSpmsiAdRouteFreq
>          MO: mvpnBgpGenMaxSrcActiveAdRoutes
>          MO: mvpnBgpGenMaxSrcActiveAdRouteFreq
> 4. Semantics:
>          InetAddress and INetAddressType objects
>              for several MOs, usage of InetAddressType unknown(0)
>              is described. In all such cases the corresponding
>              InetAddress MO value MUST be a string of length 0.
>              Plese explain that case in the respective DESCRIPTIONs
>              E.g. mvpnMrouteUpstreamNeighborAddr
>                   mvpnMrouteNextHopSourceAddr
>                   mvpnMrouteCmcastSourceAddr
>                   mvpnMrouteSourceAddr
> 
> 5. mvpnSpmsiAdvtCmcastSourceAddr OBJECT-TYPE
>       SYNTAX        InetAddress
>       ==> corresponding InetAddressType object
>           mvpnSpmsiAdvtCmcastSourceAddrType is missing
> 
> 6. Nits:
>          MO:  mvpnMrouteCmcastSourceAddrType OBJECT-TYPE
>               DESCRIPTION
>                   "A value indicating the address family of the address
>                    contained in mvpnMrouteSourceAddr.
>               s/mvpnMrouteSourceAddr/mvpnMrouteCmcastSourceAddr/
> 
> 7. Table descriptions:
>       mvpnIpmsiAdvtTable
>       mvpnSpmsiAdvtTable
>          The DESCRIPTIONs are unclear.
>              Do the tables 'contain' all advertisements or,
>              Statistics related to the advertisements?
> 
> 
> 8. Additional suggestions:
>     o For INDEX clauses of variable size where the size may potentially
>       exceed 128 octets, a statement like the following will be good.
>              Implementors need to be aware that if the total number of
>              octets in MO1, MO2 and MO3 exceeds NNN, then OIDs of column
>              instances in this row will have more than 128 sub-identifiers
>              and cannot be accessed using SNMPv1, SNMPv2c, or SNMPv3.
> 
>     o In REFERENCES be more specific if possible. Eg.
>         Current:
>             mvpnPmsiTunnelIfIndex OBJECT-TYPE
>                REFERENCE
>                    "RFC2863
>         Suggested:
>             mvpnPmsiTunnelIfIndex OBJECT-TYPE
>                REFERENCE
>                    "RFC2863 Sec. 3.1.5
> 
> 
> 
> On 2018/05/02 6:59, Glenn Mansfield Keeni wrote:
>> Hi Tsunoda,
>>     Thanks for the good work.
>> I will start reviewing this right away.
>>
>> Glenn
>>
>> On 2018/05/01 12:14, Hiroshi Tsunoda wrote:
>>> Dear Glenn,
>>>
>>> Thank you for waiting the update.
>>> I have submitted the updated version of
>>> draft-ietf-bess-mvpn-mib.
>>>
>>> Links to the draft and diff are as follows.
>>>
>>> URL:
>>> https://www.ietf.org/internet-drafts/draft-ietf-bess-mvpn-mib-06.txt
>>> Htmlized:       https://tools.ietf.org/html/draft-ietf-bess-mvpn-mib-06
>>> Htmlized: https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-mib
>>> Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-mvpn-mib-0
>>>
>>> This version contains some major changes as follows.
>>> I hope this update make the role and usage of the MIB clear for you.
>>>
>>> +----------------------------------------------------------+
>>> Changes:
>>>
>>> 1. Support for row creation in all tables is removed
>>>
>>>     Reason: The utility of row creation is dubious.
>>>     It increases complexity of the MIB implementation.
>>>     Unless an immediate need for row creation, we will go ahead
>>>     with the current draft and review the need for row creation
>>>     at a later date if and when it arises.
>>>
>>> 2. Added objects to make the role and usage of this MIB clear.
>>>
>>>     Reason: This MIB will provide the following management functions.
>>>
>>>     - Configuration of MVRF related timers
>>>
>>>     - Generation of Notifications to indicate  creation, deletion,
>>>       and/or modification of MVRFs
>>>
>>>     - Generation of Notification  when a member joins or leaves a
>>>       multicast group
>>>
>>>     - Monitoring of the following
>>>       - attributes of MVRFs of MVPNs
>>>       - PMSIs
>>>       - statistics of advertisements exchanged by a PE
>>>       - routing entries in a MVRF
>>>       - next-hops for a multicast destination in a MVRF
>>> +----------------------------------------------------------+
>>>
>>> -- tsuno
>>>
>>
>> _______________________________________________
>> MIB-DOCTORS mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/mib-doctors
> 
> _______________________________________________
> MIB-DOCTORS mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/mib-doctors
>