[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

review of draft-ietf-ccamp-lsr-mib-08.txt




Hello,

I would like to thank Bert for his input, especially
on the MIB compliances and document reference section.

Thank you,
   -Joan



draft-ietf-ccamp-gmpls-lsr-mib-08.txt
=======================================


Section 1.1 Migration Strategy
--------------------------------

1) Does the following statement refer to the
GMPLS LSR MIB module?

  "The GMPLS Label MIB module may be referenced using a row pointer from
   objects within the LSR MIB module."
                     ^^^^
As previously mentioned, using the entire MIB module name
would be clearer.


Section  4. Outline
---------------------

2)  Need to clarify the phrase "this MIB module" because
there are 2 MIB modules in this document.


 "-  Enabling GMPLS on GMPLS capable interfaces using this MIB module."

(which is referring to the GMPLS-LSR-STD-MIB module??)

...

  "-  Optionally setting up labels in the label table in this MIB module
      if the textual convention MplsLabel is not capable of holding the
      required label (for example, if the label requires more than 32
      bits to encode it), or if the operator wishes to disambiguate
      GMPLS label types."

The phrase, "in this MIB module", needs to be clarified since there
are 2 mib modules in this document.  Here, I think you are 
referring to the GMPLS-LABEL-STD-MIB, but prior to this you use
a similar phrase and are referring the the GMPLS-LSR-STD-MIB.
Please consistently specify the MIB modules by using their
names.


3) Please move section, 4.1 MIB Modules to section 1.1, Migration
Strategy.  There is much overlap in these 2 sections so please
combine them.


Section 4.1.2  Summary of GMPLS Label MIB Module
-------------------------------------------------

4)      "Generalized Labels can be of variable length and have distinct
      bit-by-bit interpretations according to the use that is made of
      them."

Please change the above to:

      "Generalized Labels can be of variable length and have distinct
      bit-by-bit interpretations depending upon how they are defined
      by the specific service provider."


5) Same section, 4.1.2  Summary of GMPLS Label MIB Module

Please remove this last sentence:

    These tables are described in the subsequent sections.


Section 5. Bidirectional LSPs
-------------------------------


6) "This MIB module..."

Please clarify which MIB module, i.e. The GMPLS-LSR-STD-MIB module

7)  Please specifically call out which term you will be using in
this document.  Beginning with the 3rd paragraph, there is much
terminology, so please add some statement such as:

"In this document, the terms 'forward' and 'reverse' will be used.


Section 6. Example of LSP Setup
-------------------------------

8) First and second sentences:
   "In this section we provide a brief example of using the MIB objects
   described in sections 7 and 8 to set up an LSP. While this example is
   not meant to illustrate every nuance of the MIB, it is intended as an
   aid to understanding some of the key concepts..."

The phrase, "every nuance of the MIB" should probably be
"every nuance of the MIB modules"  since you are referring to both
of the MIB modules (sections 7 and 8).

9) Typo:  gmplsLableTable

10) Typo:   accesible  (in several places)


Section 7.  GMPLS-LSR-STD-MIB
======================


11) smicngpro gives the following:

E: f(GMPLS-LSR-STD-MIB), (315,8) Item "ifGeneralInformationGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (316,8) Item "ifCounterDiscontinuityGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (322,8) Item "mplsInterfaceGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (323,8) Item "mplsInSegmentGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (324,8) Item "mplsOutSegmentGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (325,8) Item "mplsXCGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (326,8) Item "mplsPerfGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (327,8) Item "mplsLsrNotificationGroup" should be IMPORTed
W: f(GMPLS-LSR-STD-MIB), (358,18) For "gmplsOutSegmentTTLDecrement", syntax is identical
E: f(GMPLS-LSR-STD-MIB), (378,8) Item "ifGeneralInformationGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (379,8) Item "ifCounterDiscontinuityGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (385,8) Item "mplsInterfaceGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (386,8) Item "mplsInSegmentGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (387,8) Item "mplsOutSegmentGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (388,8) Item "mplsXCGroup" should be IMPORTed
E: f(GMPLS-LSR-STD-MIB), (389,8) Item "mplsPerfGroup" should be IMPORTed
W: f(GMPLS-LSR-STD-MIB), (404,14) For "gmplsInterfaceSignalingCaps", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (415,18) For "gmplsInterfaceRsvpHelloPeriod", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (431,18) For "gmplsInSegmentExtraParamsPtr", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (447,18) For "gmplsOutSegmentTTLDecrement", syntax is identical
W: f(GMPLS-LSR-STD-MIB), (454,18) For "gmplsOutSegmentExtraParamsPtr", syntax is identical


