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

Ignas Bagdonas <ibagdona.ietf@gmail.com> Fri, 04 November 2016 20:46 UTC

Return-Path: <ibagdona.ietf@gmail.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 47DBA129666 for <idr@ietfa.amsl.com>; Fri, 4 Nov 2016 13:46:54 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.699
X-Spam-Level:
X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 XAOCmj10bwAo for <idr@ietfa.amsl.com>; Fri, 4 Nov 2016 13:46:50 -0700 (PDT)
Received: from mail-wm0-x22e.google.com (mail-wm0-x22e.google.com [IPv6:2a00:1450:400c:c09::22e]) (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 4746F129668 for <idr@ietf.org>; Fri, 4 Nov 2016 13:46:50 -0700 (PDT)
Received: by mail-wm0-x22e.google.com with SMTP id t79so72272851wmt.0 for <idr@ietf.org>; Fri, 04 Nov 2016 13:46:50 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to; bh=4q5DRwKzPWWV6ouJDBUtnvyB+E8SOvxqVPFstwoI+mU=; b=ubMcKObxvr0lzjujQzkB2AbyaWQUnocanbjtLAtbOIxpgbq0OSFERSqmXb3+r1JqTr E0Y1YY08kyyohxuV+izXwsFrbEXYHkwxsMhgSkHLti9UDScy6PRcxt8XuNp/36tLY6fY 0E4WtdAeYJRYyK/UdVG6kw4qK5pamP4QNqT5Y+TRYZ9mZM/i2e2by0dGvXrxBshEkn/p sxd3BJXAMykekJS/fWqyzHcbr6TnhG1X2rZC6pa6ik+/qzGBOag72moxiu+MYwrTcl1U +HTG3uZG8X9NZM2E2rQH6hp0ZDgtn9js197DrIX9s/K70yQzVLZVYr7mtuPJABTtZ3T0 ymVQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=4q5DRwKzPWWV6ouJDBUtnvyB+E8SOvxqVPFstwoI+mU=; b=Hn2ABxlP1Pp6LtOzZ/Vv3HGX75kflSwPIbIgfG7ERxSlLLyc7Ay70eOopr9soN4paL FR2s2grSxvdmqlxs6MSZFB/aRH1MPO4GY4ui//x+wwij3DEcKpKUJNPvgMfmugE7c3Zv WGxpsXuGfeTyH4SGzy/vvSnNoKJsEzg0irGsujTw0JIiSc6IcdbJb1XQHguwU7wglL/s gWD0EXpZksMkiuDeX172OYP5YKv5Wudjh37M60yGcRcH6hhbh1wTbjgJVZk4+jMe50Pn 3Ifgo/h4XymcDxxSm8Q4AvIBrHDXKbNb5BVHafj2NNVEwjpIQKxz9k1lWHeg3IgFd8e4 2BMw==
X-Gm-Message-State: ABUngvdAVzxxi/oPfd9Ug93lBJo5nFJ0Yu0jcZqID1pQrattiWRQQga2ckNAQIJpjtwbgQ==
X-Received: by 10.194.158.131 with SMTP id wu3mr13156598wjb.93.1478292408392; Fri, 04 Nov 2016 13:46:48 -0700 (PDT)
Received: from [192.168.191.136] ([80.69.10.100]) by smtp.gmail.com with ESMTPSA id c7sm16113345wjk.19.2016.11.04.13.46.46 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 04 Nov 2016 13:46:47 -0700 (PDT)
To: Geoff Huston <gih@apnic.net>, "Jakob Heitz (jheitz)" <jheitz@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>
From: Ignas Bagdonas <ibagdona.ietf@gmail.com>
Message-ID: <79ca5727-d42e-d253-f929-2cd35261628a@gmail.com>
Date: Fri, 04 Nov 2016 20:46:44 +0000
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <95F42982-7DCF-46A9-A26C-71EF70DB3C59@apnic.net>
Content-Type: multipart/alternative; boundary="------------6DD2D3175DD1AFA561DECBF4"
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/C9Aq9U2vZjyhxbQVRnVu-FS3dPI>
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 20:46:54 -0000

Hi Geoff,

On your changes 6 and 7 - RFC7606 error handling requires to interpret 
only the first path attribute of the same type, ignoring the other 
instances. Within the path attribute duplicate identical community 
values are treated as one, and the case of multiple large community path 
attributes is handled by interpreting only the first path attribute. 
This has been discussed on the list around -02 or -03 revision 
timeframe. Making a deviation for multiple path attributes would be in 
violation of 7606 here.


Ignas



On 04/11/2016 19:41, 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