A Border Gateway Protocol 4 (BGP-4)
draft-ietf-idr-bgp4-26

From: Yakov Rekhter
Subject: mistake in the IANA Consideration Section of BGP spec
Date: Mon, 08 Aug 2005 07:34:44 -0700
To: iana@ietf.org, RFC Editor
Cc: Alex Zinin , Bill Fenner
    , skh@nexthop.com, yakov@juniper.net,
    tli@cisco.com

Dear IANA and RFC Editor,

There is a mistake in the IANA Consideration Section of
draft-ietf-idr-bgp4-26.txt, and as a result of this mistake
there is also a mistake in http://www.iana.org/assignments/bgp-parameters.

Specifically, BGP Message Type for NOTIFICATION should be 3,
while BGP Message Type for KEEPALIVE should be 4, not the
other way around.

Many sorry for my mistake. Please let me know what do I need
to do, if any, to correct this mistake.

Yakov.

[Ballot comment]
-------
draft-ietf-idr-bgp-implementation-01.txt
-------
Section 1 speaks about:
    Section 1.3 provides the quick survey results on inter-operability.
    Section 1.4 provides an inter-operability of the 4 implementations.
but there are no such section 1.3 and 1.4

Actually, the whole numbering of sections needs to be checked I think.

----------

Additional Comment from Pekka on 26 Nov 2004:

> Procedural:
>
> 1)
>
> The results of the implementation report do not seem to be completely
> reflected in the document.  For example:
>
>      b) SHALL NOT - Question 228, regarding section 9.2.2.2
>
>          Three vendors (Alcatel, Cisco, Laurel), answered "N" to shall
>          not (meaning they did).  One vendor (NextHop) indicated "O"
>          matching the specification.
>
>        Text:  Routes that have different MULTI_EXIT_DISC attribute
>                SHALL NOT be aggregated.
>
> ==> one implementation does not fulfill the criteria of two
> implementations of a feature, which is required prior to advancing to
> Draft Standard.  Therefore the BGP specification must be changed.
>
> I have not gone through the implementation report in detail, but it
> should be carefully reviewed to ensure that features for which there
> aren't at least two interoperable implementations are removed.
>
>
> 2) A normative reference may need to be moved over to Informative:
>
> Normative References:
>    [RFC2474] Nichols, K., et al.,"Definition of the Differentiated
>    Services Field (DS Field) in the IPv4 and IPv6 Headers", RFC2474,
>    December 1998
>
> ==> this is a Proposed Standard, and a Draft Standard cannot
> reference it normatively.
>
> Nit:
>  'Working Group Summary' in the announcement needs to be updated.
>
>

IANA Comments: IANA Actions OK. 
1 renaming of an existing registry
2 new registries
3 new subcode registries

Note:
  The BGP UPDATE messages may carry one or more Path Attributes, where
  each Attribute contains an 8-bit Attribute Type Code. IANA is already
  maintaining such a registry, entitled "BGP Path Attributes". [note to
  IANA, the registry already exists at http://www.iana.org/assign-
  ments/bgp-parameters, but should be renamed per this document. XXX to
  be removed upon RFC publication.] This document defines the following
  Path Attributes Type Codes:

I see an "XXX" in this paragraph but no where else.

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 and -02 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 and -26 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
                      -01 reviewed by Elwyn Davies, Gen-ART
draft-ietf-idr-bgp-analysis-05 reviewed by Lucy Lynch, Gen-ART
                          -06 reviewed by Elwyn Davies, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
                                      -05 reviewed by Suzanne Woolf, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

I have added reviews as comments on the individual documents, not on the ballots. There are no show-stopper issues identified, but lots of details worthy of notice (still).

Reviewed by Brian Carpenter, Gen-ART
His review:

Summary: About ready for DS, but my point on Appendix E looks like a
small bug, and there are nits.

Ow, ow, OWW! You're asking me to review a 100 page draft by our three
top routing gurus that affects the viablility of the whole Internet.
OK... but I trust Yakov, Tony and Sue a lot more than I trust myself
and so I assume that technically, this is fundamentally OK.

Substantive:
------------

The longer Abstract includes this:

>    This specification covers only the exchange of IP version 4 network
>    reachability information.

I think it would be appropriate to point to RFC 2858 for IPv6 at this
point.
It's in the Non-normative references, but not cited.

> 4.2 OPEN Message Format
...
>      My Autonomous System:
>          This 2-octet unsigned integer indicates the Autonomous System
>          number of the sender.

I think it would be better if AS Number was defined earlier, in the
definitions. But more substantially, I would expect a brief discussion
of the fact that this is a 16 bit field... that seems to be a recurrent
topic.

> Appendix E.  TCP options that may be used with BGP
...
>    If a local system TCP user interface supports setting of the DSCP
>    field [RFC2474] for TCP connections, then the TCP connection used by
>    BGP SHOULD be opened with bits 0-2 of the DSCP field set to 110
>    (binary).

I'm a bit unhappy with this. It is a specific feature of RFC 2474 that
the DSCP bits are *not* encoded and that only all 6 bits as a unit
have meaning. I believe the correct statement would be

    If a local system TCP user interface supports setting of the DSCP
    field [RFC2474] for TCP connections, then the TCP connection used by
    BGP SHOULD be opened with the DSCP field set to Class Selector 6,
    i.e. 110000 binary.

This reflects the guidance in draft-baker-diffserv-basic-classes-04.txt

> IANA Considerations

This should probably also have a reference to the AS Number registry.


Editorial:
----------

* The Abstract occurs twice, before and after the table of contents.
The first
version seems a bit long to me, and would perhaps be better positioned
as an Introduction.

* In the middle of the Non-normative References there is a useless line:
> 3563 Cooperative Agreement Between the ISOC/IETF and ISO/IEC Joint

* I wonder why the Security and IANA Considerations are positioned
as unnumbered sections after the Appendices?

* The denitter is not happy - there are some boilerplate
discrepancies, and various formatting errors:

idnits 1.51

tmp/draft-ietf-idr-bgp4-26.txt:

  Checking nits according to http://www.ietf.org/ID-Checklist.html:

    Checking conformance with RFC 3667/3668 boilerplate...

    * The document claims conformance with section 10 of RFC 2026, but
uses
      some RFC 3667/3668 boilerplate.  As RFC 3667/3668 replaces
section 10 of
      RFC 2026, you should not claim conformance with it if you have
changed
      to using RFC 3667/3668 boilerplate.

    The document seems to lack an RFC 3667 Section 5.4 Copyright Notice
    -- however, there's a paragraph with a matching beginning.
    Boilerplate error?

    (... it does have an RFC 2026 Section 10.4(C) Copyright Notice.)
    The document seems to lack an RFC 3667 Section 5.5 Disclaimer --
    however, there's a paragraph with a matching beginning. Boilerplate
    error?

    The document seems to lack an RFC 3668 Section 5, para 1 IPR
    Disclosure Acknowledgement
    The document seems to lack an RFC 3668 Section 5, para 2 IPR
    Disclosure Acknowledgement
    The document seems to lack an RFC 3668 Section 5, para 3 IPR
    Disclosure Invitation
    There are 101 instances of too long lines in the document,

    -- the longest one being 10 characters in excess of 72.


  Checking nits according to
http://www.ietf.org/ietf/1id-guidelines.txt:

    The document seems to lack a 1id_guidelines paragraph about 6 months
    document validity -- however, there's a paragraph with a matching
    beginning. Boilerplate error?

    The document seems to lack a 1id_guidelines paragraph about the list
    of current Internet-Drafts -- however, there's a paragraph with a
    matching beginning. Boilerplate error?

    The page length should not exceed 58 lines per page -
  but there was 100 longer pages, the longest (page 53) being 79 lines
    It seems as if not all pages are separated by form feeds -
  found 0 form feeds but 100 pages

  Miscellaneous warnings:

    There are 216 instances of lines with hyphenated line breaks in the
document.
    Line 385 has weird spacing: '...setting  any B...'
    Line 2105 has weird spacing: '... system  autom...'
    Line 2992 has weird spacing: '...rom the  under...'
    Line 4122 has weird spacing: '...hen all  the d...'
    Line 4262 has weird spacing: '...y, each  key a...'

    Run idnits with the --verbose option for more detailed information.

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 and -02 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 and -26 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp-analysis-05 reviewed by Lucy Lynch, Gen-ART
                          -06 reviewed by Elwyn Davies, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
                                      -05 reviewed by Suzanne Woolf, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

I have added reviews as comments on the individual documents, not on the ballots. There are no show-stopper issues identified, but lots of details worthy of notice (still).

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 and -02 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp-analysis-05 reviewed by Lucy Lynch, Gen-ART
                          -06 reviewed by Elwyn Davies, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
                                      -05 reviewed by Suzanne Woolf, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

I have added reviews as comments on the individual documents, not on the ballots. There are no show-stopper issues identified, but lots of details worthy of notice (still).

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 and -02 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
                                      -05 reviewed by Suzanne Woolf, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

I have added reviews as comments on the individual documents, not on the ballots. There are no show-stopper issues identified, but lots of details worthy of notice (still).

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
                                      -05 reviewed by Suzanne Woolf, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

Michael Patton's mibagent-survey-05 review:

Most of my concerns from an earlier review (of the -01 version) still
apply.  The review below is essentially the same with the one point
that was addressed deleted.  Given how little of my previous concerns
were addressed, I haven't bothered to try a full re-review, so there
may be additional concerns that I didn't pick up.

----------------------------------------------------------------

Summary: This draft is on the right track but has open issues,
described in the review.

Overview: While this document may well be accurate for what it
includes, it does not, in my opinion, actually document any
interoperability.  As someone whose main job is in this sphere, I do
think that these implementations do meet the interoperability
requirement.  It's just that the information in this document does not
substantiate that.


Details
-------

The wording of the Abstract is pretty sketchy and contains references.
If I understand, what it really says is just:

  This document provides a survey of SNMP agents implementing the
  BGP-4 MIB as specified in RFC1657 and updated by RFCxxxx.

