Subject: MIB Dr. review of draft-ietf-ccamp-gmpls-te-mib-13.txt

From: <[email-masked]>
Date: Tue, 14 Mar 2006 10:52:20 -0500

Tom and Adrian,

Let me apologize for my tardiness in posting the
review comments.

A great deal of work went into this latest draft.  Thank you.
Compile issues are listed first, and are followed by
review comments in order of the document.

Will have the other reviews out by tomorrow.

Thanks, Joan

Compile Issues:
===============

1)  smilint and smicngPRO complained
due to REFERENCE clause for gmplsTunnelARHopTable
needing an end quote:

   gmplsTunnelARHopTable  OBJECT-TYPE
...
     REFERENCE
    "1. Extensions to RSVP for LSP Tunnels, RFC 3209.
     2. Generalized MPLS Signaling - RSVP-TE Extensions, RFC 3473.
     3. Multiprotocol Label Switching (MPLS) Traffic Engineering (TE)
        Management Information Base (MIB), RFC 3812.

^ need end quote

   ::= { gmplsTeObjects 3 }

2) smicngPRO
E: f(GMPLS-TE-STD-MIB), (1482,6) Item "gmplsTunnelUnnumIf"
is not defined as an OBJECT-GROUP or NOTIFICATION-GROUP in
module "GMPLS-TE-STD-MIB"

   MANDATORY-GROUPS {
     gmplsTunnelGroup,
     gmplsTunnelScalarGroup,
     gmplsTunnelIsIntfcGroup  <-- please add this
     gmplsTunnelUnnumIf  <--- please remove this
   }

Also, please update the read-only conformance for gmplsTunnelUnnumIf.

   OBJECT gmplsTunnelUnnumIf
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required."

3) This is listed under compile issues because it might be
an issue depending on what is really intended.
RFC4181 "Guidelines for MIB Documents",
Section 4.8. Compliance Statements,
recommends using SYNTAX clauses to denote
a subset of values for an object.
If you don't intend to have a subset for these
then please remove the SYNTAX clause.  Please review all of these
Compliance Statements with SYNTAX, only a couple are listed
here:

   OBJECT gmplsTunnelLSPEncoding
     SYNTAX IANAGmplsLSPEncodingType
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required."

   OBJECT gmplsTunnelSwitchingType
     SYNTAX IANAGmplsSwitchingType
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required."

SYNTAX clauses such as InetAddressType and InetAddress don't
specify a subset, so need to ask if you intend
to have a subset for these such as:

   SYNTAX       InetAddressType { unknown(0), ipv4(1), ipv6(2) }

   SYNTAX       InetAddress (SIZE(0|4|16))

4) Again, wrt Section 4.8 from RFC4181, it is recommended
that if MIB authors intend to have Optional Groups that these
be noted.  This was done for the Read-only compliance but not
the Full-compliance, so would ask that all Optional Groups be
specified as recommended by Section 4.8.  Reviewing these
Compliances statements is difficult without this information.

Comments:
-----------

1) Table of Contents:

   3. The SNMP Management Framework .......................... 4

Should be:
   3. The Internet-Standard Management Framework
In other words, please change the title for section 3.

2) 5.7 Use of 32-bit and 64-bit Counters

Shouldn't the quote from RFC2863 be in quotes""?

3) 6. Cross-referencing to the gmplsLabelTable

2nd paragraph which begins:
   The hop tables in this document (gmplsHopTable, gmplsCHopTable and
   gmplsARHopTable) and the segment tables in the [RFC3813]

There is no gmplsHopTable, is this supposed to be: gmplsTunnelHopTable?
Similar comment with gmplsCHopTable and gmplsARHopTable.

4) gmplsTunnelPathComp

If the value of gmplsTunnelLSPEncoding is 'tunnelLspNotGmpls'
does this object have meaning?

5) gmplsTunnelErrorTable, would expect to see these objects
contained in an MPLS group in Compliance/Conformance Statements.

6) gmplsTunnelErrorTLVs OBJECT-TYPE

When the string size is 0 this could indicate that
no TLVs are present, right?
That would be slightly more efficient than
having to look at the first octet.
If you agree, please modify the
DESCRIPTION to reflect this.

Conformace/Compliance:
---------------------
7) Please move some of the following comment to the
   gmplsTeModuleReadOnlyCompliance DESCRIPTION
   -- The mandatory group has to be implemented by all LSRs that
   -- originate, terminate or act as transit for TE-LSPs/tunnels.
   -- In addition, depending on the type of tunnels supported, other
   -- groups become mandatory as explained below.

