AD-review comments on draft-ietf-idr-bgp4-20

Alex Zinin <zinin@psg.com> Mon, 05 May 2003 23:41 UTC

Received: from trapdoor.merit.edu (postfix@trapdoor.merit.edu [198.108.1.26]) by ietf.org (8.9.1a/8.9.1a) with ESMTP id TAA21560 for <idr-archive@ietf.org>; Mon, 5 May 2003 19:41:20 -0400 (EDT)
Received: by trapdoor.merit.edu (Postfix) id 0BC8C9122B; Mon, 5 May 2003 19:42:35 -0400 (EDT)
Delivered-To: idr-outgoing@trapdoor.merit.edu
Received: by trapdoor.merit.edu (Postfix, from userid 56) id CDABB9122D; Mon, 5 May 2003 19:42:34 -0400 (EDT)
Delivered-To: idr@trapdoor.merit.edu
Received: from segue.merit.edu (segue.merit.edu [198.108.1.41]) by trapdoor.merit.edu (Postfix) with ESMTP id 382099122B for <idr@trapdoor.merit.edu>; Mon, 5 May 2003 19:42:32 -0400 (EDT)
Received: by segue.merit.edu (Postfix) id 2267C5E29F; Mon, 5 May 2003 19:42:32 -0400 (EDT)
Delivered-To: idr@merit.edu
Received: from psg.com (psg.com [147.28.0.62]) by segue.merit.edu (Postfix) with ESMTP id 752B45E240 for <idr@merit.edu>; Mon, 5 May 2003 19:42:31 -0400 (EDT)
Received: from psg.com ([147.28.0.62] helo=127.0.0.1) by psg.com with esmtp (Exim 3.36 #1) id 19CpbJ-000FaJ-00; Mon, 05 May 2003 23:42:29 +0000
Date: Mon, 05 May 2003 16:38:15 -0700
From: Alex Zinin <zinin@psg.com>
X-Mailer: The Bat! (v1.62i) Personal
Reply-To: Alex Zinin <zinin@psg.com>
X-Priority: 3 (Normal)
Message-ID: <177177649135.20030505163815@psg.com>
To: idr@merit.edu
Cc: rtg-dir@ietf.org
Subject: AD-review comments on draft-ietf-idr-bgp4-20
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: owner-idr@merit.edu
Precedence: bulk
Content-Transfer-Encoding: 7bit

Folks,

 Please find below my AD-review comments. Hopefully they will help
 improve the document. I tried to consult Andrew's list as much as
 possible, but do feel free to point out if something has already been
 discussed and agreed upon.
 
 Thanks go to Yakov for kicking me often enough ;)

--
Alex Zinin

Some nits:
- run it by a spelling checker, please
- disable hyphenation if possible
- include boilerplates for IPR notice, Copyright notice

General comment:

  in some places I highlighted the fact that required behavior is not
  described using the 2119 language, so it is not clear if a MUST or
  SHOULD or MAY is applicable. I am sure I've missed some more places
  like this. I'd like to ask the editors to go through the doc and
  check this.

>                   A Border Gateway Protocol 4 (BGP-4)
>                       <draft-ietf-idr-bgp4-20.txt>
> 
> 
> Status of this Memo
> 
> 
...
>    The list of Internet-Draft Shadow Directories can be accessed at
>    http://www.ietf.org/shadow.html.
>
> Specification of Requirements

Nit: move Abstract here. Move requirements after the Acks.

> Abstract

Should the Abstract say that this spec covers IPv4 only?


