Subject: RFC 4271 (BGP-4) errata and issues

From: Alfred Hönes <[email-masked]>
Date: Sun, 9 Jul 2006 17:01:28 +0200 (MESZ)

Hello,
in January, the new BGP-4 RFCs have been published.
Unfortunately, I did not find the time to report the
textual issues I found in these documents until now.

This message addresses RFC 4271 (BGP-4 Protocol) authored/edited
by you, pointing out some issues I found with the RFC text, and
supplying material for an appropriate RFC Errata Note.
(Almost all issues are strictly textual / editorial in nature.)

I apologize for never having found the time to comment on
RFC 1771, the origin of many of the issues, and for not being
aware of the update in progress for the BGP documentation.

The items below are presented in RFC textual order.
Recurring issues regularly are listed only once, for the first
occurrence, unless the circumstances justified a repetition.

I use change bars ('|' in column 1) and occasionally
up/down pointing marker lines ('^^^'/'vvv') to emphasize
the location of textual issues and/or proposed corrections.
Modified text has been re-adjusted to match RFC formatting
rules, where appropriate.

(1)  "ruler line" alignment

The "ruler lines" on top of many data layout figures are misaligned
by one column, compared to the standard placement introduced by
RFC 791, recommended by RFC 1122 and repeated in the latest
"Instructions to RFC Authors".

In Section 4.1 of RFC 4271, on page 12, the figure heading,

|     0                   1                   2                   3
|     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                               |
      [...]

should be adjusted as:

|      0                   1                   2                   3
|      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                               |
      [...]

Other places, where a similar adjustment is needed:
  - Section 4.2, on page 13 and page 14 ;
  - Section 4.3, on page 16 ;
  - Section 4.5, on page 22 .

(2)  clarification

Section 4.3 of RFC 4271, on page 15/16 contains the text and figure:

      Withdrawn Routes:

         This is a variable-length field that contains a list of IP
         address prefixes for the routes that are being withdrawn from
         service.  Each IP address prefix is encoded as a 2-tuple of the
         form <length, prefix>, whose fields are described below:

                  +---------------------------+
                  |   Length (1 octet)        |
                  +---------------------------+
                  |   Prefix (variable)       |
                  +---------------------------+

<< page break >>

         The use and the meaning of these fields are as follows:

         a) Length:

            The Length field indicates the length in bits of the IP
            address prefix.  A length of zero indicates a prefix that
            matches all IP addresses (with prefix, itself, of zero
            octets).

To avoid the overloading of the frequently used word "length"
and make the terminology unambiguous, I recommend to rename the
"Length" field to "PrefixLength".
Thus, the above snippit should be changed to say:

      Withdrawn Routes:

         This is a variable-length field that contains a list of IP
         address prefixes for the routes that are being withdrawn from
         service.  Each IP address prefix is encoded as a 2-tuple of the
         form <length, prefix>, whose fields are described below:

                  +---------------------------+
|                 |   PrefixLength (1 octet)  |
                  +---------------------------+
                  |   Prefix (variable)       |
                  +---------------------------+

<< page break >>

         The use and the meaning of these fields are as follows:

|        a) PrefixLength:

|           The PrefixLength field indicates the length in bits of the
            IP address prefix.  A length of zero indicates a prefix that
            matches all IP addresses (with prefix, itself, of zero
            octets).

(3)  typo

The last paragraph on page 17 (part of Section 4.3) says:

         If the Extended Length bit of the Attribute Flags octet is set
|        to 1, the third and fourth octets of the path attribute contain
         the length of the attribute data in octets.

It should say:

         If the Extended Length bit of the Attribute Flags octet is set
|        to 1, the third and fourth octet of the path attribute contain
         the length of the attribute data in octets.

(4)  outdated specification -- deprecation needed

Text in Section 4.3, on page 18, refers to the Historic EGP:

         a) ORIGIN (Type Code 1):

               [...]

               1         EGP - Network Layer Reachability Information
                            learned via the EGP protocol [RFC904]

Perhaps, this ORIGIN value should be deprecated:

         a) ORIGIN (Type Code 1):

               [...]

               1         EGP - Network Layer Reachability Information
                            learned via the EGP protocol [RFC904]
|                           (deprecated)

(5)  clarification

The text in the lower half of page 18 repeatedly talks about
a "1-octet length field" or a "2-octet length field".
This can be misunderstood -- not all of thes fields are length
fields.
According to the maxim of using simple wording in RFC text,
the word "length" should be omitted in such context.

Therefore, in the sub-section titled

         b) AS_PATH (Type Code 2):

a) modify:

|           The path segment type is a 1-octet length field with the
            following values defined:

to say:

|           The path segment type is a 1-octet field with the following
            values defined:

b) modify:

|           The path segment length is a 1-octet length field,
            containing the number of ASes (not the number of octets) in
            the path segment value field.

            The path segment value field contains one or more AS
|           numbers, each encoded as a 2-octet length field.

to say:

|           The path segment length is a 1-octet field, containing
            the number of ASes (not the number of octets) in the path
            segment value field.

            The path segment value field contains one or more AS
|           numbers, each encoded as a 2-octet field.

(6)  clarification -- similar to (2) above

Still within Section 4.3, on page 20, the

      Network Layer Reachability Information:

contains the figure and text:

         Reachability information is encoded as one or more 2-tuples of
         the form <length, prefix>, whose fields are described below:

                  +---------------------------+
                  |   Length (1 octet)        |
                  +---------------------------+
                  |   Prefix (variable)       |
                  +---------------------------+

         The use and the meaning of these fields are as follows:

         a) Length:

            The Length field indicates the length in bits of the IP
            address prefix.  A length of zero indicates a prefix that
            matches all IP addresses (with prefix, itself, of zero
            octets).

