From nobody Wed Oct 26 08:27:46 2022 Return-Path: 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 F01B1C14F732 for ; Wed, 26 Oct 2022 08:27:44 -0700 (PDT) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: -2.106 X-Spam-Level: X-Spam-Status: No, score=-2.106 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=kumari.net Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id O3tu6BnTWVGY for ; Wed, 26 Oct 2022 08:27:40 -0700 (PDT) Received: from mail-lj1-x232.google.com (mail-lj1-x232.google.com [IPv6:2a00:1450:4864:20::232]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7B429C14F74B for ; Wed, 26 Oct 2022 08:27:40 -0700 (PDT) Received: by mail-lj1-x232.google.com with SMTP id a15so17840611ljb.7 for ; Wed, 26 Oct 2022 08:27:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kumari.net; s=google; h=to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=TumsC0e2vGO+w4yoOle1/728zyxSsUabIFI7EvTB32Y=; b=PoF7EAalHhEBhmwIU8g8ELSJyDwVpktrMvHwsFtN1GEQPwefOQPy83TqhfC/EPsJe4 FnksymJyZJ0NwUQPHSV7jBGH9iVdrS1aj+YuccXM3ZdeJZeWvuiJ3K2nexhrBKlEAlTs kXMHx10yEfPfQ6EPRH+aCSW/TSDqNV39BTtYHL6W7kPBTUrjAdcnH5mpdCwPAkDRWxvF tqrR6ZjZXWJTi2VvkfIXgX97xLP3mjS7cy/l+8vAZvt5og29QT/2zXyTCWjCtQM+v6PY Q0LCDfd0TGYI1LyhMQDnqN1k1/oIMNe37TDiPb2kSwo8XrdzhcobboVCk8GJW+SL1U+B CJAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=TumsC0e2vGO+w4yoOle1/728zyxSsUabIFI7EvTB32Y=; b=yHb/mV6JLvdmg6Iy8ZrrpJlORnKwH69qWB4qG/82yteG124Z85pCnnu1nhGmf1jbEq 4Oj14eMZZwTkn0J9S539eyN0wvgb3hrtNroPdY8s+GnoWsByfU2byeJ2Rkmf+b9Z7Db7 IR6jgNT7rTg2ypv0dA5lPsIFV7YcmTsNug0E6SwSDhG2K9ESYBQ7LbjC+4ed/0/yWc8M Gf6PaqlsQ1RcmWDbTSgQqA5vhA/eZZzGIN/SwbM1tBT/nt+2Qvm2dY5K5574/9kDJl6S DNemhXIooGyQRjp0ZiCulHBko6kMOrxAysKuvbnfgTFj2WgqV6O3v1wFV+x3PUPXV0wP 4KPQ== X-Gm-Message-State: ACrzQf2OnHNFcT9m2F/fyYL/RRo19k1JGMT379YULi6xNjAoH9Z/PvT8 Q0IIIHe4u84Ji0HZJFFr999dVqmbm/dE1UTvU598eA== X-Google-Smtp-Source: AMsMyM4njB+tSXv9TdYo4KwGqZZgX/AnJv35vqbyCRMKKPy3A2lz+pd+utrjw4TzSMdLiPZGKvplqG6nW3Qcrj6b28I= X-Received: by 2002:a2e:b60a:0:b0:26e:8ae7:3d42 with SMTP id r10-20020a2eb60a000000b0026e8ae73d42mr15782019ljn.175.1666798057953; Wed, 26 Oct 2022 08:27:37 -0700 (PDT) Received: from 649336022844 named unknown by gmailapi.google.com with HTTPREST; Wed, 26 Oct 2022 08:27:37 -0700 Mime-Version: 1.0 X-Superhuman-ID: l9psijq1.72e2453d-8db7-4372-bb3d-c88d4dae8016 X-Superhuman-Thread-ID: draft00ba8f65bc9b32ef X-Mailer: Superhuman Desktop (2022-10-25T22:06:00Z) X-Superhuman-Draft-ID: draft005d6e1f6ff36c21 From: Warren Kumari Date: Wed, 26 Oct 2022 08:27:37 -0700 Message-ID: To: draft-ietf-idr-wide-bgp-communities.all@ietf.org, "idr@ietf. org" , Alvaro Retana Content-Type: multipart/alternative; boundary="00000000000028328a05ebf1aaf5" Archived-At: Subject: [Idr] Initial review of draft-ietf-idr-wide-bgp-communities X-BeenThere: idr@ietf.org X-Mailman-Version: 2.1.39 Precedence: list List-Id: Inter-Domain Routing List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Oct 2022 15:27:45 -0000 --00000000000028328a05ebf1aaf5 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi all, I have reviewed draft-ietf-idr-wide-bgp-communities, and wrote up something similar to a directorate-style review; Alvaro suggested that I send it to the authors and WG. Apologies in advance if these questions have already been discussed in the WG - I haven't been following IDR particularly closely lately. W -------- Document: draft-ietf-idr-wide-bgp-communities Reviewer: Warren Kumari Review Date: 2022-10-25 Intended Status: Standards Track Link: https://datatracker.ietf.org/doc/draft-ietf-idr-wide-bgp-communities/ Meta comment: There are 6 authors listed. This exceeds our standard 5 (just mentioning, but the reason should be noted in the writeup or during balloting). The only mentions of Large BGP Communities are in the Operational Considerations ("Having multiple ways [...] - via the use of Standard, Extended or Large BGP Communities versus the use of BGP Wide Communities =E2=80=A6") and Security Considerations ("All the security considerations f= or BGP Communities [RFC1997], BGP Extended Communities [RFC4360], and BGP Large Communities [RFC8092] apply to BGP Community Containers."). This isn't really a problem, and I understand that these are somewhat competing documents, but it did feel a little odd... Questions and concerns: Section 2.1 - BGP Community Container Common Header: O: "The BGP Community Container Common Header is defined in Section 3.1 and contains following encoding:" C: The document would be much easier to understand if Section 3 ("BGP Community Container Attribute"), 3.1 ("BGP Community Container Attribute Common Header"), etc were to be moved here (and also if it included an example early). Section 2.1 defines part of the BGP Community format, but does it somewhat poorly, and confusingly. The actual definition is in 3.1, and it repeats a bunch of Section 2.1. This makes is somewhat confusing - there isn't really enough information in 2.1 to get a handle on how the protocol works, and so the reader is somewhat confused (or has to flip to 3.1 and then come back). A simple reorganization and removal of duplicates would make this all much much simpler to read (and would avoid having to figure if things have changed between the sections, etc.). Section 2.3. BGP Community Container Atoms O: "Atoms provide data types that can be used to encode contents of BGP Community Containers. They are in the format of TLVs and are defined later in this document in Section 5." C: This is the entirety of Section 2.3 - it's just a forward reference to Section 5. This illustrates my above point -- reorganizing the document to be either "this is the purpose and here are some examples. This is the protocol=E2=80=A6", or better yet "These are the primitives/foundation (for= mat & header, definitions, etc), here is an example of why you might use it, and this is how you assemble the parts into Wide Communities" would be much more understandable. Section 3. BGP Community Container Attribute "This document defines a BGP Path Attribute, the BGP Community Container. The attribute type code is 34. The BGP Community Container attribute is an optional, transitive BGP attribute, and may be present only once in the BGP UPDATE message." Okey dokey=E2=80=A6 but, what do I do if there are multiple BGP Community C= ontainer attributes? This is only somewhat answered in Section 8.1 ("General Error Handling for BGP Community Containers"), which says: "[RFC7606] "treat as withdraw" behavior is expected for any malformed Community Containers or malformation of their contents.". I'm *assuming* that the right behavior in this case is treat as withdraw, but I'll point out that more than one isn't actually a "malformed Community Container". The behavior here should be made more explicit. Section 3.1. BGP Community Container Attribute Common Header The table list 2 flags (T and C), and then says that bits 3..7 are reserved. AFAICT (and I may just be dumb at this point) it should either say bits "2..7" (if they are counting from 0 which is what I'd assume), or "3..8" (if counting from 1). For the definition of the flags. For the T (Transitivity bit) it says: "When not set (value 0), the community in the container is transitive across AS boundaries, but not across an administrative boundary. When set (value 1), the community in the container is transitive across all ASes. An administrative boundary, in this sense, is an arbitrary set of connected ASes, possibly under control of a single entity. How such an administrative boundary is determined is out of the scope of this document.= " Is this OK? If the document specifies the behavior for a bit, and especially one that is presumably important like this, is this level of vagueness around "possibly under control of a single entity" acceptable? Note: This may already be well known / understood in the IDR group, but it certainly wasn't clear to me exactly how far I would expect this to propagate, nor, if I managed multiple ASes, how I would delineate "this AS is part of my admin group, but *this* is the boundary, don't propagate past here". Or, is the intent that operators are expected to manually write route policy filters which match on this bit and manually filter them? I'm *assuming* that the default behavior on an eBGP session (to some other network) is to filter on this bit, but that means that I need a way to override this to say "Nah, AS42 is also me, so don't filter these". It seems that more text is needed here... Section 4.1. Community Value O: "When the high order bit of the Community Value field - I - is set, the value is IANA Registered and has a well defined meaning with underlying semantics." C: It seems like it would be nice to have a link to the registry here -- I'm *assuming** this is meaning the "Border Gateway Protocol (BGP) Well-known Communities" ( https://www.iana.org/assignments/bgp-well-known-communities/bgp-well-known-= communities.xhtml )? Section 4.4.1. Sub-Type 1, BGP Wide Community Target(s) TLV O: "Thus, if any given target matches per the semantics of that Atom for the community, the community is considered to match and the action defined by the community should be executed." C: This is probably fine, especially because this is a lowercase 'should', but it also feels like it should be something like "the action defined by the community is requested to be executed", or some "subject to administrative/operator policy" or similar. Not really a bit issue, but... 4.4.2. Sub-Type 2, BGP Wide Community Exclude Target(s) TLV O: "The semantic of the BGP Wide Community Exclude Target(s) is to match all specified Target(s) with the exception of those listed in this TLV." an= d "If the BGP Wide Community Target(s) TLV and the BGP Wide Community Exclude Target(s) TLV have conflicting semantics, priority MUST be given to the Wide Community Exclude Target(s) TLV." C: It seems like this section needs a fair bit more explanation of the semantics. Let's pretend that the "include" set (Target) contains [1,2,3,4] and "exclude" contains [3] - I'm *assuming* that the result should be the set difference of include\exclude ([1,2,4]), but this isn't stated as unambiguously as it could be... Or, if the "include" set contains [1,2,3,4] and "exclude" contains [10], is this OK? Presumably yes, but... =C2=AF\_(=E3=83=84)_/=C2=AF Section 8.2 ("BGP Wide Community Container Error Handling") If any Atom in a BGP Wide Community container's Exclude Targets TLV is unrecognized, that Wide Community MUST NOT be considered a match and no actions for that community should be processed.", but this doesn't actually cover the above case -- I might "recognize" (this is actually not very well defined in the document) [1..20], but is a value outside the "include" set acceptable? Section 5.1. Atom Type 1, The Autonomous System Number List O: " Two special values are reserved for the Autonomous System Atoms: 0x00000000 - to indicate "No Autonomous Systems". 0xFFFFFFFF - to indicate "All Autonomous Systems"." C: I'm not actually sure why we need a "No Autonomous Systems" value. AFAICT, wherever you would you this you could just leave out the option - not really important, but I'm wondering if this means that I missed something? Section 5.4. Atom Type 5, The IEEE Floating Point Number List C: I don't really understand the use-case for this, and it makes me a little worried what the expectation is, but, well, meh... Section 5.7. Atom Type 8, the UTF-8 String O: "Implementations MUST be prepared for truncated/improperly formed UTF-8 strings. When detecting such a string, the implementation should remove trailing octets of a multi-octet sequence in order to have a well-formed string. Implementations MUST be prepared to receive empty (zero-length) UTF-8 String Atoms as they may be used as Parameters." C: I'm really not sure if this is an issue, but what if I get a string of length 4, and the last octet makes it not-well formed, so I drop it=E2=80= =A6 and then the 3rd octet makes it not well formed, so I drop it as well=E2=80=A6 = and then the second octet makes it not well formed, so I drop it, and then the last octet makes it not well formed, so I drop it, and now the string is empty. This *might* not be a problem because I think that all 1 octet UTF-8 strings are, by definition, valid, but I don't actually know. Also, the "... String Atoms as they may be used as Parameters" text also makes me a little worried about the expected use case and what exactly is being created here -- do people expect that I could sent a community to e.g NTT with the UTF-8 string "Make me a sandwich"? Or "request system zeroize"? ASCII art pictures of a cat? What's the use-case? Section 6. Well Known Standard BGP Communities O: "According to RFC 1997, as well as IANA's Well-Known BGP Communities registry, the following BGP communities are defined to have global significance:" P: "At the time of this writing, the IANA's Well-Known BGP Communities registry lists the following BGP communities as having have global significance:" C: What if someone adds e.g 0xFFFFFF42 to the IANA registry? I don't really understand why the document is copying and pasting values, but if there is a reason, it should allow for future changes... Section 7. Operational Considerations C: This section feels quite short (but, I'm biased!) Are operators *expected* to use these? Or just if they want to? If they define local use ones, should they be documented? Presumably if e.g Telefonica sends NTT a Wide community saying "Please set localpref to 1", NTT can choose to ignore this, yes? What if *I* send this to NTT and I'm not one of their customers, presumably they should ignore it as well, yes? (Some of the above can be handled with something like "Operators should apply the same policies to Wide Communities that they apply to regular communities" or similar). However, this still leaves the whole "transitive" but and the not very well defined "what is an administrative boundary?" question. Perhaps the intent is that the T bit is used just like "normal" BGP transitive vs non-transitive logic? If so, that's not really clear, especially because of the new admin text. Section 8.1. General Error Handling for BGP Community Containers O: "[RFC7606] "treat as withdraw" behavior is expected for any malformed Community Containers or malformation of their contents." C: 'malformation' doesn't seem right here - perhaps "for any malformed Community Containers or with malformed contents." Section 8.2. BGP Wide Community Container Error Handling O: "If any Atom in a BGP Wide Community container's Exclude Targets TLV is unrecognized, that Wide Community MUST NOT be considered a match and no actions for that community should be processed. While the Targets TLV is meant to be inclusive, the Exclude Targets TLV is meant to be proscriptive of applying the action." C: As mentioned above, this isn't especially clear -- I think that if I only recognize targets [1, 2, 3] and I get an include (Targets TLV) of [1,2,3,4] I should still process this and only apply the ones I recognize, but if I get an exclude of [17] I should ignore the whole thing? But, there is a fair bit of guessing going on here... Also, why is "include" inclusive and "exclude" proscriptive? Section 9.2. Example Type 1 BGP Wide Community Encoding O: "The T Flag (transitive) is unset to prevent propagation of this community outside of the provider's administrative domain." C: I'm uncomfortable with "prevent propagation" without better text around how a provider is expected to set the administrative domain. From what I can tell, this doesn't mean "AS", so it's not actually clear how far it should propagate. I'd assume it is "AS or some set of ASs which are acting as one", but the Section 3.1 description is sufficiently vague that I don't know what would happen if I sent this to e.g AS209 should I expect it to propagate to AS3356? AS3561? Editorial and Nits: Abstract: O: "It is also a very common best practice for operators ..." P: "It is also a very common practice for operators..." or maybe "It is also a common best practice for operators ..." C: "very common best practice" feels redundant / reads oddly. Very much just a nit though. Section 1: O: "the encoding of community values mandates that the first two octets contain the Autonomous System number, with the next two octets containing some locally defined value." P: "... some locally defined opaque value."" C: Also a nit, but it seems like this helps clarify the scope / expected understandability of the value. Section 2.1: O: "This provides hierarchy for future Community-like features." P: "This provides a hierarchy for future Community-like features." C: Nit - missing 'a' Section 4.1: O: "When the high order bit of the Community Value field is clear, the value is Locally defined" P: "When the high order bit of the Community Value field is clear, the value is locally defined" C: Uppercase 'L' in 'Locally' --00000000000028328a05ebf1aaf5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi all,

<no-hats>
I have reviewed draft-ietf-idr-wide-bgp-communities, and wrote up somethin= g similar to a directorate-style review; Alvaro suggested that I send it to= the authors and WG.

Apo= logies in advance if these questions have already been discussed in the WG = - I haven't been following IDR particularly closely lately.

W
</no-= hats>
--------
Document:=C2=A0draft-i= etf-idr-wide-bgp-communities
Reviewer: Warren Kumari
Review Date:=C2=A02022-10-25
Intended Status: Standards Tra= ck



Meta comment:
There are 6 authors listed. This exceeds our standard 5 (just mention= ing, but the reason should be noted in the writeup or during balloting).

The only mentions of Large= BGP Communities are in the Operational Considerations ("Having multip= le ways [...] - via the use of Standard, Extended or Large BGP Communities = versus the use of BGP Wide Communities =E2=80=A6") and Security Consid= erations ("All the security considerations for BGP Communities [RFC199= 7], BGP=C2=A0 Extended Communities [RFC4360], and BGP Large Communities [RF= C8092] apply to BGP Community Containers."). This isn't really a p= roblem, and I understand that these are somewhat competing documents, but i= t did feel a little odd...


Questions and concerns:
Section 2.1 - BGP Community Container Common Header:
O: "The BGP Community Container Common Header is defined in Sect= ion 3.1 and contains following encoding:"
C: = The document would be much easier to understand if Section 3 ("BGP Com= munity Container Attribute"), 3.1 ("BGP Community Container Attri= bute Common Header"), etc were to be moved here (and also if it includ= ed an example early). Section 2.1 defines part of the BGP Community format,= but does it somewhat poorly, and confusingly. The actual definition is in = 3.1, and it repeats a bunch of Section 2.1. This makes is somewhat confusin= g - there isn't really enough information in 2.1 to get a handle on how= the protocol works, and so the reader is somewhat confused (or has to flip= to 3.1 and then come back). A simple reorganization and removal of duplica= tes would make this all much much simpler to read (and would avoid having t= o figure if things have changed between the sections, etc.).

Section 2.3.=C2=A0 BGP Community Conta= iner Atoms
O: "Atoms provide data types that = can be used to encode contents of BGP Community Containers.=C2=A0 They are = in the format of TLVs and are defined later in this document in Section 5.&= quot;

C: This is the ent= irety of Section 2.3 - it's just a forward reference to Section 5. This= illustrates my above point -- reorganizing the document to be either "= ;this is the purpose and here are some examples. This is the protocol=E2=80= =A6", or better yet "These are the primitives/foundation (format = & header, definitions, etc), here is an example of why you might use it= ,=C2=A0 and this is how you assemble the parts into Wide Communities" = would be much more understandable.


Section 3.=C2=A0 BGP Community Contain= er Attribute

"This = document defines a BGP Path Attribute, the BGP Community Container.=C2=A0 T= he attribute type code is 34.

=C2=A0=C2=A0 The BGP Community Container attribute is an optional, tr= ansitive BGP
=C2=A0=C2=A0 attribute, and may be pr= esent only once in the BGP UPDATE message."
<= br>
Okey dokey=E2=80=A6 but, what do I do if there are= multiple BGP Community Container attributes? This is only somewhat answere= d in Section 8.1 ("General Error Handling for BGP Community Containers= "), which says:
"[RFC7606] "treat a= s withdraw" behavior is expected for any malformed Community Container= s or malformation of their contents.". I'm *assuming* that the rig= ht behavior in this case is treat as withdraw, but I'll point out that = more than one isn't actually a "malformed Community Container"= ;.
The behavior here should be made more explicit.=


Section 3.1.=C2=A0 BGP Community Container Attribute Common Header
The table list 2 flags (T and C), and then says that bi= ts 3..7 are reserved. AFAICT (and I may just be dumb at this point) it shou= ld either say bits "2..7" (if they are counting from 0 which is w= hat I'd assume), or "3..8" (if counting from 1).

For the definition of the flags. For= the T (Transitivity bit) it says:
"When not = set (value 0), the community in the container is transitive across AS bound= aries, but not across an administrative boundary.
=
When set (value 1), the community in the containe= r is transitive across all ASes.=C2=A0 An administrative boundary, in this = sense, is an arbitrary set of connected ASes, possibly under control of a s= ingle entity.=C2=A0 How such an administrative boundary is determined is ou= t of the scope of this document."

<= div class=3D"">Is this OK? If the document specifies the behavior for a bit= , and especially one that is presumably important like this, is this level = of=C2=A0 vagueness around "possibly under control of a single entity&q= uot; acceptable?=C2=A0
Note: This may already be w= ell known / understood in the IDR group, but it certainly wasn't clear = to me exactly how far I would expect this to propagate, nor, if I managed m= ultiple ASes, how I would delineate "this AS is part of my admin group= , but *this* is the boundary, don't propagate past here". Or, is t= he intent that
operators are expected to manually = write route policy filters which match on this bit and manually filter them= ? I'm *assuming* that the default behavior on an eBGP session (to some = other network) is to filter on this bit, but that means that I need a way t= o override this to say "Nah, AS42 is also me, so don't filter thes= e".
It seems that more text is needed here...=
=C2=A0


Section 4.1.=C2=A0 Community Value
O: "When the high order bit of the Community Value= field - I - is set, the value is IANA Registered and has a well defined me= aning with underlying semantics."
C: It seems= like it would be nice to have a link to the registry here -- I'm *assu= ming** this is meaning the=C2=A0 "Border Gateway Protocol (BGP) Well-k= nown Communities" (https://ww= w.iana.org/assignments/bgp-well-known-communities/bgp-well-known-communitie= s.xhtml)?


=
Section 4.4.1.=C2=A0 Sub-Type 1, BGP Wide Community Target(= s) TLV
O: "Thus, if any given target matches = per the semantics of that Atom for the community, the community is consider= ed to match and the action defined by the community should be executed.&quo= t;
C: This is probably fine, especially because th= is is a lowercase 'should', but it also feels like it should be som= ething like "the action defined by the community is requested to be ex= ecuted", or some "subject to administrative/operator policy"= or similar. Not really a bit issue, but...


4.4.2.=C2=A0 Sub-Type 2, BGP = Wide Community Exclude Target(s) TLV
O: "The = semantic of the BGP Wide Community Exclude Target(s) is to match all specif= ied Target(s) with the exception of those listed in this TLV." and
=
"If the BGP Wide Community Target(s) TLV and the= BGP Wide Community Exclude Target(s) TLV have conflicting semantics, prior= ity MUST be given to the Wide Community Exclude Target(s) TLV."

C: It seems like this section = needs a fair bit more explanation of the semantics.
Let's pretend that the "include" set (Target) contains [1,2= ,3,4] and "exclude" contains [3] - I'm *assuming* that the re= sult should be the set difference of include\exclude ([1,2,4]), but this is= n't stated as unambiguously as it could be...
=
Or, if the "include" set contains [1,2,= 3,4] and "exclude" contains [10], is this OK? Presumably yes, but= ... =C2=AF\_(=E3=83=84)_/=C2=AF

Section 8.2 ("BGP Wide Community Container Error Handling"= ;) If any Atom in a BGP Wide Community container's Exclude Targets TLV = is unrecognized, that Wide Community MUST NOT be considered a match and no = actions for that community should be processed.", but this doesn't= actually cover the above case -- I might "recognize" (this is ac= tually not very well defined in the document) [1..20], but is a value outsi= de the "include" set acceptable?



Sect= ion 5.1.=C2=A0 Atom Type 1, The Autonomous System Number List

O: " Two special values are rese= rved for the Autonomous System Atoms:

=C2=A0=C2=A0=C2=A0=C2=A0 0x00000000 - to indicate "No Au= tonomous Systems".
=C2=A0=C2=A0=C2=A0=C2=A0 0= xFFFFFFFF - to indicate "All Autonomous Systems"."
=

C: I'm not actually sure why = we need a "No Autonomous Systems" value. AFAICT, wherever you wou= ld you this you could just leave out the option - not really important, but= I'm wondering if this means that I missed something?


Section 5.4.=C2= =A0 Atom Type 5, The IEEE Floating Point Number List
C: I don't really understand the use-case for this, and it makes me = a little worried what the expectation is, but, well, meh...


Section 5.7.= =C2=A0 Atom Type 8, the UTF-8 String
O: "Impl= ementations MUST be prepared for truncated/improperly formed
=C2=A0=C2=A0 UTF-8 strings.=C2=A0 When detecting such a string, = the implementation
=C2=A0=C2=A0 should remove trai= ling octets of a multi-octet sequence in order to
= =C2=A0=C2=A0 have a well-formed string.

=
=C2=A0=C2=A0 Implementations MUST be prepared to receive em= pty (zero-length) UTF-8
=C2=A0=C2=A0 String Atoms = as they may be used as Parameters."

C: I'm really not sure if this is an issue, but what i= f I get a string of length 4, and the last octet makes it not-well formed, = so I drop it=E2=80=A6 and then the 3rd octet makes it not well formed, so I= drop it as well=E2=80=A6 and then the second octet makes it not well forme= d, so I drop it, and then the last octet makes it not well formed, so I dro= p it, and now the string is empty. This *might* not be a problem because I = think that all 1 octet UTF-8 strings are, by definition, valid, but I don&#= 39;t actually know. Also, the "... String Atoms as they may be used as= Parameters" text also makes me a little worried about the expected us= e case and what exactly is being created=C2=A0 here -- do people expect tha= t I could sent a community to e.g NTT with the UTF-8 string "Make me a= sandwich"? Or "request system zeroize"? ASCII art pictures = of a cat? What's the use-case?


Section 6.=C2=A0 Well Known Standard B= GP Communities

O: "= According to RFC 1997, as well as IANA's Well-Known BGP Communities reg= istry, the following BGP communities are defined to have global significanc= e:"
P: "At the time of this writing, the= IANA's Well-Known BGP Communities registry lists the following BGP com= munities as having have global significance:"
C: What if someone adds e.g 0xFFFFFF42 to the IANA registry? I don't r= eally understand why the document is copying and pasting values, but if the= re is a reason, it should allow for future changes...



Section 7.=C2=A0 Operational Considerations
C: This section feels quite short (but, I'm biased!)
Are operators *expected* to use these? Or just if they want to? I= f they define local use ones, should they be documented?

Presumably if e.g Telefonica sends NTT a W= ide community saying "Please set localpref to 1", NTT can choose = to ignore this, yes? What if *I* send this to NTT and I'm not one of th= eir customers, presumably they should ignore it as well, yes? (Some of the = above can be handled with something like "Operators should apply the s= ame policies to Wide Communities that they apply to regular communities&quo= t; or similar).

However,= this still leaves the whole "transitive" but and the not very we= ll defined "what is an administrative boundary?" question. Perhap= s the intent is that the T bit is used just like "normal" BGP tra= nsitive vs non-transitive logic? If so, that's not really clear, especi= ally because of the new admin text.=C2=A0



Secti= on 8.1.=C2=A0 General Error Handling for BGP Community Containers
=

O: "[RFC7606] "treat as= withdraw" behavior is expected for any malformed Community Containers= or malformation of their contents."
C: '= malformation' doesn't seem right here - perhaps "for any malfo= rmed Community Containers or with malformed contents."


Section 8.2.= =C2=A0 BGP Wide Community Container Error Handling

O: "If any Atom in a BGP Wide Community con= tainer's Exclude Targets TLV is unrecognized, that Wide Community MUST = NOT be considered a match and no actions for that community should be proce= ssed.=C2=A0 While the Targets TLV is meant to be inclusive, the Exclude Tar= gets TLV is meant to be proscriptive of applying the action."
C: As mentioned above, this isn't especially clear -- = I think that if I only recognize targets [1, 2, 3] and I get an include (Ta= rgets TLV) of [1,2,3,4] I should still process this and only apply the ones= I recognize, but if I get an exclude of [17] I should ignore the whole thi= ng? But, there is a fair bit of guessing going on here...
Also, why is "include" inclusive and "exclude" = proscriptive?


=

Section 9.2.=C2=A0 Example Type 1= BGP Wide Community Encoding
O: "The T Flag (= transitive) is unset to prevent propagation of this community outside of th= e provider's administrative domain."
C: I= 'm uncomfortable with "prevent propagation" without better te= xt around how a provider is expected to set the administrative domain. From= what I can tell, this doesn't mean "AS", so it's not act= ually clear how far it should propagate. I'd assume it is "AS or s= ome set of ASs which are acting as one", but the Section 3.1 descripti= on is sufficiently vague that I don't know what would happen if I sent = this to e.g AS209 should I expect it to propagate to AS3356? AS3561?



=


<= br>

Editorial and Nits:
<= /div>
Abstract:
O: "It is also= a very common best practice for operators ..."
P: "It is also a very common practice for operators..." or may= be "It is also a common best practice for operators ..."
C: "very common best practice" feels redundant /= reads oddly. Very much just a nit though.


Section 1:
O: "the encoding of community values mandates that the first two= octets contain the Autonomous System number, with the next two octets cont= aining some locally defined value."
P: "= ... some locally defined opaque value.""
C: Also a nit, but it seems like this helps clarify the scope / expected u= nderstandability of the value.


Section 2.1:
O: &= quot;This provides hierarchy for future Community-like features."
<= /div>
P: "This provides a hierarchy for future Communit= y-like features."
C: Nit - missing 'a'= ;


Section 4.1:
O: "When the high order bi= t of the Community Value field is clear, the value is Locally defined"=
P: "When the high order bit of the Community= Value field is clear, the value is locally defined"
C: Uppercase 'L' in 'Locally'







--00000000000028328a05ebf1aaf5--