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

[Hiroshi Tsunoda <[e-mail]>]

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