Please correct the above.

Also, please change the following 2 statements (which appear twice)
to remove the SYNTAX, since this is redundant, unless there is
a reason why you prefer to leave it.

     OBJECT      gmplsInSegmentDirection 
     SYNTAX      GmplsSegmentDirection
     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      gmplsOutSegmentDirection
     SYNTAX      GmplsSegmentDirection
     MIN-ACCESS  read-only
     DESCRIPTION
       "Write access is not required. Only forward(1) needs to be
        supported by implementations that only support unidirectional
        LSPs."

Compiles cleanly with smilint.


12) DESCRIPTION
   Please change the first paragraph to (this comment applies to
   the other MIB modules also.)

             Copyright (C) The Internet Society (2005).  This version of
             this MIB module is part of RFC XXX; see the RFC itself for
             full legal notices.


13) Change the following:

     GmplsSegmentDirection
       FROM GMPLS-TC-STD-MIB                             -- GMPLSTCMIB
                      -- RFC-Editor please resolve the reference above

TO:

     GmplsSegmentDirection
       FROM GMPLS-TC-STD-MIB                             -- RFCXXXX
                      -- RFC-Editor please resolve the reference above
                      -- and remove this note.

14) Please remove:

   -- Notifications
   -- no notifications are currently defined.
   gmplsLsrNotifications OBJECT IDENTIFIER ::= { gmplsLsrStdMIB 0 }

(You don't need to change any numbering, but no sense defining
a number you don't need).

15) Please remove comments such as (this request is made
     for all MIB modules in all 3 documents):
   -- Top level components of this MIB module.
   -- Tables, Scalars
   -- Conformance
   -- GMPLS Interface Table.

   -- End of gmplsInterfaceTable

   -- In-segment table.

etc.


16)   gmplsInterfaceEntry OBJECT-TYPE

       "A conceptual row in this table is created automatically by an
        LSR for every interface capable of supporting GMPLS and which
        is configured to do so."

Please change first sentence:

       "A conceptual row in this table is created automatically by an
        LSR when the interface is capable of supporting GMPLS and is
        correctly configured to do so."

        ....

Please change the first sentence of the last paragraph:

       "The indexing is the same as that for mplsInterfaceTable."

to:

       "The indexing is the same as the mplsInterfaceTable."

17) object:    gmplsInterfaceSignalingCaps OBJECT-TYPE

     SYNTAX  BITS {
       unknown (0),
       rsvpGmpls (1),
       crldpGmpls (2), -- note the use of CR-LDP is deprecated
       otherGmpls (3)
     }


What is otherGmpls(3)??

Also, this description is confusing.  How can unknown(0) and
rsvpGmpls(1) be set simultaneously?

Additionally, how can no bits be set?  If this is a GMPLS interface,
then doesn't one of these bits HAVE to be set?

18)   gmplsInterfaceRsvpHelloPeriod OBJECT-TYPE
       "Period, in milliseconds, between sending RSVP Hello messages on
        this interface. A value of 0 indicates that no Hello messages
        should be sent on this interface."

This object is only valid if the gmplsInterfaceSignalingCaps
object is set ot rsvpGmpls.  Please update the description
to reflect that.


19)    gmplsInSegmentEntry  OBJECT-TYPE
       "An entry in this table extends the representation of an incoming
        segment represented by an entry in mplsInSegmentTable. An entry
        can be created by a network administrator or an SNMP agent, or a
        GMPLS signaling protocol.

Please use the phrase:  "via SNMP SET commands to create an entry"
in place of "SNMP agent".

20) same entry, gmplsInSegmentEntry

        Note that the storage type for this entry SHOULD be inherited
        from the corresponding entry in the mplsInSegmentTable given by
        the value of the mplsInSegmentStorageType object."

Please change to:

Note that the storage type for this entry is given by the value
of mplsInSegmentStorageType in the corresponding entry of the
mplsInSegmentTable.


21)    gmplsInSegmentDirection OBJECT-TYPE

Please use "corresponding" instead of "associated"


22)    gmplsInSegmentExtraParamsPtr  OBJECT-TYPE
     SYNTAX       RowPointer
     MAX-ACCESS   read-create
     STATUS       current
     DESCRIPTION
       "Some Tunnels will run over transports that can usefully support
        technology-specific additional parameters (for example, SONET
        resource usage). Such can be supplied from an external table and
        referenced from here. A value of zeroDotzero in this attribute
        indicates that there is no such additional information."
     DEFVAL      { zeroDotZero }
     ::= { gmplsInSegmentEntry 2 }

