The MPLS WG Archive

["Martin Dubuc" <m.dubuc@rogers.com>]

I have submitted a new version of TE link MIB to address Bert's comments. Here is my response to Bert's comments that outlines that changes that have been done to the latest version.

* * *

 

From: "Wijnen, Bert (Bert)" <bwijnen@lucent.com>
To: Martin Dubuc <m.dubuc@rogers.com>, mpls@UU.NET
Subject: AD review of: draft-ietf-mpls-telink-mib-01.txt
Date: Fri, 9 May 2003 16:35:35 +0200
MIME-Version: 1.0
X-Mailer: Internet Mail Service (5.5.2653.19)
Content-Type: text/plain;
 charset="iso-8859-1"

 

Lots of good changes have been made. This is what I still find.

 

I have now checked the example in section 7.

 

It does start off with a disclaimer that it does not show
all nuances of the MIB module. But ...

 

- When you do a createAndGo, then all row objects that do not
  have a DEFVAL need to be provided. So the way things are
  described will cause SNMP errors. If you do not want to list
  all the details, then at least I'd suggest to make a statement
  aka: appropriate values for all row objects must be passed
  for every row createAndGo action.

 

[Martin] I have defined all rows explicitly.

 

- two rows are created with createAndWait.
  I do not see they ever get activated. Was the assumption that
  they would get activted automagically, or is this an ommission?

 

[Martin] There is no reason to use createAndWait here. Have fixed it.

 

May I suggest that you make the names of your TCs start with TeLink
or some such? This to show that they are basically local TCs.
It will avoid potential future name clashes if someone wants to
define a Generic TC for a similar (or even same) concept.
Suggesteion:
   Priority             -> TeLinkPriority
   LinkProtection       -> TeLinkProtection
   SwitchingCapability  -> TeLinkSwitchingCapability
   LinkEncodingType     -> TeLinkEncodingType
This is also recommended in the draft-ietf-ops-mib-review-guidelines-01.txt

 

[Martin] Done.

 

On page 33, please remove:
      -- The mandatory groups have to be implemented
      -- by all devices supporting TE links. However, they may all
      -- be supported as read-only objects in the case where automatic
      -- configuration is supported.
This is not correct for the FullCompliance.
It is OK on page 36 (for the ReadOnlyCompliance).

 

[Martin] Removed.

 

Further comments inline (removed things where I am happy):

 

> -----Original Message-----
> From: Martin Dubuc [mailto:m.dubuc@rogers.com]
> Sent: woensdag 30 april 2003 20:09
> To: mpls@UU.NET
> Subject: RE: I-D ACTION:draft-ietf-mpls-telink-mib-00.txt
>
>
> This is my response to Bert's email which explains how I have
> addressed Bert's comments in draft-ietf-mpls-telink-mib-01.txt
> that was submitted this morining to IETF. My comments are
> embedded in the original mail (prefixed with [Martin]).
>
> Martin
>
> * * *
>
> Here is my review:
> - linelegth checker:
>    $ /bin/checkpage.awk
> <home/ietf/drafts/draft-ietf-mpls-telink-mib-01.txt
>    Long line at 770 with 77 chars
>    Long line at 771 with 76 chars
>    Long line at 1268 with 73 chars
>    -: 3 lines longer than 72 characters, max 77
>
> [Martin] Fixed.
>
Mmm... I still se:
$ /bin/checkpage.awk <draft-ietf-mpls-telink-mib-01.txt
Long line at 720 with 77 chars
Long line at 721 with 76 chars
Long line at 741 with 73 chars
-: 3 lines longer than 72 characters, max 77

 

[Martin] Fixed.

 

> - I have not yet checked sect 7.
>
See above

 

> - sect 8.1 under ifType you refernece [IanaFamily].
>   I think the reference should be to the ianaiftype.mib
>
> [Martin] You are right. Fixed.
>
Well... the reference in the reference section is incorrect.
Check the link. It should be
   http://www.iana.org/assignments/ianaiftype-mib
instead of:
   http://www.iana.org/assignments/ianatype-mib

 

[Martin] Fixed.

 

> - for my understanding, the figure in sect 8.2, can you
> explain which layers
>   in the stack are bundle(s), telink or component link. In
> fact it might be
>   good to add that explanantion to the text, so that people
> can read it too.
>
> [Martin] Actually the example is not very clear and your
> comment has triggered a flurry of mails between the authors.
> We have updated the example to make it easier for readers
> to understand. opticalTrans are component links.

 

