[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: MIB Dr. review of draft-ietf-ccamp-gmpls-te-mib-09.txt
Hi Joan,
Last but not least, the GMPLS TE MIB module.
> Here are some comments on draft-ietf-ccamp-gmpls-te-mib-09.txt.
> All 3 docs show a good deal of work.
Yes, but fortunatley most of the work appears to be editorial with only
very minor changes to the basis of the module.
Please find acks and nacks in line below.
We will also include the comments made by David and Tom Petch.
Cheers,
Adrian
> Section 1.1 Migration Strategy
> ------------------------------
>
> 1) Please remove "and OBJECT-IDENTIFIERS"
>
> Textual conventions and OBJECT-IDENTIFIERS are defined in [RFC3811]
> and [GMPLSTCMIB].
Ack
> Section 5.4 gmplsTunnelCHoptable
> ---------------------------------
>
> 2) Please capitalize Table in the Section's title.
Ack
> 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
Ack
Understood this to be optional, but planning to add size as per David's
comments
> 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
Nack
Meaningless warning message. Cannot fix without explanation.
> Disconnect of numbering in the Conformance Section
> \-3 gmplsTeConformance
> \-v-1 gmplsTeGroups
> | \-v-1 gmplsTunnelGroup
>
> 2 is missing
Ack
> 3) gmplsTunnelsConfigured OBJECT-TYPE
> SYNTAX Unsigned32
>
> SYNTAX should be Gauge32
Ack
> 4) gmplsTunnelsActive OBJECT-TYPE
> SYNTAX Unsigned32
>
> SYNTAX should be Gauge32
Ack
> 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).
Nack
"A row in this table is created when a row is created in the
mplsTunnelTable" is false.
Will attempt to reword to make this clearer.
> 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."
Nack
"network management entity" implies a remote entity such as an NMS.
Will change to:
"An entry can be created by a network administrator via SNMP SET
commands, or in response to signaling protocol events."
> 7) gmplsTunnelUnnumIf
>
> What IF-MIB object are referring to in the DESCRIPTION?
> Could you state which object in the description?
Ack
An entry in ifTable.
> 8) gmplsTunnelAttributes
> Could you specify what happens when the bit is set (i.e. 1)
> vs. 0?
Ack
Don't want to restate the protocol I-D, so this text will simply
replace "This flag indicates that" with "This flag is set to indicate
that"
> 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.
Nack
We strongly disagree with any implication that MPLS and GMPLS should be
separated in this way. GMPLS protocol extensions are likely to be
moved into MPLS implementations piecemeal and there is no clear
separation that you imply. The G in GMPLS stands for "generalized".
Can you please indicate whether this is a sticking point for you.
> 10) gmplsTunnelLinkProtection
> Same comment as with gmplsTunnelAttributes.
Ack
Same answer.
> 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.
Ack
> 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.
Ack per exchange with David.
> Please change the name of this object to:
> gmplsTunnelUpstreamRecipientForNotify
Nack
We are trying to keep to the (mythical?) 32 character limit.
How about:
gmplsTunnelUpNotifyRecipient
> 13) gmplsTunnelDownNotRecip
> Same comment as above wrt to IpAddress.
Ack
> Please change the name to gmplsTunnelDownstreamRecipientForNotify
Nack as above
How about:
gmplsTunnelDownNotifyRecipient
> 14) gmplsTunnelAdminStatusFlags (please see comments
> regarding the name of this TC and make adjustments
> to the name and DESCRIPTION of this object as appropriate).
Ack
> 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.
Nack
See discussions of rowPointer in the LSR MIB.
We need to be easily extensible to standard MIB modules that will provide
additional parameters as described in the DESCRIPTION clause. Such MIB
modules will contain tables the entries of which will each be used by
multiple tunnels.
In this context a row pointer is appropriate.
> 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.
Ack
> 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.
Ack
> 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.
Nack
Please see point 21 about name lengths.
We will insert "Fwd"
> 19) gmplsTunnelHopExpRvrsLabel and gmplsTunnelHopExpReverseLabelPtr
>
> Please change Rvrs to Reverse.
Nack
Please see point 21 about name lengths
> 20) gmplsTunnelHopLabelStatuses and gmplsTunnelARHopLabelStatuses
>
> Please change Statuses to State.
Nack
Status is the correct word. State would be incorrect.
> 21) gmplsTunnelARHopExpLable and gmplsTunnelARHopExpLabelPtr
>
> Need to add the word "Forward" in here as in
> gmplsTunnelARHopExpForwardLabel and gmplsTunnelARHopExpForwardLabelPtr
Nack
Would excede 32 characters.
Will use "Fwd"
> 22) gmplsTunnelARHopExpRvrsLabel and gmplsTunnelARHopExpRvrsLabelPtr
>
> Please change Rvrs to Reverse
Nack
See point 21
> 23) gmplsTunnelARHopProtection OBJECT-TYPE
>
> Please add what the BIT set (i.e. 1) vs. 0 means in the
> DESCRIPTION.
Ack
Limited change as before.
> 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.
Nack
Same answers as before.
Will use "Fwd"
> 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?).
Ack
> 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)?
Ack
> 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)?
Ack
> 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.
Nack
This is applicable across MPLS and GMPLS.
Error TLVs are not present in MPLS RSVP-TE at this time. They may be added
later.
> 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.
Ack
As per discussion with David.
> 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.
Ack
> 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.
Ack
> 32) gmplsTunnelUnnumIf does not appear in the ReadOnly compliance
> and it probably should.
Ack
> 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
Ack
> 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
Ack
> 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.
Ack
> 36) Please change the name IANAGmplsLSPEncoding to
> IANAGmplsLSPEncodingType. (more closely matches what's on
> IANA's website now)
Ack
> 37) Please change the name IANAGmplsGPid to
> IANAGmplsGeneralizedPid
Ack
> 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.
Nack
There are no fewer than three requests in CCAMP I-Ds that have passed WG
last call
for this space to be managed by IANA.
> Additionally, this should be renamed to
> GmplsAdminStatusInformation which more closely matches rfc3471.
Ack
> 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).
Nack
To quote the DESCRIPTION...
The relationship between the assignment of
gmplsTunnelAdminStatusFlags values and of the bit flags
in the Admin Status object or TLV within the GMPLS
signaling protocols is solely the purview of IANA and is
subject to change without notice."
However, it seems to us that making the bits match the flags in the
Admin Status object is a Good Idea (TM).
> Could delInProgress, be renamed to deleteInProgress ?
> Could adminDown be renamed to administrativelyDown?
> These names more closely match 3471.
Ack
> 40b) Please put in the specific section number on this
> reference (section 8?).
Ack