The MPLS WG Archive

Cell Relay Retreat>MPLS WG Archive>month:2002-Dec> msg00198



[Date Prev][Date Next][Thread Prev][Thread Next]  
  [Date Index][Thread Index][Author Index][Subject Index]

review of draft-ietf-mpls-te-mib-09.txt

  • From: "Thomas D. Nadeau" <tnadeau@cisco.com>
  • Date: Mon, 09 Dec 2002 11:33:15 -0500
  • Cc: "Mpls (E-mail)" <mpls@UU.NET>


>Seems that a lot of earlier comments I had made to the authors
>half a year or so ago, have not been addressed. Oh well.
>Here we go (mix of nits, and more serious things)
>
>- missing IPR section
>- Security considerations probably needs more work
>   Security ADs want you to explain what the vulnerabilities/risks
>   are and what to do against them.
>   Also for read only objects, pls list each (group of) object(s)
>   and explain what sensitivity attributes they have
>- RowStatus and STorageType objects need better text as per my comments
>   to the other MIB modules.

         Well, it is not from lack of trying. In many cases,
I would ask that you make suggestions as to what
exact text you would like to see. In particular, things like
the examples and security sections have been updated
to what we thought addressed your comments. For this
and the other MIBs you have returned comments on
you have stated that they are not up to snuff. Please
instead of going back and forth 50 times on this via
email, please just propose some text as you have
done in other DESCRIPTION clause fixes, which will
greatly speed things along.

>   And a special question...
>          "                                  If this variable
>           is set to readOnly(5), and the corresponding entry
>           is removed, then the agent must remove this row
>           shortly thereafter [RFC2579]."
>   What does that mean?
>   What is the "corresponding entry"? That is this row, is it not?
>- References need to be split in normative and informative

         I thought Arun did this. Which ones do you think
are not?

>- mplsXxxxNext, see my comments in my review of other mpls MIB modules

         Will use the description in the FTN MIB.

>- last bullet sect 4 talks about (non-)persistent tunnels.
>   as does the DESCRIPTION clause of mplsTunnelGroup...
>   but I don't get a good picture of what makes a persistent
>   or a non-persistent tunnel and what the MIB offers for that.
>- section 9, the example
>   includes mplsTunnelOwner... but it claims that those things
>   are to be part of a SET request. But... in the MIB module I see
>   the mplsTunnelOwner is read-only, so it cannot be part
>   of a SET

         Right. Need to clarify. The owner is gleaned from
who creates the entry, as is described in the TC.

>- section 9 example
>   inlcudes mplsTunnelResourcePointer and syas that it should
>   be SET to mplsTunnelResourceIndex.5
>   I think that should be mplsResourceMaxRate.5 because that
>   is the first accessible columnar object in a row that table

         Yes.

>- section 10,
>   first sentence
>   s/to one of its objetcs/to the first accesible object in a row/
>   last sentence
>   s/to the first column/to the first accessible column/
>- I wonder if mplsTunnelConfigured would be better named
>   mplsTunnelsConfigured and mplsTunnelActive better be named
>   mplsTunnelsActive
>- I ownder if mplsTunnelNotiMaxRate would be better named
>   mplsTunnelNotiInterval
>   May want to add a UNITS clause to this object anyway
>   and why is it not named mplsTunnerNotificationInterval?

         Looks like a typo.

>- mplsTunnelIndexNext
>   well, same comment as to all the mplsXxxNext objects in the
>   other MIB modules.
>   But this one has even another strange thing, namely it is
>   a Interger32 (as opposed to a Unsigned32) and it has a pretty
>   limited range and for all I can tell for no good reason.
>   In fact I see no reason for a upper limit.

         It is supposed to match the indexing of the tunnel table.

>- mplsTunnelEntry
>   Why is there a reference to RFC1700 ??? That RFC has been
>   obsoleted anyway...
>- Can someone explain to me why you use the complex indexing
>   scheme for the mplsTunnelTable, namely:
>    INDEX {
>       mplsTunnelIndexIndex,
>       mplsTunnelInstance,
>       mplsTunnelIngressLSRId,
>       mplsTunnelEgressLSRId
>    }
>
>   - first, I do not understand why it the 1st one is named
>     mplsTunnelIndexIndex instead of just mplsTunnelIndex

         Compiler complained because the type is MplsTunnelIndex.

>   - 2nd, mplsTunnelIndexIndex claims to "uniquely" identify a row
>     so then no more index objects seem needed
>   - but mplsTunnelInstance seems to also "uniquely" identify a row??
>   - and then there are yet more index objects.
>   - mplsTunnelIngresLSRId, description talks about "this value".
>     but the index object does not really have a value. It is part
>     of the OID that identifies instances in the table.
>   Why do I not understand this indexing scheme?

         We went over this during the MIB meeting. This is basically