With the xxxx to be filled in by RFCed with whatever
draft-ietf-idr-bgp4-mib-14.txt gets published as.

But the introduction then claims it's for RFC1657 only without the
updates.  And the body seems to go along with that.  So maybe what the
abstract should say is:

  This document provides a survey of SNMP agents implementing the
  BGP-4 MIB as specified in RFC1657.

or the Introduction and the relevant parts of the body also need
updating.

Note that draft-ietf-idr-bgp4-mib-14.txt claims there are errors in
RFC1657 and that it corrects them.  That means this distinction is
important!  RFC1657 is already DS, if the point of documenting
implementations is to move to full Standard, do we actually want to be
documenting interoperation of a known defective version.  I didn't
review the actual MIB, so I don't know what the changes were, but it
seems to me that a change should require the new one to come out at DS
(or even PS, if the changes are significant) and then get
interoperability of _that_ specification to move it to Standard.




The first sentence of the third paragraph of section 2 is incomplete.
I think it may have just lost one word after the section reference.
But since I can't resolve these references, I can't understand the
rest of the paragraph, and thus have no idea what it should actually
be.


At the end of 2.1 "Please note ... are deprecated." needs a reference
for where they are deprecated.  And I have no idea what the second
sentence here is saying or why it's there.


I'm not sure I really grok the tables in Section 2.5, but I think it's
telling me that each of the agents was tested against only one
manager, and no two against the same manager.  While the manager used
to test each agent may have been implemented independently, they
almost certainly tested their own agent/manager for interoperability.
I guess this meets the letter of the law on interoperability, but I'd
prefer to see cross-tests as well before I really accept that
interoperability has really been achieved.  By extrapolation back to
this section from the later---more detailed---sections, it seems that
each agent was only tested against the one that was used in its
implementation, which I'm pretty sure is not the spirit of the
interoperability requirement, even if it does meet the letter...

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-02 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

Michael Patton's mibagent-survey review:

Most of my concerns from an earlier review (of the -01 version) still
apply.  The review below is essentially the same with the one point
that was addressed deleted.  Given how little of my previous concerns
were addressed, I haven't bothered to try a full re-review, so there
may be additional concerns that I didn't pick up.

----------------------------------------------------------------

Summary: This draft is on the right track but has open issues,
described in the review.

Overview: While this document may well be accurate for what it
includes, it does not, in my opinion, actually document any
interoperability.  As someone whose main job is in this sphere, I do
think that these implementations do meet the interoperability
requirement.  It's just that the information in this document does not
substantiate that.


Details
-------

The wording of the Abstract is pretty sketchy and contains references.
If I understand, what it really says is just:

  This document provides a survey of SNMP agents implementing the
  BGP-4 MIB as specified in RFC1657 and updated by RFCxxxx.

With the xxxx to be filled in by RFCed with whatever
draft-ietf-idr-bgp4-mib-14.txt gets published as.

But the introduction then claims it's for RFC1657 only without the
updates.  And the body seems to go along with that.  So maybe what the
abstract should say is:

  This document provides a survey of SNMP agents implementing the
  BGP-4 MIB as specified in RFC1657.

or the Introduction and the relevant parts of the body also need
updating.

Note that draft-ietf-idr-bgp4-mib-14.txt claims there are errors in
RFC1657 and that it corrects them.  That means this distinction is
important!  RFC1657 is already DS, if the point of documenting
implementations is to move to full Standard, do we actually want to be
documenting interoperation of a known defective version.  I didn't
review the actual MIB, so I don't know what the changes were, but it
seems to me that a change should require the new one to come out at DS
(or even PS, if the changes are significant) and then get
interoperability of _that_ specification to move it to Standard.




The first sentence of the third paragraph of section 2 is incomplete.
I think it may have just lost one word after the section reference.
But since I can't resolve these references, I can't understand the
rest of the paragraph, and thus have no idea what it should actually
be.


At the end of 2.1 "Please note ... are deprecated." needs a reference
for where they are deprecated.  And I have no idea what the second
sentence here is saying or why it's there.


I'm not sure I really grok the tables in Section 2.5, but I think it's
telling me that each of the agents was tested against only one
manager, and no two against the same manager.  While the manager used
to test each agent may have been implemented independently, they
almost certainly tested their own agent/manager for interoperability.
I guess this meets the letter of the law on interoperability, but I'd
prefer to see cross-tests as well before I really accept that
interoperability has really been achieved.  By extrapolation back to
this section from the later---more detailed---sections, it seems that
each agent was only tested against the one that was used in its
implementation, which I'm pretty sure is not the spirit of the
interoperability requirement, even if it does meet the letter...

[Ballot discuss]
The tracker indicates that BGP-4 is being recycled at DS and the BGP-4
MIB is being recycled at PS.  If this is true, I think that it is
inappropriate to publish the implementation reports and analysis that
indicate that BGP-4 is ready for publication at STD and the BGP-4 MIB
is ready for publication at DS.  Is the proposed status for these
documents incorrect in the tracker?

I am also concerned about re-cycling or promoting a MIB that is full
of IPv4-only IpAddress objects. I realize that BGP-4 only supports
IPv4, so maybet hat is appropriate in this case?  Is there a plan to
update this MIB in the future to support multiprotocol BGP?  I don't
intend to block publication based on this issue, but I would like to
understand what the plan is going forward.

I have included some specific comments on these documents below.
Other than my questions above, these comments are all non-blocking
editorial comments.  My comments are marked with ">>".

Comments on draft-ietf-idr-bgp4-26.txt:

1. Definition of commonly used terms

>> This section is inconsistently ordered.  For example, Internal Peer
>> comes before IGP (Interior Gateway Protocol), implying that the
>> acronyms are no expanded for purposes of ordering.  However, Route
>> comes before RIB, implying that acronyms are expanded.  Either one
>> is okay with me, just be consistent.

>> The document contains the following packet layout:

      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
      +-+-+-+-+-+-+-+-+
      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> This packet layout seems needlessly confusing...  Why not do it
>> this way?:

      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
                                                      +-+-+-+-+-+-+-+-+
                                                      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |                                              |
      +-+-+-+-+-+-+-+-+                                              |
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> That is what is meant, right?  This format would better match other
>> IETF specifications.

Comments on draft-ietf-idr-bgp-implementation-02.txt:

    For every item listed (259 questions), the respondents indicated
    whether their implementation supports the Functionality/Description
    or not (Y/N) indicated by the RFC2199 [RFC2119] language. Of the 259
    questions in the survey, had two implementations giving an
    affirmative response (two "y" or "y" and "O")

>> s/RFC2199/RFC 2119/ Also, what is an "O" response?  This is
>> explained later, but it would help to explain it before this
>> section.

>> I also didn't think that it was common practice to publish
>> implementation reports as RFCs.  Is this common in the routing
>> area?

Comments on draft-ietf-idr-bgp4-experience-protocol-05.txt &
draft-ietf-idr-bgp-analysis-06.txt:

>> These documents discuss the justification for advancing BGP-4 from
>> Draft Standard to Full Standard, but the ballot seems to indicate
>> that the BGP-4 spec is being considered for DS, not STD.  I think
>> it would be misleading to publish this document in its current form
>> if BGP is not going to STD.

Comments on draft-ietf-idr-bgp4-mib-15.txt:

>> This MIB uses IpAddress objects throughout.  I realize that the
>> base BGP-4 protocol can only be used for IPv4 routing, but I wonder
>> about the wisdom of re-cycling or advancing a MIB that can only be
>> used to manage BGP-4 and not the multi-protocol version of BGP.
>> Was thought given to a combined MIB?  And, if so, why was that
>> approach rejected?  Is a later MIB update expected to cover the
>> multiprotocol case?

Comments on draft-ietf-idr-bgp-mibagent-survey-02.txt:

>> This document is an implementation report for the BGP4 MIB, but
>> according to the tracker, the BGP-4 MIB is doing to PS, not DS.  I
>> think that it would premature to publish an implementation report
>> for a protocol that isn't going to DS.

[Ballot discuss]
The tracker indicates that BGP-4 is being recycled at DS and the BGP-4
MIB is being recycled at PS.  If this is true, I think that it is
inappropriate to publish the implementation reports and analysis that
indicate that BGP-4 is ready for publication at STD and the BGP-4 MIB
is ready for publication at DS.  Is the proposed status for these
documents incorrect in the tracker?

I am also concerned about re-cycling or promoting a MIB that is full
of IPv4-only IpAddress objects. I realize that BGP-4 only supports
IPv4, so maybet hat is appropriate in this case?  Is there a plan to
update this MIB in the future to support multiprotocol BGP?

I have included some specific comments on these documents below.
Other than my questions above, these comments are all non-blocking
editorial comments.  My comments are marked with ">>".

Comments on draft-ietf-idr-bgp4-26.txt:

1. Definition of commonly used terms

>> This section is inconsistently ordered.  For example, Internal Peer
>> comes before IGP (Interior Gateway Protocol), implying that the
>> acronyms are no expanded for purposes of ordering.  However, Route
>> comes before RIB, implying that acronyms are expanded.  Either one
>> is okay with me, just be consistent.

>> The document contains the following packet layout:

      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
      +-+-+-+-+-+-+-+-+
      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> This packet layout seems needlessly confusing...  Why not do it
>> this way?:

      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
                                                      +-+-+-+-+-+-+-+-+
                                                      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |                                              |
      +-+-+-+-+-+-+-+-+                                              |
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> That is what is meant, right?  This format would better match other
>> IETF specifications.

Comments on draft-ietf-idr-bgp-implementation-02.txt:

    For every item listed (259 questions), the respondents indicated
    whether their implementation supports the Functionality/Description
    or not (Y/N) indicated by the RFC2199 [RFC2119] language. Of the 259
    questions in the survey, had two implementations giving an
    affirmative response (two "y" or "y" and "O")

>> s/RFC2199/RFC 2119/ Also, what is an "O" response?  This is
>> explained later, but it would help to explain it before this
>> section.

