Subject: BGP-17 Draft Comments...

From: Russ White <[email-masked]>
Date: Thu, 18 Apr 2002 21:12:43 -0400 (EDT)

Alvaro and I spent some time going over draft 17, and came up
with just a few comments, below. Some of them may overlap to some
degree.

:-)

Russ

__________________________________
[email-masked] CCIE <>< Grace Alone

-----

4.3 UPDATE Message Format
...

This section defines the encoding for announcing and withdrawing
paths...saying things like:

      Total Path Attribute Length:

         This 2-octet unsigned integer indicates the total length of the
         Path Attributes field in octets. Its value must allow the
         length of the Network Layer Reachability field to be determined
         as specified below.

         A value of 0 indicates that no Network Layer Reachability
         Information field is present in this UPDATE message.

But rfc2858 (Multiprotocol Extensions for BGP-4) reads:

   An UPDATE message that carries no NLRI, other than the one encoded in
   the MP_REACH_NLRI attribute, should not carry the NEXT_HOP attribute.
   If such a message contains the NEXT_HOP attribute, the BGP speaker
   that receives the message should ignore this attribute.

How can the Total Path Attribute Length be = 0 (to indicate no NLRI), but
still carry other attributes?!?!

Suggested new text - eliminate the second paragraph:

================
Total Path Attribute Length:

This 2-octet unsigned integer indicates the total length of the Path
Attributes field in octets. Its value must allow the length of the Network
Layer Reachability field to be determined as specified below.
================

A reference should be made to rfc2858 for "alternate
advertisement/withdraw mechanisms".

---

From: Pedro Roque Marques <[email-masked]>
Date: Thu, 18 Apr 2002 18:45:26 -0700 (PDT)

Russ White writes:

> Alvaro and I spent some time going over draft 17, and came up with
> just a few comments, below. Some of them may overlap to some degree.

Russ,
Some comments on your comments bellow... and generic meta-comment not
directed at you in particular...

As with software, it is quite possible to, when attempting to modify a
specification to correct some perceived problem, to introduce new
ones. So, i think we should be quite conservative when changing the
spec... sometimes inocent changes introduce backwards compatibility
problems that end up being quite problematic in the field... and once
the spec is modified one is kind of in a jam between what the spec
says and interoperability.

> 4.3 UPDATE Message Format ...

>          This 2-octet unsigned integer indicates the total length of
> the Path Attributes field in octets. Its value must allow the length
> of the Network Layer Reachability field to be determined as
> specified below.

>    An UPDATE message that carries no NLRI, other than the one
> encoded in the MP_REACH_NLRI attribute, should not carry the
> NEXT_HOP attribute.  If such a message contains the NEXT_HOP
> attribute, the BGP speaker that receives the message should ignore
> this attribute.

> How can the Total Path Attribute Length be = 0 (to indicate no
> NLRI), but still carry other attributes?!?!

There is no incompatibility between the 2 statements...

An update containing only withrawls will have a 0 attribute
length... this message won't include MP_ attributes...

An update may include MP_{UN,}REACH attributes and no non-mp NLRI...

> ================ Total Path Attribute Length:

> Attributes field in octets. Its value must allow the length of the
> Network Layer Reachability field to be determined as specified
> below.  ================

This is incorrect imho... one can have attributes and no NLRI...

I don't think there is any reason to do any changes here...

>       Marker:

>          This 16-octet field contains a value that the receiver of
> the message can predict. If the Type of the message is OPEN, or if
> the OPEN message carries no Authentication Information (as an
> Optional Parameter), then the Marker must be all ones.  Otherwise,
> the value of the marker can be predicted by some a computation
> specified as part of the authentication mechanism (which is
> specified as part of the Authentication Information) used. The
> Marker can be used to detect loss of synchronization between a pair
> of BGP peers, and to authenticate incoming BGP messages.

> I would rewrite it to say:

> This 16-octet field contains a value that the receiver of the
> message can predict.  If the Type of the message is OPEN, or if no
> authentication mechanism (which is specified as part of the
> Authentication Information Optional Parameter in the OPEN message)
> is used, then the Marker must be all ones.  Otherwise, the value of
> the marker can be predicted by a computation specified as part of
> the authentication mechanism in use.  The Marker can be used to
> detect loss of synchronization between a pair of BGP peers, and to
> authenticate incoming BGP messages.