8) Already mentioned this but was expecting to see MPLS objects
called out in a separate GROUP (e.g. objects which are valid when
gmplsTunnelLSPEncoding
has a value of 'tunnelLspNotGmpls', and other objects also).
For folks that want to support these MPLS related objects
prior to supporting GMPLS, this would be
important to understand, so the more clearly noted by the MIB the
better.

9)  NIT: In the read-only conformance there is a comment:

   -- All scalars have max access read-only

which appears in front of a bunch of objects that are
part of a table.  Please remove this comment since it
doesn't apply.

10) NIT:

--glmpsTunnelCHopTable

      ^^ typo

11) Security Section

NIT:     the network via SNMP. These are the tables and objects and their
   sensitivity/vulnerability:

   o  the gmplsTunnelTable, gmplsTunnelHopTable, gmplsTunnelARHopTable,
      gmplsTunnelCHopTable, gmplsTunnelReversePerfTable,
      gmplsTunnelErrorTable collectively show the tunnel network

Should have an "and"

   o  the gmplsTunnelTable, gmplsTunnelHopTable, gmplsTunnelARHopTable,
      gmplsTunnelCHopTable, gmplsTunnelReversePerfTable, and

^^^

      gmplsTunnelErrorTable collectively show the tunnel network

12) Normative References:
   [RFC4328]    Papadimitriou, D., Ed., "Generalized MPLS Signalling
                Extensions for G.709 Optical Transport Networks
                Control", draft-ietf-ccamp-gmpls-g709, work in progress.
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Please update.

--end of comments--

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Tue, 14 Mar 2006 12:13:13 -0500

I will update this shortly and
send you the updated version ASAP.  I will send
it to you directly, as the ID gate is closed
right now.

	Also apologies for
lack of compilation. I don't have SmicNG Pro
working here, so all I could use was
SmiLint (its a long story).

	--Tom

---

From: <[email-masked]>
Date: Wed, 15 Mar 2006 00:30:52 -0500

Hi Tom and Adrian,

Thank you for all the great updates
to this draft.  Compile issues listed
first, followed by review comments.

Thanks, 
     Joan

Compile Issues
================

GMPLS-LSR-STD-MIB
====================
1) smicngPRO
W: f(GMPLS-LSR-STD-MIB), (391,18) For 
"gmplsOutSegmentTTLDecrement", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (397,18) For 
"gmplsInSegmentExtraParamsPtr", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (404,18) For 
"gmplsOutSegmentExtraParamsPtr", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (447,14) For 
"gmplsInterfaceSignalingCaps", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (460,18) For 
"gmplsInterfaceRsvpHelloPeriod", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (474,18) For 
"gmplsInSegmentExtraParamsPtr", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (488,18) For 
"gmplsOutSegmentTTLDecrement", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (494,18) For "
gmplsOutSegmentExtraParamsPtr", syntax is identical

then please remove the SYNTAX.  Please review all of these

     OBJECT      gmplsInterfaceRsvpHelloPeriod
     SYNTAX      Unsigned32
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required."

     OBJECT      gmplsInSegmentDirection
     SYNTAX      GmplsSegmentDirectionTC
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required. Only forward(1) needs to be
        supported by implementations that only support unidirectional
        LSPs."

     OBJECT      gmplsInSegmentExtraParamsPtr
     SYNTAX      RowPointer
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required."

GMPLS-LABEL-STD-MIB
===================

1) smicngPRO and smiLint flagged line 622

E: f(GMPLS-LABEL-STD-MIB), (622,6) syntax error
E: f(GMPLS-LABEL-STD-MIB), (622,6) Syntax for OBJECT clause is
  "OBJECT" <objName> <optSyntax> <optWriteSyn> <optAccess> <desc>
E: f(GMPLS-LABEL-STD-MIB), (518,12) Group "gmplsLabelTableGroup" is 
   both a MANDATORY and conditional group for module "GMPLS-LABEL-STD-MIB"

     OBJECT       gmplsLabelRowStatus
     MIN-ACCESS   read-only
-->     SYNTAX       RowStatus { active(1) }
     DESCRIPTION
       "Support for notInService, createAndWait and notReady is not
        required."

Need to put SYNTAX clause before MIN-ACCESS clause.

2) E: f(GMPLS-LABEL-STD-MIB), (518,12) 
Group "gmplsLabelTableGroup" is both a MANDATORY and 
conditional group for module "GMPLS-LABEL-STD-MIB"

The above is flagged as an error but is actually more
pervasive than just this one error: both the
FullCompliance and the Read-only Compliance statements have
Groups listed as Mandatory but then appear as Optional.
This needs to be clarified per RFC4181 "Guidelines for MIB Documents",
Section 4.8. Compliance Statements. 

