[mpls] MIB DR review of draft-ietf-mpls-fastreroute-mib-08.txt

MIB Dr review of  draft-ietf-mpls-fastreroute-mib-08.txt

Tom, et al.

Following are the review comments from the previous version
of the MIB.  There are a few overall comments followed by
comments in order of the document. 

I have indicated that the comment has been
"DONE", "NOT DONE" or if comment is
a new comment "NEW".

Thanks,
  Joan

Overall COMMENTs
---------------

*) MIB compiles cleanly with smiLint and
smicngPRO. (applies to version 8)

*) 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.

NOT DONE. (Think we need to get this comment addressed
since many of the other comments marked NOT DONE are
related to this.)

*) 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.

DONE but not completely.  This needs to be checked throughout the
document since it is not updated consistenly, e.g.

* Several places in the document use: mplsFrrTunARHopTable
and this table was renamed mplsFrrTunnelARHopTable so 
please make this correction throughout entire document.

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.
DONE.

*) Date on the page headers do not match
date of document.
DONE. 

*) TOC
References is off by a space.

Also, the subsections 1.1, 4.1, 4.2
(maybe others) are missing from the TOC.

DONE, BUT some sections have incorrect page numbers.
Also, some spacing of off.

*) 1. Introduction

Would add RFC3811 also, i.e.
used in conjunction with [RFC3811], [RFC3812] and
[RFC3813].
DONE. 

*) NEW:  
following needs updating
s/mpls@uu.net/mpls@ietf.org/

*) 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.

DONE.

*) 4. Brief Description of the MIB Module Objects.
s/Objects./Objects
Would retitle this section:
Overview of the MIB Module

DONE. 

*)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? 

   " The specification [RFC4090] stipulates two different approaches 
    to implementing MPLS TE fast reroute: bypass and facility
    backup.  As such, the MIB module defined in this document"

s/bypass/one-to-one backup/

NOT DONE (bypass refers to a type of tunnel, not a backup method). 

"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."

DONE.

*) 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?

NOT DONE. Part of my confusion is the phrase "common practice has 
shown that LSRs do not implement both..."  Is the issue that
LSRs do not support both, or that operators choose to configure
only one method on an LSR?  In other words, could you be specific
about what the common practice is?  In my mind, this is important
to your justification of why the MIB is only going to allow one
kind of backup method on an LSR.  

*) 4.1 mplsFrrConstTable
Please use the entire name Constraints, as in
mplsFrrConstraintsTable.

DONE, BUT TOC needs to be updated with new tabel name.

*) 4.2 mplsFrrTunARHopTable
Please use mplsFrrTunnelARHopTable to match
with mplsTunnelARHopTable.

First sentence:
s/mplsTunnelARHop table/mplsTunnelARHopTable

DONE, BUT TOC and other places need to be updated with 
new table name.

*) 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?

DONE.

4.4 mplsFrrDetourTable

*) Could this be renamed to mplsFrrOne2OneDetourTable?

DONE, BUT TOC needs to be updated.

*) NEW (NIT)

   "In the mplsTunnelTable, the higher 16 bits of the tunnel instance 
    SHOULD be used as detour instance. Note that for the protected..."

s/be used as/be uses as a/

*) 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.

DONE.

*) 4.5 mplsFrrFacilityDBTable

s/mplsFrrDBTable/mplsFrrFacRouteDBTable

Could this be renamed to mplsFrrFacilty

DONE.

*) Could you state that this table is mandatory when the
facility backup method is used?

DONE.

*) 5. Handling IPv6 Tunnels

draft-ietf-ccamp-gmpls-addressing-03 does not appear
as a reference, please add to Normative References.

DONE. 

*) 6  (MIB MODULE)

*) LAST-UPDATED and REVISION is incorrect

DONE. 

*) NEW.  Please remove the REVISION/DESCRIPTION clauses
except for the latest version of the MIB.  The REVISION
clause is typically used for RFC revisions (not draft
revisions). 

*) 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.

NOT DONE.

*) Please change to yyy
::= { mplsStdMIB yyy } -- RFC-editor please fill in
                          -- yyy with value assigned by IANA,
                          -- see section 18.1 for details

DONE.

*) Please change
mplsFrrNotif to mplsFrrNotifications

DONE.

*) 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 }

NOT DONE.

*) Also, please move mplsFrrConformance under mplsFrrObjects.

NOT DONE.

