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

"Susan Hares" <shares@ndzh.com> Fri, 04 November 2016 01:04 UTC

Return-Path: <shares@ndzh.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 2FF731295FA; Thu, 3 Nov 2016 18:04:39 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.945
X-Spam-Level:
X-Spam-Status: No, score=0.945 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DOS_OUTLOOK_TO_MX=2.845] autolearn=no autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NQxwVyn-8MPV; Thu, 3 Nov 2016 18:04:37 -0700 (PDT)
Received: from hickoryhill-consulting.com (50-245-122-97-static.hfc.comcastbusiness.net [50.245.122.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 93A5A12941C; Thu, 3 Nov 2016 18:04:37 -0700 (PDT)
X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=50.36.174.111;
From: "Susan Hares" <shares@ndzh.com>
To: "'Geoff Huston'" <gih@apnic.net>, "'IETF IDR WG'" <idr@ietf.org>
References: <112dc01d235fd$57f9c370$07ed4a50$@ndzh.com> <C2DABF02-D3CB-4646-B869-FBCE5F05FDA1@apnic.net> <117ea01d23611$a28513e0$e78f3ba0$@ndzh.com> <CED07D95-A426-469C-85B4-DB2FBE52D14A@apnic.net>
In-Reply-To: <CED07D95-A426-469C-85B4-DB2FBE52D14A@apnic.net>
Date: Thu, 3 Nov 2016 21:02:22 -0400
Message-ID: <004801d23637$19dcfd20$4d96f760$@ndzh.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AQH7vYX/r5lvDTiDOvRZn6NWFj+MugIVEsE7AusR2FECKfMMF6A7okGQ
Content-Language: en-us
X-Authenticated-User: skh@ndzh.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/y5Gkh7DzwEDhXUqxa1L_ZcNMM9g>
Cc: rtg-dir@ietf.org
Subject: Re: [Idr] Review of draft-ietf-large-community-06.txt
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 04 Nov 2016 01:04:39 -0000

Geoff: 

Thank you for this amazing quick Review of the draft. 

Sue 

-----Original Message-----
From: Geoff Huston [mailto:gih@apnic.net] 
Sent: Thursday, November 3, 2016 8:14 PM
To: IETF IDR WG
Cc: rtg-dir@ietf.org; John G. Scudder; Susan Hares
Subject: Review of draft-ietf-large-community-06.txt

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