Related to this, the following groups are currently listed as
Mandatory under Full compliance, but would think
that these only apply if the device has certain
interfaces (e.g. gmplsLabelSonetSdhGroup if Sonet
interfaces are on the device), is this
an Optional Group?  What about these other groups?

       gmplsLabelPacketGroup,
       gmplsLabelPortWavelengthGroup,
       gmplsLabelFreeformGroup,
       gmplsLabelSonetSdhGroup,
       gmplsLabelWavebandGroup

3) Same comment as with the GMPLS-TE-STD-MIB wrt having
an MPLS GROUP in the compliances statements. Would be
most helpful to know what objects apply to MPLS.

Comments in order of document.
==============================
1) Table of Contents

   3. The SNMP Management Framework .................... 4
Please rename to:

    The Internet-Standard Management Framework

2) 1.1. Migration Strategy, 5th paragraph:

   the various label pointer objects in the MPLS-STD-LSR-MIB module
                                                  ^^^^
Please fix: MPLS-STD-LSR-MIB to MPLS-LSR-STD-MIB

3) Please remove "and OBJECT-IDENTIFIERS" in the last paragraph

   Textual conventions and OBJECT-IDENTIFIERS are defined in [RFC3811]
   and [GMPLSTCMIB] ...

4)    gmplsLsrModuleFullCompliance MODULE-COMPLIANCE

2 issues:
a) The following 2 objects could have an MIN-ACCESS of
  read-only since the DEFVAL is forward.

b) the DESCRIPTION is a little awkward, could this be reworded:
       "The only valid value for unidirectional LSPs is forward(1)."

This would need to be updated in the read-only section also.
(Similar comments for gmplsOutSegmentDirection).

     OBJECT      gmplsInSegmentDirection
     SYNTAX      GmplsSegmentDirectionTC
     MIN-ACCESS  read-write
     DESCRIPTION
       "Only forward(1) needs to be supported by implementations that
        only support unidirectional LSPs."

     OBJECT      gmplsOutSegmentDirection
     SYNTAX      GmplsSegmentDirectionTC
     MIN-ACCESS  read-write
     DESCRIPTION
       "Only forward(1) needs to be supported by implementations that
        only support unidirectional LSPs."

GMPLS-LABEL-STD-MIB
========================
5) gmplsLabelInterface and gmplsLabelIndex
 
I had a question regarding these 2 indices:  if there is a per-platform
label, MUST both of these values be zero?  Is there any other circumstance
in which both would or could be zero?

I was a little confused on the use of "could" and "may result" and was
wondering if this should be made a little more forceful.

6) As previously mentioned could MPLS objects be denoted
in a separate GROUP (e.g., gmplsLabelMplsLabel and possibly other
objects).

7) Please put the comment in the DESCRIPTION clause of
the gmplsLabelModuleFullCompliance.
     -- The mandatory groups have to be implemented by GMPLS LSRs
     -- claiming support for this MIB module. This MIB module is,
     -- however, not mandatory for a working implementation of a GMPLS
     -- LSR with full MIB support if the GMPLS labels in use can be
     -- represented within a 32 bit quantity.

--end of comments --

---

From: <[email-masked]>
Date: Wed, 15 Mar 2006 00:32:31 -0500

Hi Tom and Adrian,

Due to the updates to the 2 other GMPLS MIB documents
wanted to re-review this document.

I did have one minor comment which was
the naming convention used, specifically
the use of the TC on the end of the names of these
Textual Conventions.  

   GmplsFreeformLabelTC
   GmplsLabelTypeTC
   GmplsSegmentDirectionTC 

Could this "TC" be removed?  Sorry to not catch this
earlier.

The reason is that this is not consistent with
other Textual Conventions.

Thanks, 
    Joan

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Wed, 15 Mar 2006 11:16:53 -0500

> Could this "TC" be removed?  Sorry to not catch this

	The "TC" was appended to remove the ambiguity
between the variable and TC names. This is actually a
warning from SmiLint too.  Something like "Warning: object and
TC differ only in case".

> other Textual Conventions.

	I tried to make all TCs end in "TC" for
the reason stated above. I will go back and 2x check
the others and make things consistent.

	--Tom

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Tue, 21 Mar 2006 14:19:22 -0600

As per Bert's reply to this, I have made
all of the TCs in the CCAMP/GMPLS TC and LSR MIBs to have
"TC" at the end of their names to keep them consistent.

	--Tom

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Wed, 22 Mar 2006 09:36:53 -0600

On Mar 22, 2006, at 9:08 AM, <[email-masked]> wrote:

