Re: [Idr] Review of draft-ietf-large-community-06.txt

[Geoff Huston <[e-mail]>]

I should’ve added:

In terms of the quality of the document I think this is sorely needed
and is a pivotal specification for the broad acceptance of four octet 
AS numbers. I support its timely progress to publication as a standards
track specification.

Geoff



> On 4 Nov. 2016, at 11:14 am, Geoff Huston <[e-mail]> wrote:
> 
> I have reviewed this draft and have the following 9 suggestions. All of
> these fall into the category of minor suggestions. I do not believe that
> there is may major structural deficiency in this specification, and the
> document is largely ready, in my view.
> 
> Here are my specific suggestions.
> 
> 
> 1. ----------------
> 
> Title: Large BGP Communities
> 
> to be consistent with previous attribute specifications, how about
> 
> "BGP Large Communities Attribute"
> 
> i.e. switch the order of "Large BGP" to "BGP Large" to be consistent with
> earlier BGP Extended Communities specification in RFC4360
> 
> 2. ----------------
> 
> "Network operators 
> attach BGP communities to routes to identify intrinsic properties of
> these routes."
> 
> I don't think community attributes are an intrinsic property of a route
> advertisement - they are more appropriately expressed as an attached attribute 
> that expresses some desired property.
> 
> how about:
> 
> "Network operators attach BGP communities to routes to associate
> particular properties with these routes."
> 
> 3. ----------------
> 
> "and may apply to an individual route or to a group of routes."
> 
> I am confused - surely the attributes in an Update BGP message apply to the
> collection of routes contained in the Update. It cannot be applied to a 
> single route when the update itself contains multiple routes. Why not
> use the text:
> 
> "and is applied to all routes contained in a BGP Update Message where
> the Communities Attribute is included."
> 
> 4. ----------------
> 
> "[RFC1997] BGP Communities attributes are four-octet values split into
> two two-octet words."
> 
> This is not the case - RFC1997 says: "Communities are treated as 32 bit values"
> I think it would be more accurate to say:
> 
> "BGP Communities attributes are four-octet values [RFC1997]. Common use of
> this attribute type splits this single 32-bit value field into two 16-bit values."
> 
> 5. ----------------
> 
> "The BGP Extended Communities
> attribute [RFC4360] is also unsuitable, as the protocol limit of six
> octets cannot accommodate both a four-octet Global Administrator
> value and a four-octet Local Administrator value, which precludes the
> common operational practice of encoding a target ASN in the Local
> Administrator field."
> 
> 
> Thats a very long sentence. Try:
> 
> "The BGP Extended Communities attribute [RFC4360] is also unsuitable, as
> the protocol limit of six octets for each community value cannot
> accommodate both a four-octet Global Administrator value and a four-octet
> Local Administrator value. This limitation precludes the common
> operational practice of encoding a target ASN in the Local Administrator
> field."
> 
> 6. ----------------
> 
> The aggregation section contains fewer constraints then either RFC1997 or
> RFC4360. It also contains a confusing non-normative “should".
> 
> For reference, RFC4360 states: By default if a range of routes is to be
> aggregated and the resultant aggregates path attributes do not carry the
> ATOMIC_AGGREGATE attribute, then the resulting aggregate should have an
> Extended Communities path attribute that contains the set union of all the
> Extended Communities from all of the aggregated routes.  The default
> behavior could be overridden via local configuration, in which case
> handling the Extended Communities attribute in the presence of route
> aggregation becomes a matter of the local policy of the BGP speaker that
> performs the aggregation.
> 
> Why not use this formulation? i.e.
> 
> "3. Aggregation
> 
> If a set of routes is to be aggregated and the resulting aggregate route's
> path attributes do not include the ATOMIC_AGGREGATE attribute, then the
> resulting aggregate route SHOULD have a Large Communities Attribute formed
> from the set union of all the Large Community values from all of the
> aggregated set of routes.  This behavior MAY be overridden via local
> configuration, in which case handling the Large Communities attribute in
> the presence of route aggregation is determined by the local policy of the
> BGP speaker that performs the aggregation."
> 
> 7. ----------------
> 
> 4.  Canonical Representation
> 
> I am confused here - this section used an example with TWO canonical
> representations:
> 
>   "For example: 64496:4294967295:2, 64496:0:0, or (64496, 111, 222)."
> 
> 
> Conventionally, it's better to use a single canonical representation, so the
> authors should pick either a colon-delimited list, or a bracketed comma+space
> separated object.
> 
> 
> 8. ----------------
> 
> 5.  Reserved Large BGP Community values
> 
> 
> The text reads: 
> 
> "the Global Administrator values specified above could be
> used if there is a future need for them."
> 
> This is entirely unclear. Is it referring to the values that the previous
> paragraph specified as "SHOULD NOT use”? Also "could be used" is a poor
> choice of words for a normative specification.
> 
> I suggest deleting completely the paragraph that contains this quote from
> the document.
> 
> 
> 9. ----------------
> 
> The text reads: 
> 
> "The error handling of Large BGP Communities is as follows:
> 
>   o  A Large BGP Communities attribute SHALL be considered malformed if
>      its length is not a non-zero multiple of 12."
> 
> 
> I think the author is trying to dayL
> 
> "The error handling of Large BGP Communities is as follows:
> 
>   o  A Large Communities attribute SHALL be considered malformed if the
>     length of the Large Communities value, expressed in octets, is not a
>     non-zero multiple of 12."
> 
> 
> thanks,
> 
>  Geoff
>