What is meant by "external table"?  If this is an enterprise-specific
table, then is it necessary to have this object in the MIB.  In other
words, why is this MIB dictating that a row-pointer be used when
an enterprise-specific MIB may which to simply use indices.

I think this object is unnecessary, but maybe I don't understand
something.

23)   gmplsOutSegmentEntry  OBJECT-TYPE
Same comments as with gmplsInSegmentEntry

24)    gmplsOutSegmentDirection OBJECT-TYPE
Same comment as with gmplsInSegmentDirection

25)    gmplsOutSegmentTTLDecrement OBJECT-TYPE

       "This object indicates the amount by which to decrement the TTL
        of any payload packets forwarded on this segment if per-hop
        decrementing is being done.

Please change above to:

This object indicates the amount to decrement the TTL...



26)    same object's gmplsOutSegmentTTLDecrement OBJECT-TYPE

        A value of zero indicates that no decrement should be made or
        that per-hop decrementing is not in force.

Please change:  in force to enforced.

27)    gmplsOutSegmentExtraParamsPtr  OBJECT-TYPE

Same comment as with gmplsInSegmentExtraParamsPtr


28) Compliance Statements

I believe that gmplsInSegmentExtraParamsPtr and
gmplsOutSegmentExtraParamsPtr do not need to be
supported, so these maybe should be optional.


Section 8.  GMPLS-LABEL-STD-MIB
===================================
smicngPRO gives the following:

E: f(GMPLS-LABEL-STD-MIB), (21,13) Module "MPLS-TC-STD-MIB" has already been specified in IMPORTS clause


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


Compiles cleanly with smilint.

Also, disconnect in the numbering of the
gmplsLabelTable:

   |         \-v-1 gmplsLabelInterface, name "InterfaceIndexOrZero", na, current
   |           |-2 gmplsLabelIndex, Unsigned32(0..4294967295), na, current
   |           |-3 gmplsLabelSubindex, Unsigned32(0..4294967295), na, current
   |           |-4 gmplsLabelType, name "GmplsGeneralizedLabelTypes", rc, current

--->  5 is missing


   |           |-6 gmplsLabelMplsLabel, name "MplsLabel", rc, current
   |           |-7 gmplsLabelPortWavelength, Unsigned32, rc, current
   |           |-8 gmplsLabelFreeform, name "GmplsFreeformLabel", rc, current
   |           |-9 gmplsLabelSonetSdhSignalIndex, Integer32(0..4095), rc, current
   |           |-10 gmplsLabelSdhVc, Integer32(0..15), rc, current
   |           |-11 gmplsLabelSdhVcBranch, Integer32(0..15), rc, current
   |           |-12 gmplsLabelSonetSdhBranch, Integer32(0..15), rc, current
   |           |-13 gmplsLabelSonetSdhGroupBranch, Integer32(0..15), rc, current
   |           |-14 gmplsLabelWavebandId, Unsigned32, rc, current
   |           |-15 gmplsLabelWavebandStart, Unsigned32, rc, current
   |           |-16 gmplsLabelWavebandEnd, Unsigned32, rc, current
   |           |-17 gmplsLabelRowStatus, name "RowStatus", rc, current
   |           \-18 gmplsLabelStorageType, name "StorageType", rc, current



29)  DESCRIPTION
   Please change the first paragraph to (this comment applies to
   the other MIB modules also.)

             Copyright (C) The Internet Society (2005).  This version of
             this MIB module is part of RFC XXX; see the RFC itself for
             full legal notices.

30) Please remove these statements (don't need to change any numbers).

   -- Notifications
   -- no notifications are currently defined.
   gmplsLabelNotifications  OBJECT IDENTIFIER ::= { gmplsLabelStdMIB 0 }


31)   gmplsLabelTable OBJECT-TYPE


The last part of the last paragraph is awkward:

        "... Labels in the tables in other MIBs are referred to using
        row pointer into this table. The indexing of this table provides
        for arbitrary indexing and also for concatenation of labels."

Are you saying that other MIBs can use a row pointer to point to 
this table?  Why would they do that if they could just use the
indices directly?   Also, I don't follow the comment about
concatenation of labels.  Could you explain this?

32)    gmplsLabelEntry OBJECT-TYPE

The last sentence of the DESCRIPTION, please remove
the word "more" from the phrase "is more persistent."

I still don't understand how the subindex provides a way
to concatenate the labels.  Could you give an example?


33) please switch RowStatus and StorageType (this will
more closely match the other MPLS MIBs).


34)    gmplsLabelInterface OBJECT-TYPE

DESCRIPTION also needs to specify that zero may be 
used to indicate that the InterfaceIndex is not known
or that an InterfaceIndex exists but it is not important
wrt this implementation so zero is used.