>> I also didn't think that it was common practice to publish
>> implementation reports as RFCs.  Is this common in the routing
>> area?

Comments on draft-ietf-idr-bgp4-experience-protocol-05.txt &
draft-ietf-idr-bgp-analysis-06.txt:

>> These documents discuss the justification for advancing BGP-4 from
>> Draft Standard to Full Standard, but the ballot seems to indicate
>> that the BGP-4 spec is being considered for DS, not STD.  I think
>> it would be misleading to publish this document in its current form
>> if BGP is not going to STD.

Comments on draft-ietf-idr-bgp4-mib-15.txt:

>> This MIB uses IpAddress objects throughout.  I realize that the
>> base BGP-4 protocol can only be used for IPv4 routing, but I wonder
>> about the wisdom of re-cycling or advancing a MIB that can only be
>> used to manage BGP-4 and not the multi-protocol version of BGP.
>> Was thought given to a combined MIB?  And, if so, why was that
>> approach rejected?  Is a later MIB update expected to cover the
>> multiprotocol case?

Comments on draft-ietf-idr-bgp-mibagent-survey-02.txt:

>> This document is an implementation report for the BGP4 MIB, but
>> according to the tracker, the BGP-4 MIB is doing to PS, not DS.  I
>> think that it would premature to publish an implementation report
>> for a protocol that isn't going to DS.

[Ballot discuss]
The tracker indicates that BGP-4 is being recycled at DS and the BGP-4
MIB is being recycled at PS.  If this is true, I think that it is
inappropriate to publish the implementation reports and analysis that
indicate that BGP-4 is ready for publication at STD and the BGP-4 MIB
is ready for publication at DS.  Is the proposed status for these
documents incorrect in the tracker?

I have included some specific comments on these documents below.  My
comments are marked with ">>".

Comments on draft-ietf-idr-bgp4-26.txt:

1. Definition of commonly used terms

>> This section is inconsistently ordered.  For example, Internal Peer
>> comes before IGP (Interior Gateway Protocol), implying that the
>> acronyms are no expanded for purposes of ordering.  However, Route
>> comes before RIB, implying that acronyms are expanded.  Either one
>> is okay with me, just be consistent.

>> The document contains the following packet layout:

      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
      +-+-+-+-+-+-+-+-+
      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> This packet layout seems needlessly confusing...  Why not do it
>> this way?:

      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
                                                      +-+-+-+-+-+-+-+-+
                                                      |    Version    |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |    My Autonomous System      |          Hold Time          |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |                        BGP Identifier                        |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      | Opt Parm Len  |                                              |
      +-+-+-+-+-+-+-+-+                                              |
      |                                                              |
      |            Optional Parameters (variable)                    |
      |                                                              |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

>> That is what is meant, right?  This format would better match other
>> IETF specifications.

Comments on draft-ietf-idr-bgp-implementation-02.txt:

    For every item listed (259 questions), the respondents indicated
    whether their implementation supports the Functionality/Description
    or not (Y/N) indicated by the RFC2199 [RFC2119] language. Of the 259
    questions in the survey, had two implementations giving an
    affirmative response (two "y" or "y" and "O")

>> s/RFC2199/RFC 2119/ Also, what is an "O" response?  This is
>> explained later, but it would help to explain it before this
>> section.

>> I also didn't think that it was common practice to publish
>> implementation reports as RFCs.  Is this common in the routing
>> area?

Comments on draft-ietf-idr-bgp4-experience-protocol-05.txt &
draft-ietf-idr-bgp-analysis-06.txt:

>> These documents discuss the justification for advancing BGP-4 from
>> Draft Standard to Full Standard, but the ballot seems to indicate
>> that the BGP-4 spec is being considered for DS, not STD.  I think
>> it would be misleading to publish this document in its current form
>> if BGP is not going to STD.

Comments on draft-ietf-idr-bgp4-mib-15.txt:

>> This MIB uses IpAddress objects throughout.  I realize that the
>> base BGP-4 protocol can only be used for IPv4 routing, but I wonder
>> about the wisdom of re-cycling or advancing a MIB that can only be
>> used to manage BGP-4 and not the multi-protocol version of BGP.
>> Was thought given to a combined MIB?  And, if so, why was that
>> approach rejected?  Is a later MIB update expected to cover the
>> multiprotocol case?

Comments on draft-ietf-idr-bgp-mibagent-survey-02.txt:

>> This document is an implementation report for the BGP4 MIB, but
>> according to the tracker, the BGP-4 MIB is doing to PS, not DS.  I
>> think that it would premature to publish an implementation report
>> for a protocol that isn't going to DS.

[Ballot discuss]
draft-ietf-idr-bgp4-24:

  The header and the abstract need to indicate the RFC (or RFCs) that are
  obsoleted by publication of this document.

[Ballot discuss]
draft-ietf-idr-bgp4-24:

  The header and the abstract need to indicate the RFC (or RFCs) that are
  obsoleted by publication of this document.

[Ballot discuss]
-----
draft-ietf-idr-bgp-implementation-01.txt
----------

This limitation seem inappropriate.

    This document may not be modified, and derivative works of it may
    not be created, except to publish it as an RFC and to translate it
    into languages other than English.

What if someone wants to do a revised implementation report in future?

-------
draft-ietf-idr-bgp4-mib-14.txt
-------

The only REAL DISCUSS is the double quotes in the DESCRIPTION clause
of one of the REVISION clauses. Such could be fixed with an RFC-Editor
note.
But the sheer number of little things that bother me and other MIB Doctors
probably justifies to do one more revision.

1. I do not think you can state (page 25):
        -- { bgp 7 } is obsoleted
  because you still have notifications (traps) defined underneath
  that branch, al-beit deprecated.
  Maybe it is better to change
        -- { bgp 7 } is obsoleted

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 }
 
  Into something like:
        -- { bgp 7 } is deprecated. Do not allocate new objects/notifications
        --          underneath this branch.

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 } -- deprecated

2. There are some strange characters in the bulleted list in the
  Security COnsiderations.

3. If you do do (for whatever reason) a new rev, then pls add an IANA
  Considerations section in which you state that IANA does not need to do
  anything. It is part of the new www.ietf.org/ID-Checlist.html

4. The abstract is using somewhat strange SNMP/MIB language, and in fact
  furtehr in the document the language is better.
  If I had it my way, I would reword:
OLD:
  This memo is an extension to the SNMP MIB.  It obsoletes RFC 1657 and
  RFC 1269.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB was converted to use the SNMPv2 SMI, as well
  as updates references to the current SNMP framework documents.

  This memo is intended to document deployed implementations of this
  MIB in a historical context, provide clarifications of some items and
  also note errors where the MIB fails to fully represent the BGP
  protocol.  Work is currently in progress to replace this MIB with a
  new one representing the current state of the BGP protocol and its
  extensions.

NEW:
  This memo defines a portion of the Management Information Base (MIB)
  for use with network management protocols in the Internet community
  In particular, it describes managed objects used for managing the
  Border Gateway Protocol Version 4 or lower.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB module was converted to use the SMIv2
  language. This memo also updates references to the current SNMP
  framework documents.

  This memo is intended to document deployed implementations of this
  MIB module in a historical context, provide clarifications of some
  items and also note errors where the MIB module fails to fully
  represent the BGP protocol.  Work is currently in progress to
  replace this MIB module with a new one representing the current
  state of the BGP protocol and its extensions.

  This document obsoletes RFC 1269 and RFC 1657.



5. On page 5, insode the DESCRIPTION clause of the REVISION,
  you have:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using "should" verb) to initialize the counter

  That use of a double quote within a DESCRIPTION clause (which itself
  is enclosed in double quotes causes a SYNTAX ERROR.

  The/a fix is to use a single quote:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using 'should' verb) to initialize the counter

  This one is certainly BLOCKING, but can be fixed with an RFC-Editor note.


----
from a MIB doctor (Dan Romascanu): draft-ietf-idr-bgp4-mib-14.txt
------
1. "Changes from RFC 1657:

                    1) Fixed the definitions of the traps to
                        make them equivalent to their initial
                        definition in RFC 1269.

I guess that we are doing Notifications rather than Traps nowadays

2. This version of the MIB seems to support a BGP-4 specification that covers only the exchange of IP version 4 network
  reach ability information. Accordingly, the usage of the IpAddress TC is OK, but it would be nice for some text to clarify this.

3. DESCRIPTION clauses of objects with enumerated indices miss explaining each value - see for example bgpPeerState

4. Almost no REFERENCE clauses come to help the implementers - see for example the same bgpPeerState

5. No UNITS clauses for Counter objects or Integer32 objects expressing timers - e.g. bgpPeerHoldTimeConfigured

6. Is bgp 7 obsolete, or deprecated? The ASN.1 and the text are not consistent.

Although none of the above is a show stopper, my personal feeling is that the level of accuracy and documentation of the MIB module defined in this document is somehow below the usual criteria for approval by the IESG, and I would suggest that the authors are required to do an extra round and fix these problems before publication.

-------
draft-ietf-idr-bgp-mibagent-survey-01.txt
-------

0. I would change the title
      BGP v1 MIB Implementation Survey
  To
      BGP4-MIB Implementation Survey
  That is the module name of the MIB module.


