Re: [bess] MIBDoc review of draft-ietf-bess-mvpn-mib-02.txt

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

Hi Tsuno,
    Thanks for following this up.
 > Sorry, this is my fault.
No problems at all. Take your time and
send me a nice MIB for review :-)

Glenn

On 2017/06/01 18:25, Hiroshi Tsunoda wrote:
> Hi Thomas, Glenn
> 
> Sorry, this is my fault.
> 
> I think Glenn is waiting for the new revision because
> the last revision posted in March does not have essential
> improvement.
> 
> I am going to make a major update MVPN-MIB document
> in this weekend and will post the new revision at the beginning
> of the next week hopefully.
> 
> Thanks in advance,
> 
> -- tsuno
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 2017-06-01 11:16 GMT+02:00  <[e-mail]>:
>> Hi Glenn,
>>
>> Thanks for the reviews on the other mvpn-related MIB
>> (draft-ietf-bess-l2l3-vpn-mcast-mib-03).
>>
>> We would like to have an idea of what is the progress around this draft.
>> Do you know when you could do a review of the revision posted in early March
>> ?
>>
>> Thanks in advance,
>>
>> -Thomas
>>
>>
>> 2017-03-01, Hiroshi Tsunoda:
>>
>>> Dear Glenn,
>>>
>>> I posted a new revision MVPN-MIB document.
>>> This revision addresses mainly editorial parts of your comments.
>>> Although so many TBDs have remained, I hope that this becomes
>>> the re-starting point to finalize this document.
>>>
>>> URL:
>>> https://www.ietf.org/id/draft-ietf-bess-mvpn-mib-03.txt
>>> Status:
>>> https://datatracker.ietf.org/doc/draft-ietf-bess-mvpn-mib/
>>> Htmlized:
>>> https://tools.ietf.org/html/draft-ietf-bess-mvpn-mib-03
>>> Diff:
>>> https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-mvpn-mib-03
>>>
>>> Please see some notes below.
>>>
>>>> 1.  Abstract:
>>>> 1.1 Please give the full expansion of MPLS/BGP
>>>
>>>
>>> Fixed.
>>>
>>>> 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.
>>>
>>>
>>> This point will be fixed in the next revision.
>>>
>>>> 2.  Introduction
>>>> 2.1 PE - appears first time. Please give the full expansion.
>>>
>>>
>>> Fixed.
>>>
>>>> 2.2 Is the protocol for "exchanging VPN multicast" or
>>>>       "exchanging VPN multicast states"? Please fix appropriately.
>>>
>>>
>>> "exchanging VPN multicast states" is correct. Fixed.
>>>
>>>> 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.
>>>
>>>
>>> Fixed. I dropped the word "standard".
>>>
>>>>      Are the objects "generic" to PIM-MVPN and BGP-MVPN or "common"
>>>>      to  PIM-MVPN and BGP-MVPN ? Please change accordingly.
>>>
>>>
>>> This point will be fixed in the next revision.
>>>
>>>> 2.4 Please give the full expansion of the abbreviations occuring
>>>>       for the first time in the document. (MPLS, L3VPN).
>>>
>>>
>>> Fixed.
>>>
>>>> 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
>>>
>>>
>>> Partially fixed. I will give more detailed explanation in the next
>>> revision.
>>>
>>>> 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.
>>>
>>>
>>> These points will be fixed in the next revision.
>>>
>>>> 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.
>>>
>>>
>>> Partially fixed. Each box represents a table defined in this document.
>>> I fixed the labels in the boxes.
>>>
>>> More detailed explanation will be added in the next revision.
>>>
>>>> 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).
>>>
>>>
>>> These points will be fixed in the next revision.
>>>
>>>> MIB definitions:
>>>> 4. MIB syntax checking:
>>>>     smilint -s -e -l 5 mibs/MCAST-VPN-MIB 2>MCAST-VPN-MIB.txt
>>>
>>>
>>> I fixed the most of the warnings except for "index-exceeds-too-large".
>>> I think the restructuring of the MIB module may be required in order to
>>> fix "index-exceeds-too-large".  Thus, I will need more time to work on
>>> this.
>>>
>>>> 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]
>>>
>>>
>>> Fixed.
>>>
>>>> 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.
>>>
>>>
>>> Fixed.
>>>
>>>> 6.2 CONTACT-INFO
>>>>      Following the conventions (including indentation style) will
>>>>      improve the readability. (e.g. RFC4382, RFC5132).
>>>
>>>
>>> Fixed.
>>>
>>>> 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:
>>>
>>>
>>> Fixed.
>>>
>>>> 6.3 OID assignment: follow the convention of RFC4131 sec 4.5
>>>>      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.
>>>
>>>
>>> Fixed.
>>>
>>>> 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.
>>>
>>>
>>> This will be addressed in the next revision.
>>>
>>>> 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.
>>>
>>>
>>> Fixed.
>>>
>>>> 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
>>>
>>>
>>> Partially fixed. I replaced Unsigned32 by Gauge32.
>>> I will try to update description in the next revision.
>>>
>>>> 8.3 For all the following scalars:
>>>>                              mvpnMvrfNumber
>>>>                              mvpnMvrfNumberV4
>>>>                              mvpnMvrfNumberV6
>>>>                              mvpnMvrfNumberPimV4
>>>>                              mvpnMvrfNumberPimV6
>>>>                              mvpnMvrfNumberBgpV4
>>>>                              mvpnMvrfNumberBgpV6
>>>>                              mvpnMvrfNumberMldp
>>>>      same comments as 8.2.
>>>
>>>
>>> Name and SYNTAX of these scalars were fixed.
>>> Description will be fixed in the next revision.
>>>
>>>> 8.4 mvpnGenAddressFamily OBJECT-TYPE
>>>>         DESCRIPTION
>>>>             "The Address Fammily that this entry is for"
>>>>       s/Fammily/Family/
>>>
>>>
>>> Fixed.
>>>
>>>> 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')
>>>
>>>
>>> This will be addressed in the next revision.
>>>
>>>> 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.
>>>
>>>
>>> This will be addressed in the next revision.
>>>
>>>> 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
>>>
>>>
>>> These will be addressed in the next revision.
>>>
>>>> 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).
>>>>      o You must have a mvpnGenRowStorageType or the DESCRIPTION of
>>>>        mvpnGenRowStatus must indicate what will happen to the row
>>>>        after an agent restart
>>>
>>>
>>> I will try to address this comment in the next revision.
>>>
>>>> 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.
>>>
>>>
>>> I will try to address this comment in the next revision.
>>>
>>>> 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.
>>>
>>>
>>> The first comment is addressed, the second one is TBD.
>>>
>>>> 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.
>>>
>>>
>>> I rewrite this part according to the guideline described in RFC4181
>>> Sec.3.4.
>>> However, there are some TBDs in this part that should be updated according
>>> to the update in the main body of MIB module.
>>>
>>>
>>>> 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.
>>>
>>>
>>> This will be addressed in the next reivision.
>>>
>>>> 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:
>>>
>>>
>>> Fixed.
>>>
>>>> 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.
>>>
>>>
>>> In my understanding, L2L3-VPN-MCAST-MIB is designed for both
>>> L2VPN multicast and L3VPN multicast, but MVPN-MIB is designed
>>> only for L3VPN multicast. I think this is a reason why there are
>>> two separate documents.
>>>
>>> -- tsuno
>>>
>>> 2016-06-07 0:44 GMT+09:00 Jeffrey (Zhaohui) Zhang <[e-mail]>:
>>>>
>>>> Mach, Glenn,
>>>>
>>>> I've addressed most of the comments from Glenn on the l2l3 mvpn mib and
>>>> had started on addressing some comments on mvpn mib, but I've been side
>>>> tracked and am making very slow progress.
>>>>
>>>> I will try to pick it up again soon.
>>>>
>>>> Thanks.
>>>> Jeffrey
>>>>
>>>>> -----Original Message-----
>>>>> From: BESS [mailto:[e-mail]] On Behalf Of Mach Chen
>>>>> Sent: Wednesday, June 01, 2016 3:30 AM
>>>>> To: Glenn Mansfield Keeni <[e-mail]>; Benoit Claise
>>>>> <[e-mail]>; EXT - [e-mail]
>>>>> <[e-mail]>
>>>>> Cc: Jeffrey (Zhaohui) Zhang <[e-mail]>; [e-mail];
>>>>> Martin
>>>>> Vigoureux <[e-mail]>; [e-mail]
>>>>> Subject: Re: [bess] MIBDoc review of draft-ietf-bess-mvpn-mib-02.txt
>>>>>
>>>>> Hi authors,
>>>>>
>>>>> I saw the discussions between you and Glenn about l2l3 mvpn mib and you
>>>>> have already submitted the 02 version to address the comments.
>>>>>
>>>>> But I did not see any response to the below mvpn mib review and
>>>>> comments(maybe I missed something), given that we have plan to progress
>>>>> the mvpn-mib and l2l3-mvpn-mib documents together, what's your plan
>>>>> about
>>>>> this document?
>>>>>
>>>>> Best regards,
>>>>> Mach
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Glenn Mansfield Keeni [mailto:[e-mail]]
>>>>>> Sent: Wednesday, April 13, 2016 12:04 PM
>>>>>> To: Benoit Claise; [e-mail]
>>>>>> Cc: [e-mail]; Martin Vigoureux; Mach Chen; Jeffrey (Zhaohui)
>>>>>
>>>>> Zhang;
>>>>>>
>>>>>> [e-mail]
>>>>>> Subject: MIBDoc review of draft-ietf-bess-mvpn-mib-02.txt
>>>>>>
>>>>>> Hi,
>>>>>> I have been asked to do a MIB Doctors review of
>>>>>> draft-ietf-bess-mvpn-mib-02.txt.
>>>>>>
>>>>>> The comments are attached.
>>>>>> You will note that this is preliminary review. There are some generic
>>>>>
>>>>> comments
>>>>>>
>>>>>> which apply to all the scalars and tables. Please take care of those
>>>>>> and
>>>>>
>>>>> then we
>>>>>>
>>>>>> will get onto the next phase.
>>>>>>
>>>>>> Hope this helps.
>>>>>>
>>>>>>
>>>>>> Glenn
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> BESS mailing list
>>>>> [e-mail]
>>>>> https://www.ietf.org/mailman/listinfo/bess
>>>>
>>>>
>>>> _______________________________________________
>>>> BESS mailing list
>>>> [e-mail]
>>>> https://www.ietf.org/mailman/listinfo/bess
>>
>>
>>
>> _________________________________________________________________________________________________________________________
>>
>> Ce message et ses pieces jointes peuvent contenir des informations
>> confidentielles ou privilegiees et ne doivent donc
>> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu
>> ce message par erreur, veuillez le signaler
>> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
>> electroniques etant susceptibles d'alteration,
>> Orange decline toute responsabilite si ce message a ete altere, deforme ou
>> falsifie. Merci.
>>
>> This message and its attachments may contain confidential or privileged
>> information that may be protected by law;
>> they should not be distributed, used or copied without authorisation.
>> If you have received this email in error, please notify the sender and
>> delete this message and its attachments.
>> As emails may be altered, Orange is not liable for messages that have been
>> modified, changed or falsified.
>> Thank you.
>>