Subject: Review of draft-ietf-idr-bgp4-17.txt

From: "Abarbanel, Benjamin" <[email-masked]>
Date: Wed, 11 Sep 2002 10:45:06 -0400

Part I.

Technical Comments:
===================
1. P.6  section 4. Message Formats, its possible for the source BGP peer IP
layer to fragment a message such that the receiving BGP peer socket layer
would have to reassemble it. Need to mention this, since BGP implementations
are required to do this.  

2. P. 10, "Authentication Data:" section you might want to add this,
It is also possible to use MD5 (RFC2385) at the transport layer to validate
the entire BGP message.

3. P. 12, right after "Path Attributes". The following sentence should be
added to this section to clarify the scope of the Path Attribute field.
"All attributes in the Path Attribute field represent the characteristics of
all the route prefixes defined in the NLRI field of the message".

4. P.16, last paragraph in section 4.3 as stated,
 "An UPDATE message should not include the same address prefix in the
  WITHDRAWN ROUTES and Network Layer Reachability Information fields,
  however a BGP speaker MUST be able to process UPDATE messages in this
  form. A BGP speaker should treat an UPDATE message of this form as if
  the WITHDRAWN ROUTES doesn't contain the address prefix."
              
This complexity could have been avoided if withdrawn routes and NRLI
prefixes with their attributes were mutually exclusive of each other and
appeared in different update messages. If that was the case, the priority of
which field to process first would have been as simple as using "first come,
first served" message processing approach.

5. P. 20 Its not stated how we delete or modify Path Attributes associated
with NLRI prefixes.

---

From: Kaarthik Sivakumar <[email-masked]>
Date: 11 Sep 2002 11:11:51 -0400

>>> "BA" == Benjamin Abarbanel <Abarbanel> writes:

[...]

BA> 4. P.16, last paragraph in section 4.3 as stated,
BA>  "An UPDATE message should not include the same address prefix in the
BA>   WITHDRAWN ROUTES and Network Layer Reachability Information fields,
BA>   however a BGP speaker MUST be able to process UPDATE messages in this
BA>   form. A BGP speaker should treat an UPDATE message of this form as if
BA>   the WITHDRAWN ROUTES doesn't contain the address prefix."
              
BA> This complexity could have been avoided if withdrawn routes and NRLI
BA> prefixes with their attributes were mutually exclusive of each other and
BA> appeared in different update messages. If that was the case, the priority of
BA> which field to process first would have been as simple as using "first come,
BA> first served" message processing approach.

What you suggest isnt the same as what is there now. The draft says
that NLRI has higher preference to a WITHDRAWN prefix and this makes
the behavior predictable. In what you suggest the behavior isnt
predictable. Or is it? Hmm .. Maybe I am mistaken here, but it doesnt
seem like the same thing.

Kaarthik

---

From: Yakov Rekhter <[email-masked]>
Date: Wed, 11 Sep 2002 08:26:06 -0700

Ben,

> Technical Comments:

*BGP* implementations are *not* required to perform any message reassembly.
(remember this is BGP spec, not TCP and/or IP spec).

Yakov.

---

From: "John G. Scudder" <[email-masked]>
Date: Wed, 11 Sep 2002 12:55:16 -0400

Ben,

As a general comment, if you are going to split up your remarks into 
many tiny messages it would be very helpful if you would use 
descriptive subjects.  Also, FYI you seem to have an off-by-one error 
in your reading of the page numbers.

This message aggregates my replies to all your messages with the 
subject "Review of draft-ietf-idr-bgp4-17.txt".

At 10:35 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>4. In many places in the document the term Transport Connection is used, and
>in the FSM area the term TCP Connection is used. One generic term should be
>used   throughout the document in case a different transport (like SCTP) is
>used to service the BGP sessions.

As Yakov mentioned earlier, BGP is not actually 
transport-independent.  This might equally well argue in favor of 
dropping the "transport connection" pretense and just saying "TCP" 
throughout (applies to the FSM too, BTW).

A future protocol based on BGP 4 might be transport independent 
and/or run over SCTP, but I think that is far beyond our current 
scope.

>would have to reassemble it. Need to mention this, since BGP implementations

Seems to me that this is out of the scope of BGP.  It's a transport 
and network layer thing.

>first served" message processing approach.

If wishes were horses, beggars would ride.  There are plenty of ugly 
bits of the protocol which I'd change if we had a clean slate.  But 
surely something this fundamental to the operation of the 
protocol-as-deployed is not open for revision right now?

>5. P. 20 Its not stated how we delete or modify Path Attributes associated

This is implicit in the definition of "route", which discussion I 
will not repeat here, plus withdraw/replace behavior.  IMO we don't 
need to further belabor the point in the spec (which is already wordy 
enough and then some).

