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