Re: [MIB-DOCTORS] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-07

[Hiroshi Tsunoda <[e-mail]>]

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.

Please see some notes below.

URL:
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 <[e-mail]>:
> 2017-05-01 12:32 GMT+02:00 Glenn Mansfield Keeni <[e-mail]>:
>> 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