The MPLS WG Archive[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
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
|
|