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

From: <[email-masked]>
Date: Thu, 1 Jun 2017 11:16:41 +0200

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 <[email-masked]>:
>> 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:[email-masked]] On Behalf Of Mach Chen
>>> Sent: Wednesday, June 01, 2016 3:30 AM
>>> To: Glenn Mansfield Keeni <[email-masked]>; Benoit Claise
>>> <[email-masked]>; EXT - [email-masked]
>>> <[email-masked]>
>>> Cc: Jeffrey (Zhaohui) Zhang <[email-masked]>; [email-masked]; Martin
>>> Vigoureux <[email-masked]>; [email-masked]
>>> 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:[email-masked]]
>>>> Sent: Wednesday, April 13, 2016 12:04 PM
>>>> To: Benoit Claise; [email-masked]
>>>> Cc: [email-masked]; Martin Vigoureux; Mach Chen; Jeffrey (Zhaohui)
>>> Zhang;
>>>> [email-masked]
>>>> 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
>>> [email-masked]
>>> https://www.ietf.org/mailman/listinfo/bess
>>
>> _______________________________________________
>> BESS mailing list
>> [email-masked]
>> 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.

---

From: Hiroshi Tsunoda <[email-masked]>
Date: Thu, 1 Jun 2017 11:25:34 +0200

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  <[email-masked]>:
> Do you know when you could do a review of the revision posted in early March
> ?
>
> Thanks in advance,
>
> -Thomas
>
>
>>> 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.
>>>
> 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,
> This message and its attachments may contain confidential or privileged
> information that may be protected by law;

---

From: <[email-masked]>
Date: Thu, 1 Jun 2017 11:41:34 +0200

Ok, thanks for the update and for progressing this work!

-Thomas

---

From: Glenn Mansfield Keeni <[email-masked]>
Date: Sat, 3 Jun 2017 17:28:10 +0900

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

---

From: Glenn Mansfield Keeni <[email-masked]>
Date: Mon, 5 Jun 2017 19:24:10 +0900

Dear Tsuno/Zhang,
      Thanks for the good work. The document readability is
vastly improved.
The following are the comments on
     draft-ietf-bess-l2l3-vpn-mcast-mib-08.txt

Glenn

0. The MIBs
     L2L3-VPN-MCAST-TC-MIB and
     L2L3-VPN-MCAST-MIB
    compile OK. There are level-5 warnings
     mibs/L2L3-VPN-MCAST-TC-MIB:64: [5] {type-unref} warning: current  
type `L2L3VpnMcastProviderTunnelType' is not referenced in this module
     mibs/L2L3-VPN-MCAST-TC-MIB:90: [5] {type-unref} warning: current  
type `L2L3VpnMcastProviderTunnelId' is not referenced in this module
     mibs/L2L3-VPN-MCAST-TC-MIB:90: [5] {type-without-format} warning:  
type `L2L3VpnMcastProviderTunnelId' has no format specification
     mibs/L2L3-VPN-MCAST-TC-MIB:169: [5] {type-unref} warning: current  
type `L2L3VpnMcastProviderTunnelPointer' is not referenced in this module
     mibs/L2L3-VPN-MCAST-TC-MIB:198: [5] {type-unref} warning: current  