To avoid the overloading of the frequently used word "length"
and make the terminology unambiguous, again, I recommend to rename
the "Length" field to "PrefixLength".
Thus, the above snippit should be changed to say:

         Reachability information is encoded as one or more 2-tuples of
         the form <length, prefix>, whose fields are described below:

                  +---------------------------+
|                 |   PrefixLength (1 octet)  |
                  +---------------------------+
                  |   Prefix (variable)       |
                  +---------------------------+

         The use and the meaning of these fields are as follows:

|        a) PrefixLength:

|           The PrefixLength field indicates the length in bits of the
            IP address prefix.  A length of zero indicates a prefix that
            matches all IP addresses (with prefix, itself, of zero
            octets).

(7)  clarification / improved wording

Section 5.1.2, on top of page 26, says:

         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 with respect to the position of octets in the
|           protocol message).  If the act of prepending will cause an
            overflow in the AS_PATH segment (i.e., more than 255 ASes),
            it SHOULD prepend a new segment of type AS_SEQUENCE and
|           prepend its own AS number to this new segment.

The second half of this bullet, in particular the use of "will" and
the second instance of "prepend", is misleading.
I recommend to improve the wording, to say instead:

         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 with respect to the position of octets in the
|           protocol message).  If the act of prepending would cause an
            overflow in the AS_PATH segment (i.e., more than 255 ASes),
            it SHOULD prepend a new segment of type AS_SEQUENCE and
|           place its own AS number into this new segment.

(8)  word twister

Section 5.1.3, near the top of page 28, says:

   Normally, the NEXT_HOP attribute is chosen such that the shortest
   available path will be taken.  A BGP speaker MUST be able to support
|  the disabling advertisement of third party NEXT_HOP attributes in
   order to handle imperfectly bridged media.

It should say:

   Normally, the NEXT_HOP attribute is chosen such that the shortest
   available path will be taken.  A BGP speaker MUST be able to support
|  disabling the advertisement of third party NEXT_HOP attributes in
   order to handle imperfectly bridged media.

Simpler, but equivalent wording would be:

   Normally, the NEXT_HOP attribute is chosen such that the shortest
|  available path will be taken.  A BGP speaker MUST be able to suppress
|  the advertisement of third party NEXT_HOP attributes in order to
   handle imperfectly bridged media.

(9)  replicated text, and punctuation

In Section 6 on page 30, the end of the third paragraph says:

                          [...].  Before the invalid routes are deleted
   from the system, it advertises, to its peers, either withdraws for
   the routes marked as invalid, or the new best routes before the
   invalid routes are deleted from the system.

It should say:

                          [...].  Before the invalid routes are deleted
|  from the system, it advertises to its peers either withdraws for the
|  routes marked as invalid or the new best routes.

(10)  clarification / textual improvement

The first paragraph of Section 6.1, on page 31, says:

   All errors detected while processing the Message Header MUST be
   indicated by sending the NOTIFICATION message with the Error Code
   Message Header Error.  The Error Subcode elaborates on the specific
   nature of the error.

It should say:

   All errors detected while processing the Message Header MUST be
|  indicated by sending a NOTIFICATION message with Error Code
|  'Message Header Error'.  The Error Subcode elaborates on the specific
   nature of the error.

(I have added the [single] quotes to improve readability and clarity.
 You may drop this, if you dislike it.)

(11)  unexpected lack of precision

Later on on page 31, Section 6.1 says:

   If at least one of the following is true:

      - if the Length field of the message header is less than 19 or
        greater than 4096, or

      - if the Length field of an OPEN message is less than the minimum
        length of the OPEN message, or

      - if the Length field of an UPDATE message is less than the
        minimum length of the UPDATE message, or

      - if the Length field of a KEEPALIVE message is not equal to 19,
        or

      - if the Length field of a NOTIFICATION message is less than the
        minimum length of the NOTIFICATION message,

   then the Error Subcode MUST be set to Bad Message Length.  The Data
   field MUST contain the erroneous Length field.

IMHO, these specifications are unnecessarily imprecise in part;
all the concrete limit values should be specified here for clarity --
they certainly will not be changed any more in the future, for BGP-4.

(Additionally, the condition specified in the last bullet apparently
contradicts the specification in Section 6.4; that will be discussed
below -- see item (17).)

Therefore, the above snippit should better say:

   If at least one of the following is true:

      - if the Length field of the message header is less than 19 or
        greater than 4096, or

|     - if the Length field of an OPEN message is less than 29, the
        minimum length of the OPEN message, or

|     - if the Length field of an UPDATE message is less than 23, the
        minimum length of the UPDATE message, or

      - if the Length field of a KEEPALIVE message is not equal to 19,
        or

|     - if the Length field of a NOTIFICATION message is less than 21,
        the minimum length of the NOTIFICATION message,

   then the Error Subcode MUST be set to Bad Message Length.  The Data
   field MUST contain the erroneous Length field.

or even (much shorter):

   If at least one of the following is true:

      - if the Length field of the message header is less than 19 or
        greater than 4096, or

|     - if the Length field of an OPEN message is less than 29, or

|     - if the Length field of an UPDATE message is less than 23, or

      - if the Length field of a KEEPALIVE message is not equal to 19,
        or

|     - if the Length field of a NOTIFICATION message is less than 21,

   then the Error Subcode MUST be set to Bad Message Length.  The Data
   field MUST contain the erroneous Length field.

