The MPLS WG Archive[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index][Thread Index][Author Index][Subject Index] [mpls] MIB Dr review of draft-ietf-mpls-fastreroute-mib-06.txt
Tom and Riza,
Here is the review. A few overall comments followed by
comments in order of the document.
Thanks,
Joan
OVERALL COMMENTs --------------- *) MIB compiles cleanly with smiLint and smicngPRO. *) First overall comment has to do with the organization of the MIB module itself; a better way to represent the objects would be to have separate MIB modules for General, One2One and Facility. Even the scalar objects seem dependent on
which method is being used so seems that separate MIB modules would help to clarify this. Please comment. *) I tried to be very specific about this during the
commenting, but basically would ask that the names used be consistent with the MPLS-TE-STD-MIB. For example, Tunnel, Index, Instance, Ingress, Egress and other words are spelled out when used in an object's name, but in this MIB Module these are abbreviated. Just looking for some consistency with the naming conventions used in the 2 MIBs since they are tightly coupled. COMMENTS ON DOC (as they appear in the
doc)
--------------------------------------------- *) Abstract: In particular, it describes managed objects for Multiprotocol Label Switching fast rerouting. suggestion to specify the 2 fast reroute methods:
...managed objects used to support two fast reroute (FRR) methods
for
Multiprotocol Label Switching (MPLS) based traffic engineering (TE). The two methods are one-to-one backup method and facility backup method. *) Date on the page headers do not match
date of document. *) TOC References is off by a space. Also, the subsections 1.1, 4.1, 4.2
(maybe others) are missing from the TOC. *) 1. Introduction
Would add RFC3811 also, i.e.
used in conjunction with [RFC3811], [RFC3812] and [RFC3813]. *) 2. Terminology
Should state the title of the referenced docs
here. So
This document uses terminology defined in the "Multiprotocol Label Switching Architecture" [RFC3031] document and the "Fast Reroute Extensions to RSVP-TE for LSP Tunnels" [RFC4090] document. *) 4. Brief Description of the MIB Module
Objects.
s/Objects./Objects Would retitle this section: Overview of the MIB Module I am confused by the term "bypass" in this section.
Are you referring to the one-to-one backup method described in [RFC4090]? Could the terms one-to-one and facility be used consistently? "LSRs do not implement both facility and bypass methods at the same time, the Agent Compliance section in this module defined herein is divided into portions, one for each method allowing any LSR to implement only the objects applicable to the method they have implemneted." Either correct the above statement:
s/Agent Compliance/Conformance s/implemneted/implemented or reword: "LSRs do not implement both one-to-one backup method and
facility backup method at the same time, thus, the Conformance section specifies conformance based on the two fast reroute methods. This allows a developer to implement only the objects applicable to the fast reroute method supported." Additionally, RFC4090 states:
Two methods are defined here. The one-to-one backup method creates detour LSPs for each protected LSP at each potential point of local repair. The facility backup method creates a bypass tunnel to protect a potential failure point; by taking advantage of MPLS label stacking, this bypass tunnel can protect a set of LSPs that have similar backup constraints. Both methods can be used to protect links and nodes during network failure. The described behavior and extensions to RSVP allow nodes to implement either method or both and to interoperate in a mixed network. However, the draft states:
...Given that common practice has shown that LSRs do not implement both facility and bypass methods at the same time...is divided into portions, one for each method In this regard the draft differs somewhat significantly
from
RFC4090. Could this be clarified? In other words, could the draft say something like, "although [RFC4090] specifies that a node is able to support both fast reroute methods simultaneously, common practice has shown...." I would like to understand what is meant by common practice? Is it that vendors don't support both methods at the same time, or is it that operators don't mix these 2 methods in a network? *) 4.1 mplsFrrConstTable
Please use the entire name Constraints, as in mplsFrrConstraintsTable. *) 4.2 mplsFrrTunARHopTable
Please use mplsFrrTunnelARHopTable to match with mplsTunnelARHopTable. First sentence:
s/mplsTunnelARHop table/mplsTunnelARHopTable *) 4.3 mplsFrrOne2OnePlrTable
"The mplsFrrOne2OnePlrTable is an optional table that contains lists of PLRs that initiated detour LSPs to protect tunnel instances. As such, it is only required for LSRs implementing the detour backup method. In these cases, the detour LSPs are reflected in the mplsFrrDetourTable." I don't understand the above. Is this table mandatory when
the
one-to-one backup method is used? If so, shouldn't that be stated. The definition from RFC 4090 states:
"PLR: Point of Local Repair. The head-end LSR of a backup
tunnel
or a detour LSP." So the phrase "contains lists of PLRs" is confusing.
Also, the phrase "detour backup method" is confusing, are you
referring to the one-to-one backup method? *) 4.4 mplsFrrDetourTable
Could this be renamed to mplsFrrOne2OneDetourTable?
Is this table mandatory when the one-to-one backup method is used? If so, could that be stated? "This table is optional and is only required in
case
mplsFrrOne2OnePlrTable is supported." If you state that the table is mandatory for the backup method, then think the above statement is not necessary. *) 4.5 mplsFrrFacRouteDBTable
The mplsFrrDBTable s/mplsFrrDBTable/mplsFrrFacRouteDBTable
Could this be renamed to mplsFrrFacilty
Could you state that this table is mandatory when the
facility backup method is used? *) 5. Handling IPv6 Tunnels
draft-ietf-ccamp-gmpls-addressing-03 does not appear as a reference, please add to Normative References. *) 6 (MIB MODULE) *) LAST-UPDATED and REVISION is incorrect
*) DESCRIPTION:
...This MIB module is part of RFC 4327; see the RFC itself for full legal notices. needs to be changed to something
like
This version of this MIB module is part of RFC xxxx; See the RFC itself for full legal notices. -- RFC EDITOR: please replace xxxx with actual
number
-- and remove this note. *) Please change to yyy ::= { mplsStdMIB yyy } -- RFC-editor please fill in -- yyy with value assigned by IANA, -- see section 18.1 for details *) Please change mplsFrrNotif to mplsFrrNotifications *) Doesn't seem necessary to have
mplsFrrScalars OBJECT IDENTIFIER ::= { mplsFrrMIB 1 } mplsFrrObjects OBJECT IDENTIFIER ::= { mplsFrrMIB 2 } Since Scalars are objects, right?
So maybe just use mplsFrrObjects mplsFrrObjects OBJECT IDENTIFIER ::= { mplsFrrMIB 1 } mplsFrrConformance OBJECT IDENTIFIER ::= { mplsFrrMIB 2 } *) Also, please move mplsFrrConformance under mplsFrrObjects.
*) As stated above, I believe this MIB should be organized in 3 separate MIB Modules. This would help to clarify what actually needs to be supported when using One2One or Facility. The following scalars seem to be dependent on the
type of backup method: mplsFrrDetourIncoming mplsFrrDetourOutgoing mplsFrrDetourOriginating mplsFrrConfIfs mplsFrrActProtectedIfs mplsFrrConfProtectionTuns mplsFrrActProtectionTuns mplsFrrActProtectedLSPs mplsFrrNotifsEnabled mplsFrrNotifMaxRate So why are these under General Objects?
*) mplsFrrDetourIncoming
Should be put with the oneToOne objects. Should this be a Counter32 or Gauge32? Please rename to mplsFrrIncomingDetourLSPs *) mplsFrrDetourOutgoing
Should be put with the oneToOne objects. Should this be a Counter32 or Gauge32? please rename to mplsFrrOutgoingDetourLSPs *) mplsFrrSwitchover Should this be a Gauge32? *) Could mplsFrrConfIfs be
renamed to: mplsFrrConfiguredInterfaces Also, this should apply only to facilityBackup, so could be a Gauge32 or Counter32? *) mplsFrrActProtectedIfs What does the Act stand for? Is this Active or Actual? Could this be renamed to mplsFrrActiveInterfaces/mplsFrrActualInterfaces *) mplsFrrConfProtectionsTuns
Could this be renamed mplsFrrConfiguredBypassTunnels ? Also, is this a Gauge32? *) mplsFrrActProtectionTuns Could this be renamed to mplsFrrActiveBypassTunnels? Also, is this a Gauge32? *) mplsFrrActProtectedLSPs could this be renamed to: mplsFrrActiveProtectedLSPs? Also, is this a Gauge32? *) mplsFrrProtectionMethod Scalar
should probably be moved to being the first scalar. Also, I think an unknown(1) should be added. The reason is that if a device has been rebooted, due to a change from one fast reroute method to another, and if something were misconfigured, then it might be that the fast reroute method would be "unknown" until a correction was made. Obviously, "unknown" would be read-only and not settable. Would
suggest:
unknown(1), onetooneBackup(2), facilityBackup(3) *) mplsFrrNotifsEnabled. Why is this object needed? *) mplsFrrNotifMaxRate
Are there other objects which indicate if events are being thrown away due to this throttling? (Would that be useful?) Last sentence
s/notified/generated/ *) mplsFrrConstTable Please rename to mplsFrrConstraintsTable *) mplsFrrConstEntry OBJECT-TYPE
Please add a REFERENCE clause and add the reference from the DESCRIPTION clause to the REFERENCE clause. s/speciifed/specified
"contains at a tunnel ingress."
Should be contains a tunnel ingress. *) RFC 3209 Does not appear in the References. (This reference is mentioned several times) *) mplsFrrConstIfIndex
please rename to mplsFrrConstIfIndexOrZero *) mplsFrrConstTunnelInstance
s/identication/identification
*) mplsFrrConstInclAnyAffinity
OBJECT-TYPE
Please rename mplsFrrConstrainsIncludeAnyAffinity *) mplsFrrConstInclAllAffinity OBJECT-TYPE Please rename to mplsFrrConstraintsIncludeAllAffinity
*) mplsFrrConstExcl objects please rename to mplsFrrConstrainsExclude *) mplsFrrConstBandwidth
s/reserved for detour/reserved for a detour *) MplsFrrTunARHop please use Tunnel Also, other object names use Protection or Protected so please use entire word here *) mplsFrrOne2OnePlrTable
DESCRIPTION is a little bit awkward
"...that initiated the detour LSPs that traverse this node." maybe the last "that" is not needed? *)
mplsFrrOne2OnePlrEntry
states: "...An entry in this table is only created by an SNMP agent as instructed by an MPLS signaling protocol." But there are read-create objects, so want to make sure
that these read-create objects are allowed to be changed after a row has been created? Please spell out Tunnel, Ingress, Egress, Index and Instance in
these
names. This is to match the MPLS-TE-STD-MIB. *) mplsFrrOne2OnePlrRunIngrLSRId s/identity/identify
*) mplsFrrOne2OnePlrId
Please add a REFERENCE clause *) mplsFrrOne2OnePlrAvoidNAddrType mplsFrrOne2OnePlrAvoidNAddr What does the N stand for?
*) Please add REFERENCE to mplsFrrOne2OnePlrAvoidNAddr
*) mplsFrrDetourTable
Please use Tunnel, Index, Instance
*) mplsFrrDetourActive
Please add to the DESCRIPTION what true(1) means. *) mplsFrrDetourMerging
name suggestion: mplsFrrDetourMergedStatus
SYNTAX INTEGER {
none(1),
protectedTunnel(2), detour(3) } Could the above labels be
changed to: notMerged(1),
mergedWithProtectedTunnel(2) mergedWithDetour(3) Also the description talks of setting this, but
it is a read-only. *) mplsFrrFacRouteDBTable Please expand Fac to Facility, Tun to Tunnel,
Idx to Index, Inst to Instance, Ingr to Ingress and Egr to Egress. Also, Prot is used but other places in this MIB spell out Protection, or Protected. Would also take out the word Route in these object names, so mplsFrrFacRouteDBTable becomes mplsFrrFacilityDBTable *)
mplsFrrFacRouteDBTable
DESCRIPTION s/mplsFrrDBTable/mplsFrrFacRouteDBTable (The above occurs in other objects of this table, so please check the DESCRIPTIONs.) The protecting tunnel is indicated by the second two indexes (mplsTunnelIndex and mplsTunnelInstance) and represents a valid mplsTunnelEntry. Note that the tunnel The above sentence is confusing. Are you saying that
the second two indexes in this table have the same values as mplsTunnelIndex and mplsTunnelInstance? *) mplsFrrFacRouteProtIfIdx
s/applies/apply *) mplsFrrFacRouteProtTunIdx Please add a REFERENCE clause. In general, if the DESCRIPTION specifies a reference, then there should probably be a REFERENCE clause. *) Many of the DESCRIPTIONs state: "...on the specified interface as specified in the mplsFrrFacRouteIfProtIdx." Could this be reworded as: "...on the interface specified by mplsFrrFacRouteIfProtIdx." *) mplsFrrFacDBNumProtTunOnIf Please supply the specified interface's name. *) mplsFrrRacRouteDBNumProtLspOnIf Please supply the specified interface's name. *) mplsFrrFacRouteDBProtTunStatus Which protected tunnel is denoted here? *) Same question for mplsFrrFacRouteDBProtTunResvBw
*) mplsFrrFacRouteDBProtTunResvBw
Please specify which object (or objects) from the MPLS-TE-STD-MIB are being repeated. *) mplsFrrFacProtected The name doesn't say much about this notification. Could we think of a more descriptive name, e.g. (suggestion only) mplsFrrFacilityInitialBkupTunnelInvoked? s/network loading/network load
DESCRIPTION is a little confusing.
Could you just say that the notification needs to be sent prior to forwarding data ? Last paragraph:
"Note this notification only applicable..." change to: Note this notification is only applicable *) mplsFrrFacUnProtected
The name doesn't say much about this notification. Could we think of a more descriptive name, e.g. (suggestion only) mplsFrrFacilityFinalTunnelRestored? s/network loading/network load
Last paragraph: "Note this notification only applicable..." change to: Note this notification is only applicable Conformance -------------- *) Full conformance
DESCRIPTION of the one-to-one or facilty methods state: "...and is optional for those which do not."
Why is it optional? Is it possible to support these
objects in any meaningful way? *) read-only Compliance
*) Comment is misplaced since it appears before
a scalar object -- mplsFrrConstTable *) Missing from Read-only
conformance:
mplsFrrNotifsEnabled mplsFrrNotifMaxRate mplsFrrConstSetupPrio
mplsFrrconstHoldingPrio mplsFrrConstInclAnyAffinity mplsFrrConstInclAllAffinity mplsFrrConstExclAnyAffinity mplsFrrConstBandwidth mplsFrrOne2OnePlrSenderAddrType
mplsFrrOne2OnePlrSenderAddr *) 7. Security Considerations
configuration and/or performanc statistics. Administrators not wishing to reveal this information should consider these objects sensitive/vulnerable and take precautions so they are not revealed. s/performanc/performance
*) 9. Acknowledgments
Please begin this section with:
This document is a product of the MPLS Working Group. *) REFERENCES are not in order of RFC number
*) 10.1 Normative References [RFC4090] Pan, P., Swallow, G., Atlas, A., "Fast Reroute Extensions to RSVP-TE for LSP Tunnels", RFC4090, May 2005. and A. Atlas,
s/RFC4090/RFC 4090
*) [RFC3813] Srinivasan, C., Viswanathan, A. and Nadeau,
T.,
"MPLS Label Switch Router Management Information Base ", RFC 3813, June 2004 and T. Nadeau,
"Multiprotocol Label Switching (MPLS) Label Switching Router (LSR)
Management Information Base" *) [RFC3291] Daniele, M., Haberman, B., Routhier, S., and J. Schoenwaelder, "TextualConventions for Internet Network Addresses", RFC 3291, May 2002. s/TextualConventions/Textual Conventions
*) 10.2 Informative References
*) RFC3031 should probably be Normative, it is currently
listed as Informative -- end--
_______________________________________________ mpls mailing list mpls@lists.ietf.org https://www1.ietf.org/mailman/listinfo/mpls |
|