[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
MIB Dr. review of draft-ietf-ccamp-gmpls-te-mib-09.txt
Hi Tom and Adrian,
Here are some comments on draft-ietf-ccamp-gmpls-te-mib-09.txt.
All 3 docs show a good deal of work.
-Joan
draft-ietf-ccamp-gmpls-te-mib-09.txt
======================================
Section 1.1 Migration Strategy
------------------------------
1) Please remove "and OBJECT-IDENTIFIERS"
Textual conventions and OBJECT-IDENTIFIERS are defined in [RFC3811]
and [GMPLSTCMIB].
Section 5.4 gmplsTunnelCHoptable
---------------------------------
2) Please capitalize Table in the Section's title.
Section 8. GMPLS Traffic Engineering MIB Module
-----------------------------------------------
smicngPRO gives the following:
W: f(GMPLS-TE-STD-MIB), (1181,14) Item "gmplsTunnelErrorTLVs" should have
SIZE specified
W: f(GMPLS-TE-STD-MIB), (1362,13) For "gmplsTunnelDirection", syntax is
identical
W: f(GMPLS-TE-STD-MIB), (1371,13) For "gmplsTunnelPathComp", syntax is
identical
W: f(GMPLS-TE-STD-MIB), (1382,14) For "gmplsTunnelUpNotRecip", syntax is
identical
W: f(GMPLS-TE-STD-MIB), (1389,14) For "gmplsTunnelDownNotRecip", syntax is
identical
W: f(GMPLS-TE-STD-MIB), (1401,14) For "gmplsTunnelExtraParamsPtr", syntax
is identical
Disconnect of numbering in the Conformance Section
\-3 gmplsTeConformance
\-v-1 gmplsTeGroups
| \-v-1 gmplsTunnelGroup
2 is missing
| |-3 gmplsTunnelSignaledGroup
| |-4 gmplsTunnelScalarGroup
| |-5 gmplsTunnelIsIntfcGroup
| |-6 gmplsTunnelIsNotIntfcGroup
| |-7 gmplsTunnelOptionalGroup
| \-8 gmplsTeNotificationGroup
3) gmplsTunnelsConfigured OBJECT-TYPE
SYNTAX Unsigned32
SYNTAX should be Gauge32
4) gmplsTunnelsActive OBJECT-TYPE
SYNTAX Unsigned32
SYNTAX should be Gauge32
5) gmplsTunnelTable
DESCRIPTION
"The row status of an entry in this table is controlled by
mplsTunnelRowStatus in the corresponding entry in
mplsTunnelTable. That is, it is not permitted to create a row in
this table, nor to modify an existing row, when the
corresponding mplsTunnelRowStatus has value active(1)....
The sentence beginning with "That is," is awkward. Could you say
something like: A row in this table is created when a row is created
in the mplsTunnelTable. Entries in this table cannot be modified
when the corresponding mplsTunnelRowStatus has a value of active(1).
The exception....
Please also update this other places in this MIB (e.g gmplsTunnelHopTable).
6) gmplsTunnelEntry
DESCRIPTION:
"...
An entry can be created by a network administrator or by an SNMP
agent as instructed by a signaling protocol."
Please change the above to:
"An entry can be created by a network administrator via SNMP SET commands, or
by the network management entity as instructed by a signaling
protocol."
7) gmplsTunnelUnnumIf
What IF-MIB object are referring to in the DESCRIPTION?
Could you state which object in the description?
8) gmplsTunnelAttributes
Could you specify what happens when the bit is set (i.e. 1)
vs. 0?
9) gmplsTunnelLSPEncoding
The DESCRIPTION states:
...A value of 'tunnelLspNotGmpls' indicates that GMPLS signaling is
not in use. Some objects in this MIB module may be of use for
MPLS signaling extensions that do not use GMPLS signaling. By
setting this object to 'tunnelLspNotGmpls', an application may
indicate that only those objects meaningful in MPLS should be
examined....
I believe that such objects should be put in to an MPLS MIB module
and not included in this GMPLS MIB module. This is also true with
the gmplsTunnelErrorTable.
10) gmplsTunnelLinkProtection
Same comment as with gmplsTunnelAttributes.
11) gmplsTunnelPathComp
DESCRIPTION states:
This object deprecates mplsTunnelHopEntryPathComp."
The term "deprecates" is loaded. Perhaps something like,
this object takes precedence over mplsTunnelHopEntryPathComp.
In other words, for GMPLS tunnels, the value of
mplsTunnelHopEntryPathComp is meaningless.
12) gmplsTunnelUpNotRecip
Need to use InetAddressType and InetAddress. IpAddress
is not used in MIBs anymore. Please be sure to support
both v4 and v6 IP address Types. If you can't support
v6 for some reason, that should be stated clearly.
Please change the name of this object to:
gmplsTunnelUpstreamRecipientForNotify
13) gmplsTunnelDownNotRecip
Same comment as above wrt to IpAddress.
Please change the name to gmplsTunnelDownstreamRecipientForNotify
14) gmplsTunnelAdminStatusFlags (please see comments
regarding the name of this TC and make adjustments
to the name and DESCRIPTION of this object as appropriate).
15) gmplsTunnelExtraParamsPtr OBJECT-TYPE
This object does not seem appropriate for this MIB.
It would seem an enterprise MIB could do something
like this (and possibly not use a RowPointer) so
please explain what the intention is.
16) gmplsTunnelHopTable OBJECT-TYPE
The storage type for an entry in this table is inherited from
mplsTunnelHopStorageType in the corresponding entry in
mplsTunnelHopTable.
Please change to:
The storage type for this entry is given by the value
of mplsTunnelHopStorageType in the corresponding entry in the
mplsTunnelHopTable.
17) gmplsTunnelHopExpLabel OBJECT-TYPE
DESCRIPTION
"If gmplsTunnelHopLabelStatuses object indicates that a forward
label is present and gmplsTunnelHopExpLabelPtr contains the
value zeroDotZero, then the label to use on this hop is found in
object encoded within a 32-bit integer."
Think the last part of this sentence is awkward:
...to use on this hop is represented by the value of this object.
18) gmplsTunnelHopExpLabel and gmplsTunnelHopExpLabelPtr
Need to add the word "Forward" in here as in
gmplsTunnelHopExpForwardLabel and gmplsTunnelHopExpForwardLabelPtr
That way it will be consistent with the Reverse counterparts.
19) gmplsTunnelHopExpRvrsLabel and gmplsTunnelHopExpRvrsLabelPtr
Please change Rvrs to Reverse.
20) gmplsTunnelHopLabelStatuses and gmplsTunnelARHopLabelStatuses
Please change Statuses to State.
21) gmplsTunnelARHopExpLable and gmplsTunnelARHopExpLabelPtr
Need to add the word "Forward" in here as in
gmplsTunnelARHopExpForwardLabel and gmplsTunnelARHopExpForwardLabelPtr
22) gmplsTunnelARHopExpRvrsLabel and gmplsTunnelARHopExpRvrsLabelPtr
Please change Rvrs to Reverse
23) gmplsTunnelARHopProtection OBJECT-TYPE
Please add what the BIT set (i.e. 1) vs. 0 means in the
DESCRIPTION.
24) gmplsTunnelCHopLabelTable objects
Similar comments regarding the objects in this table
to those above with regard to naming
conventions (Forward and Reverse) and DESCRIPTIONs. Will not repeat the
details, please refer to above comments on the
prior 2 tables.
25) gmplsTunnelReversePerfTable
Please add to the DESCRIPTION how a user knows if
this is a bidirectional tunnel (i.e. what object
or objects in the gmplsTunnelTable need to have
what values?).
26) gmplsTunnelReversePerfPackets
Does this object represent the 32-bit
value of the least significant part of the
64-bit value (which is contained in gmplsTunnelReversePerfHCPackets)?
27) gmplsTunnelReversePerfBytes
Does this object represent the 32-bit
value of the least significant part of the
64-bit value (which is contained in gmplsTunnelReversePerfHCBytes)?
28) gmplsTunnelErrorTable OBJECT-TYPE
Since this table applies to MPLS, it should be
put in a separate MIB module which is MPLS specific. The
prefix of gmpls could be confusing.
29) gmplsTunnelErrorTLVs OBJECT-TYPE
Need to add a variable size to this string. When the string
size is 0 that would indicate that no TLVs are present, so
the DESCRIPTION should be modified to reflect this.
30) gmplsTunnelErrorHelpString OBJECT-TYPE
SYNTAX DisplayString
DisplayString is not used in newer MIBs because it does not
meet internationalization standards, better to use
SnmpAdminString or something which meets internationalization
standards.
31) Compliance
Please move some of the following comment in the
DESCRIPTION of gmplsTeModuleFullCompliance
-- 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.
32) gmplsTunnelUnnumIf does not appear in the ReadOnly compliance
and it probably should.
Section 11.2.1. IANA-GMPLS-MIB Definition
-----------------------------------------
33) Please change the name of this MIB to
IANA-GMPLS-TC-MIB
and instead of: ianaGmpls MODULE-IDENTITY
please use: ianaGmplsTcMIB MODULE-IDENTITY
34) Please add the following to the DESCRIPTION:
"Copyright (C) The Internet Society (2005). The initial version
of this MIB module was published in RFC XXXX. For full legal
notices see the RFC itself. Supplementary information
may be available on:
http://www.ietf.org/copyrights/ianamib.html
35) Please change GMPLS-TE-MIB's to GMPLS-TE-STD-MIB's [RFCXXX] (RFC-Editor
please fill in the XXX based on the RFC number for
draft-ietf-ccamp-gmpls-te-mib
and remove this note) in the
DESCRIPTION clauses of ALL TCs.
36) Please change the name IANAGmplsLSPEncoding to
IANAGmplsLSPEncodingType. (more closely matches what's on
IANA's website now)
37) Please change the name IANAGmplsGPid to
IANAGmplsGeneralizedPid
38) I believe that the TC
IANAGmplsAdminStatusFlags ::= TEXTUAL-CONVENTION
should be put in the GMPLS-TC-STD-MIB or should just
remain in the GMPLS-TE-STD-MIB. The reason is that this
is not an assigned numbers list maintained by IANA, so
should not be in the IANA GMPLS MIB. If I am incorrect
about this please let me know, but I didn't see an IANA
maintained Administrative Status Information enum.
Additionally, this should be renamed to
GmplsAdminStatusInformation which more closely matches rfc3471.
39) Several comments on the SYNTAX clause:
40a) smicngPRO gives the error
E: f(IANA-GMPLS-MIB.my), (252,22) Named bits for BITS
must be in contiguous positions
SYNTAX BITS {
delInProgress (0),
adminDown (1),
testing (2),
reflect (31)
}
RFC2578 specifies that BITS should be contiguous
(although there are exceptions to this, but this
object does not seem to be an exception).
Could delInProgress, be renamed to deleteInProgress ?
Could adminDown be renamed to administrativelyDown?
These names more closely match 3471.
40b) Please put in the specific section number on this
reference (section 8?).
-- the end --