1. The abstract states:

    This document provides of survey of BGP-4 [BGP4] protocol
    implementing RFC 1657 MIB agents according to the [BGP-v1-MIB]
    specification.

  RFC-Editor does not allow citations in the abstract. I guess it
  would be better to replace [BGP4] by (RFC yyyy) and [BGP-v1-MIB]
  by (RFC xxxx) and then make the references to the docs in question
  also [RFCyyyy] and [RFCxxxx] so that RFC-Editor can easily match
  (and fix) thenm when RFC numbers are known.

  I am confused that it states that RFC1657 MIB is implemented while
  teh agents implemented accotrding to BGP-v1-MIB. Maybe better to
  say "BGP4-MIB" (as that is the name of the MIB module.

  I do see that your questionaire speaks of RFC1657 though.
  But then the new document basically is a cleaned up (clarifications
  with fixes) of 1657. So maybe with some explanatory text this
  would still be true.
 
2. In sect 1 you sepak of RFC1657 MIB, but did I not understand correctly
  that the new draft-ietf-idr-bgp4-mib-14.txt better describes what has
  been implemented (that is at least what the abstract of that doc states)?
  If so, maybe it is better to state that report is a report on the
  implementation of the MIB module in that new document. I think your
  abstract sort of claims that too.

3. I think that section 2:

      Two or more of the implementations each implement all the objects.
      None of the implementation that responded to the survey implemented
      the read-write variables (section 1.2).  The two TRAPs as specified

  May be confusing too. It sounds as if the read-write varioables are
  not implemented at all, but if I understand it correctly, then they
  HAVE been implemented in read-only mode. Is that not so, and if yes,
  it might be good to clarify.

4. Your references to sect 1.x in section 2, probably should be references
  to sect 2.x if I understand things correctly.

[Ballot discuss]
draft-ietf-idr-bgp4
        9.1.2.1: what is the "longest route"?  The longest prefix?  The longest AS path?

draft-ietf-idr-bgp-vuln
        section 1: Address-spoofing should be listed as a possible form of damage.  Also discuss how this enables active attacks on traffic, by routing it through a modifier station.

draft-ietf-idr-bgp-analysis
        The document speaks of versions 1, 2, and 3 of BGP-4

draft-ietf-idr-bgp4-experience-protocol
        The document speaks of 134,000 prefixes; draft-ietf-idr-bgp-analysis speaks of 120K.  Which is it?

        Section 7.1.1: s/potatoe/potato/ (unless the authors subscribe to the Dan Quayle school of spelling)

        Some of the subsections of Section 17 appear as if they should be sections.

[Ballot comment]
-------
draft-ietf-idr-bgp-implementation-01.txt
-------
Section 1 speaks about:
    Section 1.3 provides the quick survey results on inter-operability.
    Section 1.4 provides an inter-operability of the 4 implementations.
but there are no such section 1.3 and 1.4

Actually, the whole numbering of sections needs to be checked I think.

[Ballot comment]
-------
draft-ietf-idr-bgp-implementation-01.txt
-------
Section 1 speaks about:
    Section 1.3 provides the quick survey results on inter-operability.
    Section 1.4 provides an inter-operability of the 4 implementations.
but there are no such section 1.3 and 1.4

Actually, the whole numbering of sections needs to be checked I think.

[Ballot discuss]
draft-idr-bgp-implementation-01:

  Why isn't this posted at http://www.ietf.org/IESG/implementation.html?

  draft-ietf-idr-bgp4-24:

  The header and the abstract need to indicate the RFC (or RFCs) that are
  obsoleted by publication of this document.

[Ballot discuss]
draft-idr-bgp-implementation-01:

  Why isn't this posted at http://www.ietf.org/IESG/implementation.html?

  draft-ietf-idr-bgp4-24:

  The header and the abstract need to indicate the RFC (or RFCs) that are
  obsoleted by publication of this document.

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp4-experience-protocol-04 reviewed by Mark Allman, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-01 reviewed by Michael Patton, Gen-ART
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

[Ballot comment]
draft-iesg-tcpmd5app
        recuse

Several of the documents have their own security analysis, instead of referring to draft-ietf-idr-bgp-vuln.  Perhaps the information should be merged (if necessary), and the extra text deleted from the other drafts.

[Ballot discuss]
draft-ietf-idr-bgp4
        9.1.2.1: what is the "longest route"?  The longest prefix?  The longest AS path?

draft-ietf-idr-bgp-vuln
        section 1: Address-spoofing should be listed as a possible form of damage.

draft-ietf-idr-bgp-analysis
        The document speaks of versions 1, 2, and 3 of BGP-4

draft-ietf-idr-bgp4-experience-protocol
        The document speaks of 134,000 prefixes; draft-ietf-idr-bgp-analysis speaks of 120K.  Which is it?

        Section 7.1.1: s/potatoe/potato/ (unless the authors subscribe to the Dan Quayle school of spelling)

        Some of the subsections of Section 17 appear as if they should be sections.

[Ballot comment]
draft-idr-bgp-implementation-01:

  The abstract and the introduction state that the the editors make no claim
  as to the accuracy of the information provided.  It would be much better to
  make a positive statement in the introduction, and say nothing in the
  abstract.  Perhaps something like:  The editors have assembled information
  provided by four implementors: Alcatel, Cisco, Laurel, and NextHop.

  draft-ietf-idr-bgp4-24:

  Please replace the first sentence of the Security Considerations with:  A
  BGP implementation MUST support the authentication mechanism specified in
  RFC 2385 [RFC2385].

  draft-ietf-idr-bgp4-mib-14:

  The security considerations section contains non-ASCII characters.

  draft-ietf-idr-bgp-vuln-00:

  Nice job.

  It would have helped me if the event tags had been explained before I
  encountered the first one in section 3.1.1.  Perhaps an additional
  paragraph in section 3 would explain them.

  In the whole document: s/IPSEC/IPsec/

  In section 1, 5th paragraph: s/msut/must/

  The lists in section 1 and section 2 have a different format.  In section 1,
  each item in the list ends with a comma (the "and" appears one entry before
  it should), and in section 2, the list members are full sentences.  I would
  like to see the same style for both lists.

  draft-iesg-tcpmd5app-00:
 
  The security Considerations say:
  >
  > The IESG believes that the variance described here will not affect
  > the security of the Internet.
  >
  I think we should insert "adversely" before "affect."

  draft-ietf-idr-bgp4-experience-protocol-04:

  In section 17.6: s/rationelle/rationale/

  Section 17 includes stuff that is not security relevant, including the
  acknowledgements section.  A minor reorganization is appropriate.

[Ballot discuss]
draft-idr-bgp-implementation-01:

  Why isn't this posted at http://www.ietf.org/IESG/implementation.html?

  Questions 212 and 213 raise a potential concern.
  >
  > Question 212 states: 
  >    "The decision process MUST either install both routes" or
  > Question 213: 
  >    "Aggregate the two routes and install the aggregated route,
  >    provided that both routes have the same value of the
  >    NEXT_HOP attribute"
  >
  None of the implementations aggregate the two routes.  What happens if a
  peer does so?  From this implementation report, we cannot tell whether such
  an implementation would cause interoperability issues.  Perhaps one of the
  implementations that was not willing to respond to a huge questionnaire can
  provide this answer.

  draft-ietf-idr-bgp4-24:

  The header and the abstract need to indicate the RFC (or RFCs) that are
  obsoleted by publication of this document.

[Ballot discuss]
-------
draft-ietf-idr-bgp4-mib-14.txt
-------
1. I do not think you can state (page 25):
        -- { bgp 7 } is obsoleted
  because you still have notifications (traps) defined underneath
  that branch, al-beit deprecated.
  Maybe it is better to change
        -- { bgp 7 } is obsoleted

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 }
 
  Into something like:
        -- { bgp 7 } is deprecated. Do not allocate new objects/notifications
        --          underneath this branch.

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 } -- deprecated

2. There are some strange characters in the bulleted list in the
  Security COnsiderations.

3. If you do do (for whatever reason) a new rev, then pls add an IANA
  Considerations section in which you state that IANA does not need to do
  anything. It is part of the new www.ietf.org/ID-Checlist.html

4. The abstract is using somewhat strange SNMP/MIB language, and in fact
  furtehr in the document the language is better.
  If I had it my way, I would reword:
OLD:
  This memo is an extension to the SNMP MIB.  It obsoletes RFC 1657 and
  RFC 1269.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB was converted to use the SNMPv2 SMI, as well
  as updates references to the current SNMP framework documents.

  This memo is intended to document deployed implementations of this
  MIB in a historical context, provide clarifications of some items and
  also note errors where the MIB fails to fully represent the BGP
  protocol.  Work is currently in progress to replace this MIB with a
  new one representing the current state of the BGP protocol and its
  extensions.

NEW:
  This memo defines a portion of the Management Information Base (MIB)
  for use with network management protocols in the Internet community
  In particular, it describes managed objects used for managing the
  Border Gateway Protocol Version 4 or lower.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB module was converted to use the SMIv2
  language. This memo also updates references to the current SNMP
  framework documents.

  This memo is intended to document deployed implementations of this
  MIB module in a historical context, provide clarifications of some
  items and also note errors where the MIB module fails to fully
  represent the BGP protocol.  Work is currently in progress to
  replace this MIB module with a new one representing the current
  state of the BGP protocol and its extensions.

  This document obsoletes RFC 1269 and RFC 1657.



5. On page 5, insode the DESCRIPTION clause of the REVISION,
  you have:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using "should" verb) to initialize the counter

  That use of a double quote within a DESCRIPTION clause (which itself
  is enclosed in double quotes causes a SYNTAX ERROR.

  The/a fix is to use a single quote:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using 'should' verb) to initialize the counter

  This one is certainly BLOCKING, but can be fixed with an RFC-Editor note.


----
from a MIB doctor (Dan Romascanu): draft-ietf-idr-bgp4-mib-14.txt
------
1. "Changes from RFC 1657:

                    1) Fixed the definitions of the traps to
                        make them equivalent to their initial
                        definition in RFC 1269.

I guess that we are doing Notifications rather than Traps nowadays

2. This version of the MIB seems to support a BGP-4 specification that covers only the exchange of IP version 4 network
  reach ability information. Accordingly, the usage of the IpAddress TC is OK, but it would be nice for some text to clarify this.

3. DESCRIPTION clauses of objects with enumerated indices miss explaining each value - see for example bgpPeerState

4. Almost no REFERENCE clauses come to help the implementers - see for example the same bgpPeerState

5. No UNITS clauses for Counter objects or Integer32 objects expressing timers - e.g. bgpPeerHoldTimeConfigured

6. Is bgp 7 obsolete, or deprecated? The ASN.1 and the text are not consistent.

Although none of the above is a show stopper, my personal feeling is that the level of accuracy and documentation of the MIB module defined in this document is somehow below the usual criteria for approval by the IESG, and I would suggest that the authors are required to do an extra round and fix these problems before publication.