*) 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?

DONE (sort of).  The scalars which apply to one-to-one backup
or facility backup should be co-located with their respective
tables.  This is just a cut-n-paste and would go a long way
to make the MIB Module more readable.

*) NEW the mplsFrrFacObjects name should be changed to
mplsFrrFacilityObjects.  The tables use the entire word
Facility so this would be for consistency.

*) 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)

DONE.

*) Last sentence
s/notified/generated/

DONE.

*) NEW for mplsFrrProtectedMethod
Please include in the DESCRIPTION clause what
error should be returned if the value of unknown(1) is
set.

*) NEW, most of the DESCRIPTION clauses in the other
objects which refer to this object (mplsFrrProtectedMethod)
need to be updated (this means most of the scalars).

*) mplsFrrSwitchover
Should this be a Gauge32?

DONE.

*) mplsFrrNotifsEnabled.
Why is this object needed?

NOT DONE.  Please answer the question wrt
RFC3413. 

*) mplsFrrNotificationsMaxRate
Are there other objects which indicate
if events are being thrown away due to this
throttling?  (Would that be useful?)

NOT DONE.  Please answer the question.  I would
think such an object (or objects) would be useful
information if throttling is done.

*) mplsFrrDetourIncoming
Should be put with the oneToOne objects.
Should this be a Counter32 or Gauge32?

NOT DONE.

*)Please rename to mplsFrrIncomingDetourLSPs
DONE.

*) mplsFrrDetourOutgoing
Should be put with the oneToOne objects.
Should this be a Counter32 or Gauge32?

NOT DONE.

*) please rename to mplsFrrOutgoingDetourLSPs
DONE.

*) Could mplsFrrConfIfs be
renamed to:
mplsFrrConfiguredInterfaces

DONE.

*)Also, this should apply only to
facilityBackup, so could be
a Gauge32 or Counter32?

NOT DONE.

*) mplsFrrActProtectedIfs
What does the Act stand for?
Is this Active or Actual?
Could this be renamed to
mplsFrrActiveInterfaces/mplsFrrActualInterfaces

DONE.

*) mplsFrrConfProtectionsTuns
Could this be renamed
mplsFrrConfiguredBypassTunnels ?

DONE.

*) NEW could DESCRIPTION say the value must be 0
and should be ignored.

*) mplsFrrActProtectionTuns
Could this be renamed to
mplsFrrActiveBypassTunnels?

DONE.

*) NEW DESCRIPTION should say, the value must be 0 and should
be ignored.

*) mplsFrrActProtectedLSPs
could this be renamed to:
mplsFrrActiveProtectedLSPs?
DONE.

*) NEW DESCRIPTION should say, the value must be 0 and should
be ignored.

*) mplsFrrConstTable
Please rename to mplsFrrConstraintsTable

DONE.

*)     mplsFrrConstEntry OBJECT-TYPE
Please add a REFERENCE clause and add the
reference from the DESCRIPTION clause to the
REFERENCE clause.

DONE.

s/speciifed/specified

DONE.

"contains at a tunnel ingress."
Should be contains a tunnel ingress.

DONE.

*) RFC 3209 Does not appear in the References.
   (This reference is mentioned several times)

DONE.

*) mplsFrrConstIfIndex
please rename to mplsFrrConstIfIndexOrZero
DONE. 

*) mplsFrrConstTunnelInstance

s/identication/identification

DONE.

*) NEW mplsFrrConstraintHopLimit
According to RFC4090 this looks to be a byte, so
why is there not a range on this?  

*) mplsFrrConstBandwidth
s/reserved for detour/reserved for a detour

DONE.

*) MplsFrrTunARHop
please use Tunnel
Also, other object names use Protection or Protected
so please use entire word here

DONE.

*) mplsFrrOne2OnePlrTable

DESCRIPTION is a little bit awkward
"...that initiated the detour LSPs that traverse this node."
maybe the last "that" is not needed?

DONE.

*) 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?

DONE.

Please spell out Tunnel, Ingress, Egress, Index and Instance in these
names.  This is to match the MPLS-TE-STD-MIB.

MOST DONE.  Inst still being used.

*) mplsFrrOne2OnePlrRunIngrLSRId

s/identity/identify

*) mplsFrrOne2OnePlrId
Please add a REFERENCE clause

DONE.

*) mplsFrrOne2OnePlrAvoidNAddrType
mplsFrrOne2OnePlrAvoidNAddr

