[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Idr] Early MIB Dr. Review of draft-ietf-idr-bgp4-mibv2-06.txt
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 at pfrc.org>
To: "Joan Cucchiara" <jcucchiara at mindspring.com>
Cc: "Dan Romascanu (E-mail)" <dromasca at avaya.com>; "David Ward"
<dward at cisco.com>; "MIB Doctors (E-mail)" <mib-doctors at ietf.org>; <idr at 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 at ietf.org
> https://www.ietf.org/mailman/listinfo/idr
_______________________________________________
Idr mailing list
Idr at ietf.org
https://www.ietf.org/mailman/listinfo/idr