Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4-mibv2-06.txt

"tom.petch" <cfinss@dial.pipex.com> Wed, 14 May 2008 10:49 UTC

Return-Path: <idr-bounces@ietf.org>
X-Original-To: idr-archive@megatron.ietf.org
Delivered-To: ietfarch-idr-archive@core3.amsl.com
Received: from [127.0.0.1] (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id B4D673A67F7; Wed, 14 May 2008 03:49:08 -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 D44D23A67F7 for <idr@core3.amsl.com>; Wed, 14 May 2008 03:49:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.19
X-Spam-Level:
X-Spam-Status: No, score=-1.19 tagged_above=-999 required=5 tests=[AWL=-0.891, BAYES_00=-2.599, MANGLED_BEEF=2.3]
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 k1TKdfXABFoz for <idr@core3.amsl.com>; Wed, 14 May 2008 03:48:57 -0700 (PDT)
Received: from mk-outboundfilter-5.mail.uk.tiscali.com (mk-outboundfilter-5.mail.uk.tiscali.com [212.74.114.1]) by core3.amsl.com (Postfix) with ESMTP id 7D58E3A67A2 for <idr@ietf.org>; Wed, 14 May 2008 03:48:56 -0700 (PDT)
X-Trace: 27031345/mk-outboundfilter-5.mail.uk.tiscali.com/PIPEX/$ACCEPTED/pipex-temporary-group/213.116.52.220
X-SBRS: None
X-RemoteIP: 213.116.52.220
X-IP-MAIL-FROM: cfinss@dial.pipex.com
X-IP-BHB: Once
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: ApsEAAJgKkjVdDTc/2dsb2JhbACLU6IgBA
X-IronPort-AV: E=Sophos;i="4.27,486,1204502400"; d="scan'208";a="27031345"
X-IP-Direction: IN
Received: from 1cust220.tnt102.lnd4.gbr.da.uu.net (HELO allison) ([213.116.52.220]) by smtp.pipex.tiscali.co.uk with SMTP; 14 May 2008 11:47:29 +0100
Message-ID: <006b01c8b5a6$e92b8080$0601a8c0@allison>
From: "tom.petch" <cfinss@dial.pipex.com>
To: Jeffrey Haas <jhaas@pfrc.org>, Joan Cucchiara <jcucchiara@mindspring.com>
References: <06f701c88b5b$22622000$6601a8c0@JoanPC> <20080503223750.GG23560@scc.mi.org>
Date: Wed, 14 May 2008 11:41:15 +0200
MIME-Version: 1.0
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook Express 6.00.2800.1106
X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2800.1106
Cc: idr@ietf.org
Subject: Re: [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
Reply-To: "tom.petch" <cfinss@dial.pipex.com>
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

As far as I can see, there is but one place where working group feedback is
requested and that is for
> 5) InetAddress/InetAddressType
but I am unclear what point you are asking about:-(

I do think that Joan's suggestion of moving SIZE into CONFORMANCE is a good one;
who knows what the RRG might come up with in order to reduce FIB size and
churn:-)

But I suspect your question is about something else; is it that RFC2545 alows 32
octet addresses? or what?  I am unclear. (I do think that specific values - 4,
16 etc - are better than 0..16 except of course for the prefix in question).

Tom Petch

----- Original Message -----
From: "Jeffrey Haas" <jhaas@pfrc.org>
To: "Joan Cucchiara" <jcucchiara@mindspring.com>
Cc: "Dan Romascanu (E-mail)" <dromasca@avaya.com>; "David Ward"
<dward@cisco.com>; "MIB Doctors (E-mail)" <mib-doctors@ietf.org>; <idr@ietf.org>
Sent: Sunday, May 04, 2008 12:37 AM
Subject: Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4-mibv2-06.txt


> Joan,
>
> Thank you for the quick review and many apologies a delayed response.
> Please find my response inline.  Once your issues and any further issues
> raised by the working group have been addressed, I will issue a new
> version of this MIB.
>
> Working group - within this response are sections marked:
> *** Working group feedback requested:
>
> Even if you're not strongly interested in MIB work, please inspect these
> sections as it may have further standardization implications.
>
> On Fri, Mar 21, 2008 at 08:54:50AM -0500, Joan Cucchiara wrote:
> > First, MIB compiler output is given, followed by "General Comments"
> > on the document, then "Specific Comments".
>
> I hadn't run smicng on this.  I will add this to my toolkit for the
> future.
>
> > SMICNG output:
> > =============
> > W: f(BGP4-MIB), (30,22) The first revision should
> > match the last update for MODULE-IDENTITY bgp
>
> Corrected.
>
> > E: f(BGP4-MIB), (1834,13) Item "bgpAfPathAttrUnknownFlags"
> > in sequence "BgpAfPathAttrUnknownEntry" has conflicting syntax specified
>
> Corrected.
>
> > E: f(BGP4-MIB), (233,13) Index item "bgpPeerAfInstance" must be
> > E: f(BGP4-MIB), (1041,13) Index item "bgpPeerAfInstance" must be
> > E: f(BGP4-MIB), (1152,13) Index item "bgpNlriIndex" must be
> > E: f(BGP4-MIB), (1153,13) Index item "bgpPeerAfInstance" must be
> > E: f(BGP4-MIB), (1362,13) Index item "bgpPeerAfInstance" must be
> > E: f(BGP4-MIB), (1371,13) Index item "bgpAdjRibsOutIndex" must be
> > E: f(BGP4-MIB), (1445,13) Index item "bgpAfPathAttrIndex" must be
> > E: f(BGP4-MIB), (1711,13) Index item "bgpAsPathIndex" must be
> > E: f(BGP4-MIB), (1712,13) Index item "bgpAsPathSegmentIndex" must be
> > E: f(BGP4-MIB), (1713,13) Index item "bgpAsPathElementIndex" must be
> > E: f(BGP4-MIB), (1825,13) Index item "bgpAfPathAttrUnknownIndex"
> > E: f(BGP4-MIB), (1826,13) Index item "bgpAfPathAttrUnknownCode"
> > defined with syntax that includes a range
>
> This was intentional.  The full range was to be made available for
> implementation since this is an opaque handle.  Did you want the
> Unsigned32 range to be added?
>
> > E: f(BGP4-MIB), (1228,5) Item "bgpNlriPrefixType" has invalid
> > value for MAX-ACCESS
>
> I don't see an error here?
>
> > W: f(BGP4-MIB), (1703,5) Sequence "BgpAsPathEntry" and
> > Row "bgpAsPathTableEntry" should have related names
>
> I have globally substituted bgpAsPathTableEntry for bgpAsPathEntry.
>
> > W: f(BGP4-MIB), (2689,5) Table "bgpRcvdPathAttrTable" and
> > Row "bgpPathAttrEntry" should have same name prefix
>
> This issue was previously existing for earlier versions of this MIB and
> cannot be changed.
>
> > 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
>
> This was intentionally done to mirror the semantics of bgpPathAttrEntry.
> This yields a MIB walk that presents common prefixes first followed by
> the peers that advertise them.
>
> > 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
>
> This table, however, wasn't in a consistent order with bgpNlriEntry.
> I've corrected that.
>
> > 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
>
> These tables contain common information that may be shared by a number of
> entries within a given table and thus represents a many to one
> relationship.  Is there a different way you would prefer to see this
> represented?
>
> > 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
>
> These issues are the result of the SMIv1 to SMIv2 porting issues that
> were largely addressed by RFC 4273.  Consensus at the time was to not
> alter the MAX-ACCESS of those objects.
>
> > 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), (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"
>
> These have been corrected to be only in the MANDATORY-GROUPS section.
>
> > E: f(BGP4-MIB), (1976,15) Group "bgpAsPathGroup" is both a MANDATORY
> > and conditional group for module "BGP4-MIB"
>
> Per feedback, this group has been made completely optional.
>
> >
> > SIMPLE WEB
> > ----------
> > Severity level requested: 6
>
> All comments from this output have been addressed above and are
> artifacts of the RFC 4273 work.
>
> > 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.
>
> Three of the four TEXTUAL-CONVENTIONs in this draft would be appropriate
> to put into a separate document: BgpIdentifierTC, BgpAfiTC and
> BgpSafiTC.  I will separate these into a separate draft for the next rev
> of this document.
>
> The fourth TEXTUAL-CONVENTION, BgpPathAttrFlagsTC seems appropriate to
> leave in this document.  I don't ever expect it to be used elsewhere.
>
> > 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.
>
> One of the most contentious portions of feedback to prior structure
> attempts for this MIB was configuration objects or read-write objects
> vs. no-config and read-only.  The working group consensus was that the
> complexity of BGP configuration, especially with regards to policy, was
> so complicated and vendor-specific that trying to represent it in a MIB
> was inappropriate.  The remaining vendor-common functionality, such as
> configuring BGP peering sessions, was the MIB would interfere with
> required vendor-specific knobs.
>
> > 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."
>
> All objects that are Integer32 are part of the SMIv1 to SMIv2 work
> completed in RFC 4273 and cannot be altered.
>
> > 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.
>
> The primary purpose of the bgpPeerAfTable is to provide most of the key
> information relating to the defintion of a BGP peering session.  This
> also gives us a place to put the small pieces of status that are
> pertinent to the status of the peering session including its status and
> other components related to its transport session.
>
> > 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.
>
> I believe operationally that keeping this information in a single table
> for walk purposes would be consistent with operator expectations based
> on common CLI output.  I don't think operators really want cluttered MIB
> walks where they have to aggregate the table output themselves to
> correlate against what they were previously getting together.  Compare
> this against bgpPeerTable.
>
> I'm certainly willing to be argued out of this if it's to the benefit of
> operators or future expandability of the MIB.
>
> > 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.
>
> I have made the suggested changes to the following objects:
> bgpPeerAfLocalAddr, bgpPeerAfRemoteAddr SIZE(4|16|20)
> bgpNlriPrefix SIZE(0..16) - note that the previous SIZE(4..20) was an
> error.
>
> I have left bgpAfPathAttrLinkLocalNextHop alone at SIZE(20) since it is
> constrained by RFC 2545.
>
> *** Working group feedback requested:
> The original defintion of the supported sizes was 4..20.  The intention
> was to support IPv4, IPv6 and to not preclude IPv6 link local peering
> sessions.  Since stronger wording is now required, could I have feedback
> on this verbiage?
>
> OBJECT bgpPeerAfLocalAddr
> SYNTAX InetAddress (SIZE(4|16|20))
> DESCRIPTION
>     "An implementation is required to support IPv4 peering
>      sessions.  An implementation MAY support IPv6 peering
>      sessions.  IPv6 link-local peering sessions MAY
>      supported by this MIB."
>
> The reason for this wording is that we've never come to proper consensus
> about ipv6-only routers.
>
> TODO:
>
> > 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.
>
> I believe I commented in each of the tables what that table had replaced
> the deprecated table.  The primary explanation of why the tables have
> been deprecated is in the RFC text - namely that the tables are being
> replaced by more flexible tables that can accomodate more reachability.
> Did you think this needed to be carried over to the Table DESCRIPTIONS?
>
> Updating each deprecated table column's DESCRIPTION with the new columns
> in the new table, especially where there is an obvious match, seemed
> excessive.  Was this what you wanted done?
>
>
> > 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/
>
> s/along//
>
> > 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.
>
> Added clarifying text at the place of first use:
>
> bgpPeerAfTable - The BGP peer table.  This table is capable of
> representing IPv6 and other address-family (Af) independent sessions.
> This table replaces the bgpPeerTable from previous versions of this MIB.
>
> bgpPrefixCountersTable - A table of per-peer per Address Family
> Identifer-Subsequent Address Family Identifier (AFI-SAFI) [RFC 4760]
> counters for prefixes.
>
> > Section 5.4
> > -----------
> >
> > For the TC could you use:
> >
> > BgpAddressFamilyIdentifierTC
> >
> > BgpSubsequentAddressFamilyIdentifierTC
> >
> > s/BgoPathAttrFlagsTC/BgpPathAttrFlagsTC
>
> Done.
>
> > 5.6 Extensions
> > -----------------
> > How do you plan to enforce using bgpExtensions for
> > other MIB Modules?
>
> Please see draft-jhaas-idr-bgp4-mibv2-community-00 as an example.
>
> > 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?
>
> The integrity of the internal structures for BGP peering sessions in an
> implmentation is a somewhat out of scope here.  Most implementations
> would not use the same index from instantiation to instantiation but
> some implementations will.  Much like other forms of SNMP
> discontinuities (i.e. counters) you're correct in that there may be some
> concern as to whether one of these abstract indices means the same thing
> from query to query.
>
> This applies also to bgpAsPathIndex and bgpAfPathAttrUnknownIndex.
>
> I believe standard practice would be to include an object that marks
> discontinuities.  Do you have a reference to a suggested discontinuity
> object that would address this problem?
>
> > 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/
>
> Done.
>
> > Specific Comments on the MIB Module
> > -----------------------------------
> >
> > BgpIdentifierTC
> >
> > *) DESCRIPTION is somewhat awkward.
>
> Very.  How about:
> "The representation of a BGP Identifier.  BGP Identifiers
>  are presented in the received network byte order.
>
>  The BGP Identifier is displayed as if it is an IP address,
>  even if it would be an illegal one."
>
> The intention is to say that the octet string contains the on-the-wire
> representation, much like an IP address.
>
> > *) 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.
>
> IpAddress would be appropriate.  Additionally, the semantics of this
> protocol field have trended somewhat away from being required to be an
> IP address to being "just a 4 octet number".
>
> I chose a new TC for that reason.
>
> > 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.
>
> Addressed above.
>
> > BgpPathAttrFlagsTC
> >
> > *) Please rename to BgpPathAttributeFlagsTC
>
> Done.
>
> >    -- 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?
>
> The objects.  I've changed this to:
>
> -- { bgp 2 } and { bgp 3 } have been deprecated and are documented
>
> > 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?)
>
> RFC 4273 object and thus cannot be changed.
>
> > 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.
>
> The original intention was to cluster the transport session information
> together.  The table has also undergone churn during design.  Does the
> following address your concerns?
>
>     BgpPeerAfEntry ::= SEQUENCE {
>         -- INDEX information
>         bgpPeerAfInstance
>             Unsigned32,
>         bgpPeerAfLocalAddrType
>             InetAddressType,
>         bgpPeerAfLocalAddr
>             InetAddress,
>         bgpPeerAfRemoteAddrType
>             InetAddressType,
>         bgpPeerAfRemoteAddr
>             InetAddress,
>
>         -- Local
>         bgpPeerAfLocalPort
>             InetPortNumber,
>         bgpPeerAfLocalAs
>             InetAutonomousSystemNumber,
>
>         -- Remote
>         bgpPeerAfRemotePort
>             InetPortNumber,
>         bgpPeerAfRemoteAs
>             InetAutonomousSystemNumber,
>         bgpPeerAfIdentifier
>             BgpIdentifierTC,
>
>         -- Session status
>         bgpPeerAfAdminStatus
>             INTEGER,
>         bgpPeerAfPeerState
>             INTEGER,
>         bgpPeerAfConfiguredVersion
>             Unsigned32,
>         bgpPeerAfNegotiatedVersion
>             Unsigned32
>     }
>
> > *) Additionally, are all the indexes necessary.
>
> I'm afraid so.
>
> RFC 1771 language originally suggested that there was a single BGP
> peering session with a remote router.  Multiple peering sessions were
> permitted, but only if the all of the endpoints for a peering session
> were different.  RFC 4271 language was altered so that multiple peering
> sessions to one of the endpoints was permitted.
>
> The instance object was requested by other working groups to cover the
> cases where multiple bgp instances were running on the same platform
> under the same administrative engine.  This provided more flexibility
> than the ENTITY-MIB would readily permit when centralized access to
> peering information was desired.
>
> > *) 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.
>
> See above for the comment about the set of information being common
> under existing CLIs.
>
> More importantly for a MIB, the INDEX represents the fact that a single
> BGP peering session is uniquely identified by that 5 tuple.
>
> I don't believe we'll get much in the way of efficiencies by doing this
> split since the index components aren't separable.
>
> > 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.
>
> I have tightened the range to be 1..4294967295.
>
> > 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.
>
> Yes.  What language would you suggest instead?
>
> > bgpPeerAfPeerState
> >
> > *) the name of this object is misleading.  This is
> > the state of the connection so please rename.
>
> The name is consistent with the previous bgpPeerState object.  What
> would you suggest instead?
>
> > bgpPeerAfAdminStatus
> > *) why is this read-only and not read-write?
>
> While this is probably a reasonable exception, working group consensus
> was to not add new writeable objects.
>
> > bgpPeerAfConfiguredVersion
> > *) why is this read-only and not read-write?
>
> This represents the value configured for the peering session.  This MIB
> does not permit the configuration of BGP peering sessions.  See comments
> above.
>
> > bgpPeerAfLastErrorReceived
> > bpgPeerAfLastErrorSent
> > *) Why did you choose to represent this as a 2 byte
> > OCTET STRING as opposed to ErrorCode and ErrorSubCode (2 objects)?
>
> I've split this into two objects.
>
> > *) General Comment for this table, may want to group all the
> > received objects together followed by the sent objects.
>
> Reordered to group received and sent together as suggested.
>
> > bpgPeerAfLastErrorReceivedText
> > bpgPeerAfLastErrorSentText
> > *) Could a REFERENCE clause be added for these 2 objects?
>
> "This object contains an implementation specific explanation of the
> error that is being reported."
>
> The appropriate reference would be vendor-specific.
>
> > 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
>
> How about the following text added to the DESCRIPTION:
> If the peer has never reached the established state, the value remains zero.
>
> > 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?
>
> No configuration is permitted in this MIB per working group consensus.
>
> > *) Also the name of this table, might be better to take out
> > the "Peer" so bgpConfiguredTimersTable?
>
> The intention had been to convey that these timers were per-peer.  If
> you strongly object I can change this.
>
> > bgpPeerAfNegotiatedTimersTable
> >
> > *) Could we consider renaming some of these tables?
> > This is bgpPeeringSession info, correct?
>
> Yes.  Let's revisit this point once we've converged on the fate of the
> peer table.
>
> > *) Timers in this and the previous table were Integer32,and
> > should be Unsigned32.
> > see rfc4181 section 4.6.1.1
>
> I believe the only Integer32 counters were ones that could not be
> changed from RFC 4273.
>
> > bgpPeerAfCountersTable
> >
> > *) Is there any Discontinuity Time Object(s) associated with
> > these counters?
>
> No.  See above for my question about how we should handle
> discontinuities.
>
> > 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.
>
> I've removed that text.
>
> > bpgPeerAfFsmEstablishedTrans
> > *) Please rename to bgpPeerAfFsmEstablishedTransitions
>
> Done.
>
> > bpgPrefixCountersTable
> >
> > *) The name says counters but the table has
> > Gauge32 objects.
>
> Counters was meant in the general English sense rather than the SNMP
> sense.  Do you have a better suggestion?
>
> > bgpPrefixInPrefixes
> > bgpPrefixInPrefixesAccepted
> > bgpPrefixOutPrefixes
> >
> > *) Could references be added to these objects
>
> Done.
>
> > *) Counters need to be pluralized.
>
> ?
>
> > *) Discontinuity for these?
>
> The discontinuity would be the same as the bgp peering session.  See
> above for questions about a general strategy for this.
>
> > *) 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?
>
> The prefix counters were a key request for the new MIB.  I'm not sure
> splitting this into a separate MIB makes sense.
>
> >    bgpRib
> >        OBJECT IDENTIFIER ::= { bgp 11 }
> >
> > *) Please rename to bpgRibObjects
>
> While the names don't necessarily make good SNMP MIB design sense, in my
> opinion they make proper protocol sense.  The BGP routing information
> base consists of network layer reachability which includes prefix and
> path attribute tuples stored in several logical views: The adjacent
> ribs-in, local rib and adjacent ribs-out.
>
> > bpgNlriTable
> >
> > *) Could REFERENCES be given for this table?
>
> Done.
>
> > *) 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.
>
> In this particular case, I think things are probably correctly grouped.
> Since this is the basis case for the messy BGP AFI-SAFI version of a
> prefix versus the InetAddressType/InetAddress version of a prefix I'll
> copy in the relevant entries:
>
>     bgpNlriEntry OBJECT-TYPE
>         SYNTAX BgpNlriEntry
>         MAX-ACCESS not-accessible
>         STATUS current
>         DESCRIPTION
>             "Information about a path to a network."
>         INDEX {
>             bgpNlriAfi,
>             bgpNlriSafi,
>             bgpNlriPrefix,
>             bgpNlriPrefixLen,
>             bgpNlriIndex,
>             bgpPeerAfInstance,
>             bgpPeerAfLocalAddrType,
>             bgpPeerAfLocalAddr,
>             bgpPeerAfRemoteAddrType,
>             bgpPeerAfRemoteAddr
>         }
>         ::= { bgpNlriTable 1 }
>
>     BgpNlriEntry ::= SEQUENCE {
>         bgpNlriIndex
>             Unsigned32,
>         bgpNlriAfi
>             BgpAddressFamilyIdentifierTC,
>         bgpNlriSafi
>             BgpSubsequentAddressFamilyIdentifierTC,
>         bgpNlriPrefixType
>             InetAddressType,
>         bgpNlriPrefix
>             InetAddress,
>         bgpNlriPrefixLen
>             InetAddressPrefixLength,
>         bgpNlriBest
>             TruthValue,
>         bgpNlriCalcLocalPref
>             Unsigned32,
>         bgpAfPathAttrIndex
>             Unsigned32,
>         bgpAfPathAttrUnknownIndex
>             Unsigned32
>     }
>
> Within BGP, a prefix is defined by the tuple <AFI, SAFI, Prefix Length,
> Prefix>.  The AFI defines the general IANA address type of the prefix.
> The SAFI, however, may imply additional semantics.  Consider the case of
> an RFC 3107 MPLS labeled prefix.  The prefix encoding is thus:
>
> <1,4,32,777,10.0.0.0>
>
> This corresponds to a labeled route with label 777 for prefix 10/8.  The
> label counts as 24 bits of length.
>
> For MIB access, the InetAddressType doesn't do us any good but is required
> by InetAddress.  AFI, as carried in BGP, may be a subset of the entries
> allowed by InetAddressType and thus I kept them different.
>
> RFC 3107 allows multiple instances of the same prefix to be advertised
> so long as they have different label stacks.  The bgpNlriIndex permits
> these multiple instances.  An extension MIB would give access to the
> label stack by AUGMENTing this table.
>
> Even for the non RFC 3107 case, IDR has been churning over the
> possibility of adding extensions that permit multiple routes for the
> past several years.  I don't relish a full set of table revisions just
> to accomodate such a change - this is meant to future proof the table
> somewhat.
>
> Does this help?
>
> > bgpAfPathAttrCount
> >
> > *) Why is this object needed?
>
> It is a common operational statistic polled from routers, especially for
> memory usage.  Since it is related to path attributes it went into the
> RIB related MIB.
>
> > *) Please keep in mind that Counter type objects need to be
> > pluralized.
>
> I renamed this bgpAfPathAttrCounter.
>
> > bgpAfPathAttrTable
> > *) Noncontiguous numbering between bgpAfPathAttrTable (4) and
> > bgpAsPathTable (6) (Number 5 is missing)
> > Should this be renumbered?
>
> Renumbered.  5 was deleted at some point during design.
>
> > bgpAfPathAttrNextHop
> >
> > *) Please rename bgpAfPathAttrNextHopAddr (to go along with
> > bgpAfPathAttrNextHopAddrType)
>
> Done.
>
> >
> > bgpAfPathAttrLinkLocalNextHop
> > *) Please add an AddrType object
> > prior to this and use the Conformance Statement to specify what is
> > supported.
>
> Done.  For the record, I asked 2 other people about this and you're the
> tie-breaking voice. :-)
>
> > bgpAfPathAttrMedPresent
> >
> > *) Please add to the DESCRIPTION an explanation of what true(1) means?
> >
>
> Done.
>
> > bgpAfPathAttrLocalPref
> >
> > *) What does the DESCRIPTION mean when it says: "this object will be
> > absent"?
>
> No instance of this object will be present in the MIB for that index.
>
> > bgpAfPathAttrAggregatorAS and bgpAfPathAttrAggregatorAddr
> >
> > *) What does the DESCRIPTION mean when it says:
> > "this object will not be present in the conceptual row"?
>
> Same as bgpAfPathAttrLocalPref.
>
> > 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.
>
> See above for questions about handling the discontinuity.
>
> > 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?
>
> Yes.  This is a problem with the object present in RFC 4273, although it
> is worse for the backwards transition notification.
>
> > bgpAfBackwardTransNotification
> >
> > *) Please spell out what "Trans" stands for?
>
> Done.
>
> > Conformance Section
> > *) If read-write objects are added to the MIB, then a read-only conformance
> > may be wanted.
>
> No read-write objects are present that were not part of RFC 4273.
>
> -- Jeff
> _______________________________________________
> Idr mailing list
> Idr@ietf.org
> https://www.ietf.org/mailman/listinfo/idr

_______________________________________________
Idr mailing list
Idr@ietf.org
https://www.ietf.org/mailman/listinfo/idr