type `L2L3VpnMcastProviderTunnelPointerType' is not referenced in this  
module

     These may be ignored.

1. The explanations for the size of the identifiers for each tunneling
    technology in TC L2L3VpnMcastProviderTunnelId
    is nicely done. These are aligned with definitions in rfc6514.
    In
            noTunnelId         (0), -- No tunnel information
    is there a specific reason to name it 'noTunnelId' instead of
    'noTunnelInfo'.

2. The TCs
        L2L3VpnMcastProviderTunnelPointerType, and
        L2L3VpnMcastProviderTunnelPointer
    Are probably not required. The value of the RowPointer [rfc2579]
    object is ' the name of the instance of the first
                accessible columnar object in the conceptual row'
    So the pointer will explicitly contain the name of the table.
    An auxilliary type object to indicate the table name is not
    necessary.
   [This is an oversight on my part. I should have noticed this earlier.]

Other Editorials:
P-2. Sec-1.
1.   para-1 makes tedious reading. Could this be improved?

2.       "Border Gateway Protocol/ MultiProtocol Label Switching (BGP/MPLS)"
      The usage of the '/' here is unclear.

3.       "and L3 VPNs.  Therefore, TCs and MOs defined"
      The context of the 'Therefore' is not clear.

4.       "The are two type"
      Probably s/The/There/ ?

On 2017/05/26 21:25, Hiroshi Tsunoda wrote:
> Dear Glenn,
> 
> Thank you for your thorough review and waiting for the updated.
> 
> I posted a new revision taking into account your latest comments.
> In the new revision, all of your comments are addressed.
> I also revised the draft to improve the readability.
> 
> https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-08.txt
> Htmlized(1):
> https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-08
> Htmlized(2):
> https://datatracker.ietf.org/doc/html/draft-ietf-bess-l2l3-vpn-mcast-mib-08
> Diff:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-08
> 
> 2017-05-02 14:44 GMT+02:00 Hiroshi Tsunoda <[email-masked]>:
>> 2017-05-01 12:32 GMT+02:00 Glenn Mansfield Keeni <[email-masked]>:
>>> Dear Tsuno/Zhang
>>>       Thanks for waiting. The review of
>>>   draft-ietf-bess-l2l3-vpn-mcast-mib-07 follows.
>>>
>>> Glenn
>>>
>>> C1. Abstract:
>>>      The draft now defines 2 MIB modules.  Please revise the abstract
>>>      and probably the title of the document too.
> 
> Fixed.
> 
>>> C2. The MIBs (L2L3-VPN-MCAST-TC-MIB, L2L3-VPN-MCAST-MIB) compile OK.
>>>      (Three {type-unref} warnings are there, may be ignored.)
> 
> I have confirmed that the latest version of MIB modules can also be
> compiled successfully.
> 
>>> C3. Page 4:
>>>        s/3.  Summary of MIB Module/
>>>          3.  Summary of MIB Modules/
> 
> Fixed.
> 
>>> C4. Page 6: L2L3VpnMcastProviderTunnelPointer DESCRIPTION
>>>        s/"Denotes a pointer to the row pertaining to a table entry/
>>>         /"This textual convention represents a pointer to a row in
>>>          the table represented by the following object of type
>>>          L2L3VpnMcastProviderTunnelPointerType./
> 
> Fixed.
> 
>>> C5. Page 7: L2L3VpnMcastProviderTunnelPointer DESCRIPTION
>>>          The explanation in the last paragraph seems out of place.
>>>          It may be removed.
> 
> Fixed.
> 
>>> C6  Page 7: L2L3VpnMcastProviderTunnelPointerType DESCRIPTION
>>>          it is unclear when the value 'null(0)' will be used.
>>>          Is this allowed only when the corresponding object of type
>>>          L2L3VpnMcastProviderTunnelPointer has a value that is a
>>>          zero-length string ? If yes, please make that clear.
> 
> 'null(0)' is the default value and indicates that the corresponding
> L2L3VpnMcastProviderTunnelPointer object is not assigned.
> In the new revision, this point is described clearly.
> 
>>> C7. Page 9: l2L3VpnMcastPmsiTunnelAttributeTable DESCRIPTION
>>>          s/created by a PE router/maintained by a PE router/
> 
> Fixed.
> 
>>> C8. Page 12: l2L3VpnMcastPmsiTunnelAttributeId
>>>           Do you really want to keep this object in L2L3-VPN-MCAST-MIB.
>>>           It will change every time a new "tunnel type" is added to
>>>           L2L3VpnMcastProviderTunnelType. That will defeat the purpose
>>>           of separating L2L3-VPN-MCAST-TC-MIB from L2L3-VPN-MCAST-MIB
>>>           It may be a good idea to define a textual convention like
>>>               L2L3VpnMcastPmsiTunnelAttributeId
>>>           in the L2L3-VPN-MCAST-TC-MIB and use that textual convention
>>>           in the L2L3-VPN-MCAST-MIB
> 
> Thank you for your suggestion. I revised the definition according to
> your suggestion.
> 
>>> C9. Page 13: l2L3VpnMcastPmsiTunnelAttributeId DESCRIPTION
>>> A.       s/Thus, the size of this object is 16 octets in IPv4/
>>>             Thus, the size of this object is 8 octets in IPv4/
> 
> Fixed.
> 
>>> B.       The last 2 paragraphs do not tie up well with the previous
>>>           paragraphs in the DESCRIPTION.
> 
> Those 2 paragraphs are removed in the new revision.
> 
>>> C.       In the last paragraph
>>>           "this object is a pair of source and group IP addresses"
>>>           is unlcear. Please clarify.
> 
> Fixed. In the new revision, this point is described as follows.
> 
> "the corresponding Tunnel Identifier is composed of
> the source IP address and the group IP address."
> 
>>> C10. Page 15: Security Considerations
>>>           I would think that any field that reveals information about
>>>           a Multicast VPN and/or its members is sensitive.
>>>           Does the l2L3VpnMcastPmsiTunnelAttributeId field reveal
>>>           information about the participating members? If yes, it must
>>>           be marked as sensitive.
> 
> I revised this point according to your suggestion.
> 
>>> C11. Editorial:
>>>
>>> A. This does not pertain to the MIBs as such,
>>>     but I am uncomfortable with the  several variations
>>>     of the phrase
>>>     "Layer 2 and Layer 3 Virtual Private Networks (VPN)
>>>      that support multicast"
>>>     that appears in the text. I do not have a solution
>>>     on hand but it would be perhaps be more readable to
>>>     define and use a simple name/notation the beast for
>>>     which the MIB is being designed (e.g. "L2L3VPNMCast").
>>>
>>> B. Same with the phrase
>>>      "Layer 2 (L2) and Layer 3 (L3) VPN (Virtual Private
>>>       Network)
>>>      it would be probably be easier on the reader if an
>>>      abbreviation like L2L3VPNs was used where ever
>>>      applicable.
> 
> Thank you for your comments.
> In the new reivsion the abbreviation "L2L3VPNMCast"
> is defined and used throughout the draft.
> 
>>> C12. P2:Para4: s/within MIB module specifications//;
> 
> Fixed.
> 
>>> C13. General:
>>> A.      The DESCRIPTION clauses could do would some more
>>>          packing. (Too much space at the end of lines)
>>> B.      Please check the articles a/an/the once again. Some
>>>          usages do not seem right to me.
> 
> Fixed.
> 
> Sincerely yours,
> 
> -- tsuno
>

---

From: Hiroshi Tsunoda <[email-masked]>
Date: Mon, 5 Jun 2017 16:40:18 +0200

Dear Glenn,

Thank you very much for your review.

I am going to resubmit a revision as soon as possible.
Please wait for a moment regarding MVPN-MIB.

-- tsuno

2017-06-05 12:24 GMT+02:00 Glenn Mansfield Keeni <[email-masked]>:
>     mibs/L2L3-VPN-MCAST-TC-MIB:64: [5] {type-unref} warning: current type
> `L2L3VpnMcastProviderTunnelType' is not referenced in this module
>     mibs/L2L3-VPN-MCAST-TC-MIB:90: [5] {type-unref} warning: current type
> `L2L3VpnMcastProviderTunnelId' is not referenced in this module
>     mibs/L2L3-VPN-MCAST-TC-MIB:90: [5] {type-without-format} warning: type
> `L2L3VpnMcastProviderTunnelId' has no format specification
>     mibs/L2L3-VPN-MCAST-TC-MIB:169: [5] {type-unref} warning: current type
> `L2L3VpnMcastProviderTunnelPointer' is not referenced in this module
>     mibs/L2L3-VPN-MCAST-TC-MIB:198: [5] {type-unref} warning: current type
> `L2L3VpnMcastProviderTunnelPointerType' is not referenced in this module
>
>     These may be ignored.
>