just the RSVP-TE indexing required.

>   I think it would be good to include in the DESCRIPTION clause of
>   either the mplsTunnleEntry or of the indices a very good explanation
>   as to how the indexing works here.

         Yes. We agreed to improve the description as an action
from the MIB meeting.

>- I have trouble with the use of DisplaySTring (as mentioned half a year
>   ago). These days we want people to use UTF8 based strings if the strings
>   are intended for human use. So either SnmpAdminString or some other
>   UTF8-based string. Several TCs have been defined to replace DisplayString

         Yes, will use SnmpAdminString.

>- I wonder why the mplsTunnelName is ever to be equal to ifName.
>   The link between an entry in the mplsTunnelTable and the ifTable is
>   made by the mplsIfTunnelIndex which is the better way indeed to make
>   the link.

         The idea is that in cases where the tunnel is an ifEntry too,
this makes sense so they are associated.

>- It seems to me that mplsIfTunnelDescription could have a DEFVAL
>   of the empty (zero lenght) string
>- It seems to me that mplsTunnelIsIf and mplsTunnelIfIndex are overlapping.
>   A value of zero in the mplsTunnelIfIndex means that this tunnel does
>   not correspond to an interface. A nonzero value means that it does
>   correspond to an interface. So why is mplsTunnelIsIf needed? So that
>   people can have conflicting stuff that needs to be chacked and all that?

         The idea is that you can quickly determine the type of
tunnel. I suppose that we could eliminate this object and just
state that if ifIndex = 0 that it is NOT an ifIndex.

>- mplsTunnelSessionAttributes
>   you may want to check the formatting in the DESCRIPTION clause
>- mplsTunnelOwner is a read-only object and yet the description clause
>   talks about that it cannot be modified when RowStatus is active.
>   That seems redundant text. A read-only object can NEVER be changed

         Again, this should be the same as that in the TC that we
discussed (the agent sets this value).

>- I see mplsTunnelHopTableIndex to be read-create while
>   mplsTunnelARHopTableIndex and mplsTunnelCHopTableIndex are read-only
>   Is that not strange?

         Nope. The actual route hop table is what is spit out
by the CSPF computation; not operator programmable.

>- I do not understand mplsTunnelPrimaryInstance, but that is probably linked
>   to me not understanding the indexing scheme.
>- mplsTunnelPathChanges
>   s/paths has changed/path has changed/ or
>   s/path has changed/paths have changed/ ??
>- mplsTunnelTotalUptime
>   so is this an aggregate value of multiple entries in this table?

         This is the up time of all LSP instances of the tunnel.

>   and if so of how many? Is that of all entries with the same
>   mplsTunnelIndexIndex, or what?

         Yes, the same group of tunnel instances (the same primary
tunnel index).

>   And then... if there are 4 entries aggregated, do all 4 entries then
>   have the same value?
>   And what is the upTime of each entry?

         mplsTunnelPrimaryTimeUp shows the time of the primary
instance that this tunnel

         mplsTunnelTotalUpTime is the aggregate time of all
instances of this tunnel (head).

         mplsTunnelInstanceUpTime is the time this instance has
been up.

>   I am confused here as to what the purpose of this object is and how
>   it is computed. Under what circumstances would it "not be available"?

         It might not be available if no instances exist for
the tunnel. I suppose it might be simpler to just remove this
text, but the thinking was that we specified an initial value
for when no tunnel instances (other than 0) existed.

>- mplsTunnelHopListIndexNext
>   well, same comment as to all the mplsXxxNext objects in the
>   other MIB modules.
>   But this one has even another strange thing, namely it is
>   a Interger32 (as opposed to a Unsigned32) and it has a pretty
>   limited range and for all I can tell for no good reason.
>   In fact I see no reason for a upper limit.
>- mplsTunnelHopTable.
>   again I do not (from the DESCRIPTION clauses) understand the indexing
>   scheme. It seems to have two "seconday" indices. Should that be a secondary
>   and a tertiary? Must be me, since I am not an mpls expert ;-)
>- I assume that the mplsTunnelHopAddrType and related objects get replaced
>   with a common new TC as discussed at the MPLS MIB meeting in Atlanta

         Yes.

>- mplsTunnelHopType
>   this sort of ENUMERATION seems to occur a few times, so that it seems to
>   me that you want to create a TC for it.

         Okay.

>- mplsTunnelHopPathOptionName should be UTF8 based and not DisplayString

         SnmpAdminString okay?

