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