---

From: Hiroshi Tsunoda <[email-masked]>
Date: Tue, 6 Jun 2017 18:22:12 +0200

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.

> This point will be fixed in the next revision.

Fixed. "Provide Edge" router is correct.

> This point will be fixed in the next revision.

Fixed. "common" is correct.

> Partially fixed. I will give more detailed explanation in the next revision.

I have added some more explanation in this revision.

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

> This will be addressed in the next revision.

I have added some references but more are required.
I will keep working on this.

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

>>      if you want to dynamically change that value, otherwise,

Fixed. Now, "MAX-ACCESS    read-create" is used.

>>     monitoring but not configuration.

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

---

From: Glenn Mansfield Keeni <[email-masked]>
Date: Mon, 12 Jun 2017 21:47:32 +0900

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

---

From: Hiroshi Tsunoda <[email-masked]>
Date: Wed, 21 Jun 2017 19:35:05 +0200

Dear Glenn,

Thank you for your comments. I posted a new revision (-09) as follows.

URL:
https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-09.txt
Htmlized:
https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-09
Htmlized:
https://datatracker.ietf.org/doc/html/draft-ietf-bess-l2l3-vpn-mcast-mib-09
Diff:
https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-09

In the new revision, all of your comments are addressed and
the following additional changes are made.