-------
draft-ietf-idr-bgp-mibagent-survey-01.txt
-------

0. I would change the title
      BGP v1 MIB Implementation Survey
  To
      BGP4-MIB Implementation Survey
  That is the module name of the MIB module.


1. The abstract states:

    This document provides of survey of BGP-4 [BGP4] protocol
    implementing RFC 1657 MIB agents according to the [BGP-v1-MIB]
    specification.

  RFC-Editor does not allow citations in the abstract. I guess it
  would be better to replace [BGP4] by (RFC yyyy) and [BGP-v1-MIB]
  by (RFC xxxx) and then make the references to the docs in question
  also [RFCyyyy] and [RFCxxxx] so that RFC-Editor can easily match
  (and fix) thenm when RFC numbers are known.

  I am confused that it states that RFC1657 MIB is implemented while
  teh agents implemented accotrding to BGP-v1-MIB. Maybe better to
  say "BGP4-MIB" (as that is the name of the MIB module.

  I do see that your questionaire speaks of RFC1657 though.
  But then the new document basically is a cleaned up (clarifications
  with fixes) of 1657. So maybe with some explanatory text this
  would still be true.
 
2. In sect 1 you sepak of RFC1657 MIB, but did I not understand correctly
  that the new draft-ietf-idr-bgp4-mib-14.txt better describes what has
  been implemented (that is at least what the abstract of that doc states)?
  If so, maybe it is better to state that report is a report on the
  implementation of the MIB module in that new document. I think your
  abstract sort of claims that too.

3. I think that section 2:

      Two or more of the implementations each implement all the objects.
      None of the implementation that responded to the survey implemented
      the read-write variables (section 1.2).  The two TRAPs as specified

  May be confusing too. It sounds as if the read-write varioables are
  not implemented at all, but if I understand it correctly, then they
  HAVE been implemented in read-only mode. Is that not so, and if yes,
  it might be good to clarify.

4. Your references to sect 1.x in section 2, probably should be references
  to sect 2.x if I understand things correctly.

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-01 reviewed by Michael Patton, Gen-ART
Note: this doc describes RFC 1657 implementations, not bgp4-mib-14 implementations.
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

[Ballot discuss]
-------
draft-ietf-idr-bgp4-mib-14.txt
-------
1. I do not think you can state (page 25):
        -- { bgp 7 } is obsoleted
  because you still have notifications (traps) defined underneath
  that branch, al-beit deprecated.
  Maybe it is better to change
        -- { bgp 7 } is obsoleted

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 }
 
  Into something like:
        -- { bgp 7 } is deprecated. Do not allocate new objects/notifications
        --          underneath this branch.

        bgpTraps          OBJECT IDENTIFIER ::= { bgp 7 } -- deprecated

2. There are some strange characters in the bulleted list in the
  Security COnsiderations.

3. If you do do (for whatever reason) a new rev, then pls add an IANA
  Considerations section in which you state that IANA does not need to do
  anything. It is part of the new www.ietf.org/ID-Checlist.html

4. The abstract is using somewhat strange SNMP/MIB language, and in fact
  furtehr in the document the language is better.
  If I had it my way, I would reword:
OLD:
  This memo is an extension to the SNMP MIB.  It obsoletes RFC 1657 and
  RFC 1269.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB was converted to use the SNMPv2 SMI, as well
  as updates references to the current SNMP framework documents.

  This memo is intended to document deployed implementations of this
  MIB in a historical context, provide clarifications of some items and
  also note errors where the MIB fails to fully represent the BGP
  protocol.  Work is currently in progress to replace this MIB with a
  new one representing the current state of the BGP protocol and its
  extensions.

NEW:
  This memo defines a portion of the Management Information Base (MIB)
  for use with network management protocols in the Internet community
  In particular, it describes managed objects used for managing the
  Border Gateway Protocol Version 4 or lower.

  The origin of this memo is from RFC 1269 "Definitions of Managed
  Objects for the Border Gateway Protocol (Version 3)", which was
  updated to support BGP-4 in RFC 1657.  This memo fixes errors
  introduced when the MIB module was converted to use the SMIv2
  language. This memo also updates references to the current SNMP
  framework documents.

  This memo is intended to document deployed implementations of this
  MIB module in a historical context, provide clarifications of some
  items and also note errors where the MIB module fails to fully
  represent the BGP protocol.  Work is currently in progress to
  replace this MIB module with a new one representing the current
  state of the BGP protocol and its extensions.

  This document obsoletes RFC 1269 and RFC 1657.



5. On page 5, insode the DESCRIPTION clause of the REVISION,
  you have:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using "should" verb) to initialize the counter

  That use of a double quote within a DESCRIPTION clause (which itself
  is enclosed in double quotes causes a SYNTAX ERROR.

  The/a fix is to use a single quote:
                    7) The following objects have had their DESCRIPTION
                        clause modified to remove the text that suggested
                        (using 'should' verb) to initialize the counter

  This one is certainly BLOCKING, but can be fixed with an RFC-Editor note.


----
from a MIB doctor (Dan Romascanu): draft-ietf-idr-bgp4-mib-14.txt
------
1. "Changes from RFC 1657:

                    1) Fixed the definitions of the traps to
                        make them equivalent to their initial
                        definition in RFC 1269.

I guess that we are doing Notifications rather than Traps nowadays

2. This version of the MIB seems to support a BGP-4 specification that covers only the exchange of IP version 4 network
  reach ability information. Accordingly, the usage of the IpAddress TC is OK, but it would be nice for some text to clarify this.

3. DESCRIPTION clauses of objects with enumerated indices miss explaining each value - see for example bgpPeerState

4. Almost no REFERENCE clauses come to help the implementers - see for example the same bgpPeerState

5. No UNITS clauses for Counter objects or Integer32 objects expressing timers - e.g. bgpPeerHoldTimeConfigured

6. Is bgp 7 obsolete, or deprecated? The ASN.1 and the text are not consistent.

Although none of the above is a show stopper, my personal feeling is that the level of accuracy and documentation of the MIB module defined in this document is somehow below the usual criteria for approval by the IESG, and I would suggest that the authors are required to do an extra round and fix these problems before publication.

-------
draft-ietf-idr-bgp-mibagent-survey-01.txt
-------

0. I would change the title
      BGP v1 MIB Implementation Survey
  To
      BGP4-MIB Implementation Survey
  That is the module name of the MIB module.