MUCH MUCH better now. If you added the ifIndex number in the
boxes in teh fugures as well, that would improve it even more.
and I wonder, if it makes sense to use the same ifIndex
numbers in section 7 and then also point to the figure.
That allows people to better link between objects and a
visual picture of it. Just a thought.

 

[Martin] ifIndex have been added to figure in Section 8.2.
Examples in Section 7 and Section 8.2 use the same identifiers.

 

> - May I recommend that
>      teLinkIpAddrType             InetAddressType,
>      teLinkIpAddr                 InetAddress,
>      teLinkRemoteIpAddr           InetAddress,
>   Be renamed to:
>      teLinkAddressType             InetAddressType,
>      teLinkLocalAddress            InetAddress,
>      teLinkRemoteAddress           InetAddress,
>   First of all, there can be an unnumbered addresstype (unknown), so then
>   it is not an IpAddress.
>   Second, I think that the first address is the local address (that is on
>   the device where the instance of the object lives, no?
>
>   Further I wonder.. if it is an unnumbered link, it has an identifier somehow
>   does it not. Why would we not put that identifier in the address in that case
>   and describe what it looks like. In fact I wonder if instead of  the
>   InetAddress.... TCs would it not be better to use TeHopAddres... TCs from the
>   MPLS-TC-MIB? It allows for TeHopAddressUnnum as an address
>
>   In any event, I would not write:
>     teLinkIpAddrType OBJECT-TYPE
>       SYNTAX        InetAddressType
>       MAX-ACCESS    read-create
>       STATUS        current
>       DESCRIPTION
>         "For IPv4 and IPv6 numbered links, this object represents the
>          IP address type associated with the TE link. For
>          unnumbered links, a value of unknown(0) must be used."
>   But rather something aka:
>     teLinkAddressType OBJECT-TYPE
>       SYNTAX        InetAddressType
>       MAX-ACCESS    read-create
>       STATUS        current
>       DESCRIPTION
>         "The type of Internet address for the TE link. Only IPv4 and
>          IPv6 and unknown (for unnumbered links) need to be supported."
>  
>   Then forther in the Address itself I would do something aka (not that
>   the first sentence is REQUIRED as per RFC3291):
>     teLinkLocalAddress OBJECT-TYPE
>       SYNTAX        InetAddress
>       MAX-ACCESS    read-create
>       STATUS        current
>       DESCRIPTION
>         "The local Internet address for the TE link. The type of this
>          address is determined by the value of the teLinkAddressType
>          object. For an unnumbered TE link, the address represents an
>          unnumbered interface in this format:
>
>               octets   contents               encoding
>               1-4      unnumbered interface   network-byte order
>         "
>   I stole some of the text from MPLS-TC-MIB for now
>
> [Martin] I have changed the name teLinkIpAddrType to teLinkAddressType per
> your recommendation. However, I have not changed teLinkIpAddr and
> teLinkRemoteIpAddr because these fields are really meant to be IP addresses.
> If address type is unnumbered, then these fields should not be used (the
> teLinkIncomingIfId and teLinkOutgoingIfId fields should be used instead).
> It looks like this is not clear enough in the MIB, so
> I have added text to clarify this point.
>
Mmm... I see. So I still would change the definition to something
as follows (the 2nd sentence is manadatory according to RFC3291,
and it is also important to indicate what the value of this object is
(zero length string) in case of unnumberd (i.e. unknown type) link):

 

teLinkLocalIpAddr OBJECT-TYPE
   SYNTAX        InetAddress
   MAX-ACCESS    read-create
   STATUS        current
   DESCRIPTION
       "The Local Internet Address for numbered links. The type of this
        address is determined by the value of the teLinkAddressType
        object.

 

        For IPv4 and IPv6 numbered links, this object represents the
        local IP address associated with the TE link. For an unnumbered
        link, the local address is of type unknown and this this object
        is set to the zero length string and the teLinkOutgoingIfId
        object then identifies the unnumbered 'address'."
   ::= { teLinkEntry 2 }

 

[Martin] Updated accordingly. Have also updated teLinkRemoteIpAddr.

 

> - Would it make sense to elaborate a bit in the DESCRIPTION clause of
>   teLinkProtectionType and explain what the values exactly mean?
>   Or if they are explained in that GMPLS-OSPF doc that you
>   refernce, then at least make that statement.
>
> [Martin] The values are specified in GMPLS-OSPF, but are actually described in
> GMPLS-ROUTING. I have added this statement (and the new reference).
>
Mmm... might have better been added to the TC now that you made it a TC.

 

[Martin] It is not a TC. Only used in one place.

 

> - When I see a table like this:
>    teLinkDescriptorTable
>    TeLinkDescriptorEntry ::= SEQUENCE {
>       teLinkDescriptorId           Unsigned32,
>       teLinkEncodingType           INTEGER,
>       teLinkDescrPriority          Unsigned32,
>       teLinkMinReservableBandwidth Unsigned32,
>       teLinkMaxReservableBandwidth Unsigned32,  
>       teLinkDescrRowStatus         RowStatus,
>       teLinkDescrStorageType       StorageType
>   Then I always wonder if we cannot make it easier for people to see which
>   objects are indeed in that table. The first object name teLinkDescriptorId
>   clealrly is in this table. Then we have one teLinkEncodingType that is not
>   so clear. Then we have one that is not as clear as the 1st one, but still
>   gives some clue. Then we have 2 that give not clue, then we have 2 more
>   that give some clue. How about naming them:
>    TeLinkDescriptorEntry ::= SEQUENCE {
>       teLinkDescriptorId                Unsigned32,
>       teLinkDescrEncodingType           INTEGER,
>       teLinkDescrPriority               Unsigned32,
>       teLinkDescrMinReservableBandwidth Unsigned32,
>       teLinkDescrMaxReservableBandwidth Unsigned32,  
>       teLinkDescrRowStatus              RowStatus,
>       teLinkDescrStorageType            StorageType
>   If you can agree with that, then pls check you other tables. The same
>   issue/concern comes back in a few other tables.
>   In fact our mib guidelines document does discuss this issue.
>
> [Martin] Although it is not obvious why some entries do not have the same table
> prefix, some others have been shortened because of the 32 character limit
> warning for identifiers reported by smilint. I have changed the
> identifiers so that they have the same prefix but stayed within
> limit of 32 characters.
>
Mmm... the 32 limit is "recommended", but not a hard limit (that is why smilint
gives a warning and not an error). The hard limit is 64. And we have in fact
relaxed quite a bit over the last year or so on this. Again, this is clearly
described in the mib guidelines document.

 

I would really like the teSrlgTable to have prefixes that start with
teSrlg instead of just srlg. That will avoid future possible name
conflicts much easier.

 

[Martin] Added 'te' prefixes to srlg rows.

 

> - Your teLinkNotifEnable would be better named:
> teLinkNotificationsEnabled
>
> [Martin] Changed.
>
Thanks

 

> - I worry about the number of notifications that can get generated per minute
>   or per second? Can you say something about it? Do we need an abject that
>   lets an NMS configure how many notification an agent can send at most per
>   minute?
>
> [Martin] is only one type of notification in the TE link MIB module.
> This notification can only occur if the provider uses bundled link and will
> be generated only when there is a mismatch in a link bundle. Since link bundles
> and TE links are provisioned by users and not on a regular basis, and since
> mismatches are not likely to be common, the number of notifications should be
> extremely small. I do not think there is a need to throttle
> these notifications.
>
If possible, might want to add some text to DESCRIPTION clause. Will help
people understand it better.

 

[Martin] See below.

 

> - linkBundleMismatch
>   I do not understand the DESCRIPTION clause at all. Specifically, if an error
>   is found and the notification is sent, then the 1 second later, the same
>   situation probably still exists. Is another notification sent?
>   Should such a problem not be detected when someone creates an antry in
>   a table and should create operation not be prevented from succeeding?
>
> [Martin] I expected that the notification would be sent only when the
> mismatch was detected. We could prevent a create to succeed if we
> detect a mismatch, but it may be easier to just flag the error.
> I don't know how others feel about this.
>
In general it is better to return an error on an SNMP SET when someone
tries to do somthing bad instead of letting it pass and generate a
notification. Note that the error would go back to the offender, while
you have no idea where the notification goes.

 

[Martin] I don't have a strong case for this notification. I agree that
it is better to flag the error as it appears. Furthermore, if we keep
this notification, we need another one to notify when condition is cleared.
This becomes a bit too involved. For this reason, I have decided to remove
this notification.

 

>    I wonder if you do not need ipv4z and/or ipv6z.  As long as you have
>    evaluated this (with the WG) and decide that you do not need it, then
>    fine.
>
> [Martin] The issue of what to do with ipv4z and ipv6z has not been raised
> by the WG or by the 3 implementations that we know of the MIB.
>
So maybe the WG SHOULD wonder about this. The question is not so much
about if it has already been implemented or not. The question is if
Traffic Engineered links can be created over Link Local or Site Local.
interfaces/links. (By the way I know that site local is being
abandoned by IPv6 WG).

 

[Martin] I have raised this issue on the MPLS mailing list.

 

> - I have trouble with:
>       OBJECT      teLinkRowStatus
>       SYNTAX      INTEGER { active(1), notInService(2),
>                             createAndGo(4), destroy(6) }
>       DESCRIPTION
>           "The notReady(3) state need not be supported."
>
>   I think what you want to specify is that createAndWait is not needed.
>   Since you allow all objects to be changed in 'active' mode, the
>   notInService may not be needed either. (I am assuming here that I
>   did understand your intent... which I am not sure of). In that case
>   something like the following example from RFC3289 would be better:
>
>     OBJECT diffServClfrStatus
>     SYNTAX RowStatus { active(1) }
>     WRITE-SYNTAX RowStatus { createAndGo(4), destroy(6) }
>     DESCRIPTION
>        "Support for createAndWait and notInService is not required."
>
> [Martin] The notInService is required since it is one of the state that
> allows writes (see explanation a few points above). notReady
> and createAndWait are not required. For read-only, only active is
> required. I have updated text in this respect.
>
Mmm... in the example in section 7, you DO show that you would do a
createAndWait for the teLinkTable, don't you?
So this COMPLIANCE statement seems to be in conflict with that.
So pls decide what it is or should be and then we need to
make a proper specification that is in sync with that.

 

It seems to me, that for your teLinkRowStatus in the rev 01 document
(assuming you do want createAndWait as per section 7) that the
statement on teLinkRowStatus should be removed.
If indeed you do not want/need createAndWait (but then sect 7 needs
an update), then it could be changed to:

 

     OBJECT      teLinkRowStatus
     SYNTAX      INTEGER { active(1), notInService(2) }
     WRITE-SYNTAX RowStatus { notInService(2), createAndGo(4),
                              destroy(6) }
     DESCRIPTION
         "The notReady(3) and createAndWait(5) states need
          not be supported."

 

[Martin] No createAndWait is required. See above.

 

> - I have trouble with:
>       OBJECT      teLinkStorageType
>       SYNTAX      INTEGER { other(1) }
>       DESCRIPTION
>           "Only other(1) needs to be supported."
>
>   I think I could live with a volatile only, but the 'other' value
>   is completely unspecified and seems to make no sense to me at all.
>   What is you intention here?
>
> [Martin] I picked this from the LSR MIB. I don't think there is a good
> reason to limit to one single value. I have removed this conformance
> statement.
>
I hope LSR MIB will remove it too!!

 

> - In security considerations...
>   - Do All tables have the same considerations? If so you may want
>     to make that explicit.
>
> [Martin] Yes, they all have the same considerations. All info in this
> MIB module are used for routing purposes. To me, they have the
> same sensitivity. Except for IP addresses, which may actually be used
> in some sort of attack if known. This is why I have a separate clause
> regarding IP addresses.
>
>   - In general, our security ADs probably want some more specificity.
>     A statement like "all tables ....." sounds as if you are
>     opting for an easy out.
>
> [Martin] It may sounds like I am opting for an easy out, but the fact of
> the matter is, all tables have the same considerations.
>
If you make an EXPLICIT statement that they ALL have the same
security considerations, then you show that you have thought about
it and have made a conscious decision (or so I think). E.g. something
like "All the tables in this MIB module have routing information in
them and so they all have the same security attributes."

 

[Martin] Made this explicit.

 

>   - On th eread-only objects... are there no issues with bandwith info?
>     Or priorities? On protection?
>     would that not reveal info that people don't want to loose?
>
> [Martin] The problem here is where you draw the line. Every MIB object
> is vulnerable and may reveal info that would be better kept secret. I have
> read the security guidelines thoroughly and my interpretation is that we
> should uncover anything that is sensitive to exposure of end-user
> personal information or may lead to potential disruption of service.
> There isn't anything in TE link MIB that relates to end-user personal
> information. There is potential for disruption of service if someone
> changes attributes in any table of the MIB because this affects routing
> (this is my first statement) and there may be disruption of service if the
> IP address are used in some form of spoofing or attack (this is my second
> statement). I don't know that there is much else to say according to the
> spirit of the security guidelines.
>
I am just trying to help here. I know Security ADs are often picky these
days. We'll see what they say when it gets to IESG. You can also send
and email to them beforehand and check if they are OK with what you have.
Feel free to copy this discussion.

 

Bert