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