-   Update the definition and description of L2L3VpnMcastProviderTunnelType
    Describe the enumerated values and the corresponding
    tunnel type.
    I added new valule, transportTunnel (8) defined in [RFC7524].

 - Add some description related to transportTunnel (8)
   into the defi L2L3VpnMcastProviderTunnelId

 - Update the description of l2L3VpnMcastPmsiTunnelAttributeFlags
   according to [RFC7902]

 -  Added a new object, l2L3VpnMcastPmsiTunnelAttributeAddlFlags.
    This is defined in [RFC7902]

I also removed some paragraph from 1.1 Terminology section,
because those seem verbose.

Please see some other notes below.

Fixed.

>   [This is an oversight on my part. I should have noticed this earlier.]

These TCs were removed and related descriptions in other parts
in the document were updated.

>      The usage of the '/' here is unclear.

Updated.
In the new revision, BGP/MPLS VPN is explained in para-1, and
multicast support in BGP/MPLS Layer 2 and Layer 3 is explained in para-2
separately.

>      The context of the 'Therefore' is not clear.

Fixed. I removed the related sentence because it seemed not
required already.

>      Probably s/The/There/ ?

Fixed. Thank you for pointing it out.

-- tsuno

---

From: Glenn Mansfield Keeni <[email-masked]>
Date: Mon, 26 Jun 2017 16:52:39 +0900

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:
>> https://www.ietf.org/internet-drafts/draft-ietf-bess-mvpn-mib-04.txt
>> 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
>>

---

From: Hiroshi Tsunoda <[email-masked]>
Date: Tue, 27 Jun 2017 08:34:02 +0200

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-masked]>:
>>> Htmlized:https://datatracker.ietf.org/doc/html/draft-ietf-bess-mvpn-mib-04
>>> Diff:
>>>> Partially fixed. I will give more detailed explanation in the
>>>> nextrevision.
>>>
>>>
>>> Eight tables can be categorized into two groups: tables forconfiguration
>>> and
>>> tables for monitoring.
>>> In this revision, the diagram representing the relationship amongtables
>>> is
>>>
>>>>>     o Please confirm that the values in the row itself will not
>>>>> bechanged
>>> 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