The MPLS WG Archive

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



[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: "Wijnen, Bert (Bert)" <bwijnen@lucent.com>
  • Date: Mon, 9 Dec 2002 14:48:23 +0100

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.
  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
- mplsXxxxNext, see my comments in my review of other mpls MIB modules

- 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
- 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
- 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?
- 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.
- 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
  - 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? 
  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.
- 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
- 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.
- 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?
- 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
- I see mplsTunnelHopTableIndex to be read-create while 
  mplsTunnelARHopTableIndex and mplsTunnelCHopTableIndex are read-only
  Is that not strange?
- 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?
  and if so of how many? Is that of all entries with the same 
  mplsTunnelIndexIndex, or what?
  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?
  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"?
- 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
- 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.
- mplsTunnelHopPathOptionName should be UTF8 based and not DisplayString
- 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?
- 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?
- 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
- 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
- MODULE COMPLIANCE
  Maybe a Full and a ReadOnly compliance?
  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
  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? One seems sufficient to me.
  And why not spell out Intfc as Interface?
Thanks,
Bert