35)   gmplsLabelIndex OBJECT-TYPE
     SYNTAX        Unsigned32 (0..4294967295)

36a) I was expecting the SYNTAX to be IndexInteger due
to the gmplsLabelIndexNext in the DESCRIPTION.
Please explain why the use of zero is needed.  You
explain the use of zero in the gmplsLabelInterface
and gmplsLabelSubindex indices.


36b)Please change the "may" to "should" in this statement:
        A management application may read the gmplsLabelIndexNext
        object to find a suitable value for this object."


37)    gmplsLabelType OBJECT-TYPE

        of gmplsMplsLabel (1) denotes that a 23 bit MPLS packet label is
        present, but does not describe whether this is signaled using
        MPLS or GMPLS.

Is this 23 bit value correct?  Is this a FrameRelay DLCI label?


38)   gmplsLabelRowStatus OBJECT-TYPE
MUST explain which columns must be properly configured
before the row can be made active (as per RFC2579)


39) Please clarify in the DESCRIPTION of each of
the following groups, if the group is optional 
or needs to be supported by such implementations:
     GROUP gmplsLabelPacketGroup
     DESCRIPTION
       "This group extends gmplsLabelTableGroup for implementations that
        support packet labels."

     GROUP gmplsLabelPortWavelengthGroup
     DESCRIPTION
       "This group extends gmplsLabelTableGroup for implementations that
        support port and wavelength labels."

     GROUP gmplsLabelFreeformGroup
     DESCRIPTION
       "This group extends gmplsLabelTableGroup for implementations that
        support freeform labels."

     GROUP gmplsLabelSonetSdhGroup
     DESCRIPTION
       "This group extends gmplsLabelTableGroup for implementations that
        support SONET or SDH labels."

     GROUP gmplsLabelWavebandGroup
     DESCRIPTION
       "This group extends gmplsLabelTableGroup for implementations that
        support Waveband labels."


40)   gmplsLabelModuleROCompliance MODULE-COMPLIANCE

Please change the name to: gmplsLabelModuleReadOnlyCompliance
to be consistent with the other MPLS and GMPLS MIB modules.

41) The ReadOnly compliance for gmplsLabelRowStatus
does NOT have a MIN-ACCESS of read-only.  Is this 
intentional?

42) Also, (related to the above)
     OBJECT       gmplsLabelRowStatus
     SYNTAX       RowStatus {
       active(1),
       notInService(2)
     }


     WRITE-SYNTAX RowStatus {
       active(1),
       notInService(2),
       createAndGo(4),
       destroy(6)
     }
     DESCRIPTION
       "Support for notInService, createAndWait and notReady is not
        required."

The DESCRIPTION clause conflicts with the WRITE-SYNTAX.
Please clarify the intentions for this ReadOnly compliance
object.



Section 12.2 Informational References
---------------------------------------

43a) The title of this section should be
Informative.


43b) Please move the following from 
Informative to Normative.

   [RFC3471]         Berger, L. (Editor), "Generalized Multi-Protocol
                     Label Switching (GMPLS) Signaling Functional
                     Description", RFC 3471, January 2003.

   [RFC3472]         Ashwood-Smith, P., Berger, L. (Editors),
                     "Generalized MPLS Signaling - CR-LDP Extensions",
                     RFC 3472, January 2003.

   [RFC3473]         Berger, L. (Editor), "Generalized MPLS Signaling -
                     RSVP-TE Extensions", RFC 3473 January 2003.

   [GMPLSSonetSDH]   Mannie, E., Papadimitriou, D. (Editors),
                     "Generalized Multi-Protocol Label Switching
                     Extensions for SONET and SDH Control",
                     draft-ietf-ccamp-gmpls-sonet-sdh, work in progress.

43c)  The above reference now appears to be RFC 3946.

Please change the above (NOTE also the changes in the
reference itself, more on the Reference format next):

   [GMPLS-SONET]   Mannie, E. and D. Papadimitriou, "Generalized Multi-
                   Protocol Label Switching (GMPLS) Extensions for
                   Synchronous Optical Network (SONET) and Synchronous
                   Digital Hierarchy (SDH) Control", RFC 3946, October
                   2004.



44) There are a number of references (as discovered by
Bert) that aren't in the RFC expected format.
Please review all the references in this document and
the other GMPLS MIB docs.


Here are some to change. One suggestion is to copy a reference
(if possible) from an existing RFC.  Sometimes a date is missing
and sometimes the authors/editors names do not appear as
expected.  Also, titles need to appear exactly as on
the RFC.  Please check the other GMPLS docs also.
(NOTE: see also draft-rfc-editor-rfc2223bis-08.txt)