>- mplsTunnelResourceIndexNext
>   well, same comment as to all the mplsXxxNext objects in the
>   other MIB modules.
>   But this one has even another strange thing, namely it is
>   a Interger32 (as opposed to a Unsigned32) and it has a pretty
>   limited range and for all I can tell for no good reason.
>   In fact I see no reason for a upper limit.
>- mplsTunnelResourceMaxRate and mplsTunnelResourceMeanRate
>   could use a UNITS clause
>   And... you say it is bits per second, while the MplsBitRate TC claims
>   that it is Kbps... oh well
>- mplsTunnelResourceMaxRate and mplsTunnelResourceMeanRate
>   I don't think I udnerstand what the OID copying is all about and
>   how an agent determines what to do? Or is it the management station
>   who does this?

         The text you are referring to is:

          This object is copied to an instance of
           mplsTrafficParamMaxRate in mplsTrafficParamTable
           the OID of which is copied into the corresponding
           mplsInSegmentTrafficParamPtr.

         this refers to the fact the values should be copied down
to those see in the LSR MIB's resource table. This is now
moot, as we are removing the LSR MIB's table.  I will
remove the aforementioned text.   In fact, this text appears
all over the place in the resource table, so I will fix.

>- mplsTunnelResourceFrequency  OBJECT-TYPE
>    SYNTAX       INTEGER {
>                      unspecified(1),
>                      frequent(2),
>                      veryFrequent(3)
>   I see another occurence in another table. So a good candidate for a TC
>   But I also wonder what "frequent" and "veryFrequent" means?
>   To me they seem very very vague  terms, no?

         I believe this comes from CR-LDP and does not apply
for RSVP-TE (you would use unspecified). See the reference
of CR-LDP Specification, Section 4.3 for more details.  I
would actually prefer to ditch this and the other CR-LDP
tables to make things simpler...

>- mplsTunnelResourceWeight    OBJECT-TYPE
>    SYNTAX       Unsigned32(0..255)
>    MAX-ACCESS   read-create
>    STATUS       current
>    DESCRIPTION
>         "The relative weight for using excess bandwidth above
>           its committed rate.  The value of 0 means that
>           weight is not applicable for the CR-LSP."
>   I see this multiple times. Good candidate for a TC

         I only see it twice. Can we skip this?

>- mplsTunnelARHopAddrType and related objects
>   I assume this will be replaced by new TCs as discussed in atlanta
>- mplsTunnelCHopAddrType and related objects
>   I assume this will be replaced by new TCs as discussed in atlanta
>- I would suggest to just do counter 64 instead of both 64 and 32
>   for objects like
>       mplsTunnelPerfPackets           Counter32,
>       mplsTunnelPerfHCPackets         Counter64,
>       mplsTunnelPerfBytes             Counter32,
>       mplsTunnelPerfHCBytes           Counter64
>   In general we also speak about octets instead of bytes, but I can
>   live with  bytes.
>- I wonder if mplsTunnelCRLDPResFlags would not be better represented
>   with a BITS construct instead of an INTEGER enumeration

         We have been over this several times. The representation is
based on what is found in the CR-LDP protocol. The WG preferred
to leave the value this way, so I would prefer to leave it (unless
we can ditch all of the CR-LDP related stuff).

>- MODULE COMPLIANCE
>   Maybe a Full and a ReadOnly compliance?

         Will add.

>   See also my comment to other mpls MIB modules. It links to the
>   SYNTAX and WRITE-SYNTAX of some of the objects as well, specifically
>   the RowStatus objects
>- Pls see also my note in other MIB Module comments about the
>   'other' for StirageType objetcs. It seems really weird to have
>   that in the compliance.
>- It is not good to give a group of objects a name that indicates
>   optionality. A gorup is just a group. If it is optional or
>   manadatory, that should become clear in the MODULE COMPLIANCE
>   statement, but not in the name (and/or description clause) of the group

         Okay.

>   Some future MODULE COMPLIANCE may want to make that group mandatory.
>   An OBJECT GROUP is a "logical grouping" of objects. The requirement
>   for mandatory of conditional or optional implementation is to be
>   specified in the MODULE-COMPLIANCE.
>- I do not understand why there is both mplsTunnelIsIntfcGroup and a
>   mplsTunnelIsNotIntfcGroup?

         Because some objects make sense only for when the tunnel entry
is implemented as an ifIndex.

>One seems sufficient to me.

         This is moot if we just remove the IsIf object from the
tunnelEntry.

>   And why not spell out Intfc as Interface?

         Brevity? *)  We actually wanted to be consistent with
the rest of the MIB, which I think uses "if".

         --Tom


>Thanks,
>Bert