>
> Hi Tom,
>
> I think we have a disconnect somewhere...when I said
> that the use of the "TC" was not consistent, I meant with
> TCs in general (not just in GMPLS MIBs), so would prefer
> to see this removed.
>
> I looked at the use of the Textual Conventions in the
> GMPLS MIBs and saw that one still is the same as
> an Object (except in case) which was gmplsLabelType.
> (I do think there was more overlap originally but names have
> changed since then).

	There was, and I am fixing that now. Almost
done with the TE module and I will send them all over
for your review.  I updated the others yesterday.

> My suggestion would be to remove the "TC" suffix from the
> TCs and change the name of the overlapping gmplsLabelType object
> to something else (gmplsTypeOfLabel).
>
> I do realize this is a style issue, so not a show stopper, only a  
> humble
> suggestion.

	I understand your point and I respectfully declined
to change them, but agreed (based on Bert's post) to make
them consistent throughout the modules.

	--Tom

>
> Thanks,
>   Joan
>
>
>
> ----- Original Message -----
> From: Thomas D. Nadeau <[email-masked]>
> To: <[email-masked]>
> Cc: <[email-masked]>; Wijnen, Bert (Bert) <[email-masked]>;  
> Dan
> Romascanu (E-mail) <[email-masked]>; Adrian Farrel (E-mail)
> <[email-masked]>; Bill Fenner (E-mail)  
> <[email-masked]>; Alex
> Zinin (E-mail) <[email-masked]>
> Sent: Tuesday, March 21, 2006 3:19 PM
> Subject: Re: MIB Dr. review of draft-ietf-ccamp-gmpls-tc-mib-09.txt
>
>
>>

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Wed, 22 Mar 2006 10:44:47 -0600

> first, followed by review comments.

	Fixed these.

> GMPLS-LABEL-STD-MIB

	Fixed all of the above.

> most helpful to know what objects apply to MPLS.

	Can we talk about this one in more detail?
I am confused about what you are asking for specifically
here.

> Comments in order of document.

	OK, I updated the text a bit. please look at it.

> wondering if this should be made a little more forceful.

	Yes, I think we should change this to SHOULD.

> in a separate GROUP (e.g., gmplsLabelMplsLabel and possibly other

	 Can you please explain why we would do this
instead of or in addition to what we are doing now?
I am confused since we are already requiring the objects
from the MPLS RFCs in the mandatory MODULE statements.

>      -- represented within a 32 bit quantity.

	OK.

	--Tom

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Wed, 22 Mar 2006 11:28:44 -0600

> Let me apologize for my tardiness in posting the

	Let me apologize for my tardiness in
replying to you. *)

> Will have the other reviews out by tomorrow.

	Done.

>        "Write access is not required."

	done.

>    SYNTAX       InetAddress (SIZE(0|4|16))

	I found about 6 more, and fixed them.

> Compliances statements is difficult without this information.

	I don't understand. There are 5 groups, two of which
are specified as Mandatory, and the other three are explained
to be optional and why below that.

> Comments:

	Perhaps, but we were just following
what was approved in section 6 of RFC3813:

6.  Use of 32-bit and 64-bit Counters

    64-bit counters are provided in this MIB module for high speed
    interfaces where the use of 32-bit counters might be impractical.  
The
    requirements on the use of 32-bit and 64-bit counters (copied
    verbatim from [RFC2863]) are as follows.

    For interfaces that operate at 20,000,000 (20 million) bits per
    second or less, 32-bit byte and packet counters MUST be supported.
    For interfaces that operate faster than 20,000,000 bits/second, and
    slower than 650,000,000 bits/second, 32-bit packet counters MUST be
    supported and 64-bit octet counters MUST be supported.  For
    interfaces that operate at 650,000,000 bits/second or faster, 64-bit
    packet counters AND 64-bit octet counters MUST be supported.

> contained in an MPLS group in Compliance/Conformance Statements.

	Can you explain what "an MPLS group" would be
specifically?

> DESCRIPTION to reflect this.

         "The sequence of interface identifier TLVs reported with the
         error by the protocol code. The interpretation of the TLVs and
         the encoding within the protocol are described in the
         references. A value of zero in the first octet indicates  
that no
         TLVs are present."

---

From: <[email-masked]>
Date: Wed, 22 Mar 2006 10:08:08 -0500

Hi Tom,

I do realize this is a style issue, so not a show stopper, only a humble
suggestion.

Thanks,
  Joan

----- Original Message -----
From: Thomas D. Nadeau <[email-masked]>
To: <[email-masked]>
Cc: <[email-masked]>; Wijnen, Bert (Bert) <[email-masked]>; Dan
<[email-masked]>; Bill Fenner (E-mail) <[email-masked]>; Alex