(12)  clarification / textual improvement -- similar to (10)

The first paragraph of Section 6.2, on page 31, says:

|  All errors detected while processing the OPEN message MUST be
|  indicated by sending the NOTIFICATION message with the Error Code
   OPEN Message Error.  The Error Subcode elaborates on the specific
   nature of the error.

It should say:

|  All errors detected while processing an OPEN message MUST be
|  indicated by sending a NOTIFICATION message with Error Code
|  'OPEN Message Error'.  The Error Subcode elaborates on the specific
   nature of the error.

(13)  clarification / textual improvement -- similar to (10) again

The first paragraph of Section 6.3, on page 32, says:

|  All errors detected while processing the UPDATE message MUST be
|  indicated by sending the NOTIFICATION message with the Error Code
   UPDATE Message Error.  The error subcode elaborates on the specific
   nature of the error.

It should say:

|  All errors detected while processing an UPDATE message MUST be
|  indicated by sending a NOTIFICATION message with Error Code
|  'UPDATE Message Error'.  The error subcode elaborates on the specific
   nature of the error.

(14)  clarification

The third paragraph on page 33 (within Section 6.3) says:

|  If any of the well-known mandatory attributes are not recognized,
   then the Error Subcode MUST be set to Unrecognized Well-known
   Attribute.  The Data field MUST contain the unrecognized attribute
   (type, length, and value).

It should say:

|  If any of the attributes marked as well-known are not recognized,
   then the Error Subcode MUST be set to Unrecognized Well-known
   Attribute.  The Data field MUST contain the unrecognized attribute
   (type, length, and value).

(15)  clarifications and corrections

The 5th + ff. paragraphs on page 33 (within Section 6.3) say:

   If the NEXT_HOP attribute field is syntactically incorrect, then the
   Error Subcode MUST be set to Invalid NEXT_HOP Attribute.  The Data
   field MUST contain the incorrect attribute (type, length, and value).
   Syntactic correctness means that the NEXT_HOP attribute represents a
|  valid IP host address.

|  The IP address in the NEXT_HOP MUST meet the following criteria to be
   considered semantically correct:

|     a) It MUST NOT be the IP address of the receiving speaker.

This text should say:

   If the NEXT_HOP attribute field is syntactically incorrect, then the
   Error Subcode MUST be set to Invalid NEXT_HOP Attribute.  The Data
   field MUST contain the incorrect attribute (type, length, and value).
   Syntactic correctness means that the NEXT_HOP attribute represents a
|  valid (unicast) IP interface address.

|  The IP address in the NEXT_HOP attribute MUST meet the following
   criteria to be considered semantically correct:

|     a) It MUST NOT be an IP address of the receiving speaker.

(Restriction to *unicast* addresses seems essential; IP addresses
 are bound to *interfaces*; a host may have multiple IP addresses.)

(16)  suspected inconsistency

The first paragraph on page 34 (still within Section 6.3) says:

   If the UPDATE message is received from an external peer, the local
   system MAY check whether the leftmost (with respect to the position
   of octets in the protocol message) AS in the AS_PATH attribute is
   equal to the autonomous system number of the peer that sent the
   message.  If the check determines this is not the case, the Error
   Subcode MUST be set to Malformed AS_PATH.

This specification -- when taken literally -- contradicts the
optimization specified in Appendix F.4 !
To resolve this issue, perhaps the above check should only be
performed if the first path segment is an AS_SEQUENCE, and *not*
in the case of an AS_SET.

Please check!

(17)  suspected inconsistency

Section 6.4, on page 34, says:

|  If a peer sends a NOTIFICATION message, and the receiver of the
|  message detects an error in that message, the receiver cannot use a
|  NOTIFICATION message to report this error back to the peer.  Any such
   error (e.g., an unrecognized Error Code or Error Subcode) SHOULD be
   noticed, logged locally, and brought to the attention of the
   administration of the peer.  The means to do this, however, lies
   outside the scope of this document.

This specification apparently is in contradiction to Section 6.1,
where the last bullet (cf. item (11) above) triggers sending of an
error NOTIFICATION in response to a NOTIFICATION with illegal Length.

Please check!

(18)  clarification / textual improvement -- similar to (10) above

Section 6.5, on page 34, says:

   If a system does not receive successive KEEPALIVE, UPDATE, and/or
   NOTIFICATION messages within the period specified in the Hold Time
|  field of the OPEN message, then the NOTIFICATION message with the
|  Hold Timer Expired Error Code is sent and the BGP connection is
   closed.

It should better say:

   If a system does not receive successive KEEPALIVE, UPDATE, and/or
   NOTIFICATION messages within the period specified in the Hold Time
|  field of the OPEN message, then a NOTIFICATION message with Error
|  Code 'Hold Timer Expired' is sent and the BGP connection is closed.

(19)  clarification / textual improvement -- similar to (10) above

Section 6.6, on page 35, says:

   Any error detected by the BGP Finite State Machine (e.g., receipt of
|  an unexpected event) is indicated by sending the NOTIFICATION message
|  with the Error Code Finite State Machine Error.

It should better say:

   Any error detected by the BGP Finite State Machine (e.g., receipt of
|  an unexpected event) is indicated by sending a NOTIFICATION message
|  with Error Code 'Finite State Machine Error'.

(20) clarification / textual improvement -- similar to (10) above

a) Section 6.7, on page 35, says:

   In the absence of any fatal errors (that are indicated in this
   section), a BGP peer MAY choose, at any given time, to close its BGP
