Hi Glenn, Thank you for your review and valuable comments. I am going to look at your comments in detail and revise the draft.
Best regards, -- tsuno 2017-06-26 9:52 GMT+02:00 Glenn Mansfield Keeni <[email protected]>: > Hi Tsuno, > Thanks for waiting. I have done one pass of the draft. > The comments are attached. > Please note that we probably need to do some more design > considerations on the MIB - there are some issues that > need to be addressed before we can arrive at a reasonably > stable version of the MIB itself. > > Glenn > > > On 2017/06/12 21:47, Glenn Mansfield Keeni wrote: >> >> Hi Tsuno, >> Got this. I will be working on this. It is massive- >> 40 pages. I hope to get back to you on this by the end >> of next week. >> >> Cheers, >> >> Glenn >> On 2017/06/07 1:22, Hiroshi Tsunoda wrote: >>> >>> Hi Glenn, >>> >>> I posted a new revision (-04) of MVPN-MIB document. >>> In this revision, "Summary of MIB Module" has been rewritten. >>> I hope this change improves the readability. >>> >>> URL: >>> https://www.ietf.org/internet-drafts/draft-ietf-bess-mvpn-mib-04.txt >>> Htmlized: https://tools.ietf.org/html/draft-ietf-bess-mvpn-mib-04 >>> >>> Htmlized:https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-mib-04 >>> Diff: >>> https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-mvpn-mib-04 >>> >>> Please see notes below for other changes. >>> >>> 2017-03-01 16:00 GMT+01:00 Hiroshi Tsunoda <[email protected]>: >>>>> >>>>> 1. Abstract: >>>>> 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. >>> >>> >>> Fixed. "Provide Edge" router is correct. >>> >>>>> 2. Introduction >>>>> 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. >>> >>> >>> Fixed. "common" is correct. >>> >>>>> 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 >>>> nextrevision. >>> >>> >>> I have added some more explanation in this 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. >>>>> 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. >>>>> 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). >>> >>> >>> I have rewritten "Sec.3.1 Summary of MIB Module". >>> Eight tables can be categorized into two groups: tables forconfiguration >>> and >>> tables for monitoring. >>> In this revision, the diagram representing the relationship amongtables >>> is >>> >>> divided to two separated diagrams based on the roles of tables. >>> >>>>> MIB definitions: >>>>> 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. >>> >>> >>> I have added some references but more are required. >>> I will keep working on this. >>> >>>>> 8. MOs. >>>>> 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 >>> >>> >>> I will try to update description in the next revision. >>> >>>>> 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 >>>>> bechanged >>>>> after creation. ( you do not have a 'modifiedMvrfConfig') >>> >>> >>> The name has been changed into mvpnGenMvrfStatusChange. >>> The name of the related object (mvpnGenOperStatusChangeTime) has >>> also been changed into mvpnGenMvrfStatusChangeTime. >>> >>>>> 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. >>> >>> >>> Fixed. Now, "MAX-ACCESS read-create" is used. >>> >>>>> 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 >>> RFC4181Sec.3.4. >>> However, there are some TBDs in this part that should be updatedaccording >>> 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. >>> >>> >>> In this revision, I have added additional ReadOnly compliance >>> Now, there are following two MODULE-COMPLIANCE statements >>> are defined in this module. >>> - mvpnModuleFullCompliance >>> - mvpnModuleReadOnlyCompliance >>> >>> Best regards, >>> >>> -- tsuno >>> >>> _______________________________________________ >>> BESS mailing list >>> [email protected] >>> https://www.ietf.org/mailman/listinfo/bess >>> >> >> _______________________________________________ >> BESS mailing list >> [email protected] >> https://www.ietf.org/mailman/listinfo/bess > > _______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
