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

"Jakob Heitz (jheitz)" <jheitz@cisco.com> Fri, 04 November 2016 21:55 UTC

Return-Path: <jheitz@cisco.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 EAA2E1297C3 for <idr@ietfa.amsl.com>; Fri, 4 Nov 2016 14:55:53 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -16.018
X-Spam-Level:
X-Spam-Status: No, score=-16.018 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-1.497, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
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 uIH5rPOVeppv for <idr@ietfa.amsl.com>; Fri, 4 Nov 2016 14:55:51 -0700 (PDT)
Received: from alln-iport-7.cisco.com (alln-iport-7.cisco.com [173.37.142.94]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B45591297D4 for <idr@ietf.org>; Fri, 4 Nov 2016 14:55:51 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=18602; q=dns/txt; s=iport; t=1478296551; x=1479506151; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=KGStD/vF96DrTqSX8B+8zQL3kXxfqyhFK785Z7hokoI=; b=Qiva+AoT0dgmDzfAUfN5SKHbKmik3mTXGNzRC/RHq4pkEYXRmZ5+nnup AW8ijyfBkrIcAr69I5ZBaRhEMfOFgSvNUIqREI5NjwIrXAxj+cd3C+7RM c1VzIk3p7Dd5CEVPBC079Pg+Z6DUR4bshy0wiItVc4SfIeUw5I4wdcIk/ U=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0AVAQAsAx1Y/5ldJa1SChkBAQEBAQEBAQEBAQcBAQEBAYMuAQEBAQEfWHwHjTGXAJRGgggdC4V7AhqBfj8UAQIBAQEBAQEBYiiEYQEBAQQBAQEgBA06CwwEAgEIEQQBAQECAiMDAgICJQsUAQgIAgQBDQUIDIhEDq8tjHkBAQEBAQEBAQEBAQEBAQEBAQEBAQEXBYEJhTaEVYQfBTqCbYJcAQSIS4YIi1ABiT6GeoF1jhyHKYV4hAMBHjdsgyOBfnKFCYEvgQwBAQE
X-IronPort-AV: E=Sophos;i="5.31,445,1473120000"; d="scan'208";a="344340987"
Received: from rcdn-core-2.cisco.com ([173.37.93.153]) by alln-iport-7.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2016 21:55:50 +0000
Received: from XCH-ALN-011.cisco.com (xch-aln-011.cisco.com [173.36.7.21]) by rcdn-core-2.cisco.com (8.14.5/8.14.5) with ESMTP id uA4LtoIK011436 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Fri, 4 Nov 2016 21:55:50 GMT
Received: from xch-aln-014.cisco.com (173.36.7.24) by XCH-ALN-011.cisco.com (173.36.7.21) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Fri, 4 Nov 2016 16:55:49 -0500
Received: from xch-aln-014.cisco.com ([173.36.7.24]) by XCH-ALN-014.cisco.com ([173.36.7.24]) with mapi id 15.00.1210.000; Fri, 4 Nov 2016 16:55:49 -0500
From: "Jakob Heitz (jheitz)" <jheitz@cisco.com>
To: Job Snijders <job@ntt.net>, Geoff Huston <gih@apnic.net>
Thread-Topic: [Idr] Review of draft-ietf-large-community-06.txt
Thread-Index: AQHSNjBz6sRcMeSZxEiIR579im5tsaDIY72wgAB9DlKAAK2JAIAAA2sAgAAGW4D//8SNUA==
Date: Fri, 04 Nov 2016 21:55:49 +0000
Message-ID: <8a293ce4fc134657aa98134b5017d92e@XCH-ALN-014.cisco.com>
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> <6c5d83aa1d6a4a04b651b8f14f4b445b@XCH-ALN-014.cisco.com> <40D942F5-0710-46D1-BF09-76C827377479@cisco.com> <95F42982-7DCF-46A9-A26C-71EF70DB3C59@apnic.net> <20161104195346.GK961@Vurt.local> <20161104201631.GA35942@Vurt.lan>
In-Reply-To: <20161104201631.GA35942@Vurt.lan>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.32.152.140]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/VJLLJxsEBjonOlanyGDZQO3l5uk>
Cc: "idr@ietf.org" <idr@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 21:55:54 -0000

All good, except one. You added:

   There is no significance to the order
   ...
   of the BGP Large Communities attributes if two or more such
   attributes are encoded in the BGP path attribute payload of a single
   BGP Update message.

This is in conflict with RFC 7606:

       If any other attribute (whether
       recognized or unrecognized) appears more than once in an UPDATE
       message, then all the occurrences of the attribute other than the
       first one SHALL be discarded and the UPDATE message will continue
       to be processed.

I would leave out any mention of multiple attributes.
What this is meant to say is that the order of community values within
the attribute is irrelevant. I put that in, because it has caused
confusion in the past. I was thinking to add that "a router may reorder
the community values when forwarding to the next router", because 
this point was the actual source of confusion in more than one instance.

Thanks,
Jakob.