|  connection by sending the NOTIFICATION message with the Error Code
   Cease.  However, the Cease NOTIFICATION message MUST NOT be used when
   a fatal error indicated by this section does exist.

It should better say:

   In the absence of any fatal errors (that are indicated in this
   section), a BGP peer MAY choose, at any given time, to close its BGP
|  connection by sending a NOTIFICATION message with Error Code 'Cease'.
   However, the Cease NOTIFICATION message MUST NOT be used when a fatal
   error indicated by this section does exist.

b) Later on on page 35, at the end of Section 6.7, the RFC says:

               [...].  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 the neighbor a NOTIFICATION message
|  with the Error Code Cease.  The speaker MAY also log this locally.

It should say:

               [...].  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 the neighbor a NOTIFICATION message
|  with Error Code 'Cease'.  The speaker MAY also log this locally.

(21)  improper wording

The first two paragraphs of Section 6.8, on page 35, say:

|  If a pair of BGP speakers try to establish a BGP connection with each
|  other simultaneously, then two parallel connections well be formed.
   If the source IP address used by one of these connections is the same
   as the destination IP address used by the other, and the destination
   IP address used by the first connection is the same as the source IP
   address used by the other, connection collision has occurred.  In the
   event of connection collision, one of the connections MUST be closed.

   Based on the value of the BGP Identifier, a convention is established
|  for detecting which BGP connection is to be preserved when a
   collision occurs.  [...]

The RFC text should say:

|  If a pair of BGP speakers tries to establish a BGP connection with
|  each other simultaneously, then two parallel connections will be
   formed.
   If the source IP address used by one of these connections is the same
   as the destination IP address used by the other, and the destination
   IP address used by the first connection is the same as the source IP
   address used by the other, connection collision has occurred.  In the
   event of connection collision, one of the connections MUST be closed.

   Based on the value of the BGP Identifier, a convention is established
|  for determining which BGP connection is to be preserved when a
   collision occurs.  [...]

(22)  unnecessarily complicated spec.

The first bullet on page 36 (within Section 6.8) says:

      1) The BGP Identifier of the local system is compared to the BGP
         Identifier of the remote system (as specified in the OPEN
         message).  Comparing BGP Identifiers is done by converting them
         to host byte order and treating them as 4-octet unsigned
         integers.

Since the BGP Identifier *is* defined as a 4-octet unsigned integer
(cf. page 5 of the RFC), the second sentence is redundant; it gives
trivial implementation details (partially inappropriate for platforms
operating intrinsically in network byte order) that should not be
part of a protocol specification.
(BTW, network byte order has the significant advantage that unsigned
 integers can always be compared as octect strings -- e.g. using POSIX
 strncmp() -- *without* any host dependant byte order conversion!)
Thus, preferrably that second sentence might better be deleted.

(23)  incomplete specification / clarification

The second and the third bullet on page 36 (within Section 6.8) say:

      2) If the value of the local BGP Identifier is less than the
         remote one, the local system closes the BGP connection that
         already exists (the one that is already in the OpenConfirm
|        state), and accepts the BGP connection initiated by the remote
         system.

      3) Otherwise, the local system closes the newly created BGP
         connection (the one associated with the newly received OPEN
         message), and continues to use the existing one (the one that
|        is already in the OpenConfirm state).

This text does not fully reflect the preceding explanations (on p. 35)
and also should be clarified a bit to more precisely name the subject
connection.
Thus, the RFC should better say:

      2) If the value of the local BGP Identifier is less than the
         remote one, the local system closes the BGP connection that
|        already exists (the one that is already in the OpenConfirm [or
|        OpenSent] state), and accepts the new BGP connection initiated
         by the remote system.
                                          ^^^^^
      3) Otherwise, the local system closes the newly created BGP
         connection (the one associated with the newly received OPEN
         message), and continues to use the existing one (the one that
|        is already in the OpenConfirm [or OpenSent] state).

(24)  clarification / textual improvement -- similar to (10)

The final paragraphh of Section 6.8, on page 36, says:

   Closing the BGP connection (that results from the collision
|  resolution procedure) is accomplished by sending the NOTIFICATION
|  message with the Error Code Cease.

It should say:

   Closing the BGP connection (that results from the collision
|  resolution procedure) is accomplished by sending a NOTIFICATION
|  message with Error Code 'Cease'.

(25)  apparently redundant variable in the BGP-4 FSM

Section 8, on page 37, lists as a required session attribute:

      2) ConnectRetryCounter

and says (on mid-page 37):

   ....  The ConnectRetryCounter indicates the number of times a BGP
   peer has tried to establish a peer session.

Later on in the RFC (i.e. throughout the whole Section 8, on pages
53 to 75), there are only two different actions specified that
operate on the ConnectRetryCounter:

    - set to zero,
and
    - increments by 1.

But, surprisingly, the value of that counter is *never* read or
tested in this specification.
In particular, this counter value is not mentioned in the context
of peer oscillation damping, where it might be expected.
I also could not locate in any other RFC the (abbreviated) term,
"ConnectRetryCount" -- even with embedded white space.

Therefore, the above sentence on mid-page 37 needs to be amended,
telling the intended use of the ConnectRetryCounter.

Please check.

(26)  text misalignment

Section 8.1.3, on page 46, says:

      Event 12: DelayOpenTimer_Expires

         Definition: An event generated when the DelayOpenTimer expires.

|                    Status:     Optional

         [...]

It should say:

      Event 12: DelayOpenTimer_Expires

         Definition: An event generated when the DelayOpenTimer expires.

|        Status:     Optional

         [...]

