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 protected]>: > 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. >> >> 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
