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

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 for configuration and
tables for monitoring.
In this revision, the diagram representing the relationship among tables 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 be changed
      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 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.

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

Reply via email to