[Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4-mibv2-06.txt
"Joan Cucchiara" <jcucchiara@mindspring.com> Fri, 21 March 2008 13:57 UTC
Return-Path: <idr-bounces@ietf.org>
X-Original-To: ietfarch-idr-archive@core3.amsl.com
Delivered-To: ietfarch-idr-archive@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 5CBC33A6DD9; Fri, 21 Mar 2008 06:57:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -97.496
X-Spam-Level:
X-Spam-Status: No, score=-97.496 tagged_above=-999 required=5 tests=[AWL=-1.476, BAYES_40=-0.185, FH_RELAY_NODNS=1.451, HELO_MISMATCH_ORG=0.611, RDNS_NONE=0.1, TRACKER_ID=2.003, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id q+qnp5w0wNqG; Fri, 21 Mar 2008 06:57:18 -0700 (PDT)
Received: from core3.amsl.com (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id 5CDBA3A6D74; Fri, 21 Mar 2008 06:57:18 -0700 (PDT)
X-Original-To: idr@core3.amsl.com
Delivered-To: idr@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id A3F3A3A6D67; Fri, 21 Mar 2008 06:57:16 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id nSe1elrc1aBm; Fri, 21 Mar 2008 06:57:15 -0700 (PDT)
Received: from elasmtp-galgo.atl.sa.earthlink.net (elasmtp-galgo.atl.sa.earthlink.net [209.86.89.61]) by core3.amsl.com (Postfix) with ESMTP id 9242F3A6953; Fri, 21 Mar 2008 06:57:14 -0700 (PDT)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=dk20050327; d=mindspring.com; b=AzTTjsmBOY+g/ttLjhKRP37adpQ/iEVktl/sXHDQ7hj7GsNJPbYBOUaw5iPs57S1; h=Received:Message-ID:From:To:Cc:Subject:Date:MIME-Version:Content-Type:Content-Transfer-Encoding:X-Priority:X-MSMail-Priority:X-Mailer:X-MimeOLE:X-ELNK-Trace:X-Originating-IP;
Received: from [141.154.113.33] (helo=JoanPC) by elasmtp-galgo.atl.sa.earthlink.net with esmtpa (Exim 4.67) (envelope-from <jcucchiara@mindspring.com>) id 1Jchhs-0004SA-9h; Fri, 21 Mar 2008 09:54:54 -0400
Message-ID: <06f701c88b5b$22622000$6601a8c0@JoanPC>
From: Joan Cucchiara <jcucchiara@mindspring.com>
To: jhaas@pfrc.org
Date: Fri, 21 Mar 2008 08:54:50 -0500
MIME-Version: 1.0
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook Express 6.00.2900.2180
X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.2180
X-ELNK-Trace: 4d68bbe9cb71969ea344cf2d1a8e60840a9da525759e26543e4eca31154f766dbe1ed98b79e4787e900f0771c95f4286350badd9bab72f9c350badd9bab72f9c
X-Originating-IP: 141.154.113.33
Cc: "Dan Romascanu (E-mail)" <dromasca@avaya.com>, David Ward <dward@cisco.com>, "MIB Doctors (E-mail)" <mib-doctors@ietf.org>, idr@ietf.org
Subject: [Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4-mibv2-06.txt
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/pipermail/idr>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: idr-bounces@ietf.org
Errors-To: idr-bounces@ietf.org
Jeff, This is an early MIB Dr. review of draft-ietf-idr-bgp4-mibv2-06.txt. First, MIB compiler output is given, followed by "General Comments" on the document, then "Specific Comments". Thanks, Joan SMICNG output: ============= W: f(BGP4-MIB), (30,22) The first revision should match the last update for MODULE-IDENTITY bgp E: f(BGP4-MIB), (1834,13) Item "bgpAfPathAttrUnknownFlags" in sequence "BgpAfPathAttrUnknownEntry" has conflicting syntax specified E: f(BGP4-MIB), (233,13) Index item "bgpPeerAfInstance" must be defined with syntax that includes a range E: f(BGP4-MIB), (1041,13) Index item "bgpPeerAfInstance" must be defined with syntax that includes a range E: f(BGP4-MIB), (1152,13) Index item "bgpNlriIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1153,13) Index item "bgpPeerAfInstance" must be defined with syntax that includes a range E: f(BGP4-MIB), (1228,5) Item "bgpNlriPrefixType" has invalid value for MAX-ACCESS E: f(BGP4-MIB), (1362,13) Index item "bgpPeerAfInstance" must be defined with syntax that includes a range E: f(BGP4-MIB), (1371,13) Index item "bgpAdjRibsOutIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1445,13) Index item "bgpAfPathAttrIndex" must be defined with syntax that includes a range W: f(BGP4-MIB), (1703,5) Sequence "BgpAsPathEntry" and Row "bgpAsPathTableEntry" should have related names E: f(BGP4-MIB), (1711,13) Index item "bgpAsPathIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1712,13) Index item "bgpAsPathSegmentIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1713,13) Index item "bgpAsPathElementIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1825,13) Index item "bgpAfPathAttrUnknownIndex" must be defined with syntax that includes a range E: f(BGP4-MIB), (1826,13) Index item "bgpAfPathAttrUnknownCode" must be defined with syntax that includes a range W: f(BGP4-MIB), (2689,5) Table "bgpRcvdPathAttrTable" and Row "bgpPathAttrEntry" should have same name prefix W: f(BGP4-MIB), (1148,13) Row "bgpNlriEntry" does not have a consistent indexing scheme - index items from current table must come after index items from other tables W: f(BGP4-MIB), (1362,13) Row "bgpAdjRibsOutEntry" does not have a consistent indexing scheme - cannot specify an index item from additional "base row" bgpPeerAfEntry, since can have only one "base row" which is bgpNlriEntry W: f(BGP4-MIB), (1445,13) Row "bgpAfPathAttrEntry" does not have a consistent indexing scheme - index item bgpAfPathAttrIndex from base row bgpNlriEntry is not defined as an index item W: f(BGP4-MIB), (1711,13) Row "bgpAsPathTableEntry" does not have a consistent indexing scheme - index item bgpAsPathIndex from base row bgpAfPathAttrEntry is not defined as an index item W: f(BGP4-MIB), (1825,13) Row "bgpAfPathAttrUnknownEntry" does not have a consistent indexing scheme - index item bgpAfPathAttrUnknownIndex from base row bgpNlriEntry is not defined as an index item W: f(BGP4-MIB), (3144,19) Variable "bgpPeerRemoteAddr" in notification "bgpEstablishedNotification" is an index for a table W: f(BGP4-MIB), (3158,19) Variable "bgpPeerRemoteAddr" in notification "bgpBackwardTransNotification" is an index for a table E: f(BGP4-MIB), (1970,15) Group "bgpTimersGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1973,15) Group "bgpCountersGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1976,15) Group "bgpAsPathGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1979,15) Group "bgpBaseGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1982,15) Group "bgpErrorsGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1985,15) Group "bgpPeerAfGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" E: f(BGP4-MIB), (1988,15) Group "bgpAfPathAttributesGroup" is both a MANDATORY and conditional group for module "BGP4-MIB" SIMPLE WEB ---------- Severity level requested: 6 Line Severity Problem 30 3 revision date after last update 77 3 revision for last update is missing 2218 5 warning: index element `bgpPeerRemoteAddr' of row `bgpPeerEntry' should be not-accessible in SMIv2 MIB 2689 5 warning: row identifier `bgpPathAttrEntry' should have the same prefix as table identifier `bgpRcvdPathAttrTable' 5 warning: index element `bgpPathAttrDestNetwork' of row `bgpPathAttrEntry' should be not-accessible in SMIv2 MIB 5 warning: index element `bgpPathAttrPeer' of row `bgpPathAttrEntry' should be not-accessible in SMIv2 MIB 2817 5 warning: index element `bgp4PathAttrIpAddrPrefix' of row `bgp4PathAttrEntry' should be not-accessible in SMIv2 MIB 5 warning: index element `bgp4PathAttrIpAddrPrefixLen' of row `bgp4PathAttrEntry' should be not-accessible in SMIv2 MIB 5 warning: index element `bgp4PathAttrPeer' of row `bgp4PathAttrEntry' should be not-accessible in SMIv2 MIB 3177 5 warning: notification `bgpEstablished' is not reverse mappable 3191 5 warning: notification `bgpBackwardTransition' is not reverse mappable ID-NITS -------- Did NOT run through ID-NITS since it is early review. Comments: --------- General Comments: ----------------- 1) With regard to the Textual Conventions, has there been any thoughts to a separate TC document? The reason is that, there is a strong indication that more BGP MIB Modules will be forthcoming, thus, having a separate TC document would help in 2 possible ways: A. would help to clarify where the TCs were (i.e in a document named (for example) BGP4-TC-MIB ) and B. may encourage other MIB Module authors to add to the document with their TCs. 2) There aren't any read-write objects in the current MIB Module (except deprecated objects). I expect this will change in future versions of the document, but it did impact my review, since this kind of information is good to know. This will likely also impact the Conformance Section of the MIB, since a read-only Conformance might be wanted by the working group. Not to mention, either StorageType objects or verbage will need to be added. 3) Many objects were using Integer32 (timers for example) and should probably be Unsigned32. Here is relevant text from RFC4181: "If the value range is between 0..2147483647 (inclusive), and the value of the information being modelled does not increase above the maximum value nor decrease below the minimum value, then: - Unsigned32 is RECOMMENDED; - INTEGER, Integer32, and Gauge32 are acceptable." 4) I have suggested a different structure to this MIB. Currently, bgp instance, remote peer and peering session information are in the same table (bgpPeerAfTable). Many tables then AUGMENT this table. The suggestion is to separate bgpPeerAfTable into 3 tables: bgp instance information in one table remote peer information into another table, and peering session information into a 3rd table. This structure would then have a ripple effect throughout the MIB such that you will probably see some counters which AUGMENT the table with bgp instance info, some which would AUGMENT the bgp session, etc. Since other MIB Modules within BGP are to build on this MIB, then having such a flat structure is going to be cumbersome. This is only a suggestion, and since it is an early review of the MIB would like to take the opportunity to put this forth as a discussion point. (Also, my suggestion may not be the best structure for the MIB, but hopefully that would be worked out through discussion.) 5) InetAddress/InetAddressType the place for specifying the supported address types and addresses is in the conformance, not in the MIB Module. If there is a change in the supported types, then the objects with those types, would all need to be deprecated, but if it is in the conformance then only the conformance would need to be deprecated and rewritten. The exception being if the address is an index (then a SIZE should be used) see RFC4001 for details. Please see the DIFF-SERV-MIB (rfc3289) as an example of using the Conformance statements to denote Address Type and size. 6) Deprecated and Obsoleted Objects One of the other MIB Doctors (David T. Perkins) suggested to me the following and I would like to pass this on as a suggestion to you > RFC 2578 has language, sich as that found in section 10.2: > (3) A STATUS clause value of "current" may be revised as "deprecated" > or "obsolete". Similarly, a STATUS clause value of "deprecated" > may be revised as "obsolete". When making such a change, the > DESCRIPTION clause should be updated to explain the rationale. > <snip> > For deprecated items, I start the DESCRIPTION text > with something like.. > "This item is DEPRECRATED. This is done because..." > I then leave the original DESCRIPTION text. For obsoleted > items, I generally remove the original DESCRIPTION cause > text. Some of the DESCRIPTION clauses have been updated to various degrees so you are definitely on the right path, but it did seem some places where skipped and so the deprecated and obsoleted objects should be reviewed to ensure that the DESCRIPTIONs have been modified. Specific Comments ----------------- Section 4. Overview "Multi-protocol extensions [RFC4760] were introduced along which allowed advertisement of reachability such as IPv6 [RFC2545], MPLS Labeled routes [RFC3107], etc." *) Awkward (suggest rewording) s/introduced along which allowed/introduced, along with/ Section 5.2 Tables *) What does Af mean? (Address Family ?) *) What does AFI adn SAFI stand for? Okay, I see Address Family Identifier and Subsequent Address Family Identifier. Please spell out entire meaning prior to abbreviation. Section 5.4 ----------- For the TC could you use: BgpAddressFamilyIdentifierTC BgpSubsequentAddressFamilyIdentifierTC s/BgoPathAttrFlagsTC/BgpPathAttrFlagsTC 5.6 Extensions ----------------- How do you plan to enforce using bgpExtensions for other MIB Modules? WRT bgpAfPathAttrIndex, in the event of a reboot of the router, or a restart of the BGP subsystem, is all the information in the tables that reference or use this object/index lost? In other words, this object becomes an index for other tables. In the event of a restart of BGP or restart of the router, or restart of the SNMP Agent, etc. this object gets re-initialized, but what happens to the indexes that use this object? Section 6.1 --------------- *) Awkward: "Note that conducting BGP peering sessions over transport protocols other that TCP over IP are out of scope of the current BGP specifications." s/that/than/ Specific Comments on the MIB Module ----------------------------------- BgpIdentifierTC *) DESCRIPTION is somewhat awkward. *) Could InetAddress be used instead of creating a new TC? In other words, you seem to be saying this is an IPv4 Address, but I am not sure what is specific to BGP wrt to this. *) Is this the correct Reference? RFC4271(?) The definition in the Terminology section of RFC4271 was useful. BgpAfiTC *) rename BgpAddressFamilyIdentifierTC *) Please spell out what AFI stand for (both first time in the document text and first time in the MIB Module. BgpSafiTC *) rename BgpSubsequentAddressFamilyIdentifierTC *) Please spell out what SAFI stand for (both first time in the document text and first time in the MIB Module. BgpPathAttrFlagsTC *) Please rename to BgpPathAttributeFlagsTC -- bgp 2 and 3 have been deprecated and are documented -- elsewhere in this MIB *) Are you referring to the protocol versions 2 and 3, or do you mean the MIB objects that are specific to these versions have been deprecated? bgpIdentifier SYNTAX IpAddress *) According to RFC4181 IpAddress should not be used. Can InetAddress/InetAddressType or even InetAddressIPv4 be used? (Or is the BgpIdentifierTC is more appropriate?) INDEX Clause for bgpPeerAfEntry and the order of the objects in the entry *) I don't understand the ordering here. The Indexes need to uniquely identify an entry in the table and they should be at the beginning of the table. *) Additionally, are all the indexes necessary. *) Looking this over, I'd like to make a suggestion that this table be made into two (or more?) tables. This one table represents the "bgp instances" on this router AND the remote peers AND peering sessions. My suggestion would be to have a table for just the "bgp instances" to include ConfiguredVersion, AdminStatus, PeerState, Local Info, etc. and the other table for the Remote Peer Information (and perhaps, another for peering session information?) Maybe another way to think of this is when information would be available. "bgp Instances" should have information prior to Peer Information, right? bgpPeerAfInstance *) Needs to have a range beginning with 1. The DESCRIPTION is a little confusing, but I think you are saying that if there is only one BGP routing process then it is recommended that this index have the value of 1. bgpPeerAfPeerState *) the name of this object is misleading. This is the state of the connection so please rename. bgpPeerAfAdminStatus *) why is this read-only and not read-write? bgpPeerAfConfiguredVersion *) why is this read-only and not read-write? bgpPeerAfLastErrorReceived bpgPeerAfLastErrorSent *) Why did you choose to represent this as a 2 byte OCTET STRING as opposed to ErrorCode and ErrorSubCode (2 objects)? *) General Comment for this table, may want to group all the received objects together followed by the sent objects. bpgPeerAfLastErrorReceivedText bpgPeerAfLastErrorSentText *) Could a REFERENCE clause be added for these 2 objects? bpgPeerAfFsmEstablishedTime *) Do peering sessions always reach the Established State? If not, what is the value of this object (zero?). That should probably be added to the DESCRIPTION bpgPeerAfConfiguredTimersTable *) General comment (getting back to the issue of separating the tables into "bgp instance" vs. "remote peer info" vs "peering session info", this table would seem to largely be part of the bpg instance information. I don't understand why these objects are read-only? Is configuration not allowed via SNMP for these objects? *) Also the name of this table, might be better to take out the "Peer" so bgpConfiguredTimersTable? bgpPeerAfNegotiatedTimersTable *) Could we consider renaming some of these tables? This is bgpPeeringSession info, correct? *) Timers in this and the previous table were Integer32,and should be Unsigned32. see rfc4181 section 4.6.1.1 bgpPeerAfCountersTable *) Is there any Discontinuity Time Object(s) associated with these counters? DESCRIPTION: "...This object should be initialized to zero when the connection is established" *) Counters by definition cannot be reset. Please see rfc 2578.txt. You could create a TimeStamp object which is set to the time when the FSM transitioned to the established state. bpgPeerAfFsmEstablishedTrans *) Please rename to bgpPeerAfFsmEstablishedTransitions bpgPrefixCountersTable *) The name says counters but the table has Gauge32 objects. bgpPrefixInPrefixes bgpPrefixInPrefixesAccepted bgpPrefixOutPrefixes *) Could references be added to these objects *) Counters need to be pluralized. *) Discontinuity for these? *) This is "new" to the MIB and doesn't appear in rfc4273. Is it needed for BGP4's basic deployment? Was there any consideration to putting this into a separate MIB Module? bgpRib OBJECT IDENTIFIER ::= { bgp 11 } *) Please rename to bpgRibObjects bpgNlriTable *) Could REFERENCES be given for this table? *) I didn't understand the indexing and the indexes do not match the entry of this table. Very difficult to review this one and the bgpAdjRibsOutTable. Please fix the indexes to match the table entry. bgpAfPathAttrCount *) Why is this object needed? *) Please keep in mind that Counter type objects need to be pluralized. bgpAfPathAttrTable *) Noncontiguous numbering between bgpAfPathAttrTable (4) and bgpAsPathTable (6) (Number 5 is missing) Should this be renumbered? bgpAfPathAttrNextHop *) Please rename bgpAfPathAttrNextHopAddr (to go along with bgpAfPathAttrNextHopAddrType) bgpAfPathAttrLinkLocalNextHop *) Please add an AddrType object prior to this and use the Conformance Statement to specify what is supported. bgpAfPathAttrMedPresent *) Please add to the DESCRIPTION an explanation of what true(1) means? bgpAfPathAttrLocalPref *) What does the DESCRIPTION mean when it says: "this object will be absent"? bgpAfPathAttrAggregatorAS and bgpAfPathAttrAggregatorAddr *) What does the DESCRIPTION mean when it says: "this object will not be present in the conceptual row"? bgpAsPathIndex *) In the event of a reboot of the router, or a restart of the BGP subsystem, is all the information in the tables that reference or use this object/index lost? In other words, this object becomes an index for other tables. In the event of a restart of BGP or restart of the router, or restart of the SNMP Agent, etc. this object gets re-initialized, but what happens to the indexes that use this object? bgpAfPathAttrUnknownIndex *) same question as above. bgpAfEstablishedNotification *) Would the FSM ever be in a state that would fluxuate in and out of the established state? In other words, would this notification ever get flooded to the network? bgpAfBackwardTransNotification *) Please spell out what "Trans" stands for? Conformance Section *) If read-write objects are added to the MIB, then a read-only conformance may be wanted. ---------------end _______________________________________________ Idr mailing list Idr@ietf.org https://www.ietf.org/mailman/listinfo/idr
- [Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4… Joan Cucchiara
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… Jeffrey Haas
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… Joan Cucchiara
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… tom.petch
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… Jeffrey Haas
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… tom.petch
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… Jeffrey Haas
- Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-… Joan Cucchiara
- [Idr] RFC 1657 MIB errors/corrections Jeffrey Haas
- [Idr] Factoring the bgpPeerAfTable in BGP MIBv2 Jeffrey Haas
- [Idr] BGP MIBv2 discontinuity objects Jeffrey Haas
- Re: [Idr] BGP MIBv2 discontinuity objects Joan Cucchiara
- Re: [Idr] BGP MIBv2 discontinuity objects Jeffrey Haas
- Re: [Idr] BGP MIBv2 discontinuity objects Joan Cucchiara
- Re: [Idr] [MIB-DOCTORS] BGP MIBv2 discontinuity o… Jeffrey Haas
- Re: [Idr] [MIB-DOCTORS] BGP MIBv2 discontinuity o… Jeffrey Haas
- Re: [Idr] BGP MIBv2 discontinuity objects Jeffrey Haas