What does the N stand for?

DONE.

*) Please add REFERENCE to mplsFrrOne2OnePlrAvoidNAddr

DONE.

*) mplsFrrDetourTable

Please use Tunnel, Index, Instance

DONE.

*) mplsFrrDetourActive
Please add to the DESCRIPTION what true(1) means.

DONE.

*) 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)

DONE.

*)Also the description talks of setting this, but
it is a read-only.

Please just get rid of the phrase "set to" 

NOT DONE.

*) NEW, mplsFrrOne2OneDetourMergedDetourInst

Same comment as above.  Please just the the value is, as
the phrase "set to" is confusing with read-only objects.

*) 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.

DONE.

Would also take out the word Route in these
object names, so mplsFrrFacRouteDBTable becomes
mplsFrrFacilityDBTable

DONE.

*) mplsFrrFacRouteDBTable
DESCRIPTION
s/mplsFrrDBTable/mplsFrrFacRouteDBTable
(The above occurs in other objects of this
table, so please check the DESCRIPTIONs.)

DONE.

             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?

DONE.

*) mplsFrrFacRouteProtIfIdx
s/applies/apply

DONE. 

*) mplsFrrFacRouteProtTunIdx
Please add a REFERENCE clause.  In general, if the
DESCRIPTION specifies a reference, then there should
probably be a REFERENCE clause.

DONE.

*) 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."

DONE.

*) mplsFrrFacDBNumProtTunOnIf
Please supply the specified interface's name.

DONE.

*) NEW mplsFrrFacilityDBNumProtectingTunnelOnIf
s/speficied/specified/

*) mplsFrrRacRouteDBNumProtLspOnIf
Please supply the specified interface's name.

DONE. 

*) mplsFrrFacRouteDBProtTunStatus
Which protected tunnel is denoted here?

DONE.

*) Same question for mplsFrrFacRouteDBProtTunResvBw

DONE.

*) mplsFrrFacRouteDBProtTunResvBw
Please specify which object (or objects) from
the MPLS-TE-STD-MIB are being repeated.

DONE.

*) mplsFrrFacProtected
The name doesn't say much about this notification.
Could we think of a more descriptive name, e.g.
(suggestion only) mplsFrrFacilityInitialBkupTunnelInvoked?

DONE.

s/network loading/network load

DONE.

DESCRIPTION is a little confusing.
Could you just say that the notification needs to
be sent prior to forwarding data ?

DONE.

Last paragraph:
"Note this notification only applicable..."
change to:
Note this notification is only applicable

DONE. 

*) mplsFrrFacUnProtected
The name doesn't say much about this notification.
Could we think of a more descriptive name, e.g.
(suggestion only) mplsFrrFacilityFinalTunnelRestored?

DONE, okay suggestion only.

s/network loading/network load

DONE.

Last paragraph:
"Note this notification only applicable..."
change to:
Note this notification is only applicable

DONE.

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? 

NOT DONE.

read-only Compliance
----------------------- 
*) Comment is misplaced since it appears before
a scalar object
       -- mplsFrrConstTable

DONE.

*)  Missing from Read-only conformance:
mplsFrrNotifsEnabled
mplsFrrNotifMaxRate

mplsFrrConstSetupPrio
mplsFrrconstHoldingPrio
mplsFrrConstInclAnyAffinity
mplsFrrConstInclAllAffinity
mplsFrrConstExclAnyAffinity
mplsFrrConstBandwidth

mplsFrrOne2OnePlrSenderAddrType
mplsFrrOne2OnePlrSenderAddr

DONE.

*) 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

DONE.

*) 9. Acknowledgments

Please begin this section with:
This document is a product of the MPLS Working Group.

NOT DONE.

*) REFERENCES are not in order of RFC number

NOT DONE.

*) 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
DONE.

*)    [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"

NOT DONE.

*)    [RFC3291]  Daniele, M., Haberman, B., Routhier, S., and J.
              Schoenwaelder, "TextualConventions for Internet Network
              Addresses", RFC 3291, May 2002.

s/TextualConventions/Textual Conventions

DONE (N/A).

*) 10.2 Informative References

*) RFC3031 should probably be Normative, it is currently
listed as Informative

DONE.
-- end--

_______________________________________________
mpls mailing list
mpls@ietf.org
http://www.ietf.org/mailman/listinfo/mpls