1. The abstract states:

    This document provides of survey of BGP-4 [BGP4] protocol
    implementing RFC 1657 MIB agents according to the [BGP-v1-MIB]
    specification.

  RFC-Editor does not allow citations in the abstract. I guess it
  would be better to replace [BGP4] by (RFC yyyy) and [BGP-v1-MIB]
  by (RFC xxxx) and then make the references to the docs in question
  also [RFCyyyy] and [RFCxxxx] so that RFC-Editor can easily match
  (and fix) thenm when RFC numbers are known.

  I am confused that it states that RFC1657 MIB is implemented while
  teh agents implemented accotrding to BGP-v1-MIB. Maybe better to
  say "BGP4-MIB" (as that is the name of the MIB module.

  I do see that your questionaire speaks of RFC1657 though.
  But then the new document basically is a cleaned up (clarifications
  with fixes) of 1657. So maybe with some explanatory text this
  would still be true.
 
2. In sect 1 you sepak of RFC1657 MIB, but did I not understand correctly
  that the new draft-ietf-idr-bgp4-mib-14.txt better describes what has
  been implemented (that is at least what the abstract of that doc states)?
  If so, maybe it is better to state that report is a report on the
  implementation of the MIB module in that new document. I think your
  abstract sort of claims that too.

3. I think that section 2:

      Two or more of the implementations each implement all the objects.
      None of the implementation that responded to the survey implemented
      the read-write variables (section 1.2).  The two TRAPs as specified

  May be confusing too. It sounds as if the read-write varioables are
  not implemented at all, but if I understand it correctly, then they
  HAVE been implemented in read-only mode. Is that not so, and if yes,
  it might be good to clarify.

4. Your references to sect 1.x in section 2, probably should be references
  to sect 2.x if I understand things correctly.

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-24 reviewed by Brian Carpenter, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-01 reviewed by Michael Patton, Gen-AT
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

[Ballot discuss]
Problem identified by Michael Patton:
draft-ietf-idr-bgp4-mib claims to fix problems with the RFC 1657 MIB.
draft-ietf-bgp-mibagent-survey is somewhat inconsistent, but seems to claim to survey conformance to the RFC 1657 MIB.
We have advanced "fixed" documents from Proposed to Draft before, but have done so only after being assured that interoperable implementations of the "fixes" existed.
Do we have that assurance here, and if yes, shoudn't that assurance go into the mibagent-survey document?

[Ballot comment]
draft-ietf-idr-bgp-implementation-01 reviewed by Mary Barnes, Gen-ART
draft-ietf-idr-bgp4-mib-14 reviewed by John Loughney, Gen-ART
draft-ietf-idr-bgp-vuln-00 reviewed by Kent Crispin, Gen-ART
draft-ietf-idr-bgp-mibagent-survey-01 reviewed by Michael Patton, Gen-AT
draft-iesg-tcpmd5app-00 reviewed by Spencer Dawkins, Gen-ART

[Ballot comment]
In draft-ietf-idr-bgp4-24.txt, an informative reference to RFC1930 seems likely to be
useful, especially since that reserves specific ASNs as private use.

The abstract in draft-ietf-idr-bgp4-mib-14.txt says:

  This memo is an extension to the SNMP MIB.  It obsoletes RFC 1657 and
  RFC 1269..

That doesn't seem quite right.  1657, which this obsoletes, says:

This memo defines a portion of the Management Information Base (MIB)
  for use with network management protocols in the Internet community.
  In particular, it describes managed objects used for managing the
  Border Gateway Protocol Version 4 or lower [1, 2].

An extension to the SMI, maybe, instead of the snmp mib?

In draft-idr-bgp-implementationt:

The abstract states the the editors make no claim as to the accuracy
of the information provided.  This is repeated in section 1. Not blocking
but this seems strange in an implementation report. 

In draft-ietf-idr-bgp4-experience-protocol-04.txt , I assume the RFC Editor
will talk to them about "hot potatoe" spellings in US terms.  This sentence:

Seemingly more intuitive references that fall outside the vegetable
  kingdom refer to cold potatoe routing as "best exit routing", and hot
  potatoe routing as "closest exit routing", though vegetable.

also looked like it might have gotten interrupted in mid-edit.

I note the this document says that folks use IPSEC to protect BGP
in some circumstances, where the TCPmd5 variance says that
IPSec and TLS  are rarely if ever used.  Not blocking, just thought it worth
noting that these don't quite agree.  Hopefully this means folks are looking
at how to augment TCPmd5.

[Ballot comment]
draft-ietf-idr-bgp-mibagent-survey:
The Security Considerations should be something more detailed than "This document does not address any security issues".  If anything, those words are an admission that something more should be written!  A reading of RFC 3552 would explain why I think this could be better, even if there's not much in this document that has a security impact.

draft-ietf-idr-bgp4-experience-protocol:
draft-ietf-idr-bgp-vuln:
References need to be split normative/informative.

[Ballot comment]
Some comments that should be addressed when the documents are revised.

draft-ietf-idr-bgp4-experience-protocol:

From my previous review:

>  I'm generally quite happy with this document. The only comment I had was about
>  the way the 2119 requirements language is used. In some places (simple search
>  for "MUST" would reveal), the document reads as defining protocol procedures,
>  which should be in the protocol spec. If the intention is to quote the spec,
>  the wording should be changed accordingly.

>  Refs to SBGP and SoBGP should be filled in.


There are still one place where the above problem exists:

> 7.1.2.  Sending MEDs to BGP Peers

>    [BGP4] allows MEDs received from any EBGP peers by a BGP speaker to
>    be passed to its IBGP peers.  Although advertising MEDs to IBGP peers
>    is not a required behavior, it is a common default.  MEDs received
>    from EBGP peers by a BGP speaker MUST NOT be sent to other EBGP
>    peers.

The document also needs new boilerplates per 3667/3668.


draft-ietf-idr-bgp-implementation-01.txt:

> 1. Introduction
>   
>    This revision of the BGP-4 standard [BGP4] updates the BGP standard
>    [RFC1771] to be in alignment with the deployments of the BGP-4
>    protocols.  BGP-4 as deployed in the Internet encompasses both this
>    base specification and additional specifications such as TCP MD5
>    [RFC2385], BGP Route Reflectors [RFC 2796], BGP Confederations 
>    [RFC3065], and BGP Route Refresh [RFC 2918]. 

Don't "This revision of the BGP-4 standard" and "this base specification"
give the reader an impression that this document is the revision and the
base specification?

> 2.3 BGP Implementation Identification

The origins of the implementations are still not specified.


draft-ietf-idr-bgp-analysis-05.txt:
> 2.1.  Key Features
...
>    One of the most important path attributes is the Autonomous System
>    Path, or AS_PATH. Autonomous System's (AS) reachability information
                      ^^^^^^^^^^^^^^^^^^^^^^^^

Is this really what you meant here? The original text said "AS reachability
info...", I commented if is should have been "As", i.e. whether it should read
"as ... info travereses... it is augmented..."

>    traverses the Internet, this information is augmented by the list of
>    autonomous systems that have been traversed thus far, forming the
>    AS_PATH.
...

> 6.1.  Link bandwidth and CPU utilization
...
>
>            BW = O((N + A) * P)
>
>    The following table illustrates the typical amount of bandwidth

The table is missing in the text.

> 6.1.1.  CPU utilization
...
>    During the periods of network instability, BGP routers within the
>    network are generating routing updates that are exchanged using the
>    BGP UPDATE messages.  The greatest overhead per UPDATE message occurs
>    when each UPDATE message contains only a single network. It should
>    pointed out that in practice routing changes exhibit strong locality

"it should BE pointed out"

When you revise the draft, please include the new boilerplates required
by 3667/3668.

draft-ietf-idr-bgp4-24.txt:

needs new boilerplates

AD-review comments on draft-ietf-idr-bgp4-experience-protocol-03.txt:

I'm generally quite happy with this document. The only comment I had was about
the way the 2119 requirements language is used. In some places (simple search
for "MUST" would reveal), the document reads as defining protocol procedures,
which should be in the protocol spec. If the intention is to quote the spec,
the wording should be changed accordingly.

Refs to SBGP and SoBGP should be filled in.

AD-review comments on draft-ietf-idr-bgp-implementation-00.txt:

Several comments inline below.

> 1.1 General
>   
>    This draft of BGP-4 attempts to bring BGP standard as described in

Seems like this is out of context.

>    RFC 1771 in alignment with the deployments of the BGP-4 protocols. 
>    The changes with RFC 1771 are listed in the appendix A of[BGP4]. 
>    BGP-4 as deployed in the Internet encompasses both this base
>    specification and additional specifications such as TCP MD5
>    [RFC2385], BGP Route Reflectors [RFC 2796], BGP Confederations 
>    [RFC3065], and BGP Route Refresh [RFC 2918]. 
>           
>    BGP as a widely deployed cornerstone of Internet technology
>    continues to add additional functionality as the needs within the
>    Internet require. This survey had 259 detailed questions on the
>    compliances with the standard.  3 implementers (Cisco, Laurel,
>    NextHop) sent in implementation reports.  Sections X - Y provides
>    the compilation of those results.
>     
>    X implementers who responded below indicating inter-operability with
>    other implementations.  Of these X implementations, Y also indicated
>    the length of the survey was as problem. The editor recommends that
>    other methods, such as enlisting existing testing vendors be
>    employed to gather more implementation report.
>     
>    Section Z provides the quick survey results on inter-operability. 
>   
> 
> 
> Hares & Retana          Expires - August 2004                [Page 3]
>
>
>
>
> ?                draft-ietf-idr-bgp-implementation-00    February 2004
> 
> 

X, Y, and Z need to be filled out... ;)

> 1.2 Full Survey result summary
>     
>    Significant Differences
>     
>    All 259 survey points had two "y" or "y" and "O" except the
>    following:

At this point of the document, it is not clear what "y" and "0" are.

>     

Generally, if we don't have at least two implementations for a specific feature,
it can't stay in the spec. The granularity of the BGP implementation report
is finer than on a per-feature basis, but we still need to see if the spec needs
to be fixed accordingly. More specific comments below:

>      MUST - Question 214
>       
>      Question 214 about aggregation of  routes. section 9.1.4 had a "N"
>      response from 3 implementers indicating that they install both
>      routes, and 1 yes.

when I look at section 2.43.214, I see:

      Alcatel Y/N/O/Comments: Y
      Cisco  Y/N/O/Comments: Y
      Laurel  Y/N/O/Comments: Y
      NextHop Y/N/O/Comments: Y

wrong section?

>      SHALL NOT - Question 228, regarding section 9.2.2.2
>       
>      Three vendors (Alcatel, Cisco, Laurel), answered "N" to shall not
>      (meaning they did).  One vendor (NextHop) indicate "y" matching
>      the specification.
>       
>        text: Routes that have different MULTI_EXIT_DISC attribute SHALL
>        NOT be aggregated.

If this is considered to be important for protocol correctness, then the WG
may consider softening the requirements language and changing it to "SHOULD
NOT"--apparently some implementers found that under certain circumstances
such routes still need to be aggregated.

>      SHOULD - 2 in appendix F (questions 257, 258)
>       
>      Three vendors said no, one vendor said yes to question 257.  All
>      four vendors indicated no to question 258. (Please note that
>      Appendix F is an optional text section)
>       
>        Text: section F.2 - A BGP speaker which needs to withdraw a
>        destination and send an update about a more specific or less 
>        specific route SHOULD combine them into the same UPDATE message. 
>       
>        Text: Section F.6: The last instance (rightmost occurrence) of
>        that AS number is kept.

Assuming that the appendices are not a normative part of the spec, the
2119 language should not be used there.

...
> 1.4 Implementations and interoperability
>   
>    Short informal summary of implementers reporting implementations and
>    inter-operability
>     
>      [This section will be added later but will have the format below ]
>
>                    Alcatel Cisco Laurel  NextHop 
>        Alcatel                               
>        Cisco                                 
>        Laurel                                                 
>        NextHop                                       

the above table needs to be filled out.

> 1.5 BGP Implementation Identification

This section does not specify the origin of code

>    1.5.0 Alcatel
>   
>    1.5.1 Cisco
>    Implementation Name/Version: Cisco BGP Implementation, 12.0(27)S
>    Date: 11/26/2003
>   
>    1.5.2 Laurel
>   
>    1.5.3 NextHop Technologies
>    Implementation Name/Version: Gated NGC 2.0, 2.2
>    Date: January 2004 
> 
> 
> Hares & Retana          Expires - August 2004                [Page 5]
>
>
>
>
> ?                draft-ietf-idr-bgp-implementation-00    February 2004
> 
> 
>   
> 2. BGP4 Implementation Report
>   
>    For every item listed, the respondents indicated whether their
>    implementation supports the Functionality/Description or not (Y/N)
>    according to the RFC2119 [3] language indicated.

"according to the RFC 2119" should probably be removed. The implementation
either decides to support something the 2119 language prescribes in some form or
it doesn't.

AD-review comments on draft-ietf-idr-bgp-analysis-04.txt:


I have some technical comments regarding periods of stability and instability in
the Internet, BGP steady state, and the security considerations, plus several
editorial remarks.

See inline below, please.

> 2.1.  Key Features
...
>    One of the most important path attributes is the Autonomous System
>    Path, or AS_PATH.  AS reachability information traverses the
                        ^^
                        "As"

