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
Inline
> -----Original Message-----
> From: Thomas D. Nadeau [mailto:tnadeau@cisco.com]
> Sent: maandag 9 december 2002 17:33
> To: Wijnen, Bert (Bert)
> Cc: Mpls (E-mail)
> Subject: Re: 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.
>
> 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.
>
OK, so here we go. I thought I had passed this on before.
First, I'd like to see an answer to the following piece of text
that is in every StorageType TC. What does this mean?
" 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?
Anyway, let us look at how the STorageType TC is defined (RFC2579):
StorageType ::= TEXTUAL-CONVENTION
STATUS current
DESCRIPTION
"Describes the memory realization of a conceptual row. A
row which is volatile(2) is lost upon reboot. A row which
is either nonVolatile(3), permanent(4) or readOnly(5), is
backed up by stable storage. A row which is permanent(4)
can be changed but not deleted. A row which is readOnly(5)
cannot be changed nor deleted.
If the value of an object with this syntax is either
permanent(4) or readOnly(5), it cannot be written.
Conversely, if the value is either other(1), volatile(2) or
nonVolatile(3), it cannot be modified to be permanent(4) or
readOnly(5). (All illegal modifications result in a
'wrongValue' error.)
Every usage of this textual convention is required to
specify the columnar objects which a permanent(4) row must
at a minimum allow to be writable."
SYNTAX INTEGER {
other(1), -- eh?
volatile(2), -- e.g., in RAM
nonVolatile(3), -- e.g., in NVRAM
permanent(4), -- e.g., partially in ROM
readOnly(5) -- e.g., completely in ROM
}
It says that a row which is either nonVolatile(3), permanent(4)
or readOnly(5), is backed up by stable storage.
That means that after a reboot (or restart of SNMP agent) such a row
should re-appear. If for other reasons such a row needs to go away
(for example since the LSP is removed via some LDP protocol mechanism),
then the row of course can and should be removed.
If the row is readOnly, the SNMP SET command cannot change or delete
the row. So not SET command for that row succeeds.
To me it means that there is no need to say anything about readOnly
storageType in your descrition clauses, or there must be something
special that I do not understand.
Now, if the row is permanent, then you cannot delete it (via SNMP SET).
But the row can be changed. That is if such is wanted/needed. Since
some people may interpret "permanent" as "cannot be changed", the
last para tells you that a DESCRIPTION clause (for an object with syntax
of StoraggeType) MUST (is required to) specify which columns must be
writable for such a permanent row.
If you do not "require" any write access, then a simple DESCRIPTION
clause for all your StorageType objects could be:
DESCRIPTION
"The storage type for this conceptual row. Conceptual rows
having the value 'permanent' need not allow write-access to any
columnar objects in the row."
RFC2574 has example of STorageType where some (special) columns DO
need to be writable, even for permanent rows.
Hope this helps.
For the RowStatus, Good examples are again in for example rfc3289
DESCRIPTION
"The status of this conceptual row. All writable objects in this
row may be modified at any time."
But then I see that in many cases you do NOT allow changes while the
row is active, so then it could be something aka:
DESCRIPTION
"The status of this conceptual row. None of the writable objects
in this row may be modified when this row is 'active'."
Or if some can and some otehrs cannot, then for example something like
DESCRIPTION
"The status of this conceptual row. Writable objects a and b
in this row may be modified at any time, but other wrietable
objects in this row may not be modified when this row is 'active'."
Now, be carefull, because when objects are not allowed to be modified when
the row is 'active', and when you specify (in MODULE COMPLIANCE) that
one only has to support createAndGo for the WRITE-SYNTAX, then people
cannot (in such an implementation) set the row to notInService, and
so that then means that changes to the row can only be made by
deleting (destroy) and recreating the rows.
Does this help?
> >- References need to be split in normative and informative
>
> I thought Arun did this. Which ones do you think
> are not?
>
Comon Tom, this is easy to find, and I did mention it for each doc.
But here is the summary, these docs do not have them split:
draft-ietf-mpls-tc-mib-05.txt
draft-ietf-mpls-ftn-mib-05.txt
draft-ietf-mpls-lsr-mib-09.txt
draft-ietf-mpls-te-mib-09.txt
these are OK w.r.t. reference split
draft-ietf-mpls-ldp-mib-09.txt
draft-ietf-mpls-mgmt-overview-02.txt
draft-ietf-mpls-bundle-mib-04.txt
Although the SNMP references are not split properly, see my other
emails about how to split those.
> >- mplsXxxxNext, see my comments in my review of other mpls
> MIB modules
>
> Will use the description in the FTN MIB.
>
Well, note that
- I had pointed out some conflicts in the description clauses in
those objects and TCs in that MIB
- I had suggested to use a common approach (similar as in diffserv
MIB module (rfc3289). So that means taking the TCs out
into the mpls TC MIB and using the same TC in all objects of
that type.
Hope I was clear on that. If not, let me know and I can propose
exact text and TCs to be specified and used.
> >- 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.
>
Well.. try to be very clear. When you say that you are listing objects
as they would be passed in a SET request, then it seems incorrect
to specify any read-only objects.
Also... you have the index (indices) included too. I udnerstand you
want to do that... maybe you want to mark them with an asterisk or such
and then explain they are index/indices and so that they are not
separate varBinds in the SET PDU but that they make up the
index part of each object in the varBinds in the SET PDU.
Or some such.
> >- 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.
>
OK, looking forward to that, hopefully I then understand.
Is it just me who does not understand? I doubt it in fact.
> >- 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.
>
And so when people do SET on ifName or on mplsTunnelName, then
the agent needs to make sure they stay in sync?
What if in one SET request (I will admit it is kind of stupid) one
send a SET for ifName of "abcd" and a SET for mplsTunnelName of "efgh"
I see all sorts of potential problems with this.
And is it the MIB who wants to presrcibe this?
I know thay ifName was meant for operators to use. So you are forcing
them to use same name for mplsTunnel, why can they not use
ifName : Interface1 for MPLS
mplsTunnelName : mpls Tunnel to Linschoten (this is where I live ;-))
Oh well... I won't loose sleep over it.... I just wonder about
the complexities it may cause (for in mu view no good reason)
> >- 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.
>
Having 2 objects to represnt kind of the same thing always
adds complexity in terms of "what to do when they conflict"
or "how to make sure they stay in sync".
Is it just me who sees these type of concerns?
> >- 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).
>
Right... but pls remove the piece of text that says that it cannot
be changed when row is active, because that piece if nonsense and
confusing.
> >- 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.
>
Got you. Maybe say something about how it gets filled in,
and then people (like myself) might not need to wonder as much
Not everyone is an MPLS expert on all these things (think about
the (generic) apps developers we talked about in Atlanta. They
want to quickly understand it too.
> >- 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.
>
That is not a clear answer to my question
My interpreation of your answer is that the answer is: YES
> > 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).
>
First of all, it would be good to make that very clear in the
DESCRIPTION clauses. One can kind of conclude that, but it is
a very strange thing to see in a MIB table, so make it clear
that it is intentional.
But it seems BAD to me.
> > 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.
>
So you are again not answering my first question: do all 4
entries have the same value? And if so that looks BAD to me.
But maybe it is just me. Any other opinions?
> > 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.
>
Or add the explanation as to when the condition can occur.
In general, whenever a independent (that is someone who was
not involved in the devlopment of the protocol and/or the MIB)
reviewer has a question in the form of "how does that work?" or
"Do I understand this and that (in)correctly?", then that often
indicates that it is a good idea to add explanatory text in
some of the DESCRIPTION clauses. So that not every new reader
needs to go to the mailing lists to ask the same questions
> >- 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?
>
Yep.
> >- 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.
>
OK, thanks
> >- 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...
>
Either ditch it, or add some of the above explanation to the
DESCRIPTION clause. And if we keep multiple occurences, as I
said, those are primary candidates for a TC
> >- 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?
>
You can... but that is what TCs are for, namely descrinbing once
what is used in multiple (more than 1) places.
Since it occures only twice and in same MIB module, I can
live with it, if you find it too much trouble and if no-one else
cares.
> >- 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).
>
That is a WG question
> >- 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.
>
It seems I am not clear. This is what you have in your MIB module:
mplsTunnelIsIntfcGroup OBJECT-GROUP
OBJECTS { mplsTunnelIsIf }
STATUS current
DESCRIPTION
"Objects needed to implement tunnels that are
interfaces."
::= { mplsTeGroups 5 }
mplsTunnelIsNotIntfcGroup OBJECT-GROUP
OBJECTS { mplsTunnelIsIf }
STATUS current
DESCRIPTION
"Objects needed to implement tunnels that are not
interfaces."
::= { mplsTeGroups 6 }
They are EXACTLY the same set of (one single) object.
That to me means there is only ONE group and not TWO.
You then go on in the MODULE-COMPLIANCE to state (in a DESCRITION
clause) that in some cases the value is true or false.
You can write that in the single DESCRIPTION clause in the
MODULE-COMPLIANCE anyway,
So again, it completely escapes me why you think you need
two groups with the same content.
> >One seems sufficient to me.
>
> This is moot if we just remove the IsIf object from the
> tunnelEntry.
>
That is true too, but I'd like you to read the above
and try to understand why I think my viepoint makes sense and
what is unb the rev 9 of the document does not.
> > 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".
>
Well, then be consistent, Either that or spell it out,
But... this is a real NIT and I won't loose sleep over it.
Bert
> --Tom
>
>
> >Thanks,
> >Bert
>
>
|
|