> 3. Summary of Operation
...
>    This document uses the term `Autonomous System' (AS) throughout.  The
>    classic definition of an Autonomous System is a set of routers under
>    a single technical administration, using an interior gateway protocol
>    (IGP) and common metrics to determine how to route packets within the
>    AS, and using an inter-AS routing protocol to determine how to route
>    packets to other ASs. Since this classic definition was developed, it
>    has become common for a single AS to use several IGPs and sometimes
>    several sets of metrics within an AS. The use of the term Autonomous
>    System here stresses the fact that, even when multiple IGPs and met-
>    rics are used, the administration of an AS appears to other ASs to
>    have a single coherent interior routing plan and presents a consis-
>    tent picture of what destinations are reachable through it.

Ed: Since 'AS' has been defined before, do we need to repeat the
definition here?

...
>    peer in the same AS is referred to as an internal peer. Internal BGP
>    and external BGP are commonly abbreviated IBGP and EBGP.

Ed: These two have been defined before too

...
> Care must be taken to
>    ensure that the interior routers have all been updated with transit
>    information before the BGP speakers announce to other ASs that tran-
>    sit service is being provided.

What does the last sentence really mean from the implementation
perspective? It used to mean the BGP/IGP synchronization check. Now
that iBGP everywhere is assumed, how do we check this condition?

>    This document specifies the base behavior of the BGP protocol. This
>    behavior can and is modified by extention specifications.  When the
Ed: "extension"

>    protocol is extended the new behavior is fully documented in the
>    extention specifications.
Ed: "extension"

> 3.1 Routes: Advertisement and Storage
> 
>    For the purpose of this protocol, a route is defined as a unit of
>    information that pairs a set of destinations with the attributes of a
>    path to those destinations. The set of destinations are systems whose
>    IP addresses are contained in one IP address prefix carried in the
>    Network Layer Reachability Information (NLRI) field of an UPDATE mes-
>    sage, and the path is the information reported in the path attributes
>    field of the same UPDATE message.
Ed: Repeated definition again
...
>    If a BGP speaker chooses to advertise the route, it MAY add to or
>    modify the path attributes of the route before advertising it to a
>    peer.

The intent here is to say that it's ok to modify the attribute set of
a previously received route when it's announced further. The way it
reads though is that self-originated routes are also within the
context and MAY sounds like you don't have to add attributes when
announcing those.

...

>    Changing attribute of a route is accomplished by advertising a
>    replacement route. The replacement route carries new (changed)
>    attributes and has the same NLRI as the original route.

"same NLRI" implies the same prefix, but not the NLRI field, which can
be different (containing other routes), should the use of this term be
normalized throughout the document?

> 4.2 OPEN Message Format
> 
>    After a TCP is established, the first message sent by each side is an

"TCP connection"

> 5. Path Attributes
...
>    If a path with recognized transitive optional attribute is accepted
>    and passed along to other BGP peers and the Partial bit in the
>    Attribute Flags octet is set to 1 by some previous AS, it is not 

'MUST NOT' here?

> set
>    back to 0 by the current AS. Unrecognized non-transitive optional
>    attributes MUST be quietly ignored and not passed along to other BGP
>    peers.
...
>    The same attribute (attribute with the same type) can not appear more
>    than once within the Path Attributes field of a particular UPDATE
>    message.

What should an implementation do if this happens?

>    The mandatory category refers to an attribute which MUST be present
>    in both IBGP and EBGP exchanges if NLRI are contained in the UPDATE

Ed: "if the NLRI field is contained" instead?

> 5.1.2 AS_PATH
...
>       b) When a given BGP speaker advertises the route to an external
>       peer, then the advertising speaker updates the AS_PATH attribute
>       as follows:
> 
>          1) if the first path segment of the AS_PATH is of type
>          AS_SEQUENCE, the local system prepends its own AS number as the
>          last element of the sequence (put it in the leftmost position).

'Leftmost position'... isn't this still open for interpretation? How
about wording this relative to the position of the octets in the
protocol message?

>          If the act of prepending will cause an overflow in the AS_PATH
>          segment, i.e. more than 255 ASs, it is legal

What's the recommended behavior here?

>          to prepend a new
>          segment of type AS_SEQUENCE and prepend its own AS number to
>          this new segment.


> 5.1.4 MULTI_EXIT_DISC
> 
> 
>    The MULTI_EXIT_DISC is an optional non-transitive attribute which is
>    intended to be used on external (inter-AS) links to discriminate
>    among multiple exit or entry points to the same neighboring AS.  The
>    value of the MULTI_EXIT_DISC attribute is a four octet unsigned num-
>    ber which is called a metric. All other factors being equal, the exit
>    point with lower metric SHOULD be preferred. If received over EBGP,
>    the MULTI_EXIT_DISC attribute MAY be propagated over IBGP to other
>    BGP speakers within the same AS. The MULTI_EXIT_DISC attribute

seems that a reference to 9.1.2.2 is due here, as using MED in local
route calculation and not propagating it further is dangerous

>    received from a neighboring AS MUST NOT be propagated to other neigh-
>    boring ASs.
> 
>    A BGP speaker MUST IMPLEMENT a mechanism based on local configuration
                        ^^^^^^^^^lower-case
                        
>    which allows the MULTI_EXIT_DISC attribute to be removed from a
>    route. This MAY be done prior to determining the degree of preference

what's the recommended behavior here?

>    of the route and performing route selection (decision process phases
>    1 and 2).
> 
>    An implementation MAY also (based on local configuration) alter the
>    value of the MULTI_EXIT_DISC attribute received over EBGP.  This MAY
>    be done prior to determining the degree of preference of the route

what's the recommended behavior here?

> 5.1.5 LOCAL_PREF
...
> A BGP speaker SHALL calculate the degree of preference for
>    each external route based on the locally configured policy, and

Should we be more honest here and say that the implementation must
allow the admin to SET the degree of preference through the local
policy to influence the best-path selection process, i.e., I don't
think any implementation really *calculates* it.

> 5.1.6 ATOMIC_AGGREGATE
...
>    A BGP speaker that receives a route with the ATOMIC_AGGREGATE
>    attribute MUST NOT make any NLRI of that route more specific (as
>    defined in 9.1.4) when advertising this route to other BGP speakers.

Since deaggregation is not described in this document, do we need this
para?

>   A BGP speaker that receives a route with the ATOMIC_AGGREGATE
>    attribute needs to be cognizant of the fact that the actual path to
>    destinations, as specified in the NLRI of the route, while having the
>    loop-free property, may not be the path specified in the AS_PATH
>    attribute of the route.

What does this really mean from the implementation perspective?

> 5.1.7 AGGREGATOR
> 
> 
>    AGGREGATOR is an optional transitive attribute which MAY be included
>    in updates which are formed by aggregation (see Section 9.2.2.2). A
>    BGP speaker which performs route aggregation MAY add the AGGREGATOR

What's the recommended behavior here? Include or not, and under what
circumstances?

> 6. BGP Error Handling.
...
>    The phrase "the BGP connection is closed" means that the TCP connec-
>    tion has been closed, the associated Adj-RIB-In has been cleared, and
>    that all resources for that BGP connection have been deallocated.
>    Entries in the Loc-RIB associated with the remote peer are marked as
>    invalid. The fact that the routes have become invalid is passed to
>    other BGP peers before the routes are deleted from the system.

What does "the fact is passed" mean? Should we instead say that local
route recalculation happens and peers are sent either updated best
routes or withdrawals?

> 6.2 OPEN message error handling.
...
>    If the Autonomous System field of the OPEN message is unacceptable,
>    then the Error Subcode is set to Bad Peer AS. The determination of
>    acceptable Autonomous System numbers is outside the scope of this
>    protocol.

Shouldn't we say that configuration based detection should be
supported, i.e., when remote-as is configured for the peer?

...
>   If the BGP Identifier field of the OPEN message is syntactically
>    incorrect, then the Error Subcode is set to Bad BGP Identifier.  Syn-
>    tactic correctness means that the BGP Identifier field represents a
>    valid IP host address.

Is "valid IP host address" defined somewhere, btw?

> 6.3 UPDATE message error handling.
> 
> 
>    All errors detected while processing the UPDATE message are indicated
>    by sending the NOTIFICATION message with Error Code UPDATE Message
>    Error. The error subcode elaborates on the specific nature of the
>    error.

"are indicated..." is this a MUST, SHOULD, or MAY?
...
>    If the ORIGIN attribute has an undefined value, then the Error Sub-
>    code is set to Invalid Origin Attribute. The Data field contains the
>    unrecognized attribute (type, length and value).

Curious: do we really have to drop a session on this condition? Given
that the attribute was syntactically correct and the TLV was not
broken, so the stream is still in sync and we can move on? Of course,
if this is what current implementations do, we have no other choice.

...
>    If the UPDATE message is received from an external peer, the local
>    system MAY check whether the leftmost AS in the AS_PATH attribute is

Same comment about 'leftmost'... Maybe we should define this somewhere
in the beginning of the spec?

...
>    The NLRI field in the UPDATE message is checked for syntactic valid-
>    ity. If the field is syntactically incorrect, then the Error Subcode
>    is set to Invalid Network Field.

Should we give more data on what syntactic validity means in this case
so people behave consistently?

> 6.7 Cease.
...
> If the BGP speaker decides to terminate its BGP
>    connection with a neighbor because the number of address prefixes
>    received from the neighbor exceeds the locally configured upper
>    bound, then the speaker MUST send to the neighbor a NOTIFICATION mes-
>    sage with the Error Code Cease.

Should we also say that when the peer decides to discard incoming
prefixes, this event should be logged locally?


> 8. BGP Finite State machine

General comment: I would _really_ appreciate more people looking at this
section.

>    The optional Session attributes are listed below. These optional
>    attributes may be supported either per connection or per local sys-
>    tem:
> 
>         1) Delay Open flag

Where's the description of this flag and how/when is it set? Same for
others below. Should we have a brief description for each attribute?

>         2) Open Delay Timer
>         3) Perform automatic start flag
>         4) Perform automatic stop flag
>         5) Passive TCP establishment flag
>         6) Perform BGP peer oscillation damping flag
>            (which will be denoted as stop_peer_flap in text)
>         7) Idle Hold timer
>         8) Perform Collision detect in Established flag
>         9) Accept connections from un-configured peers
>        10) Track TCP state flag
>        11) Send NOTIFICATION without an OPEN flag
> 
Suggestion: to make reading of the FSM description below easier, we
could "merge" the multiword flag names and normalize them, e.g.
'perform automatic start flag' to 'PerformAutoStart flag'. 'Passive
TCP establishment flag' to 'PassiveTCPEstablishment flag',
'stop_peer_flap' to 'StopPeerFlag'.

> 8.1.1 Administrative Events
> 
> 
>    Please note that only Event 1 (manual start) and Event 2 (manual
>    stop) are mandatory administrative events. All other administrative
>    events are optional. The optional attributes do not have to be sup-
>    ported. However, if these attributes are supported, the state of the
>    flags should be as indicated.

'flags should be as indicated' does not give a clear understanding of
what they are used for. Should the events be sanity-checked by
checking those attributes? what's the recommended behavior when the
flags are in a different state?

>        Event3: Automatic start
> 
>               Definition: Local system automatically starts the
>                           BGP connection.
> 

When is this event generated by the system? Under what conditions?
>               Status:     Optional depending on local system.
> 
>               Optional
>               attributes: 1) Perform automatic start flag SHOULD be set
>                              if this event occurs.
>                           2) if the passive Passive TCP establishment flag

passive Passive?

>        Event5: Automatic start with passive TCP flag
> 
>               Definition: Local system automatically starts the
>                           BGP connection with the passive flag
>                           enabled.  The passive flag indicates
> 

Same question about generation conditions
..
>        Event23: Open collision dump
> 
>               Definition: An event generated administratively
>                           when a connection collision has been
>                           detected while processing an incoming
>                           OPEN message and this connection has been
>                           selected to disconnected. See Section
'to be disconnected'
>                           6.8 for more information on collision
>                           detection.
> 
>                           Event23 is an administrative based only
'based on'?
>                           implementation specific policy. This
>                           Event may occur if the FSM is implemented
>                           as two linked state machines.
> 
> 
>               Status:     Optional, depending on local system
> 
>               Optional
>               Attributes: If the state machine is to process this
>                           attribute in Established state,
>                            1) Peform Collision detect in Established
'Perform'
>                                flag SHOULD be set.

...

>        Event25: NotifMsg
> 
>               Definition: An event is generated when a
>                           NOTIFICATION messages is received and
message
>                           the error code is anything but
>                           "version error".
> 
>               Status:     Mandatory


> 8.2.1 FSM Definition
> 
> 
>    BGP MUST maintain a separate FSM for each configured peer, Each BGP
>    peer paired in a potential connection unless configured to remain in
>    the idle state, or configured to remain passive, will attempt to  to
to to

>    connect to the other.  For the purpose of this discussion, the active
>    or connect side of the TCP connection (the side of a TCP connection
'active or connecting'?

>    sending the first TCP SYN packet) is called outgoing.  The passive or
>    listening side (the sender of the first SYN ACK) is called an incom-
>    ing connection (see Section 8.2.1.1 on the terms active and passive
>    below).
> 
>    A BGP implementation MUST connect to and listen on TCP port 179 for
>    incoming connections in addition to trying to connect to peers.  For
>    each incoming connection, a state machine MUST be instantiated.

Is this true for implementations that resolve connection collision
through one FSM with two transport connections?

> 8.2.1.1 Terms "active" and "passive"
> 
> 
>    The terms active and passive have been in our vocabulary for almost a
>    decade and have proven useful.  

Ed: The style here is quite different from the rest of the document
(i.e., personalization), plus time values tend become outdated with
time :)

> 8.2.1.2 FSM and collision detection
> 
> 
>    There is one FSM per BGP connection.  Prior to determining what peer
>    a connection is associated with there may be two connections for a
>    given peer.  There SHOULD be no more than one connection per peer.

Is above "SHOULD" normative? I.e., should be "should" instead?

>   The collision detection identifies the case where there is more than
>    one connection per peer and provides guidance for which connection to
>    get rid of.  When this occurs, the corresponding FSM for the connec-
>    tion that is closed SHOULD be disposed of.
> 

BTW, I think the specification would really benefit from a section
that describes processing of incoming transport connections.

> 8.2.2 Finite State Machine
> 
> 
>       Idle state:
> 
>          Initially BGP is in the Idle state.

Not BGP, but the peer FSM, right?

> 
>          In this state BGP refuses all incoming BGP connections.  No

all incoming connections from that peer?

> 
>          resources are allocated to the peer. In response to a
>          manual start event(Event1) or an automatic start
>          event(Event3), the local system:
>             - initializes all BGP resources,
all BGP resources or only those needed for the peer?
also, what does 'initialize' mean here?

>             - sets ConnectRetryCnt (the connect retry counter) to zero

Seems we have inconsistency in FSM parameter naming here.

>         In response to a manual start event with the passive TCP connection
>         flag (Event 4) or automatic start with the passive TCP connection
>         flag (Event 5), the local system:
>             - initializes all BGP resources,
>             - sets ConnectRetryCnt (the connect retry counter) to zero,
>             - starts the Connect Retry timer with initial value,
>             - listens for a connection that may be initiated by
>               the remote peer, and
>             - changes its state to Active.

Ditto comments here

>         The method of preventing persistent peer oscillation is
>         outside the scope of this document.

So we have these events, but we don't define how to handle them?

>         Any other events [Events 9-12, 15-28] received in the Idle state
>         does not cause change in the state of the local system.

'do not cause changes'  ?

>         In response to a manual stop event [Event2], the local system:
>            - drops the TCP connection,
>            - releases all BGP resources,
>            - sets ConnectRetryCnt (the connect retry count) to zero
>            - sets the Connect Retry timer to zero, and

sets timer to zero? 'Stops the timer' instead?

>            - changes its state to Idle.
...

>         If the BGP port receives a valid TCP connection indication
BGP port?
>         [Event 14], the TCP connection is processed and
>         the connection remains in the Connect state.
> 
>         If the TCP connection receives an invalid indication [Event 15]:

TCP connection receives?

>         the local system rejects the TCP connection and the connection
>         remains in the Connect state.
> 
>         If the TCP connection succeeds [Event 16 or Event 17],
>         the local system checks the Delay Open flag prior to
>         processing. If the Delay Open flag is set, the local system:
>              - sets the Connect Retry timer to zero,
"stops" instead?

>              - set the Open Delay timer to the initial value, and

sets

>              - stays in the Connect state.
>         If the Delay Open flag is not set, the local system:
>              - sets the Connect Retry timer to zero,
stops

>              - completes BGP initialization

What does the above really mean?

...
>         the Open Delay Timer. If the Open Delay timer is running,
>         the local system:
>             - restarts the connect retry time with initial value,
>             - stops the Open Delay timer and resets value to zero,
>             - continues to listen for a connection that may be
>               initiated by the remote BGP peer, and
>             - changes its state to Active.
>         If the open Delay timer is not running, the local system:
>            - sets the Connect Retry timer to zero,
>            - drops the TCP connection,
>            - releases all BGP resources, and
all BGP resources?

>            - changes its state to Idle.
> 
>         If an OPEN message is received with the Open Delay timer is
>         running [Event 20], the local system:
>            - sets the Connect Retry timer to zero,
>            - completes the BGP initialization,
What does it mean?

>            - stops and clears the Open Delay timer (sets the value to zero),
>            - sends an OPEN message,
>            - sends a KEEPALIVE message,
>            - If the hold timer value is non-zero,
>                    - start the keepalive timer to inital value,
"starts"... start to initial value?

>                    - reset the hold timer to the negotiated value,
Resets

>              else if hold timer value is zero,
>                    - reset the keepalive timer, and

resets

>                    - reset the hold timer value to zero
resets

>            - and changes its state to OpenConfirm.
> 

OK, I'll stop reviewing the FSM text here and will skip to the next
section. Given the number of English grammar mistakes, it is clear to
me that either it has not been sufficiently reviewed or even read by
someone carefully enough or the comments have not been incorporated.
Please address.

...
> 9. UPDATE Message Handling
> 
> 
>    An UPDATE message may be received only in the Established state.
What if it is received in another state?

...
> 9.1 Decision Process
> 
> 
>    The Decision Process selects routes for subsequent advertisement by
>    applying the policies in the local Policy Information Base (PIB) to
>    the routes stored in its Adj-RIBs-In. The output of the Decision Pro-
>    cess is the set of routes that will be advertised to peers; the
>    selected routes will be stored in the local speaker's Adj-RIB-Out
RIB-Out or RIBs-out (plural)?

>    according to policy.
> 
>    The selection process is formalized by defining a function that takes
>    the attribute of a given route as an argument and returns either (a)
>    a non-negative integer denoting the degree of preference for the
>    route, or (b) a value denoting that this route is ineligible to be
>    installed in LocRib and will be excluded from the next phase of route

Loc-RIB
>    selection.
...
>    The Decision Process operates on routes contained in the Adj-RIB-In,
Adj-RIBs-In (plural) ?
>    and is responsible for:

> 9.1.1 Phase 1: Calculation of Degree of Preference
...
>       If the route is learned from an external peer, then the local BGP
>       speaker computes the degree of preference based on preconfigured
>       policy information. If the return value indicates that the route
>       is ineligible, the route MAY NOT serve as an input to the next
>       phase of route selection; otherwise the return value is used as
>       the LOCAL_PREF value in any IBGP readvertisement.

So, AFAIK, the major implementations do not follow this step
(calculating the degree of preference, and then announcing). Instead,
implementations allow setting the LOCAL_PREF value locally, which is
taken into consideration during the best path selection, and is also
reannounced further.

Also "is used" is not specific enough. Is it SHOULD or MUST?

> 9.1.2 Phase 2: Route Selection
...
>    If the AS_PATH attribute of a BGP route contains an AS loop, the BGP
>    route should be excluded from the Phase 2 decision function.  AS loop
>    detection is done by scanning the full AS path (as specified in the
>    AS_PATH attribute), and checking that the autonomous system number of
>    the local system does not appear in the AS path.  Operations of a BGP
>    speaker that is configured to accept routes with its own autonomous
>    system number in the AS path are outside the scope of this document.

If we're checking for an AS loop here (in Phase 2) as opposed to
during the UPDATE message sanity checking, the route is already
received and accepted in the peer's Adj-RIB-In. Those implementations
I know don't even install such routes in the RIB...

> 9.1.2.2 Breaking Ties (Phase 2)
...
>       Similarly, neighborAS(n) is a function which returns the neighbor
>       AS from which the route was received.  If the route is learned via
>       IBGP, and the other IBGP speaker didn't originate the route, it is
>       the neighbor AS from which the other IBGP speaker learned the
>       route. If the route is learned via IBGP, and the other IBGP
>       speaker originated the route, it is the local AS.

What if the route is locally originated?

> 9.1.4 Overlapping Routes
...
>    When overlapping routes are present in the same Adj-RIB-In, the more
>    specific route takes precedence, in order from more specific to least
>    specific.
> 
Doesn't this happen at the packet forwarding stage?

> 
>    The set of destinations described by the overlap represents a portion
>    of the less specific route that is feasible, but is not currently in
>    use.  If a more specific route is later withdrawn, the set of desti-
>    nations described by the overlap will still be reachable using the
>    less specific route.
> 
>    If a BGP speaker receives overlapping routes, the Decision Process
>    MUST consider both routes based on the configured acceptance policy.
>    If both a less and a more specific route are accepted, then the Deci-
>    sion Process MUST either install both the less and the more specific
Install where?

>    routes or it MUST aggregate the two routes and install the aggregated
>    route, provided that both routes have the same value of the NEXT_HOP
>    attribute.

anyone really does the latter?

>    If a BGP speaker chooses to aggregate, then it SHOULD either include
>    all AS used to form the aggreagate in an AS_SET or add the
>    ATOMIC_AGGREGATE attribute to the route.  This attribute is now pri-
>    marily informational.  With the elimination of IP routing protocols
>    that do not support classless routing and the elimination of router
>    and host implementations that do not support classless routing, there
>    is no longer a need to deaggregate.  Routes SHOULD NOT be de-aggre-
>    gated.  A route that carries ATOMIC_AGGREGATE attribute in particular
>    MUST NOT be de-aggregated. That is, the NLRI of this route can not be
>    made more specific. Forwarding along such a route does not guarantee
>    that IP packets will actually traverse only ASs listed in the AS_PATH
>    attribute of the route.

Since we don't do deaggregation any more, should we remove the
discussion about it completely and indicate in the "changes" section
that deaggregation has been deprecated?

> 9.2 Update-Send Process
...
>    When a BGP speaker receives an UPDATE message from an internal peer,
>    the receiving BGP speaker SHALL NOT re-distribute the routing infor-
>    mation contained in that UPDATE message to other internal peers,
>    unless the speaker acts as a BGP Route Reflector [RFC2796].

Suggest to put "unless..." in brackets () to make it more apparent
that this is not a normative ref.

> 9.2.1.1 Frequency of Route Advertisement
>    Since fast convergence is needed within an autonomous system, either
>    (a) the MinRouteAdvertisementInterval used for internal peers SHOULD
>    be shorter than the MinRouteAdvertisementInterval used for external
>    peers, or (b) the procedure describe in this section SHOULD NOT apply
>    for routes sent to internal peers.

It sounded like MinRouteAdvertisementInterval was an architectural
constant, but now it sounds like either this is a timer that can be
assigned different settings or there are two constants:
MinRouteAdvIntIBGP and MinRouteAdvIntEBGP.

> 9.2.2.2 Aggregating Routing Information
> 

Hmmm... I expected to see in this section some text talking about when
and how an aggregate would be announced, i.e., when an aggregate
prefix is configured, and more specific routes are present, the
aggregate is announced, when no specifics are left--withdraw the
aggregate. I haven't found anything on this topic...


> 9.3 Route Selection Criteria
>
>    Generally speaking, additional rules for comparing routes among sev-
>    eral alternatives are outside the scope of this document. There are
>    two exceptions:
> 
>       - If the local AS appears in the AS path of the new route being
>       considered, then that new route can not be viewed as better than
>       any other route (provided that the speaker is configured to accept
>       such routes). If such a route were ever used, a routing loop could
>       result.
> 
>       - In order to achieve successful distributed operation, only
>       routes with a likelihood of stability can be chosen. Thus, an AS
>       SHOULD avoid using unstable routes, and it SHOULD NOT make rapid
>       spontaneous changes to its choice of route. Quantifying the terms
>       "unstable" and "rapid" in the previous sentence will require expe-
>       rience, but the principle is clear.
> 

Where does this (the second one) fit within and how does this affect
the route selection criteria?

>    Care must be taken to ensure that BGP speakers in the same AS do not
>    make inconsistent decisions.

How? What does this mean for the implementor?

> 9.4 Originating BGP routes
> 
>    A BGP speaker may originate BGP routes by injecting routing informa-
>    tion acquired by some other means (e.g. via an IGP) into BGP. A BGP
>    speaker that originates BGP routes assigns the degree of preference
> 

"assigns the degree of preference"... how do the implementations
really do that?

> 10 BGP Timers
...
>    The suggested default value for the MinRouteAdvertisementInterval is
>    30 seconds.

This was described as a parameter, not a timer. Further, it was
earlier suggested that it should be shorter for iBGP than it is for
eBGP. I'd expect the document to specify the recommended value for
both.

> IANA Considerations
...
>    All extensions to this protocol, including new message types and Path
>    Attributes MUST only be made using the Standards Action process
>    defined in [RFC2434].

This section should include the description of each registry that
needs to be created (if needed) and maintained by IANA, as well as the
allocation policy that is in the text already.

<EOM>