Subject: RE: AD review of draft-ietf-mpls-ldp-mib-10.txt

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Sat, 10 May 2003 15:08:03 +0200

Answers to your questions:
> >2. I see at several place in DESCRIPTION clause of StorageType
> >   objects this text:
> >      DESCRIPTION
> >       "The storage type for this conceptual row.  Conceptual rows
> >        having the value 'permanent(4)' MAY allow write-access to any
> >        columnar objects in the row, except for setting the
> >        mplsLdpEntityRowStatus to 'destroy(6)'."
> >   And that is very weird. It seems to allow to change the storagetype
> >   object itself (which is a nono). And the cannot destroy is also
> >   already covered by being a permanent entry.
> >   The required text from RFC2579 (Storage Type TC) is something aka:
> >       "The storage type for this conceptual row.  Conceptual rows
> >        having the value 'permanent(4)' need not allow write-access
> >        to any columnar objects in the row."
> >   Which you have in some other cases, and which I think expresses
> >   the same and is correct accoring to RFC2579 requirements.
> >   mplsLdpEntityAtmLRStorageType has it specified correctly!
> >
> 
> Sorry, but am confused by this, if the type is permanent(4), then
> the agent does not have to support any subsequent sets (modifications)
> to objects in this row, which includes setting the RowStatus object
> to destroy, is this correct?
> 
> So then once a row is permanent(4), then the agent does not need to provide
> a way to remove/change this row using SNMP, is this correct?
> 
> We can rephrase this using the text suggested above, but just want to
> make sure we understand ;)
> 
The values of StorageType MUST be controled as the prescribed behaviour
in the StorageType TC. From 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."

