Jeffrey, Thanks for the update. Will get back to you after a detailed review is done.
Glenn On 2016/04/16 21:47, Jeffrey (Zhaohui) Zhang wrote: > Glenn, > > Thanks for your comments. I've addressed most of your comments in the new > revision: > > URL: > https://www.ietf.org/internet-drafts/draft-ietf-bess-l2l3-vpn-mcast-mib-03.txt > Status: > https://datatracker.ietf.org/doc/draft-ietf-bess-l2l3-vpn-mcast-mib/ > Htmlized: > https://tools.ietf.org/html/draft-ietf-bess-l2l3-vpn-mcast-mib-03 > Diff: > https://www.ietf.org/rfcdiff?url2=draft-ietf-bess-l2l3-vpn-mcast-mib-03 > > Please see below. > >> 1. Abstract: >> 1.1 A sentence on how the managed objects will be used by >> applications for operations, monitoring and management >> would be good. > > I had thought this would be standard/obvious for all MIB objects - the > read-write ones are used to control how a device works, and the read-only > ones are used for monitoring. Do I really need to say it explicitly? > > I see RFC 4382 has the following: > > This memo defines a portion of the Management Information Base (MIB) > for use with network management protocols in the Internet community. > In particular, it describes managed objects to configure and/or > monitor Multiprotocol Label Switching Layer-3 Virtual Private > Networks on a Multiprotocol Label Switching (MPLS) Label Switching > Router (LSR) supporting this feature. > > Is it enough to say something similar? For example: > > In particular, it describes common managed objects used to configure > and/or monitor both L2 and L3 VPN Multicast. > >> >> 2. Introduction >> 2.1 Please give the full expansion of the abbreviations >> appearing for the first time. (PE, VPLS,..) > > Fixed. > >> >> 2.2 The terminology section is a bit terse. Explaining the >> terms that are used, nicely with reference to the protocol >> documents will improve readability. >> e.g. >> - PMSI, I-PMSI, S-PMSI, provider tunnels > > As the paragraph alluded to, this MIB needs to be understood in the general > context of L2/L3 multicast VPN and providing good explanation of the terms is > not attempted. The references for the terms are the the RFCs for the relevant > technologies. > > Having said that, I'll explain PMSI a bit further. > >> 2.3 Is there a difference between >> "multicast in Layer 2 and Layer 3 VPNs , defined by >> RFC 7117 and RFC 6513/6514" >> used in the DESCRIPTION in the MODULE-IDENTITY >> and >> "multicast in BGP/MPLS L2 or IP VPN" >> used in the DESCRIPTION of L2L3VpnMcastProviderTunnelType ? >> If these are the same, it will be helpful to stick to the >> same expression. If these are not the same, the dictinction >> should be clarified. > > No difference. I was using "Layer 3" or "L3" but it was pointed out that the > layer 3 VPN is often referred to IP VPN in other RFCs and I was advised to > change it accordingly. Looks like I did not change all the cases. > > On the other hand, I noticed that RFC 4382 does use "Layer 3 VPN" so I'll > change it back. > >> >> >> 3. Summary of MIB Module. >> An overview of the L2L3-VPN-MCAST-MIB will be good- the >> structure of the MIB, short descriptions of the table(s) >> including usage of the table(s) for management and/or by >> other MIB(s). > > I had that, but have added one sentence about the only table. > >> >> MIB definitions: >> 4. MIB syntax checking: >> smilint -s -e -l 5 mibs/L2L3-VPN-MCAST-MIB 2>L2L3-VPN-MCAST-MIB.txt > > I used simpleweb's validation tool but looks like I did not use the strictest > level of validation. I've now fixed the following issues and verified. > >> >> mibs/L2L3-VPN-MCAST-MIB:63: [4] {hyphen-in-label} warning: named number >> `rsvp-p2mp' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:64: [4] {hyphen-in-label} warning: named number >> `ldp-p2mp' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:65: [4] {hyphen-in-label} warning: named number >> `pim-asm' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:66: [4] {hyphen-in-label} warning: named number >> `pim-ssm' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:67: [4] {hyphen-in-label} warning: named number >> `pim-bidir' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:68: [4] {hyphen-in-label} warning: named number >> `ingress-replication' must not include a hyphen in SMIv2 >> mibs/L2L3-VPN-MCAST-MIB:69: [4] {hyphen-in-label} warning: named number >> `ldp-mp2mp' must not include a hyphen in SMIv2 > > See later question/comments below. > >> mibs/L2L3-VPN-MCAST-MIB:215: [5] {group-unref} warning: current group >> `l2L3VpnMcastOptionalGroup' is not referenced in this module >> mibs/L2L3-VPN-MCAST-MIB:4: [5] {import-unused} warning: identifier >> `NOTIFICATION-TYPE' imported from module `SNMPv2-SMI' is never used >> mibs/L2L3-VPN-MCAST-MIB:5: [5] {import-unused} warning: identifier >> `Unsigned32' imported from module `SNMPv2-SMI' is never used >> mibs/L2L3-VPN-MCAST-MIB:8: [5] {import-unused} warning: identifier >> `NOTIFICATION-GROUP' imported from module `SNMPv2-CONF' is never used >> mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier >> `TruthValue' imported from module `SNMPv2-TC' is never used >> mibs/L2L3-VPN-MCAST-MIB:11: [5] {import-unused} warning: identifier >> `RowStatus' imported from module `SNMPv2-TC' is never used >> mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier >> `TimeStamp' imported from module `SNMPv2-TC' is never used >> mibs/L2L3-VPN-MCAST-MIB:12: [5] {import-unused} warning: identifier >> `TimeInterval' imported from module `SNMPv2-TC' is never used >> mibs/L2L3-VPN-MCAST-MIB:15: [5] {import-unused} warning: identifier >> `SnmpAdminString' imported from module `SNMP-FRAMEWORK-MIB' is never used >> mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier >> `InetAddress' imported from module `INET-ADDRESS-MIB' is never used >> mibs/L2L3-VPN-MCAST-MIB:18: [5] {import-unused} warning: identifier >> `InetAddressType' imported from module `INET-ADDRESS-MIB' is never used > > Removed the above unused imports. > >> >> 5. REFERENCE clauses: Please use REFERENCE clauses liberally. >> Wherever possible, provide references for objects used in >> the MIB. The references will point to specific sections/ >> sub-sections of the RFCs defining the protocol for which the >> MIB is being designed. It will greatly improve the readability >> of the document. > > Added. > >> >> 6. IMPORTS clause >> MIB modules from which items are imported must be cited and >> included in the normative references. >> The conventional style is >> mplsStdMIB >> FROM MPLS-TC-STD-MIB -- [RFC3811] > > Added. > >> >> 7. Please update the MODULE-IDENTITY. (There are no syntantic errors.) >> 7.1 CONTACT-INFO >> Following the conventions (including indentation style) will >> improve the readability. (e.g. RFC4382, RFC5132). >> Will be good if it does not overflow into the next page. > > Fixed. > >> >> 7.2 REVISION clause: follow the convention recommended in RFC4181 >> sec 4.5 >> REVISION "200212132358Z" -- December 13, 2002 >> DESCRIPTION "Initial version, published as RFC yyyy." >> -- RFC Ed.: replace yyyy with actual RFC number & remove this note: > > Fixed. > >> 7.3 OID assignment: follow the convention recommended in RFC4181 >> sec 4.5 i >> replace >> ::= { experimental 99 } -- number to be assigned >> by >> ::= { <subtree> XXX } >> -- RFC Ed.: replace XXX with IANA-assigned number & remove this note >> <subtree> will be the subtree under which the module will be >> registered. >> > > I kept "experimental 99" so that I could continue to use mib tools to > validate; but I added notes for the editor to replace them as you indicated. > >> >> 8. Specific MO and TC related comments. >> L2L3VpnMcastProviderTunnelType ::= TEXTUAL-CONVENTION >> STATUS current >> DESCRIPTION >> "Types of provider tunnels used for multicast in >> BGP/MPLS L2 or IP VPN." >> SYNTAX INTEGER { unconfigured (0), >> rsvp-p2mp (1), >> ldp-p2mp (2), >> pim-asm (3), >> pim-ssm (4), >> pim-bidir (5), >> ingress-replication (6), >> ldp-mp2mp (7) >> >> o Would be nice to align the enumeration labels with the >> labels in the protocol document RFC 6514 unless there is >> a good reason for not doing so. (You will have to take >> care of the smi compilation errors too; '-' is not allowed ). > > Are spaces allowed? I don't know so I used hyphen. For now I replace with > things like rsvpP2mp. > Or could/should I just remove the definitions, so that if a new type is > defined in the future there is no need to update the MIB? > >> >> 8.1 l2L3VpnMcastPmsiTunnelAttributeEntry OBJECT-TYPE >> SYNTAX L2L3VpnMcastPmsiTunnelAttributeEntry >> MAX-ACCESS not-accessible >> STATUS current >> DESCRIPTION >> "An entry in this table corresponds to an PMSI attribute >> that is advertised/received on this router. >> For BGP-based signaling (for I-PMSI via auto-discovery >> procedure, or for S-PMSI via S-PMSI A-D routes), >> they are just as signaled by BGP (RFC 6514 section 5, >> 'PMSI Tunnel attribute'). >> For UDP-based S-PMSI signaling for PIM-MVPN, >> they're derived from S-PMSI Join Message >> (RFC 6513 section 7.4.2, 'UDP-based Protocol').. >> >> Note that BGP-based signaling may be used for >> PIM-MVPN as well." >> o Fix the ".." in "'UDP-based Protocol').." above. >> o Please give the reference for this Table. >> Is it- "PMSI Tunnel attribute" in RFC 6513 Sec.4 ? >> "PMSI Tunnel attribute" in RFC 6514 Sec.5 ? >> both? >> Any other pointers? > > Fixed. > >> >> 8.2 l2L3VpnMcastPmsiTunnelAttributeFlags OBJECT-TYPE >> SYNTAX OCTET STRING (SIZE (1)) >> MAX-ACCESS not-accessible >> STATUS current >> DESCRIPTION >> "For UDP-based S-PMSI signaling for PIM-MVPN, this is 0. >> For BGP-based I/S-PMSI signaling, this is the Flags >> field in PMSI Tunnel Attribute of the corresponding >> I/S-PMSI A-D route." >> ::= { l2L3VpnMcastPmsiTunnelAttributeEntry 1 } >> o Please confirm that the above is a complete enumeration of the >> types of signalling. >> o RFC 6514 Sec.5 says that the Flags field indicates >> "Leaf Information Required". That is useful information. >> Please include in the description. > > The intent is to simply return the octet value of the flags field, w/o > listing individual bits like "Leaf Information Required". More bits could be > defined in the future but the MIB would not change. > > Is that OK? > >> >> 8.3 l2L3VpnMcastPmsiTunnelAttributeId OBJECT-TYPE >> SYNTAX OCTET STRING ( SIZE (0..37) ) >> MAX-ACCESS not-accessible >> STATUS current >> DESCRIPTION >> "For UDP-based S-PMSI signaling for PIM-MVPN, the first >> four or sixteen octets of this attribute are filled with >> the provider tunnel group address (IPv4 or IPv6).. >> For BGP-based I/S-PMSI signaling, this is the Tunnel >> Identifier >> Field in PMSI Tunnel Attribute of the corresponding I/S-PMSI >> A-D route." >> o Check the size specifications. The specs above say it can be >> all sizes 0..37. That is not clear from the DESCRIPTION clause. >> o Fix the ".." in "(IPv4 or IPv6).." above. >> o RFC 6514 Sec 5. PMSI Tunnel Attribute gives the Tunnel Identifiers >> for mLDP, PIM-SM, PIM-SSM, BIDIR-PIM,Ingress Replication,MP2MP. >> It appears that the sizes (range) for each case will be different. >> Please clarify that, and if there are discrete sizes, specify >> accordingly. > > Depending on the tunnel type, there could be different sizes. Future tunnel > types could have other sizes that not specified today. I was thinking to just > give a size range so that it is flexible. Is that ok? > >> >> >> 8.3 l2L3VpnMcastPmsiTunnelPointer OBJECT-TYPE >> SYNTAX RowPointer >> MAX-ACCESS read-only >> STATUS current >> DESCRIPTION >> "If the tunnel exists in some MIB table, this is the >> row pointer to it." >> o "some MIB table" : specify which MIB table. > > I can give an example, like mplsTunnelTable [RFC 3812]. It could be whatever > table that a tunnel may be put into. > >> o In what case will the tunnel exist and in what case will it not? > > If a device supports mplsTunnelTable and the tunnel is represented there, > then it exists. > >> o What will be the behaviour if the above condition is not satisfied? > > A null pointer should be given. > >> >> 8.4 l2L3VpnMcastPmsiTunnelIf OBJECT-TYPE >> SYNTAX RowPointer >> MAX-ACCESS read-only >> STATUS current >> DESCRIPTION >> "If the tunnel has a corresponding interface, this is the >> row pointer to the ifName table." >> o DESCRIPTION looks incorrect. Please fix it. Do you want to say >> this object points to the corresponding row in the ifTable? > > Yes. Fixed. > >> o In what case does the TunnelIf exist and in what case will it not? > > Some tunnels may not have a corresponding interface. > >> o What will be expected if the tunnel does not have a corresponding >> interface? > > Null row pointer. > >> >> 9. The Security Considerations section does not follow the Security >> Guidelines for IETF MIB Modules >> http://trac.tools.ietf.org/area/ops/trac/wiki/mib-security. >> Please fix. > > I was really hoping that it would not have to be that tedious. SNMP/MIB > security should be no different from the CLI security - once you secure the > infrastructure then what's more to do? > > I'll need more time to work on this. Let me try to address the issues in the > other mib first and come back to this. > >> >> >> 10.ID-nits >> 10.1 Checking nits according to http://www.ietf.org/id-info/checklist : >> >> --------------------------------------------------------------------------- >> >> ** There are 4 instances of too long lines in the document, the >> longest one >> being 3 characters in excess of 72. > > I fixed some but there still three too long lines: > > l2L3VpnMcastPmsiTunnelAttributeType L2L3VpnMcastProviderTunnelType, > > l2L3VpnMcastGroups OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance 1} > l2L3VpnMcastCompliances OBJECT IDENTIFIER ::= {l2L3VpnMcastConformance 2} > > Should I break them into different lines or just keep them as is? Any example > of expected indentation if I break the lines? > >> >> 10.2 Checking references for intended status: Proposed Standard >> >> --------------------------------------------------------------------------- >> >> == Missing Reference: 'RFC 7117' is mentioned on line 76, but not >> defined >> 'described in [RFC6513, RFC6514, RFC 7117] and other documents >> tha...' > > I hope I understood and fixed it (removing the space in "RFC 7117"). > >> >> 11. There is another WIP MVPN-MIB in draft-ietf-bess-mvpn-mib-02.txt >> MVPN-MIB has objects that refer to L2L3-VPN-MCAST-MIB. >> Is there a good reason for not merging the 2 documents? I have not seen >> any discussion or explanation on this. I may have missed it. Please >> clarify or, give some pointers. > > As mentioned in the introduction: > > this memo describes managed objects common to both VPLS > Multicast [RFC7117] and MVPN [RFC6513, RFC6514]. > > MVPN-MIB is for MVPN. There was another VPLS Multicast MIB in the work and > both would reference common objects defined in this MIB. > > Thanks! > Jeffrey > >> -----Original Message----- >> From: BESS [mailto:[email protected]] On Behalf Of Glenn Mansfield >> Keeni >> Sent: Tuesday, April 12, 2016 2:28 AM >> To: Benoit Claise <[email protected]>; EXT - [email protected] >> <[email protected]> >> Cc: Jeffrey (Zhaohui) Zhang <[email protected]>; [email protected]; Martin >> Vigoureux <[email protected]>; [email protected]; Mach Chen >> <[email protected]> >> Subject: [bess] MIBDoc review of draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt >> >> Hi, >> I have been asked to do a MIB Doctors review of >> draft-ietf-bess-l2l3-vpn-mcast-mib-02.txt. >> My knowledge of L2L3VPN Multicast is limited to the reading >> of this document and browsing through the documents referred >> to in the draft and bess-wg mailing list archives.( read "shallow"). >> So some of the doubts and questions may sound trivial or >> strange. Please bear with me and help me help you make >> this into a better document :-) >> >> The comments are attached. >> >> Glenn >> > > _______________________________________________ > BESS mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/bess > _______________________________________________ BESS mailing list [email protected] https://www.ietf.org/mailman/listinfo/bess