Please change:
   [RFC2026]         S. Bradner, "The Internet Standards Process --
                     Revision 3", RFC 2026, October 1996.

to:

   [RFC2026]        Bradner, S., "The Internet Standards Process --
                    Revision 3," BCP 9, RFC 2026, October 1996.

Please change:

   [RFC2863]         McCloghrie, K. and F. Kastenholtz, "The Interfaces
                     Group MIB", RFC 2863, June 2000.

to (author name misspelled):

   [RFC2863]   McCloghrie, K. and F. Kastenholz,  "The Interfaces Group
               MIB", RFC 2863, June 2000.

Please change:

   [RFC3032]         Rosen, E. et al, "MPLS Label Stack Encoding",
                     RFC 3032, January 2001.

to:

   [RFC3032]   Rosen, E., Tappan, D., Fedorkow, G., Rekhter, Y.,
               Farinacci, D., Li, T., and A. Conta, "MPLS Label Stack
               Encoding", RFC 3032, January 2001.


Please change:

   [RFC3212]         Jamoussi, B., Aboul-Magd, O., Andersson, L.,
                     Ashwood-Smith, P., Hellstrand, F., Sundell, K.,
                     Callon, R., Dantu, R., Wu, L., Doolan, P., Worster,
                     T., Feldman, N., Fredette, A., Girish, M., Gray,
                     E., Halpern, J., Heinanen, J., Kilty, T., Malis,
                     A., and P. Vaananen, "Constraint-Based LSP Setup
                     using LDP", RFC 3212, December 2001."

to:

   [RFC3212] Jamoussi, B., Ed., Andersson, L., Callon, R., Dantu, R.,
             Wu, L., Doolan, P., Worster, T., Feldman, N., Fredette, A.,
             Girish, M., Gray, E., Heinanen, J., Kilty, T., and A.
             Malis,  "Constraint-Based LSP Setup using LDP", RFC 3212,
             January 2002.


Please change:

   [RFC3411]         Harrington, D., Presuhn, R., and B. Wijnen, "An
                     Architecture for Describing Simple Network
                     Management Protocol (SNMP) Management Frameworks",
                     RFC 3411, December 2002.

to:
   [RFC3411]             Harrington, D., Presuhn, R., and B. Wijnen, "An
                         Architecture for Describing Simple Network
                         Management Protocol (SNMP) Management
                         Frameworks", STD 62, RFC 3411, December 2002.

Please change:

   [RFC3413]         Levi, D., Meyer, P., Stewart, B., "SNMP
                     Applications", RFC 3413, December 2002.
to:
   [RFC3413]   Levi, D., Meyers, P. and B. Stewart, "Simple Network
               Management Protocol (SNMP) Applications", STD 62, RFC
               3413, December 2002.


Please change:
   [RFC3443]         Agarwal, P. and Akyol, B., "Time To Live (TTL)
                     Processing in Multi-Protocol Label Switching
                     (MPLS) Networks", RFC 3443, January 2003.

to:
   [RFC3443]         Agarwal, P. and B. Akyol, "Time To Live (TTL)
                     Processing in Multi-Protocol Label Switching
                     (MPLS) Networks", RFC 3443, January 2003.


Please change:
   [RFC3471]         Berger, L. (Editor), "Generalized Multi-Protocol
                     Label Switching (GMPLS) Signaling Functional
                     Description", RFC 3471, January 2003.


to:
   [RFC3471]        Berger, L., Editor, "Generalized Multi-Protocol
                    Label Switching (GMPLS) Signaling Functional
                    Description", RFC 3471, January 2003.


Please change:

   [RFC3472]         Ashwood-Smith, P., Berger, L. (Editors),
                     "Generalized MPLS Signaling - CR-LDP Extensions",
                     RFC 3472, January 2003.

to:
   [RFC3472]         Ashwood-Smith, P. and L. Berger, Editors, 
                     "Generalized Multi-Protocol Label 
                     Switching (GMPLS) Signaling Constraint-based 
                     Routed Label Distribution Protocol (CR-LDP) 
                     Extensions", RFC 3472, January 2003.

Please change:

   [RFC3473]         Berger, L. (Editor), "Generalized MPLS Signaling -
                     RSVP-TE Extensions", RFC 3473 January 2003.

to:
   [RFC3473]        Berger, L., Editor, "Generalized Multi-Protocol
                    Label Switching (GMPLS) Signaling - Resource
                    ReserVation Protocol-Traffic Engineering (RSVP-TE)
                    Extensions", RFC 3473, January 2003.



--- the end ----