So it say that a row which is permanent can be changed but cannot be
deleted. So no need for you to repeat that you cannot do a destroy
(in fact it may just confuse people).
The TC also already prescribes that a row CAN BE CHANGED, so you do 
not have to repeat that write-access MAY be allowed.
The last para in the above description state that you MUST specify if there
are any objects that MUST be allowed to be written. In your case, I think
there are none. (RFC3414, usmUserTable is an examble where some object
MUST allow write access, but most tables don't have such situations).

So I think my proposed text is:
  shorter, more precise, and in-line with all other such similar 
  descriptions in other MIB modules.

> >3 I see:
> >         mplsFecAddrPrefixLength    InetAddressPrefixLength,
> >         mplsFecAddrFamily          InetAddressType,
> >         mplsFecAddr                InetAddress,
> >  I believe that RFC3291 suggests to put InetAddressType BEFORE
> >  any objects that are to be intepreted within the context of
> >  the specific addressType. so the sequence should probaly be:
> >         mplsFecAddrFamily          InetAddressType,
> >         mplsFecAddrPrefixLength    InetAddressPrefixLength,
> >         mplsFecAddr                InetAddress,
> >
> >  I would also check the DESCRIPTION clauses with RFC3291.
> >  I am not so sure it is all in sync with prescriptions of RFC3291
> >  I am specifically worried about the reference to RFC1700 (which is
> >  obsolete anyway, probably you better refer to
> >      http://www.iana.org/assignments/address-family-numbers
> >  if that is what you want). But is it not just IPv4 and IPv6
> >  addresses that you support (at least that is what compliance
> >  seems to state?
> 
> 
> The reference to ADDRESS-FAMILY is because the LDP Spec (3036) uses
> in a few TLV and they do specify [RFC1700] as the reference.  Here
> is the Specific example from the LDP Spec (rfc3036):
> 
> 
> "Prefix FEC Element value encoding:
> 
>        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
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |  Prefix (2)   |     Address Family            |     PreLen    |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |                     Prefix                                    |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>       Address Family
>          Two octet quantity containing a value from ADDRESS FAMILY
>          NUMBERS in [RFC1700] that encodes the address family for the
>          address prefix in the Prefix field."
> 
> 
> 
> We do not mean to cause confusion in the MIB, BUT,  the LDP Spec
> which is "Standards Track" document refers to the obsoleted document
> (RFC1700) and the MIB is actually refering to the LDP Spec.
> 
That is because when RFC3036 was issued, the RFC3232 did not yet exist.
For new documents, we want people to use the newer RFC if at all possible.

> In the MIB we are trying to use descriptions based on the LDP Spec so that
> folks can use this MIB to configure values for the LDP TLVs.  
> If we change this description too much, then the reference back to the LDP 
> Spec is lost.
> 
Why would that be lost? 
I can see it MAY cause some confusion, and I now also am a bit confused.
In the MIB module you keep saying that only IPv4 and IPv6 addresses are
to be supported (or unnumberd, but that is not part of AddressFamily at 
all, so that is another confusing factor).
The AddressFamily has MANY MORE address types than just IPv4 or IPv6.
So... if they all need to be supported, then you have conflicting
(or at least confusing) text in the MIB document. If it is only IPv4/v6,
then INET-ADDRESS-MIB numbers/values happen to be the same as 
addressFamily numbers.

> Originally we did use the ADDRESS-FAMILY-NUMBERS TC but changed this
> for the INET-ADDRESS-MIB which is better for MIB purposes.
> 
Yep, I remember I (strongly) suggested that. Probably I WAS ALREADY
confused back then cause you spoke about just IPv4/IPv4 support.

So what is the story on that. Does the LDP protocol support all those
addresses and did you just limit the MIB support? That would not be
correct I think.

> Please let us know if we can keep the DESCRIPTION as it is or change
> it?  
> 
Depends on answers to my above questions back to you.

> >
> >4. How many notifications can be generated per second?
> 
> Are you asking for a worst case scenario?  
> 
> In a network where LDP is configured on each device (and not being
> reconfigured) would not expect notifications to be generated once sessions
> are established.  So would not quantify X notifications per second.
> 
> Maybe another way to ask this question is: how long does the
> average LDP Session last?  
> 
> Would say most last longer than a second, but
> would be good to get operator experience here.
> 
The thing is that we do not want the network and the NMS to be flooded
(congested) with notifications. If there is a true story that they 
will be infrequent, then just add that explanation, and we're OK.
Do remember that a NMS may be receiveing such notification from many
managed devices. So if you had one a second per managed device, and
you had to manage 1000s of devices, then there is potential still for
NMS overload/congestion.
So some words to make people feel comfortable is goodness. If the
evaluation is that there might be too many notifications, then we
may need to add a throttle object.

> >   I worry based on the description clauses. I see a risk that it
> >   could be many if not implemented properly or with a proper
> >   throttle? Can you add some text about that?
> 
> There are not huge amounts of LDP sessions on a device and
> these (one established) are fairly stable unless an operator 
> reconfigures or something goes wrong with the network, but this
> would fall into a worst case scenario.
> 
> Certainly, would be good to get some operator experience here, but 
> in my experience, sessions (once established) are fairly stable as
> long as the network's physical connections are not being changed and 
> LDP is not being reconfigured.
> 
OK, so add some text to explain situation is under control (at least
theoretically). If implementation/deployment experience tells us
later that we were wrong, then we can add a throttle object later.
> >
> >5. I worry about a statement in a RowStatus DESCRIPTION clause
> >   (it is doen several times):
> >            NOTE:  This RowStatus object should
> >            have the same value of the 'mplsLdpEntityRowStatus'
> >            related to this entry."
> >   I will agree that that is the ideal situation. But the rowStatus
> >   should always represent the real status of the row itself.
> >   If that is not consistent with some other row in another table,
> >   then that is bad, but it should not be a cause for the 
> >   rowStatus of thos column to take an invalid value. I guess
> >   you did not mean to say it should... but that is how people
> >   might read it. 
> 
> Okay, I understand I think.  We have had many issues with folks not
> understanding the relationship of these tables and essentially, any
> table that has a configurable row is related to a row in the 
> mplsLdpEntityTable.  This is outside of the scope of RowStatus though,
> so will reword this to remove the "should".
> 
Right. It might be better to have some text in the ...Table or ...Entry
DESCRIPTION clause that explains the relationships. It might also be
good to have some of that text in narative text before the MIB MODULE
itself. 
Let me warn you that experience has told us that if table relationships
become too complex and if too many tables have to be in sync all the time,
that such has been very troublesome. 
We often found it easier to tell people that if entry x in table A 
was not in sycn with entry x in table B, then some operation
would not work... but we would not force the agent to make sure that 
all tables were kept in sync all the time. Just giving you some
background info here.

> >
> >Nits (some more serious than otehrs)...
> >would be nice if they can be fixed:
> >
> >
> >- mplsLdpSesKeepaliveTime
> >  What does value of zero mean?
> 
> The value cannot be zero.  This has to be a nonzero, 2 octet
> value, which represents the amount of seconds between keep alive
> messages negoatiated between the entity and the peer for that
> specific session.
> 
> The mplsLdpEntityKeepAliveHoldTimer is the object which 
> configures this value.  This Peer also sends a value and
> these are negotiated to be the smaller of the 2 values.
> 
> This object represents the negotiated value.
> 
> Do we put a range on this even though it is read-only and
> we already have a range on the configurable counter-part?
> 
I would. That shows management Apps what the expected valid values 
are. It seems that I would put the same limits(range) as the
one that configures it.

Interesting explanation by the way. I always wonder why such explanations
do not get provided in the object DESCRIPTION clauses. Now things all
of a sudden make much more sense to me. Why would you assume that 
operators (who monitor these things) would easily understand all thise
details?

> 
> The range will be the same range as for the
> mplsLdpEntityKeepAliveHoldTimer object which is configurable.
> 
> >
> >- mplsLdpHelloAdjacencyHoldTimeRem
> >  could use a UNITS clause
> 
> We debated this.  If there are special values (in this case for
> 0 and for 0xffff/65535)  then if we have a UNITS "seconds" 
> is it clear that these other values are not seconds??
> 
Mmm... but you have a SYNTAX of TimeInterval... I think I meant
mplsLdpHelloAdjacencyHoldTime and there indeed it has the special values.
Sorry.. guess it was late at night.

> 
> >  I also wonder, since 0xffff has a special meaning,does that mean
> >  that that is also the max value? If so, a range of (0..65535) 
> >  would make good sense.
> 
> The object which is used to configure this value for the entity
> is mplsLdpEntityHelloHoldTimer which does have a range.
> 
> SO same question as above, do we put a range on this object even if
> it is read-only and we already have the range for the configurable
> counter-part?
> 
As stated above, I would. If you had done so, I would not have had to
ask this question. And I bet I am not the only one who would wonder.

Again, you are giving extra explantion that might be helpfull for
other/future readers as well. Might want to add that too.

> 
> 
> >
> >- I see:
> >       mplsOutSegmentLdpLspTable OBJECT-TYPE
> >         SYNTAX      SEQUENCE OF MplsOutSegmentLdpLspEntry
> >         MAX-ACCESS  not-accessible
> >         STATUS      current
> >         DESCRIPTION
> >             "A table of LDP LSP's which
> >             map to the InSegment Table in the
> >             the LSR MIB's."
> >  s/InSegment/OutSegment/ ??
> >  s/MIB's/MIB/ ??
> >
> >- I may have said this before. Could the mplsLdpSesUp and mplsLdpSesDown
> >  notification not be combined to mplsLdpSesStateChange?
> >  The included mplsLdpSesState object will identify if it is an Up or a
> >  Down, will it not?
> 
> 
> Yes, this came up.  We wanted to leave this because we do know NMS apps which
> do not decode the notifications too much but resolve by saying this notification
> matches this other notification.  So one notification causes 
> an alarm and the other notification clears it.  Not sure if it matters too
> much in this situation so we can change it as suggested above.
> 
I am not too worried. I just brought it up to make sure people had
thought about it. So up to you and the WG.
I could check if the collective set of MIB Doctors has consensus on this
(I would not be surprised if they do not). 

> Thanks again for the review.
> 
You're welcome. And thanks for the speedy follow up.

Bert
>   -Joan

---

From: "Thomas D. Nadeau" <[email-masked]>
Date: Mon, 12 May 2003 08:17:58 -0400

> > reconfigured) would not expect notifications to be generated once 
> > sessions are established.  So would not quantify X 
> notifications per 
> > second.
> > 
> > Maybe another way to ask this question is: how long does 
> the average 
> > LDP Session last?
> > 
> will be infrequent, then just add that explanation, and we're 
> OK. Do remember that a NMS may be receiveing such 
> notification from many managed devices. So if you had one a 
> second per managed device, and you had to manage 1000s of 
> devices, then there is potential still for NMS 
> overload/congestion. So some words to make people feel 
> comfortable is goodness. If the evaluation is that there 
> might be too many notifications, then we may need to add a 
> throttle object.

	Bert, 

	I don't understand what you are getting at here.
How can any one device know what the other devices in the
network are doing? Given your description above, you seem to
be advocating that every notification have a throttling value
associated with it. Although I totally agree that this is something 
that is often appropriate on a per-box-basis (indeed my boxes do this), 
having to put this into every new notification seems difficult to
implement at best. Imagine if you have accidentally not set 
these values across sevearal MIBs to be inconsistent? Imagine
the fun the agent has to go throught to appropriately throttle
each MIB's notifications.

	--Tom