I prefer the original text as it makes it clear that if you do not use
authentication then the marker is all ones... since to my knowledge
the bgp authentication field is not used, the first phrase in the
original text makes it quite clear what to do.

> ----

> 4.3

> The UPDATE message always includes the fixed-size BGP header, and
> also includes the other fields as shown below (note, some of the
> shown fields may not be present in every UPDATE message):

> I assume that the fixed length fields must always be included, or
> there is not way to parse the update format. In light of this, it
> might be better to change this to state: "....(note, some of the
> shown variable length fields may not be present....)"

I think the original text is just fine...

> ----

> 7.

> version number each supports. If an open attempt fails with an Error
> Code OPEN Message Error, and an Error Subcode Unsupported Version
> Number, then the BGP speaker has available the version number it
> tried, the version number its peer tried, the version number passed
> by its peer in the NOTIFICATION message, and the version numbers
> that it supports.

> This sentence doesn't seem to make any sense.

Makes perfect sense to me... tells one to keep around the versions
that where exchanged for diagnostics purposes...

> Maybe:

> If an open attempt fails bceause of a version number mismatch, then
> the local BGP speaker should receive an Error Code OPEN Message
> Error with an Error Subcode Unsupported Version Number. On receiving
> this notification, the local BGP speaker should have available the
> version number it tried, the version number its peer tried, and the
> version numbers it supports.

Version negotiation is not being used at the moment... as far as i
know nobody uses version < 4 anymore... so why change the document ?

> 9.2.1.1

>    Two UPDATE messages sent from a single BGP speaker that advertise
> feasible routes to some common set of destinations received from
> external peers must be separated by at least
> MinRouteAdvertisementInterval. Clearly, this can only be achieved
> precisely by keeping a separate timer for each common set of
> destinations. This would be unwarranted overhead. Any technique
> which ensures that the interval between two UPDATE messages sent
> from a single BGP speaker that advertise feasible routes to some
> common set of destinations received from external peers will be at
> least MinRouteAdvertisementInterval, and will also ensure a constant
> upper bound on the interval is acceptable.

> Didn't we determine (on list) that this was true of unfeasible
> destinations as well?

I think that the only consensus on this issue is that there is no
consensus... this is one of those areas where each vendor just has to
apply it's own judgement, imho.

AFAIK no vendor follows the above recomendation anyway... with
different equipment exibiting different behaviours without causing
interoperability issues.

> ----

> 9.2.1.3 Jitter

>    To minimize the likelihood that the distribution of BGP messages
> by a given BGP speaker will contain peaks, jitter should be applied
> to the timers associated with MinASOriginationInterval, Keepalive,
> and MinRouteAdvertisementInterval.

> Hmmm.... Maybe:

> "To minimize the likelihood that the distribution of BGP messages by
> a given BGP speaker will cluster around a particular time, causing a
> short term spike in traffic which could overwhelm the network or the
> peer, ...."

I don't see a reason to make this change... the "peaks" argument above
is good enought imho.

  Pedro.

---

From: Russ White <[email-masked]>
Date: Thu, 18 Apr 2002 22:24:27 -0400 (EDT)

> As with software, it is quite possible to, when attempting to
> modify a specification to correct some perceived problem, to
> introduce new ones. So, i think we should be quite
> conservative when changing the spec... sometimes inocent
> changes introduce backwards compatibility problems that end
> up being quite problematic in the field... and once the spec
> is modified one is kind of in a jam between what the spec

Agreed.... Responses within.... :-)

> I think the original text is just fine...

But, just adding the words "variable length" makes it clearer
what fields we are talking about not being in the update. Reading
through the entire draft, you would know that, but it's easier to
read if it's clear here.

> Version negotiation is not being used at the moment... as far
> as i know nobody uses version < 4 anymore... so why change
> the document ?

Well, we might change versions in the future, so it would be nice
if this text were clear to someone just implementing. 

> I don't see a reason to make this change... the "peaks"
> argument above is good enought imho.

But what do you mean by "peaks?" That's just the word I was
trying to kill off, so as to provide a "more complete" or "more
correct" explanation.

:-)

Russ