(27)  textual improvement / clarification

Section 8.2.1.3, on page 52, says:

   Each of the optional timers (DelayOpenTimer and IdleHoldTimer) has a
   group of attributes that are:

      - flag indicating support,
      - Time set in Timer
      - Timer.

|  The two optional timers show this format:

      DelayOpenTimer: DelayOpen, DelayOpenTime, DelayOpenTimer
      IdleHoldTimer:  DampPeerOscillations, IdleHoldTime,
                      IdleHoldTimer

It should perhaps better say:

   Each of the optional timers (DelayOpenTimer and IdleHoldTimer) has a
   group of attributes that are:

      - flag indicating support,
      - Time set in Timer
      - Timer.

|  These attributes of the two optional timers are designated as
|  follows:

      DelayOpenTimer: DelayOpen, DelayOpenTime, DelayOpenTimer
      IdleHoldTimer:  DampPeerOscillations, IdleHoldTime,
                      IdleHoldTimer

(28)  textual improvement / clarification

Section 8.2.2, on page 53, says:

|     In this state, BGP FSM refuses all incoming BGP connections for
      this peer.  No resources are allocated to the peer.  In response
      to a ManualStart event (Event 1) or an AutomaticStart event (Event
      3), the local system:

        - initializes all BGP resources for the peer connection,

|       - sets ConnectRetryCounter to zero,

        - starts the ConnectRetryTimer with the initial value,

|       - initiates a TCP connection to the other BGP peer,

The wording of this text needs to be corrected, and made consistent
with the wording used in other parts of Section 8.

The text should say:

|     In this state, BGP FSM refuses all incoming BGP connections from
      this peer.  No resources are allocated to the peer.  In response
      to a ManualStart event (Event 1) or an AutomaticStart event (Event
      3), the local system:

        - initializes all BGP resources for the peer connection,

|       - sets the ConnectRetryCounter to zero,

        - starts the ConnectRetryTimer with the initial value,

|       - initiates a TCP connection to the BGP peer,