>    Internet, this information is augmented by the list of autonomous
>    systems that have been traversed thus far, forming the AS_PATH.  The
>    AS_PATH allows straightforward suppression of the looping of routing
>    information.  In addition, the AS_PATH serves as a powerful and
>    versatile mechanism for policy-based routing.
>
>    BGP enhances the AS_PATH attribute to include sets of autonomous
>    systems as well as lists via the AS_SET attribute.  This extended
>    format allows generated aggregate routes to carry path information
>    from the more specific routes used to generate the aggregate.  It
>    should be noted however, that as of this writing, AS_SETs are rarely
>    used in the Internet [ROUTEVIEWS].
>
>
>
> 2.2.  BGP Algorithms
>
>
>    BGP uses an algorithm that is neither a pure distance vector
>    algorithm or a pure link state algorithm.  It is instead a modified
>    distance vector algorithm referred to as a "Path Vector" algorithm
>    that uses path information to avoid traditional distance vector
>    problems.  Each route within BGP pairs destination with path
>    information to that destination.  Path information (also known as
>    AS_PATH information) is stored within the AS_PATH attribute in BGP.
>    This allows BGP to reconstruct large portions of overall topology
>    whenever required.

I've always been uncomfortable with documents saying that BGP reconstructs the
overall topology. It doesn't really do this like the link-state protocols do,
for example.

>    BGP uses an incremental update strategy in order to conserve
>    bandwidth and processing power.  That is, after initial exchange of
>    complete routing information, a pair of BGP routers exchanges only
>    changes to that information.  Such an incremental update design
>    requires reliable transport between a pair of BGP routers to function
>    correctly.  BGP solves this problem by using TCP for reliable
>    transport.

Should also note that use of TCP as the transport mechanism helps control
congestion and CPU utilization.

>
>    In addition to incremental updates, BGP has added the concept of
>    route aggregation so that information about groups of networks may be
>
>
>
> Meyer and Patel                                  Section 2.2.  [Page 5]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>    aggregated and sent as a single Network Layer Reachability (NLRI).

this doesn't read well. Neither "reachability" nor "information" are countable.
"Single prefix" instead?

>    Finally, note that BGP is a self-contained protocol.  That is, BGP
>    specifies how routing information is exchanged both between BGP
>    speakers in different autonomous systems, and between BGP speakers
>    within a single autonomous system.
>
>
>
> 2.3.  BGP Finite State Machine (FSM)
>
>
>    The BGP FSM is a set of rules that are applied to a BGP speaker's set
>    of configured peers for the BGP operation.  A BGP implementation
>    requires that a BGP speaker must connect to and listen on TCP port
>    179 for accepting any new BGP connections from its peers.  The BGP
>    Finite State Machine, or FSM, must be initiated and maintained for
>    each new incoming and outgoing peer connections.  However, in steady
>    state operation, there will be only one BGP FSM per connection per
>    peer.
>
>    There may exist a temporary period where in a BGP peer may have
>    separate incoming and outgoing connections resulting into two
>    different BGP FSMs for a peer (instead of one).  This can be resolved
>    following BGP connection collision rules defined in the [BGP4].
>
>    Following are different states of BGP FSM for its peers:
>
>    IDLE:          State when BGP peer refuses any incoming
>                    connections.
>
>    CONNECT:        State in which BGP peer is waiting for
>                    its TCP connection to be completed.
>
>    ACTIVE:        State in which BGP peer is trying to acquire a
>                    peer by listening and accepting TCP connection.
>
>    OPENSENT:      BGP peer is waiting for OPEN message from its
>                    peer.
>
>    OPENCONFIRM:    BGP peer is waiting for KEEPALIVE or NOTIFICATION
>                    message from its peer.
>
>    ESTABLISHED:    BGP peer connection is established and exchanges
>                    UPDATE, NOTIFICATION, and KEEPALIVE messages with
>                    its peer.
>
>    There are different BGP events that operate on above mentioned states
>
>
>
> Meyer and Patel                                  Section 2.3.  [Page 6]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>    of BGP FSM for its peers.  These BGP events are used for initiating and
>    terminating peer connections.  They also assist BGP in identifying any
>    persistent peer connection oscillations and provide a mechanism
>    for controlling them.
>
>    Following are different BGP events:
>
>    Manual Start:          Manually start the peer connection.
>
>    Manual Stop:            Manually stop the peer connection.
>
>    Automatic Start:        Local system automatically starts the peer
>                            connection.
>
>    Manual start with
>    passive TCP flag:      Local system administrator manually starts the
>                            peer connection with peer in passive mode.
>
>    Automatic start
>    with passive TCP flag:  Local system administrator automatically starts
>                            the peer connection with peer in passive mode.
>
>    Automatic start
>    with bgp_stop_flap
>    option set:            Local system administrator automatically starts
>                            the peer connection with peer oscillation
>                            damping enabled.
>
>    Automatic start with
>    bgp_stop_flap option
>    set and passive TCP
>    establishment
>    option set:            Local system administrator automatically starts
>                            the peer connection with peer oscillation
>                            damping enabled and with peer in passive mode.
>
>    Automatic stop:        Local system automatically stops the
>                            BGP connection.
>
>    Both, Manual Start and Manual Stop are mandatory BGP events.  All
>    other events are optional.
>

Ummmm... the above are "administrative" events only. There are other events in
the FSM, and most of other events are mandatory.

>
>
>
>
>
>
>
>
> Meyer and Patel                                  Section 2.3.  [Page 7]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
> 3.  BGP Capabilities
>
>
>    The BGP Capability mechanism [RFC2842] provides an easy and flexible
>    way to introduce new features within the protocol.  In particular,
>    the BGP capability mechanism allows peers to negotiate various
>    optional features during startup.  This allows the base BGP protocol
>    to contain only essential functionality, while at the same time
>    providing a flexible mechanism for signaling protocol extensions.
>
>
>
> 4.  BGP Persistent Peer Oscillations
>
>
>    Ideally, whenever a BGP speaker detects an error in any peer
>    connection, it shuts down the peer and changes its FSM state to IDLE.
>    BGP speaker requires a Start event to re-initiate its idle peer
>    connection.  If the error remains persistent and BGP speaker
>    generates Start event automatically then it may result in persistent
>    peer flapping.  However, although peer oscillation is found to be
>    wide-spread in BGP implementations, methods for preventing persistent
>    peer oscillations are outside the scope of base BGP protocol
>    specification.
>
>
>
> 5.  Implementation Guidelines
>
>
>    A robust BGP implementation is work conserving.  This means that if
>    the number of prefixes is bound, arbitrarily high levels of route
>    change can be tolerated with bounded impact on route convergence for
>    occasionally changes in generally stable routes.

"occasional"?

>
>    A BGP implementation under high load conditions should empty as much
>    inbound routing updates from its input streams, processing only the
>    most recent route if the route for a given NLRI changes multiple
>    times.  TCP also provides blocking on the writes on the sender side.
>    A BGP implementation under load should expect blocks on write calls
>    and send only the most recent routes when sockets unblock rather than
>    sending entire history.
>
>    A robust implementation of BGP should have the following
>    characteristics:
>          1.  It is able to operate in almost arbitrarily high levels
>              of route flap without loosing peerings (failing to send
>              keepalives) or loosing other protocol adjacencies as a
>
>
>
> Meyer and Patel                                    Section 5.  [Page 8]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>              result of BGP load.
>
>          2.  Instability of a subset of routes should not affect the
>              route advertisements or forwarding associated with the set
>              of stable routes.
>
>          3.  High levels of instability and peers of different CPU speed
>              or load resulting in faster or slower processing of routes
>              should not cause instability and should have a bounded
>              impact on the convergence time for generally stable routes.
>
>    Numerous robust BGP implementations exist.  Producing a robust
>    implementation is not a trivial matter but clearly achievable.
>
>
>
>
> 6.  BGP Performance characteristics and Scalability
>
>
>    In this section, we provide "order of magnitude" answers to the
>    questions of how much link bandwidth, router memory and router CPU
>    cycles the BGP protocol will consume under normal conditions.  In
>    particular, we will address the scalability of BGP and its
>    limitations.
>
>
>
> 6.1.  Link bandwidth and CPU utilization
>
>
>    Immediately after the initial BGP connection setup, BGP peers
>    exchange complete set of routing information.  If we denote the total
>    number of routes in the Internet by N, the mean AS distance of the
>    Internet by M (distance at the level of an autonomous system,
>    expressed in terms of the number of autonomous systems), the total
>    number of unique AS paths by A, and assume that the networks are
>    uniformly distributed among the autonomous systems, then the worst
>    case amount of bandwidth consumed during the initial exchange between
>    a pair of BGP speakers is
>
>            BW = O(N + (M * A))
>
>    The following table illustrates the typical amount of bandwidth
>    consumed during the initial exchange between a pair of BGP speakers
>    based on the above assumptions (ignoring bandwidth consumed by the
>    BGP Header).  For purposes of the estimates here, we will calculate
>    BW = 4 * (N + (M * A)).
>
>
>
> Meyer and Patel                                  Section 6.1.  [Page 9]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>    # NLRI      Mean AS Distance      # AS's    Bandwidth (MR)
>    ----------  ----------------      ------    ----------------
>    40,000      15                    400        184,000  bytes
>    100,000      10                    10,000    800,000  bytes
>    120,000      10                    15,000    1,080,000 bytes
>    140,000      15                    20,000    1,760,000 bytes
>
>    [note that most of this bandwidth is consumed by the NLRI exchange]

