[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
Adrian
I am still unsure about MSB/LSB in IANAGmplsAdminStatusFlags. This has reflect
(31) while RFC3471 has reflect as bit 0.
As Joan says, "The BITS as specified look backwards when viewing the TLV".
This is fine if one is MSB, the other LSB (although that might be worth adding a
note about).
Tom Petch
----- Original Message -----
From: <jcucchiara@mindspring.com>
To: "Adrian Farrel" <adrian@olddog.co.uk>; <tnadeau@cisco.com>;
<ccamp@ops.ietf.org>
Cc: <bwijnen@lucent.com>; <jcucchiara@mindspring.com>
Sent: Friday, September 30, 2005 4:13 AM
Subject: Re: MIB Dr. review of draft-ietf-ccamp-gmpls-te-mib-09.txt
>
> hi Adrian,
>
> Please see comments inline below, I have snipped parts where we
> are in agreement.
>
> thanks, joan
>
>
>
> ----- Original Message -----
> From: Adrian Farrel <adrian@olddog.co.uk>
> To: <tnadeau@cisco.com>; <ccamp@ops.ietf.org>; <jcucchiara@mindspring.com>
> Cc: <bwijnen@lucent.com>; <jcucchiara@mindspring.com>
> Sent: Thursday, September 22, 2005 8:59 AM
> Subject: 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 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.
>
>
> The object is defined as:
>
>
> gmplsTunnelDirection OBJECT-TYPE
> SYNTAX INTEGER {
> forward (0),
> bidirectional (1)
> }
> MAX-ACCESS read-create
> STATUS current
> DESCRIPTION
> "Whether this tunnel carries forward data only (is
> unidirectional) or is bidirectional.
>
> Values of this object other than 'forward' are meaningful
> only if gmplsTunnelLSPEncoding is not set to
> 'tunnelLspNotGmpls'."
> DEFVAL { forward }
> ::= { gmplsTunnelEntry 8 }
>
>
> The following (part of the ReadOnly compliance statement) is causing
> the warning because the SYNTAX defined in the object and the
> SYNTAX below are the same. (Following the OBJECT are a
> few alternatives.)
>
> OBJECT gmplsTunnelDirection
> SYNTAX INTEGER {
> forward (0),
> bidirectional (1)
> }
> MIN-ACCESS read-only
> DESCRIPTION
> "Only forward (0) is required."
>
>
> If you do intend for the SYNTAX in both the OBJECT-TYPE and
> the OBJECT to be the same, then removing it from the OBJECT
> would get rid of the warning. This is not a blocking issue, but
> it is duplicate info that does not need to be duplicated.
> So here is what the OBJECT statement would like:
>
> OBJECT gmplsTunnelDirection
> MIN-ACCESS read-only
> DESCRIPTION
> "Only forward (0) is required."
>
> Another alternative would be to specify forward(0) only since
> this is what the DESCRIPTION is specifying, however, it is
> likely that you are probably allowing both, so I don't think this
> is what you intend, but this would also solve the compiler warning:
>
> OBJECT gmplsTunnelDirection
> SYNTAX INTEGER {
> forward (0),
> }
> MIN-ACCESS read-only
> DESCRIPTION
> "Only forward (0) is required."
>
>
>
>
> > > 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.
>
> Yes, I should have said, "network management entity (e.g., snmp agent).
>
>
> > Will change to:
> >
> > "An entry can be created by a network administrator via SNMP SET
> > commands, or in response to signaling protocol events."
> >
>
> good.
>
> > > 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.
> >
>
> Could you specify the ifType(s)?
>
> > > 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.
> >
>
> As was suggested in an email on draft-ietf-ccamp-gmpls-lsr-mib, could
> you put these objects in a separate object group? Also, please separate
> MPLS and MPLS-TE objects into separate object groups.
>
>
> >
> > > 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
>
> How about gmplsTunnelUpstreamNotifyRecipient
>
> As for the 32 char limit, we've already addressed that, so won't
> repeat that discussion.
>
> >
> > > 13) gmplsTunnelDownNotRecip
> > > Same comment as above wrt to IpAddress.
> >
> > Ack
> >
> > > Please change the name to gmplsTunnelDownstreamRecipientForNotify
> >
> > Nack as above
> > How about:
> > gmplsTunnelDownNotifyRecipient
>
> How about gmplsTunnelDownstreamNotifyRecipient
>
>
> >> > 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
>
> In other objects in this MIB module are named using the entire
> word Reverse ( e.g.
> GmplsTunnelReversePerfEntry ::= SEQUENCE {
> gmplsTunnelReversePerfPackets Counter32,
> gmplsTunnelReversePerfHCPackets Counter64,
> gmplsTunnelReversePerfErrors Counter32,
> gmplsTunnelReversePerfBytes Counter32,
> gmplsTunnelReversePerfHCBytes Counter64
> })
>
> So please either change these to all use Rvrs or instead of Rvrs use
> the entire word, Reverse. Similarly with the use of Fwd and Forward,
> if you decide to use Reverse, then please use the entire word, Forward.
> If you decide to use Rvrs, then please use Fwd. I prefer the words
> spelled out but just please be consistent.
>
>
>
> > > 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"
>
> See discussion above wrt Rvrs and Reverse.
>
> >
> > > 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.
> >
>
> Discussed previously about separate OBJECT groups.
>
> > > 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
> > > -----------------------------------------
> >
> > > 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.
>
> Is this going to be managed by IANA soon? (just curious)
>
> >
> > > 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).
> >
>
> Here is the TLV that the object represents:
>
> 0 1 2 3
> 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> |R| Reserved |T|A|D|
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> Reflect (R): 1 bit
> Reserved: 28 bits
> Testing (T): 1 bit
> Administratively down (A): 1 bit
> Deletion in progress (D): 1 bit
>
> The BITS as specified look backwards when viewing the TLV.
> Are the BITS correctly specified?
>
> I consulted with other MIB Doctors and the following was suggested:
> SYNTAX BITS {
> delInProgress (0),
> adminDown (1),
> testing (2),
> reserved3(3), -- 0 on send, ignored on receive
> reserved4(4), -- 0 on send, ignored on receive
> ..
> reserved30(30), -- 0 on send, ignored on receive
> reflect (31)
> }
>
> Would this be acceptable?
>
>
> > > Could delInProgress, be renamed to deleteInProgress ?
> > > Could adminDown be renamed to administrativelyDown?
> > > These names more closely match 3471.
> >
> > Ack
> >
>
>
>