(There's no "other" peer involved, just the calling peer and the local.)

(29)  word omission

Section 8.2.2, on page 54, says:

   Connect State:

|     In this state, BGP FSM is waiting for the TCP connection to be
      completed.

It should say:

   Connect State:

|     In this state, the BGP FSM is waiting for the TCP connection to be
      completed.
                    ^^^^^

(30)  word omission

Section 8.2.2, in the first line of page 55, says:

|       - sets ConnectRetryCounter to zero,

To make the wording consistent with other parts of Section 8,
it should say:

|       - sets the ConnectRetryCounter to zero,
              ^^^^^

(31)  expected operation not specified

Section 8.2.2, in line 5..8 of page 55, specifies:

      In response to the ConnectRetryTimer_Expires event (Event 9), the
      local system:

        - drops the TCP connection,

        - restarts the ConnectRetryTimer,

        [...]

>From the context, it might perhaps be expected that in this case
additionally the ConnectRetryCounter should be incremented by one.
This is not specified here.  Shouldn't it?
But also cf. (25) above !!

A similar issue arises repeatedly in the remainder of section 8.2.2,
whenever a restart of the ConnectRetryTimer is specified.

(32)  textual improvement

Section 8.2.2, in line 10 of page 55, says:

|       - initiates a TCP connection to the other BGP peer,

As in (28) above, it should say:

|       - initiates a TCP connection to the BGP peer,

(33)  suspected improper specification

Later on onpage 55, Section 8.2.2 says:

      If the BGP FSM receives a TcpConnection_Valid event (Event 14),
      the TCP connection is processed, and the connection remains in the
      Connect state.

I strongly suspect that, according to the explanations given
in Section 8.2.1.2, Event 14 will not "happen" in the BGP FSM
in the Connect state, but instead in another, second BGP FSM.
If this is true, the above sentence should perhaps be deleted.

Please check.

(34)  incomplete specification

Near the bottom of page 56, Section 8.2.2 says:

      If an OPEN message is received while the DelayOpenTimer is running
      (Event 20), the local system:

In accordance with the text style in other parts of Section 8,
it should give a more complete specification for this case,
and say:

      If an OPEN message is received while the DelayOpenTimer is running
|     (Event 20), all fields are checked for correctness. If there are
|     no errors in the OPEN message, the local system:

(35)  intended purpose unclear

Section 8.2.2, near the bottom of page 57, specifies:

|       - increments the ConnectRetryCounter by 1,

        - performs peer oscillation damping if the DampPeerOscillations
          attribute is set to True, and

|       - changes its state to Idle.

According to the text on page 53, in the Idle state no resources
are allocated to the peer, i.e. there is no actual instantiation
of the FSM with its attributes.  But the ConnectRetryCounter is
one of these attributes (cf. page 37).  According to item (25),
the value of ConnectRetryCounter is not used anywhere, and it
hence apparently makes no sense to increment it just before
deleting the FSM instance.

This issue recurs repeatedly in the remainder of Section 8.2.2.

(36)  word omission

Section 8.2.2, in line 2+3 of page 59, says:

|     In this state, BGP FSM is trying to acquire a peer by listening
      for, and accepting, a TCP connection.

It should say:

|     In this state, the BGP FSM is trying to acquire a peer by
      listening for, and accepting, a TCP connection.

(37)  word omission -- like (30)

Section 8.2.2, in line 12 on page 55, says:

|       - sets ConnectRetryCounter to zero,

To make the wording consistent with other parts of Section 8,
it should say:

|       - sets the ConnectRetryCounter to zero,
              ^^^^^

(38)  clarification / textual improvement

Section 8.2.2, in mid-page 59 says:

      In response to a ConnectRetryTimer_Expires event (Event 9), the
      local system:

        - restarts the ConnectRetryTimer (with initial value),

|       - initiates a TCP connection to the other BGP peer,

For the first bullet, see (31) above.
As in the last part of (28) above, the second bullet should say:

|       - initiates a TCP connection to the BGP peer,

(39)  typo

In the final line at the bottom of page 59, Section 8.2.2 says:

|       - sends the OPEN message to its remote peer,

It should say:

|       - sends an OPEN message to its remote peer,
               ^^^^

(40)  textual improvement

Section 8.2.2, in line 3/4 of page 60 says:
                                       vvvvvv
|     A HoldTimer value of 4 minutes is also suggested for this state
      transition.

It should say:

|     A HoldTimer value of 4 minutes is suggested for this state
      transition.

(41)  suspect specification

In the final line at the bottom of page 60, Section 8.2.2 specifys:

        - restarts the ConnectRetryTimer (with the initial value),

Subsequently, the state is changed to Idle (page 61).
In Idle state, the BGP FSM is not actually instantiated
and hence cannot handle timer expiry ( event 9).
Thus, this specufucation apparently does not make sense.

Please check.

(42)  bad wording

At mid-page 61, Section 8.2.2 syas:

        - if the HoldTimer value is non-zero,

|           - starts the KeepaliveTimer to initial value,
                                       ^^^^
It should perhaps say:

        - if the HoldTimer value is non-zero,

|           - starts the KeepaliveTimer with its initial value,

(43)  word omission

On page 63, Section 8.2.2 says:

   OpenSent:

      In this state, BGP FSM waits for an OPEN message from its peer.

It should say:

   OpenSent:

      In this state, the BGP FSM waits for an OPEN message from its
      peer.

(44)  textual improvement

At mid-page 63, Section 8.2.2 says:

        - sends the NOTIFICATION with a Cease,

It should better say:

        - sends a NOTIFICATION with Error Code 'Cease',

This issue recurs near the bottom of page 63, and -- including
slight variants -- 8 more times later on in the text.

(45)  misleading wording

Section 8.2.2, in line 7 on page 65, says:

|       - sets a KeepaliveTimer (via the text below)
              ^^^
It should say:

|       - sets the KeepaliveTimer (via the text below)

(There's only one such timer in the BGP FSM instance.)

(46)  textual improvement

Section 8.2.2, 6 lines from the bottom of page 66, says:

|       - sends the NOTIFICATION with the Error Code Finite State
          Machine Error,

It should say:

|       - sends a NOTIFICATION with Error Code 'Finite State Machine
|         Error',

(47)  textual improvement

Section 8.2.2, at the top of page 68, says:

|       - sends the NOTIFICATION message with the Error Code Hold Timer
          Expired,

It should say:

|       - sends a NOTIFICATION message with Error Code 'Hold Timer
          Expired',

(48)  word omission

Section 8.2.2, at mid-page 68, says:

      In the event of a TcpConnection_Valid event (Event 14), or the
      success of a TCP connection (Event 16 or Event 17) while in
|     OpenConfirm, the local system needs to track the second
      connection.

It should say:

      In the event of a TcpConnection_Valid event (Event 14), or the
      success of a TCP connection (Event 16 or Event 17) while in
|     OpenConfirm state, the local system needs to track the second
      connection.

(49)  improper sequence of specification

Section 8.2.2, on page 69/70, contains two blocks of bulleted items
the preconditions of which must be tested in reverse order for
obtaining logically correct behaviour.
Hence, the text block on page 69,

      If the local system receives a valid OPEN message (BGPOpen (Event
      19)), the collision detect function is processed per Section 6.8.
      If this connection is to be dropped due to connection collision,
      the local system:

        - sends a NOTIFICATION with a Cease,

        - sets the ConnectRetryTimer to zero,

        - releases all BGP resources,

        - drops the TCP connection (send TCP FIN),

        - increments the ConnectRetryCounter by 1,

        - (optionally) performs peer oscillation damping if the
          DampPeerOscillations attribute is set to TRUE, and

        - changes its state to Idle.

should be moved behind the immediately subsequent text block
(extending from page 69 to page 70):

      If an OPEN message is received, all fields are checked for
      correctness.  If the BGP message header checking (BGPHeaderErr
      (Event 21)) or OPEN message checking detects an error (see Section
      6.2) (BGPOpenMsgErr (Event 22)), the local system:

        - sends a NOTIFICATION message with the appropriate error code,

        - sets the ConnectRetryTimer to zero,

        - releases all BGP resources,

        - drops the TCP connection,

        - increments the ConnectRetryCounter by 1,

<< page break >>

        - (optionally) performs peer oscillation damping if the
          DampPeerOscillations attribute is set to TRUE, and

        - changes its state to Idle.

(Additionally, various notes from above apply.)

(50)  unclear specification

Following the two text blocks shown in item (49) above,
Section 8.2.2, on page 70, says:

      If, during the processing of another OPEN message, the BGP
      implementation determines, by a means outside the scope of this
      document, that a connection collision has occurred and this
      connection is to be closed, the local system will issue an
      OpenCollisionDump event (Event 23).  [...]

Apparently, this text is inappropriate, since the RFC indeed does
contain specification text detailing collision detection and
resolution, i.e. determining which connection is to be closed.

(51)  indentation flaw

Section 8.2.2, at mid-page 71, says:

        - sets the ConnectRetryCounter to zero, and

|        - changes its state to Idle.
        ^^
It should say:

        - sets the ConnectRetryCounter to zero, and

|       - changes its state to Idle.

(52)  textual improvement, and punctuation

Immediately below the text mentioned in item (51) above,
Section 8.2.2 says:

      In response to an AutomaticStop event (Event 8), the local system:

|       - sends a NOTIFICATION with a Cease,

|       - sets the ConnectRetryTimer to zero

        - deletes all routes associated with this connection,

Incorporation arguments from above,
it should say:

      In response to an AutomaticStop event (Event 8), the local system:

|       - sends a NOTIFICATION with Error Code 'Cease',

|       - sets the ConnectRetryTimer to zero,

        - deletes all routes associated with this connection,

(53)  event logic

On mid-page 72, Section 8.2.2 says:

      If the KeepaliveTimer_Expires event occurs (Event 11), the local
      system:

        - sends a KEEPALIVE message, and

|       - restarts its KeepaliveTimer, unless the negotiated HoldTime
|         value is zero.

As far as I can see, in this case the latter condition is
precluded by the established dependencies.
Hence, teh RFC should say:

      If the KeepaliveTimer_Expires event occurs (Event 11), the local
      system:

        - sends a KEEPALIVE message, and

|       - restarts its KeepaliveTimer.

(54)  typo

The second paragraph of Section 9.1.1, on page 77, says:

|  The Phase 1 decision function is a separate process,f which completes
   when it has no further work to do.
                                                       ^^
It should say:

|  The Phase 1 decision function is a separate process, which completes
   when it has no further work to do.

(55)  textual improvement

At the end of Section 9.1.1, on page 77, RFC 4271 says:

      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 the route is
      ineligible, the route MAY NOT serve as an input to the next phase
      of route selection; otherwise, the return value MUST be used as
      the LOCAL_PREF value in any IBGP readvertisement.

      The exact nature of this policy information, and the computation
      involved, is a local matter.

The sequence of sentences should be reordered to make it more logical,
and the indentation level adjusted to mirror the role of the sentences
(relative to the preceding text).
Thus, the RFC should say:

      If the route is learned from an external peer, then the local BGP
      speaker computes the degree of preference based on preconfigured
      policy information.  The exact nature of this policy information,
      and the computation involved, is a local matter.

   If the return value indicates the route is ineligible, the route
   MAY NOT serve as an input to the next phase of route selection;
   otherwise, the return value MUST be used as the LOCAL_PREF value
   in any IBGP readvertisement.

(56)  missing forward reference

In Section 9.1.2, the second paragraph on page 78,

   If the NEXT_HOP attribute of a BGP route depicts an address that is
   not resolvable, or if it would become unresolvable if the route was
   installed in the routing table, the BGP route MUST be excluded from
   the Phase 2 decision function.

makes use of the terms "resolvable" vc. "unresolvable" not yet
introduced into the text until that place.
Perhaps, a forward reference should be added to resolve this issue.
Thus, the text should better say:

   If the NEXT_HOP attribute of a BGP route depicts an address that is
   not resolvable, or if it would become unresolvable if the route was
   installed in the routing table, the BGP route MUST be excluded from
|  the Phase 2 decision function (see Section 9.1.2.1 for details).

(57)  pseudocode issue, 1st instance

The pseudocode in Section 9.1.2.2 c), on page 81:

       for m = all routes still under consideration
|          for n = all routes still under consideration
               if (neighborAS(m) == neighborAS(n)) and (MED(n) < MED(m))
                   remove route m from consideration

erroneously admits the reflexive case (n = m) for the inner loop,
which should be excluded.
Therefore, the RFC should say:

       for m = all routes still under consideration
|          for n = all other routes still under consideration
               if (neighborAS(m) == neighborAS(n)) and (MED(n) < MED(m))
                   remove route m from consideration

(58)  pseudocode issue, 2nd instance

A similar issue as the one noted in item (57) above also holds for
Section 9.1.2.2 e), where the text on page 82 says:

         for m = all routes still under consideration