Is the caption for column 3 correct? It says "# AS's", which reads as "number of
AS's" while it seems it should be the number of unique paths.
>
>    BGP was created specifically to reduce the size of the set of NLRI
>    entries which have to be carried and exchanged by border routers.
>    The aggregation scheme, defined in RFC 1519 [RFC1519], describes the
>    provider-based aggregation scheme in use in today's Internet.
>
>    Due to the advantages of advertising a few large aggregate blocks
>    instead of many smaller class-based individual networks, it is
>    difficult to estimate the actual reduction in bandwidth and
>    processing that BGP has provided over BGP-3.  If we simply enumerate
>    all aggregate blocks into their individual class-based networks, we
>    would not take into account "dead" space that has been reserved for
>    future expansion.  The best metric for determining the success of
>    BGP's aggregation is to sample the number NLRI entries in the
>    globally connected Internet today and compare it to projected growth
>    rates before BGP was deployed.
>
>    At the time of this writing, the full set of exterior routes carried
>    by BGP is approximately 120,000 network entries [ROUTEVIEWS].
>
>
>
> 6.1.1.  CPU utilization
>
>
>    An important and fundamental feature of BGP is that BGP's CPU
>    utilization depends only on the stability of the Internet.  If the
>    Internet is stable, then the only link bandwidth and router CPU
>    cycles consumed by BGP are due to the exchange of the BGP KEEPALIVE
>    messages.  The KEEPALIVE messages are exchanged only between peers.
>    The suggested frequency of the exchange is 30 seconds.  The KEEPALIVE
>    messages are quite short (19 octets), and require virtually no
>    processing.  As a result, the bandwidth consumed by the KEEPALIVE
>    messages is about 5 bits/sec.  Operational experience confirms that
>    the overhead (in terms of bandwidth and CPU) associated with the
>    KEEPALIVE messages should be viewed as negligible.
>
>    During periods of Internet instability, changes to the reachability
>    information are passed between routers in UPDATE messages.  The

While theoretically the text is correct and we could talk about periods of
stability and instability of the Internet, I wonder if this text is still
applicable from the practical perspective. I.e., the continuous churn that the
Internet BGP speakers experience ensures that they practically always have
something to process.

Probably instead of talking about periods of "Internet stability" and "Internet
instability" we could talk about something like "periods of stable state among
BGP speakers when they do no have new updates to communicate to each other".

>
> Meyer and Patel                                Section 6.1.1.  [Page 10]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>    greatest overhead per UPDATE message occurs when each UPDATE message
>    contains only a single network.  It should be pointed out that in
>    practice routing changes exhibit strong locality with respect to the
>    AS path.  That is, routes that change are likely to have common AS
>    path.  In this case, multiple networks can be grouped into a single
>    UPDATE message, thus significantly reducing the amount of bandwidth
>    required (see also Appendix F.1 of [BGP4]).
>
>    Since in the steady state the link bandwidth and router CPU cycles
>    consumed by the BGP protocol are dependent only on the stability of
>    the Internet, it follows that BGP should have no scaling problems in
>    the areas of link bandwidth and router CPU utilization.

Not sure I follow here. What is meant by the "steady state" here? If it means
convergence on a stable topology after the initial exchange of updates, then why
talk about "stability of the Internet"? In other words, if the Internet is
unstable then can we say that the BGP speakers are in steady state? In the lack
of topology changes, it seems that the consumption should instead depend on the
number of peers (affects KEEPALIVE processing) and the number of persistently
oscillating routes potentially present in the network...

> This assumes
>    that as the Internet grows,  the overall stability of the inter-AS
>    connectivity of the Internet can be controlled.

please specify how it is assumed to be "controlled", e.g. through operational
practices.

> In particular, while
>    the size of the IPv4 Internet routing table is bounded by O(232 * M),

232 or 2^32?

>    (where M is a slow-moving function describing the AS
>    interconnectivity of the network),

slow-moving or slow-growing?

> no such bound can be formulated
>    for the dynamic properties (i.e., stability) of BGP.  Although, the
>    dynamic properties of the network cannot be quantitatively bounded,
>    they can be controlled within BGP.  Beyond certain changes in the
>    network, BGP can start to suppress such changes using BGP Route Flap
>    Damping [RFC2439], pacing of its route updates, or BGP would be
>    unable to keep up with the changes and force suppression of multiple
>    changes over very short periods by causing the BGP peer socket to
>    block on the sender.
>
>
>
> 6.1.2.  Memory requirements
>
>
>    To quantify the worst case memory requirements for BGP, we denote the
>    total number of networks in the Internet by N, the mean AS distance
>    of the Internet by M (distance at the level of an autonomous system,
>    expressed in terms of the number of autonomous systems), the total
>    number of unique AS paths as A.  Then the worst case memory
>    requirements (MR) can be expressed as
>
>
>            MR = O(N + (M * A))
>
>
>    Since a mean AS distance M is a slow moving function of the
>    interconnectivity ("meshiness") of the Internet, for all practical
>    purposes the worst case router memory requirements are on the order
>    of the total number of networks in the Internet times the number of
>    peers the local system is peering with.  We expect that the total
>    number of networks in the Internet will grow much faster than the
>
>
>
> Meyer and Patel                                Section 6.1.2.  [Page 11]
> ?
> INTERNET-DRAFT            Expires: March 2004            September 2003
>
>
>    average number of peers per router.  As a result, BGP's memory
>    scaling properties are linearly related to the total number of
>    networks in the Internet.
>
>    The following table illustrates typical memory requirements of a
>    router running BGP.  We denote average number of routes advertised by
>    each peer as N, the total number of unique AS paths as A, the mean AS
>    distance of the Internet as M (distance at the level of an autonomous
>    system, expressed in terms of the number of autonomous systems),
>    number of bytes required to store a route as R, and number of bytes
>    required to store one AS in an AS path as P.  It is assumed that each
>    network is encoded as four bytes, each AS is encoded as two bytes,
>    and each networks is reachable via some fraction of all of the peers
>    (# BGP peers/per net).  For purposes of the estimates here, we will
>    calculate MR = ((N * R) + (M * A) * P)
>
>
>      # Networks  Mean AS Distance # AS's # BGP peers/per net Memory Req (MR)
>      ----------  ---------------- ------ ------------------- --------------
>      100,000          20        3,000        20            1,040,000
>      100,000          20        15,000        20            1,040,000
>      120,000          10        15,000        100            75,000,000
>      140,000          15        20,000        100          116,000,000
>
>
>    In analyzing BGP's memory requirements, we focus on the size of the
>    forwarding table (and ignoring implementation details).  In
>    particular, we derive upper bounds for the size of the forwarding
>    table.

The above doesn't look like calculations for a forwarding table--the FIB doesn't
normally contact AS-PATH info or all possible paths to a destination.

>
> 11.  Security Considerations
>
>
>    This document presents an analysis of the BGP protocol and as such
>    presents no new security implications for BGP.

I'm afraid the IESG will expect to see some more here. Specifically, I would
suggest that the document talks about available security mechanisms, and the
analysis of how they affect processing overhead, BW, and scalability.
Certainly a pointer to the bgp-vuln document would be useful.

comments from a rtg-dir member (Russ):

Abstract:

This information is sufficeint to construct a graph of the AS connectivity
from which routing loops may be pruned and some policy decisions at the AS
level may be enforced.

UPDATE Message Format:

The information in the UPDATE message can be used to construct a graph
describing the relationships of the various Autonomous Systems.

In both cases this is true, I suppose, but in neither case does this really
describe what the AS Path is used for, right? I would think we'd want to
describe it less in terms of a "graph of the connectivity in the
internetwork," and more in terms of "a graph of the path through Autonomous
Systems ued to reach the destination advertised." It could be confusing,
since there isn't anyplace where we discuss building a graph of
inconnectivity between the Autonomous Systems....

-----

Forwarding Paradigm:

This document uses the term "Autonomous System" (AS)  throughout....

This entire paragraph is a repeat--I'd leave it just in the definitions.

-----

Forwarding Paradigms:

The initial data flow....

This paragraph has two different thoughts in it, one about incremental
updates, and the other about keeping data that you've received. It seems
like just putting a return after "as the routing tables change."

-----

Forwarding Paradigms:

The paragraph starting "KEEPALIVE messages" should, I think, be moved up
above the section on route exchange. I don't know why, it just seems less
like it's jumping all over the place that way.

-----

3.1 Routes: Advertisement and Storage

It almost seems like the section about The initial data flow should maybe
be put entirely under this section someplace (?).

The first paragraph in this section is really a definition of a route vs a
prefix, and should probably be in the definitions.

The paragraph "Changing attribute of a route...." needs a "the," or
attribute needs an "s."

-----

3.2 Routing Information Bases

b) Loc-RIB....

I think it might be useful to state the contents of the Loc-RIB are
actually installed in the local routing table, and thus used for forwarding
packets on this router. I don't see anyplace this connection is made
explicit, it seems more like it's implicit throughout the doc.

-----

Page 18, a) LOCAL_PREF

"....to inform other peers...." should be "....to inform its other
peers...."

-----

Network Layer Reachability Information

"This varibale length field contains a list of IP address prefixes."

I think we can kill "address" here.

a) Length

"The Length field inidicates...." The sentence can start with
"Indicates..."

b) Prefix

"The Prefix field indicates...." The sentence can start with
"Indicates...."

-----

Network Layer Reachability Information

"An UPDATE message can list multiple routes to be withdrawn...."

Actually, we don't withdraw routes, we withdraw prefixes, right? The next
paragraph shows this confusion, by talking about routes without attributes,
but routes are prefixes combined with attributes, so.... They aren't
routes, they're prefixes. You remove routes based on withdrawn prefixes, I
think.

------

5. Path Attributes

"Well-known attributes MUST be recognized by all BGP implementations."

This sentence, as strange as it may sound, implies it's the attributes
fault if the BGP implementation doesn't recogonize it, that it's up to the
attribute definers to, in some way, make certain that BGP implementations
will recognize it. I think it should probably be worded the other way
'round:

"BGP implementations MUST recognize all well-known attributes."

-----

5. Path Attributes

"All well-known attributes MUST be passed along (after proper updating, if
necessary) to other BGP peers."

This just seems a little rough. Maybe this:

"Once a BGP peer has updated any well-known attributes, it MUST pass these
attributes in any updates it transmits to its peers."

-----

5.1.1 ORIGIN

"Its value SHOULD NOT be changed by any other speaker."

I really think this should be "MUST NOT." I can't think of any reason it
wouldn't be, except in the case of aggregation, and that case could be
mentioned here as the only known exception (?).

AD-review comments:

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)
>                     
>
>
> 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.