> -----Original Message-----
> From: Idr [mailto:idr-bounces@ietf.org] On Behalf Of Job Snijders
> Sent: Friday, November 04, 2016 1:17 PM
> To: Geoff Huston <gih@apnic.net>
> Cc: idr@ietf.org
> Subject: Re: [Idr] Review of draft-ietf-large-community-06.txt
> 
> Hi everyone,
> 
> Attached is the rfcdiff html file that represents the current -06 -> -07
> changes with the work from Jakob and Geoff and some nits that were
> picked by John Scudder.
> 
> I should note the following, in "Reserved BGP Large Community values"
> the 'reasons' behind each reserved value were removed. Some in the
> working group made a case that it could be good to provide a
> justification what makes these numbers special.
> 
> Other then that, i feel that these changes have improved the document's
> flow and readability.
> 
> Kind regards,
> 
> Job
> 
> On Fri, Nov 04, 2016 at 08:53:46PM +0100, Job Snijders wrote:
> > On Sat, Nov 05, 2016 at 06:41:32AM +1100, Geoff Huston wrote:
> > > Jakob,
> > >
> > > Many thanks for sending this.
> > >
> > > I have used Word to mark up my reactions to this proposed revisions
> > > for your further review as this tool has a good track changes option -
> > > the PDF of the marked up text is attached to this note.
> > >
> > > My suggested changes to your proposed test are:
> > >
> > > 1) change to the Introduction to alter the description of Communities
> > > to conform more exactly to the language used in RFC 1997
> > >
> > > 2) change to the considerations of RFC4360 in the Introduction. (I
> > > also note that RFC4360 calls the Global Administrator and Local
> > > Administrator parts of the community value “sub-fileds” rather than
> > > “fields”, while this draft calls these elements just “fields” )
> > >
> > > 3) the set of values is explicitly called out to be “unordered” in the
> > > Introduction
> > >
> > > 4) The description of the attribute fields is altered to be consistent
> > > with RFC4360
> > >
> > > 5) the treatment of the Global Administrator value is unclear. I have
> > > proposed removing the last two sentences and propose changing the MUST
> > > to a SHOULD, as, later in the draft in Section 6, it is noted that the
> > > Global Administrator field can be any value!
> > >
> > > 6) I have explicitly called out that the values are in any order, and
> > > allowed for two or more individual Large Communities attributes to be
> > > contained in a single BGP Update message, again in any order. If this
> > > is not your intent then you should look at this part of section 2 and
> > > tighten up the wording.
> > >
> > > 7) you have not explicitly disambiguated between duplicate instances
> > > of the same attribute and duplicate instances of the same Community
> > > value. I have offerred some text to clarify this to the level of
> > > values.
> > >
> > > 8) I have proposed changing the MUST to a SHOULD in rthe canonical
> > > representation section
> > >
> > > 9) The sentences about the use of a colon make no sense given that you
> > > have not specified a colon separator in the preceeding paragraph. You
> > > probably need to either drop the reference to the colon, or add a
> > > colon as part of the Canonical Representation. I have proposed the
> > > former.
> > >
> > > 10) - in section 6 I changed the “may” to a “MAY” - seemed like the
> > > right thing to do.
> > >
> > > 11) I added a paragraph in the Security Considerations section
> > > explicitly that the values in this attribute are not specifically
> > > protected and then pointed to BGP security (just because an ASN is
> > > used in a community attribute, that does not mean that the ASN in
> > > question attached that attribute, or that the ASN will honour that
> > > attribute. - its just a signal without any integrity protection)
> > >
> > > 12) I have added my name as a reviewer - hope that’s ok! :-)
> > >
> > > I _assume_ that the atomic aggregator mess will be fixed up by the
> > > action of making this an historic attribute, so I have left the text
> > > on aggregation unaltered.
> > >
> > > Regards,
> > >
> > >   Geoff
> > >
> > >
> > >
> > > > On 5 Nov. 2016, at 1:20 am, Jakob Heitz (jheitz) <jheitz@cisco.com> wrote:
> > > >
> > > > Geoff,
> > > >
> > > > Here is my proposed revision. I made all the changes, except the ATOMIC_AGGREGATE. If after the latest
> discussion, you still want it, I'll make that change too.
> > > >
> > > > Thanks,
> > > > Jakob.
> > > >
> > > >
> > > > Begin forwarded message:
> > > >
> > > >>> -----Original Message-----
> > > >>> From: Idr [mailto:idr-bounces@ietf.org] On Behalf Of Geoff Huston
> > > >>> Sent: Thursday, November 03, 2016 5:14 PM
> > > >>> To: IETF IDR WG <idr@ietf.org>
> > > >>> Cc: rtg-dir@ietf.org; Susan Hares <shares@ndzh.com>
> > > >>> Subject: [Idr] 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
> > > >>>
> > > >>> _______________________________________________
> > > >>> Idr mailing list
> > > >>> Idr@ietf.org
> > > >>> https://www.ietf.org/mailman/listinfo/idr
> > > > <draft-ietf-idr-large-community.xml><draft-ietf-idr-large-community-07.txt><draft-ietf-idr-large-community-07-
> diff-from-06.html>
> > >
> >
> > > _______________________________________________
> > > Idr mailing list
> > > Idr@ietf.org
> > > https://www.ietf.org/mailman/listinfo/idr
> >
> > _______________________________________________
> > Idr mailing list
> > Idr@ietf.org
> > https://www.ietf.org/mailman/listinfo/idr