|            for n = all routes in still under consideration
                 if (cost(n) is lower than cost(m))
                     remove m from consideration

where it should say:

         for m = all routes still under consideration
|            for n = all other routes in still under consideration
                 if (cost(n) is lower than cost(m))
                     remove m from consideration

(59)  typo

The 3rd paragraph of section 9.2.1.1, on page 85, says:

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

It should say:

   Since fast convergence is needed within an autonomous system, either
   (a) the MinRouteAdvertisementIntervalTimer used for internal peers
   SHOULD be shorter than the MinRouteAdvertisementIntervalTimer used
|  for external peers, or (b) the procedure described in this section
   SHOULD NOT apply to routes sent to internal peers.

(60)  misleading text (?) / clarification

I strongly suspect that the final sentence in Section 9.2.2.1,
on page 86,

                [...].  In practice, this is not likely to be a problem
         because once an IP packet arrives at the edge of a group of
         autonomous systems, the BGP speaker is likely to have more
         detailed path information and can distinguish individual paths
|        from destinations.

in fact should say:

                [...].  In practice, this is not likely to be a problem
         because once an IP packet arrives at the edge of a group of
         autonomous systems, the BGP speaker is likely to have more
         detailed path information and can distinguish individual paths
|        to different destinations (that it had aggregated).

(61)  missing words, punctuation, and textual enhancements

In Section 9.2.2.2, on page 87, the rule that says:

      ORIGIN attribute:
|        If at least one route among routes that are aggregated has
         ORIGIN with the value INCOMPLETE, then the aggregated route
