Subject: [MIB-DOCTORS] MIB Dr. review of draft-ietf-mpls-fastreroute-mib-11.txt

From: "Joan Cucchiara" <[email-masked]>
Date: Sat, 28 Feb 2009 03:13:47 -0500

Hi Tom and Kiran,

Thanks for this latest version.  A good deal
of work went into this, so thank you for that.

The MIBs compile cleanly with smicngPRO and smiLint.

IDNITS should be rechecked as one (or more)
of the boilerplates changed recently.

General comments followed by specific comments.

Thanks,
  -Joan

General Comment
----------------

*) Should have caught this earlier, but please
request mib-2 as a sub-id for these MIBs (and not mplsStdMIB)

Please see RFC4181 "Guidelines for Authors and
Reviewers of MIB Documents" Section 4.5 for details about
the assignment.

The IANA Considerations sections will also need to be updated
to reflect this, as well as the MIB Modules.

Specific Comments:
------------------------

*) Section 4.1
NIT (typo)
s/the fast reroute method/the fast reroute methods/

*) Section 4.1.1

NIT
s/This table is MUST/This table MUST/

*) Section 4.2.1

NIT:  This is the first place that PLRs is used, please
spell out what PLR stands for and then show abbreviation.

*) Section 4.2.2

NIT:
s/This table is MUST/This table MUST/

*) Section 4.2.3

Still not sure the example is correct

Anyway, here's my interpretation using all decimal values,
BUT please check this over:

      In mplsFrrOne2OneDetourTable:

      {
        mplsFrrOne2OnePlrTunnelIndex           = 1,
        mplsFrrOne2OnePlrTunnelDetourInstance  = 6553601,

        --
        --    100 << 16 | 1 == 6553601
        --
        --   100 is  mplsFrrOne2OnePlrTunnelDetourInstance
        --   from mplsFrrOne2OnePlrTable
        --
        --   shift 16 to put this into the high order bits
        --   1  is mplsFrrOne2OnePlrTunnelIndex  (need to OR
        --      the index value into low order bits)

        -- To get all detours of a protected tunnel we could do
        -- a snmpwalk of the mplsFrrOne2OneDetourEntry
        -- where mplsFrrOne2OnePlrTunnelIndex == 1
        -- mplsFrrOne2OnePlrTunnelDetourInstance == 6553601

        -- The first value would be: mplsFrrOne2OneDetourActive.1.6553501

*) Section 4.3.1

NIT

s/Backup tunnels defined/Backup tunnels are defined/

Awkward:

"The protecting tunnel is defined to exist on the
PLR in the MPLS TE Fast reroute specification."

Also, please site a reference from the
Reference section.

6.1 MPLS-FRR-GENERAL-STD-MIB Module

*) mplsFrrGeneralSwitchover

Counters should end in "s"

Also, not sure that this name reflects what
is described, could a different name be
used, e.g. mplsFrrGeneralIngressTunnelInstances ?

*) mplsFrrGeneralTunnelARHopProtectType and
mplsFrrGeneralTunnelArHopProtectTypeInUse.

Is the reference for these objects correct?

I didn't understand why these have a SYNTAX of BITS, so
wanted to look at the Reference.

*) MPLS-FRR-ONE2ONE-STD-MIB

*)     mplsFrrOne2OneScalars       OBJECT IDENTIFIER
                            ::= { mplsFrrOne2OneObjects 1 }

Please remove the mplsFrrOne2OneScalars sub-branch
and just use mplsFrrOne2OneObjects.   This will be
more consistent with the other 2 MIB Modules.

*) mplsFrrIncomingDetourLSPs, mplsFrrOutgoingDetourLSPs,
mplsFrrOne2OneDetourOriginating, mplsFrrActiveProtectedLSPs

SYNTAX and DESCRIPTION clauses need to be updated
since these objects now refers only to one2one method.

*) mplsFrrOne2OneScalarsGroup

NIT:
s/Objects that are required geneneral/Objects that are required for general/

*) MPLS-FRR-FACILITY-STD-MIB

*) All the Scalars

Some SYNTAX and all the DESCRIPTION Clauses need to be updated
to reflect that these scalars will only be present
for facility backup method.

*) mplsFrrFacilityDBProtectingTunnelStatus

NIT:

last line in the DESCRIPTION Clause

s/as/has/

*)     mplsFrrFacilityInitialBkupTunnelInvoked

 NIT:
DESCRIPTION Clause

Please remove the last paragraph in the DESCRIPTION
clause, this notification only applies to facility backup method.

*)     mplsFrrFacilityModuleFullCompliance MODULE-COMPLIANCE

       MODULE -- this module
           MANDATORY-GROUPS {
                             mplsFrrFacilityDBGroup,
                             mplsFrrFacilityNotificationsGroup
                            }

       ::= { mplsFrrFacilityCompliances 1 }

Are you missing mplsFrrFacilityScalarGroup ?

*)     mplsFrrFacilityModuleReadOnlyCompliance MODULE-COMPLIANCE

Are you missing the
      MODULE MPLS-FRR-GENERAL-STD-MIB -- MPLS FRR Generic MIB
         MANDATORY-GROUPS {
                             mplsFrrGeneralScalarGroup,
                             mplsFrrGeneralTunnelARHopGroup,
                             mplsFrrGeneralConstraintsGroup
         }

*) 7. Security Considerations

NIT
s/This MIB module/These MIB modules/

*) Please include a mention all objects which have read-create or
read-write STATUS in this section.

*) 8. IANA Considerations

As mentioned under the "General Comments" this section
and subsections are going to need to be updated to
request a sub-id from mib-2.