Dear Tsuno,
     Thanks for the good work.
I will review and get back to you.

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

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


_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess

Reply via email to