|        MUST have the ORIGIN attribute with the value INCOMPLETE.
|        Otherwise, if at least one route among routes that are
         aggregated has ORIGIN with the value EGP, then the aggregated
         route MUST have the ORIGIN attribute with the value EGP.  In
|        all other cases,, the value of the ORIGIN attribute of the
         aggregated route is IGP.

should better say:

      ORIGIN attribute:
|        If at least one route among the routes that are aggregated has
         ORIGIN with the value INCOMPLETE, then the aggregated route
|        MUST have INCOMPLETE as its ORIGIN attribute value.
|        Otherwise, if at least one route among the routes that are
         aggregated has ORIGIN with the value EGP, then the aggregated
         route MUST have the ORIGIN attribute with the value EGP.  In
|        all other cases, the value of the ORIGIN attribute of the
         aggregated route is IGP.

(62)  textual improvement / clarification

The second paragraph of Section 10, on page 90, says:

   Two optional timers MAY be supported: DelayOpenTimer, IdleHoldTimer
   by BGP (see Section 8).  Section 8 describes their use.  The full
   operation of these optional timers is outside the scope of this
   document.

It should better say:

|  Two optional timers MAY be supported by BGP (see Section 8):
|  DelayOpenTimer and IdleHoldTimer.  Section 8 describes their use.
   The full operation of these optional timers is outside the scope of
   this document.

(63)  word omission

In Appendix A, the last line on page 92,

|     Clarification of BGP FSM.

should say:

|     Clarification of the BGP FSM.

(64)  outdated wording

In Appendix B, the sentence at the end of the 2nd paragraph on page 93,

                                                   [...].  These
|  abilities allow BGP-4 to support the proposed supernetting scheme
   [RFC1518, RFC1519].
                                       ^^^^^^^^^^
uses oudated wording.  This might easily be corrected.
The text should say:
                                       vvvvvv
                                                   [...].  These
|  abilities allow BGP-4 to support the CIDR supernetting scheme
   [RFC1518, RFC1519].

(65)  completeness, and improved wording

The final paragraph of Appendix D, on page 94,

   Note that quite often BGP, as specified in RFC 1105, is referred to
   as BGP-1; BGP, as specified in [RFC1163], is referred to as BGP-2;
|  BGP, as specified in RFC 1267 is referred to as BGP-3; and BGP, as
|  specified in this document is referred to as BGP-4.

should be enhanced and corrected to say:

   Note that quite often BGP, as specified in RFC 1105, is referred to
   as BGP-1; BGP, as specified in [RFC1163], is referred to as BGP-2;
|  BGP, as specified in [RFC1267] is referred to as BGP-3; and BGP, as
|  specified in [RFC1771] and this document is referred to as BGP-4.
                ^^^^^^^^^^^^^^

(66)  word omission

The first paragraph of Appendix E, on page 94,

   If a local system TCP user interface supports the TCP PUSH function,
   then each BGP message SHOULD be transmitted with PUSH flag set.
|  Setting PUSH flag forces BGP messages to be transmitted to the
   receiver promptly.

should say:

   If a local system TCP user interface supports the TCP PUSH function,
   then each BGP message SHOULD be transmitted with PUSH flag set.
|  Setting the PUSH flag forces BGP messages to be transmitted to the
   receiver promptly.

(67)  specification conflict

As noted in item (16) above, the optimization specified
in Appendix F.4, on page 96, conflicts with text in Section 6.3,
on page 34, if the latter is taken literally.

(68)  outdated Ref.

Within the Security Considerations (which are, unfortunately,
an unnumbered section!), the 4th paragraph on page 98 says:

   Obviously, each should key also be chosen to be difficult for an
|  attacker to guess.  The techniques specified in RFC 1750 for random
   number generation provide a guide for generation of values that could
   be used as keys.  [...]

Long before RFC 4271, RFC 4086 = BCP 106 has been published, which
obsoleted RFC 1750.  Therefore, RFC 4271 should reference RFC 4086
in the text above, and an appropriate Reference (Informative, or even
Normative) should be added on page 101/102.
The above text should say:

   Obviously, each should key also be chosen to be difficult for an
|  attacker to guess.  The techniques specified in [RFC4086] for random
   number generation provide a guide for generation of values that could
   be used as keys.  [...]

(69)  incomplete IANA Considerations

Within the IANA Considerations (which are, unfortunately, an
unnumbered section!), I miss the listing of, and maintenance
guidance for, the BGP-4 Open Message Attribute Types sub-registry.

This concludes my notes on RFC 4271.
(The list has grown to an unexpected size, during elaboration!)

Please comment.

A lot of the items above seem to be appropriate for an Errata Note.

Preferrably, you should submit an Author's Errata Note to the
RFC Editor's RFC Errata web pages, making freely use of the
material supplied above.  But if you like, I can formally
submit an Errata Note on my own, with your advice and consent.

The remainder of the notes above should be kept in mind for use
in a future update of the document, for advancement of BGP-4 to
Full Standard.

Best regards,
  Alfred Hönes.

-- 

+------------------------+--------------------------------------------+
| TR-Sys Alfred Hoenes   |  Alfred Hoenes   Dipl.-Math., Dipl.-Phys.  |
| Gerlinger Strasse 12   |  Phone: (+49)7156/9635-0, Fax: -18         |
| D-71254  Ditzingen     |  E-Mail:  [email-masked]                     |
+------------------------+--------------------------------------------+

P.S: I have written this message in significant hurry, shortly before
     leaving for holidays, so please excuse any typos left in my text,
     and be patient if you send a reply and expect further feedback
     from me.