At 10:52 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>1. P.28, 2nd to last paragraph. The line that reads, "To be semantically
>correct, the IP address in the NEXT_HOP must not be the IP address of the
>receiving speaker, and the NEXT_HOP IP address must either be the sender's
>IP address (used to establish the BGP session), or the interface associated
>with the NEXT_HOP IP address must share a common subnet with the receiving
>BGP speaker..."
>
>This is not always true, what if the current ASBR BGP router is advertising
>an external AS route (to a IBGP Peer) whose NEXT_HOP IP address is the IP
>address of the EBGP peer in the other AS?

Doesn't apply.  Right before the text you quoted is this: "Semantic 
correctness applies only to the external BGP links, and only when the 
sender and the receiving speaker are one IP hop away from each other."

At 10:52 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>2. P. 29, Section 6.3. Add this rule near the attribute rules.
>"Multiple prefixes that require the same attribute type with different
>values must never appear in the same update message".

This is implicit in the message format and therefore redundant -- you 
don't have to forbid something which is just plain impossible.  IMO 
redundancy is to be avoided, eschewed and shunned except where the 
point being emphasized is subtle and easily missed (which does not 
apply in this case IMO).

At 10:52 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>3. P. 29, Section 6.5, Please rewrite this sentence from:
>     "If a system does not receive successive KEEPALIVE and/or UPDATE
>      and/or NOTIFICATION messages within the period specified in the Hold
>      Time field of the OPEN message ..."
>
>  To This:
>     "If a system does not receive successive KEEPALIVE and/or UPDATE
>      and/or any other BGP message within the period specified in the Hold
>      Time field of the OPEN message ..."

I disagree as previously discussed (see archives, I won't repeat myself here).

If this change were to be made anyway, it would be important to 
update the language of 4.4 ("KEEPALIVE Message Format") to make clear 
that only updates and keepalives can be relied upon to reset the hold 
time.

At 10:52 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>4. P. 31, section 6.8, Please rewrite this sentence from:
>    "Comparing BGP Identifiers is done by treating them as (4-octet long)
>     unsigned integers."
>             
>  To This:
>    "Comparing BGP Identifiers is done by treating them as large numbers
>based
>     on their IP Address type (e.g. IPv4, IPv6, etc.)."

The BGP identifier is a 4-byte quantity, not a variable quantity.  I 
think the sentence is fine as written.

At 10:58 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>3. P. 13, EGP, are there other EGP protocols other than BGP that are in use?
>If not, change EGP to BGP.

From the references section:

    [1] Mills, D., "Exterior Gateway Protocol Formal Specification",
    RFC904, April 1984.

I suppose it might not hurt to stick a "[1]" in the origin code 
section to add clarity.  The conflation of EGP-the-protocol and 
EGP-the-class-of-protocols is unfortunate, but we're stuck with it 
now.

At 11:05 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>4. P. 50,  line, "If a route in Loc-RIB is excluded from a particular
>Adj-RIB-Out the previously advertised route in that Adj-RIB-Out must be
>withdrawn from service by means of an UPDATE message (see 9.2)."
>
>Would like to rephrase the sentence for clarity,
>"If a route in Loc-RIB is excluded from a particular Adj-RIB-Out and was
>previously advertised via Adj-RIB-Out, it must be withdrawn from service by
>means of an UPDATE message (see 9.2)."

If we're going for total claribility here, I would like to ask what 
other way there is to advertise a route, since the proposed wording 
implies that there are options other than placing it in Adj-RIB-Out. 
I'd say either s/via Adj-RIB-Out// or simply keep the original 
wording.

At 11:05 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>5. P. 50, section 9.1.4, The two fragments of this sentence are redundant
>and don't say anything new or simplify the content. Just keep one fragment.
>"A route describing a smaller set of destinations (a longer prefix) is said
>to be more specific than a route describing a larger set of destinations (a
>shorted prefix); similarly, a route describing a larger set of destinations
>(a shorter prefix) is said to be less specific than a route describing a
>smaller set of destinations (a longer prefix)."

I'm glad we seem to agree about the undesirability of redundancy. 
However I would say that in this case, while the sentence is 
uncomfortably wordy every part of it contributes.  The first clause 
defines "more specific" the second, "less specific".

I'll grant you that the third and fourth parentheses could be dropped.

At 11:05 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>6. P. 52, section 9.2.1.1 Change this sentence for clarity,
>    "This rate limiting procedure applies on a per-destination basis,
>although 
>     the value of MinRouteAdvertisementInterval is set on a per BGP peer
>basis."
>
>  To This:
>    "This rate limiting procedure applies on a per-destination basis,
>although   
>     the value of MinRouteAdvertisementInterval is set on a BGP router (same
>     value for all peers) basis."

I disagree.

At 11:05 AM -0400 9/11/02, Abarbanel, Benjamin wrote:
>8. Since there is going to be an FSM spec, do we need to have FSM
>    descriptions in this spec. Maybe the FSM section should be delete.

Yep.

Regards,

--John

---

From: Yakov Rekhter <[email-masked]>
Date: Wed, 18 Sep 2002 11:02:56 -0700

John,

> throughout (applies to the FSM too, BTW).

That seems like a reasonable suggestion.

> and/or run over SCTP, but I think that is far beyond our current 

Agreed.

Yakov.