Re: [Idr] AD Review of draft-ietf-idr-wide-bgp-communities-08

[Alvaro Retana <[e-mail]>]

Ping!

On December 23, 2022 at 6:37:35 AM, Alvaro Retana ([e-mail])
wrote:



Dear authors:

Thank you for your continued efforts with this document!

I have many comments (see inline) that I will want to see addressed before
starting the IETF Last Call.

(1) In general, I found that the naming is not consistent across the
document.
    For example, "BGP Wide Community", "Wide Community", "Wide community",
and
    "side community" are all used.  The same treatment applies to the name
of
    the TLVs, Atoms, etc.  I mentioned this below, but may have missed
several
    occurrences.  Please take a look.

(2) Should this document be marked as replacing
draft-ietf-idr-registered-wide-
    bgp-communities?  I know draft-ietf-idr-registered-wide-bgp-communities
and
    draft-ietf-idr-wide-bgp-communities were both adopted at about the same
    time...


Besides my notes, please also address Warren's review [1].

Thanks!

Alvaro.


[1] https://mailarchive.ietf.org/arch/msg/idr/aW8XfPljF0YlrptpXPWf9bj96w4/


[Line numbers from idnits.]


2 IDR Working Group                                         R. Raszuk, Ed.
3 Internet-Draft                                                Individual
4 Intended status: Standards Track                            J. Haas, Ed.
5 Expires: 12 January 2023                                Juniper Networks
6                                                           A. Lange, Ed.
7                                                                   Nokia
8                                                        B. Decraene, Ed.
9                                                                  Orange
10                                                               S. Amante
11                                                             Apple, Inc.
12                                                                P. Jakma
13                                          Huawei Ireland Research Centre
14                                                            11 July 2022

[major] Shepherd/Chairs: There are more than the recommended number of
authors in the front page (rfc7322).  Please work to reduce the list to 5
(or less).  Some are already identified as Editors.  Others can be added to
the Contributors section.



...
19 Abstract
...
28   This document defines a new encoding which will enhance and simplify
29   what can be accomplished today with the use of BGP communities.  The
30   most important addition this specification makes over currently
31   defined BGP communities is the ability to specify and advertise an
32   operator's parameters for execution It also provides an extensible
33   platform for any future community encoding requirements.

[nit] Suggestion>
   This document defines a new encoding that enhances and simplifies
   what can be accomplished with BGP communities. This specification's
   most important addition is the ability to specify and advertise an
   operator's parameters for execution. It also provides an extensible
   platform for any future community encoding requirements.



35 Requirements Language

37   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
38   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
39   document are to be interpreted as described in RFC 2119 [RFC2119].

[major] Use the template from rfc8174.



...
128 1.  Introduction
...
145   The authors believe that defining a new BGP Path Attribute, with the
146   ability to contain locally defined parameters will enhance the
147   current level of network policies, as well as simplify BGP policy
148   management.  The proposed encoding will also facilitate the delivery
149   of new network services without the need to define a new BGP
150   extension for each new application.

[minor] "The authors believe..."

This document is the product of the WG...

s/The authors believe that defining a new BGP Path Attribute/The definition
of a new BGP Path Attribute


[nit] Some of the claims (in this paragraph, for example) are written in
future tense (will enhance...will facilitate), but this specification is
the present.  Consider "enhances...facilitates...".



152   When defining any new type of tool there is always a unique
153   opportunity to specify a subset of well recognized behaviors.  Lists
154   of the current most commonly used BGP communities, as well as
155   creation of a new registry for future definitions will be described
156   in an extension-specific document.

[minor] It seems that this paragraph may have been overtaken by events:
there's no "extension-specific document", the registries are created here,
etc..



...
166 2.1.  BGP Community Container Common Header
...
177   The BGP Community Container Common Header is defined in Section 3.1
178   and contains following encoding:

[minor] Please keep the description/specification of the encoding in §3.1.
No need to specify/describe things more than once (and with different
words).


180   Container Type:
181       Container Type 1, BGP Wide Community is defined in this document.

183   Flags:
184       Flags control common behavior including the transitivity of the
185       Container.

187   Length:
188       Length of the Container contents.

[] The description in §3.1 is more complete than this.  Adding partial
descriptions may result in confusion.



190 2.2.  Community Containers

192   This document defines one Community Container with the following
193   encoding:

[minor] Please keep the description/specification of the encoding in §4.
No need to specify/describe things more than once (and with different
words).



...
230 3.  BGP Community Container Attribute
...
235   The BGP Community Container attribute is an optional, transitive BGP
236   attribute, and may be present only once in the BGP UPDATE message.

[major] What should a receiver do if the attribute is present more than
once?


[major] Allowing only one Community Container doesn't permit the ability to
create both local *and* external policies for a given prefix.  All BGP Wide
Communities would either be transitive or not.  If transitive, any local
policy would be propagated.  There is a mention (in §10.1) of the ability
to remove unwanted received communities, but the document says nothing
about controlling the advertisement by filtering.  Should this
functionality exist?  Please add some text.



...
242 3.1.  BGP Community Container Attribute Common Header

244   Containers always start with the following common header:

246      0                   1                   2                   3
247      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
248     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
249     |             Type              |    Flags  |C|T|   Reserved    |
250     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
251     |            Length             |
252     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

254                     Figure 1: Common container header

256   This document defines container type 1.  See the Section 11 for
257   information on additional type registration policies.

[minor] Just to confirm: the definition in this section applies to all
types, not just type 1, is that correct?

If so, it would be nice if the specification of the common header was
presented first, including the general description of *all* fields.  This
sentence (add a pointer to Section 4) should then be placed at the end of
the section.



259    +======+=======+==================================================+
260    | Bit  | Value | Meaning                                          |
261    +======+=======+==================================================+
262    |  T   |   0   | Not Transitive across administrative boundaries. |
263    +------+-------+--------------------------------------------------+
264    |      |   1   | Transitive across AS/administrative boundaries.  |
265    +------+-------+--------------------------------------------------+
266    |  C   |   0   | Not transitive across confederation boundaries.  |
267    +------+-------+--------------------------------------------------+
268    |      |   1   | Transitive across confederation boundaries.      |
269    +------+-------+--------------------------------------------------+
270    | 3..7 |   -   | RESERVED - MUST be zero when originated and      |
271    |      |       | SHOULD be ignored upon receipt.                  |
272    +------+-------+--------------------------------------------------+

[major] Should IANA manage the potential allocation of the reserved bits?
If so, please include the request for a new registry in §11.



...
278   Bit 0 (T bit) Transitivity bit:

280      When not set (value 0), the community in the container is
281      transitive across AS boundaries, but not across an administrative
282      boundary.

[major] "transitive across AS boundaries, but not across an administrative
boundary"

This definition is confusing, specially for the case where the
"administrative boundary" is defined as only containing the local AS -- the
texts seems to contradict itself: transitive across AS boundaries, but not
"this" AS boundary.  Please clarify.



284      When set (value 1), the community in the container is transitive
285      across all ASes.  An administrative boundary, in this sense, is an
286      arbitrary set of connected ASes, possibly under control of a
287      single entity.  How such an administrative boundary is determined
288      is out of the scope of this document.

[major] Please pull out the definition of an "administrative boundary" from
the description of the bit being set.  Should it be defined in terms of an
"administrative domain" (mentioned in §9.2) instead?



290   Bit 1 (C bit) Confederation bit:

292      The confederation bit is used to manage the propagation scope of a
293      given BGP Wide Community across confederation boundaries.

[minor] s/given BGP Wide Community/given container

The Wide Community is just one type.


[minor] Please add a Normative reference to rfc3065.



295      When not set (value 0) community is not transitive across
296      confederation sub-AS boundary.  When set (value of 1) indicates
297      that community in a given container is transitive across
298      confederation boundary.

[major] "confederation sub-AS"

rfc3065 talks about a Member-AS, not a sub-AS.  Please be consistent with
existing terminology.


[major] A "confederation sub-AS boundary" and "confederation boundary" are
used apparently to mean the same thing.  But I don't think they do.
Specifically, a "confederation boundary" seems to imply exiting a
confederation, which is different from advertising something from one
Member-AS to another.  Please clarify.


[major] What about the interaction between the T and C bits?  Specifically,
if the T bit is set ("transitive across all ASes") and the C bit is not set
("not transitive across confederation sub-AS boundary").  A Member-AS is an
AS, so it would fit in the description of "all ASes".



...
303   The Length field represents the total length in octets of a given
304   container's contents.

[major] ...not including the common header, right?  Given that it could be
interpreted that the container starts with the header, it may be worth
clarifying.



...
329 4.1.  Community Value

331   Community Value: 4 octets

[major] §11.5 uses "Community Type".



333   The Community Value indicates what set of actions a router is
334   requested to take upon reception of a route containing this
335   community.  The semantics of this value depend on whether this is a
336   private/local community or IANA registered.

338   When the high order bit of the Community Value field - I - is set,
339   the value is IANA Registered and has a well defined meaning with
340   underlying semantics.  See the documentation for each Registered BGP
341   Wide Community for its semantics and validation requirements.

343   When the high order bit of the Community Value field is clear, the
344   value is Locally defined and has semantics solely within the control
345   of the AS defining that community.  The Context AS Number provides
346   the namespace in which this Community Value is interpreted.  It is
347   that AS's responsibility to provide the semantics and validation
348   requirements for the BGP Wide Community.

350   See Section 11.5 for code point space partitioning.

[major] Given how the ranges are allocated, the definition of the I bit is
not providing any additional information.  It is also not correct to say
that it indicates that the value is "IANA Registered and has a well defined
meaning with underlying semantics" for two reasons: (1) values in the
experimental range are not recorded by IANA, and (2) there may be undefined
values...

It may be too late to remove the I bit -- and simply let the 32-bit field
reflect the value. [The suggestion below doesn't use it.]


[major] "...clear, the value is Locally defined and has semantics solely
within the control of the AS defining that community."

This phrase seems to indicate a willingness for the Wide Community to not
be propagated outside the Context AS, or only be used inside it, or maybe
both.  Should there be a scope indicated with the T and C bits if this type
of Community Value is used?  What should a router do if the Context ASN
doesn't correspond to the local ASN?  Are there special considerations for
confederations?

[This question is related to the filtering question above.]


[major] What should a receiver do if it encounters an unknown Community
Value?


[] Suggestion>
   The Community Value indicates what set of actions a router is
   requested to take upon reception of a route containing this
   community.  All new BGP Wide Community Values MUST be defined
   with specific semantics and  validation requirements.

   Some Community Values are reserved for Private or Experimental
   Use (Section 11.5).  When these values are used, the Context AS
   Number provides the namespace in which this Community Value is
   interpreted.  The definition of the semantics and validation
   requirements and their communication are out of scope.

   [Text about scoping and handling of the Context ASN.]

   [Text about handling of unknown values.]



352 4.2.  Source AS Number
...
356   The Autonomous System number indicates the AS originating this BGP
357   Wide Community.

[major] Should this field always have a value?  If so, please Normatively
indicate so.  What should a receiver do if the field is empty (zero)?
Should there be any additional verification -- for example, comparing the
AS_PATH to the Source ASN?



359 4.3.  Context AS Number
...
363   This identifies the AS that provides the semantics to interpret this
364   Community.

[major] It is clear from above that this field should be considered when a
Private/Experimental value is used.  But, what about when registered values
are used?  Please indicate what should be done (I assume ignore, but better
to be explicit).


[major] Should the Context and Source ASNs always match?  If locally
defined Community Values are to be used locally, then that should be the
case, right?  When would it be ok for the values to not match?



366 4.4.  BGP Wide Community TLVs
...
382   Sub-Type:
383       The sub-type of the BGP Wide Community TLV.  A given Sub-Type
384       MUST NOT appear more than once.

[nit] These are not sub-TLVs... Just a nit.


[major] What should a receiver do if a given TLV appears more than once?



...
392 4.4.1.  Sub-Type 1, BGP Wide Community Target(s) TLV

[minor] Even if only one Target can be configured, should the name of this
TLV be generalized by using "Targets" (and not "Target(s)"?  It would make
the text clearer.



...
398   BGP Wide Community Targets define the matching criteria for the
399   community.  A given wide community may have a number of targets to
400   which it applies.  The semantics of these targets will vary on a per-
401   community basis.  Depending on the definition of the community,
402   targets may be optional.

[nit] s/wide community/Wide Community/g

Most of the text used the capitalized version.  Please be consistent.



404   Wide Community Targets consist of a series of Atoms that have "match
405   any" semantics.  Thus, if any given target matches per the semantics
406   of that Atom for the community, the community is considered to match
407   and the action defined by the community should be executed.

[nit] s/Wide Community Targets/BGP Wide Community Targets


[minor] It may just be me, but I had to look up what "match any" referred
to.



...
411   If the semantics of a given Atom is undefined for the community in
412   question, this Atom MUST be ignored.

[major] If reuse is possible, what does it mean for an Atom to be
"undefined for the community"?  Does this sentence require that new
Community Values mention which Atoms are applicable (defined) to them?  And
that if a document defines a new Atom it then has to list all the
applicable Communities?



...
419 4.4.2.  Sub-Type 2, BGP Wide Community Exclude Target(s) TLV

421   The BGP Wide Community Exclude Target(s) TLV (Sub-Type 2) contains a
422   list of Atoms.

[minor] A "list of Atoms" seems to be, well, a list of Atoms...instead of
"series of Atom TLVs", as the specification in §4.4.1 says.  Please be
consistent and precise in the specification.



424   Wide Community Exclude Targets define criteria by which the community
425   is considered to NOT match.  Depending on the semantics of the BGP
426   Wide Community, Exclude Target(s) may be optional.

[nit] s/Wide Community Exclude Targets/BGP Wide Community Exclude Targets


[major] "NOT" is not an rfc2119 keyword and will require to be written in
lowercase -- even if the intent is to add emphasis.



...
439   If the BGP Wide Community Target(s) TLV and the BGP Wide Community
440   Exclude Target(s) TLV have conflicting semantics, priority MUST be
441   given to the Wide Community Exclude Target(s) TLV.

[major] "conflicting semantics...priority MUST be given"

I think you may mean "conflicting results" (or something like that) as the
semantics (include vs exclude) are already conflicting.  To be clear, I
mean that the Targets TLV matched on a set of targets, but the Exclude
Targets TLV also matches on the same.  Is that an example of that you mean?

If so, then it seems to me that the result is to ignore the matches from
the Targets TLV and act (exclude) on the matched from the Exclude Targets
TLV.  I'm sure there must be a succinct way of specifying the behavior
clearly.

All this comes from the fact that "priority" implies (to me) that the
Targets may be considered *after* the Exclude Targets.



...
449 4.4.3.  Sub-Type 3, BGP Wide Community Parameter(s) TLV

451   The BGP Wide Community Parameter(s) TLV (Sub-Type 3) contains a list
452   of Atoms.

[] "list of Atoms" -- the mention below of "an ordered set of Atom
sub-TLVs" is a better description.



...
461   Care must be taken when using Atoms with list semantics.  If the
462   desired behavior is a single or limited number of instances of that
463   type, this should be documented as part of the use case of that BGP
464   Wide Community.

[major] "should be documented"

Other mentions of documentation requirements use MUST, but this one uses a
non-normative statement.  Why the difference?  Should this mention be
Normative too?



...
478 4.4.4.  Usage

480   The detailed interpretation of the targets or parameters SHALL be
481   provided when describing given community type in a separate document
482   or when locally defined by an operator.

[] This section seems to just be a restatement of the requirements listed
above.  Please remove it.



484 5.  BGP Community Container Atoms
...
504   The Type field contains a value of 1-254.  The values 0 and 255 are
505   reserved for future use.  The TLV types are to be assigned and
506   maintained by IANA registry; see Section 11.2.

[major] No need to include any IANA-related text here.



...
512   Supported format of the TLVs can be:

[minor] These are the defined TLVs, but other may be (eventually)
supported.  In any case, if the Atoms are defined in the following sections
it seems redundant to list them here.

514     Type  1: Autonomous System Number List.
515     Type  2: IPv4 Prefix (1 octet prefix length + prefix) List.
516     Type  3: IPv6 Prefix (1 octet prefix length + prefix) List.

[major] The TLVs are called "IPvx Prefix List" -- no patenthesis for
clarification.



...
526   In the following sections defining the different Atoms, validation
527   rules for the Length of the Atom will be presented.  If the Length of
528   the Atom does not match the rules for that Atom, it SHALL be
529   considered malformed.  (See Section 8.)

[nit] The first sentence seems to be missing something...

Suggestion>
   The following sections define the Atoms and the validation rules
   for their Length.



531   In general, Atoms of List type have the semantics of sets.  Duplicate
532   entries SHOULD NOT be present and MAY be removed by a BGP Speaker
533   propagating the Lists.  The presence of duplicate entries have no
534   additional semantics.

[major] "SHOULD NOT be present"

This phrase brings up the question of when is it ok to include duplicate
entries...but I think that the statement should not be normative.

Suggestion>
   In general, Atoms of List type have the semantics of sets.  The
   presence of duplicate entries have no additional semantics, and
   they MAY be removed by a BGP Speaker propagating the Lists.



536 5.1.  Atom Type 1, The Autonomous System Number List

538   This Atom represents a list of Autonomous System numbers, each 4
539   octets in size.  When encoding two-octet ASes, the first two octets
540   of this four-octet value MUST be filled with zeros.  The minimum
541   Length of this Atom is 4 octets.  The Length MUST be a multiple of 4.

[major] "When encoding two-octet ASes, the first two octets of this
four-octet value MUST be filled with zeros."

This encoding is already in the base spec (rfc6793 formally Updated
rfc4271) so it doesn't need to be specified here again.  Note also that the
specification of the Source/Context ASN doesn't include this behavior
specification -- it is not needed there either because it is part of the
base spec.



543   Two special values are reserved for the Autonomous System Atoms:

[major] "values are reserved"

In general, the concept of reserving values carries the impression that
they have been set aside in a registry.  That is not the case here.

Suggestion>
   When used in an Autonomous System Number List Atom, the following
   values have special meaning:



...
548 5.2.  Atom Types 2 and 3, The IPv4 and IPv6 Prefix Lists

550   This Atom represents a list of IPv4 or IPv6 prefixes.  IPv4 and IPv6
551   Prefix Atom values are encoded in the same format used by BGP NLRI in
552   Section 4.3 of [RFC4271].

[minor] s/This Atom represents/These Atoms represent


[minor] s/IPv4 and IPv6 Prefix Atom/IPv4 and IPv6 Prefix List Atom


[minor] s/encoded in the same format used by BGP NLRI in Section 4.3 of
[RFC4271]./encoded as specified for the BGP NLRI in Section 4.3 of
[RFC4271].



...
562   The Prefix Length for IPv4 prefixes MUST be in the range of 0..32.

564   The Prefix Length for IPv6 prefixes MUST be in the range of 0..128.

[major] If using the same specification from rfc4271 there's no need to
respecify the behavior here.



566   The Length field must be able to accommodate the list of prefixes
567   according to the encoding rules.  If the Length cannot fully
568   accommodate the required number of octets to encode the Prefix Length
569   and the Prefix, the Atom SHALL be considered malformed.  (See section
570   Section 8

[major] "Length cannot fully accommodate the required number of octets"

Without knowing upfront the number of prefixes in the list, how can it be
determined that the Length is wrong?  If just one prefix is included then
there's no issue, but more than one can result in the next value (or Atom)
being overrun/underrun.

The length of the NLRI is not explicitly encoded in an UPDATE, but rfc4271
offers a way to calculate it.  In this case, a calculation (if possible)
could be used to asses the validity of the Length.



...
589 5.5.  Atom Type 6, The Neighbor Class List

591   The Neighbor Class list Atom represents a list of Neighbor classes,
592   each 4 octets in size.  Neighbor class currently can contain three
593   values:

595   Peer (1):
596       This class is typically applied to sessions where a transit-free
597       relationship exists between two providers.

599   Customer (2):
600       This class is typically applied to sessions where the remote end
601       of the session is operated by a customer.

603   Upstream (3):
604       This class is typically applied to sessions where the remote end
605       of the session is operated by a network from which you receive
606       transit routes.

[major] RFC9234 defined the concept of a Role, which seems to align with
the Neighbor Classes defined here.  Why wasn't that work reused?



...
611 5.6.  Atom Type 7, The User-defined Class List
...
619   Examples of User-defined Class properties include geography (East,
620   West), continent (North America, Asia, Europe), etc.  Similar to the
621   [RFC1997] BGP Communities, it is necessary that the Context AS
622   provide a registry of the value and the semantics of a given
623   community.

[minor] Add: "; how this is done is out of scope"



...
642 6.  Well Known Standard BGP Communities

644   According to RFC 1997, as well as IANA's Well-Known BGP Communities
645   registry, the following BGP communities are defined to have global
646   significance:

648        0xFFFF0000   planned-shut         [draft-francois-bgp-gshut]
649        0xFFFFFF01   NO_EXPORT            [RFC1997]
650        0xFFFFFF02   NO_ADVERTISE         [RFC1997]
651        0xFFFFFF03   NO_EXPORT_SUBCONFED  [RFC1997]
652        0xFFFFFF04   NOPEER               [RFC3765]

[major] There are more communities listed in the registry.  Instead of
listing them, a reference to the registry may be more appropriate.


654   This document recommends for simplicity as well as for avoidance of
655   backward compatibility issues that the continued use of BGP Standard
656   Community Path Attribute, type 8, as defined in [RFC1997] and
657   [RFC3765] to distribute non-Autonomous System specific Well-Known BGP
658   Communities.

[minor] Where are global vs "non-Autonomous System specific" WKCs
categorized?  Given that the recommendation is not Normative, would it be
ok to extend it to all WKCs?


660   For the same reason, this document does not intend to obsolete the
661   currently defined and deployed BGP Extended Communities.

[nit] "document does not intend" -- this document is being written now, if
it didn't obsolete then there's no point in mentioning what it didn't do.


[] Suggestion>
   A number of Well-Known BGP Communities are documented by IANA
   [Informative-Reference-to-the-Registy].  It is recommended to
   continue using the BGP Community Attribute [RFC1997] for the
   propagation of these Well-Known Communities.



663 7.  Operational Considerations

665   Having multiple ways to propagate locally assigned BGP Communities -
666   via the use of Standard, Extended or Large BGP Communities versus the
667   use of BGP Wide Communities - may seem to potentially cause problems
668   when considering propagation of conflicting actions.  However, even
669   at present, an operator may append such Communities with conflicting
670   information.

[minor] Please add references.


[major] This first paragraph doesn't sound like much operational guidance
-- it reads: "this new container can cause problems, but you can shoot
yourself in the foot already!".

Perhaps, something like this might be better:

Suggestion>
   Multiple mechanisms exist for an operator to propagate actions into the
   network.  Besides BGP Wide Communities, an operator can use the BGP
   Community Attribute [rfc1997], the BGP Extended Communities Attribute
   [rfc4360], or the BGP Large Communities Attribute [rfc8092] to achieve
   similar objectives.  Care should be taken when using more than one of
   these tools.  The interaction between the different community attributes
   is out of the scope of this document.



672   Implementations SHOULD provide mechanisms to control the order of
673   processing and manipulation of the varying types of BGP communities.
674   With such a mechanism, operators will have the ability to control the
675   outcome of potentially conflicting actions.

[major] What does "manipulation" include?


[major] What about filtering?  I ask because §10.2 says that "the Community
Value the Source AS may provide sufficient context to strip unwanted or
unexpected communities." -- but there is no corresponding
requirement/recommendation for an implementation to provide that
functionality.


[] Perhaps put the sentence about the interaction being out of scope at the
end of the second paragraph.



...
679 8.1.  General Error Handling for BGP Community Containers

681   [RFC7606] "treat as withdraw" behavior is expected for any malformed
682   Community Containers or malformation of their contents.

[nit] s/[RFC7606] "treat as withdraw" behavior/The "treat as withdraw"
behavior [RFC7606]


[major] "is expected" -- can we make this Normative?  SHOULD/MUST?



684   Each Community Container type may have additional validation rules,
685   including permitted length of Atoms.  Failure to conform to those
686   additional rules MUST also be treated as a malformed Community
687   Container.

[] I asked several times about the behavior specified...it wasn't clear to
me that the result would be to treat the container/community as malformed.
Please indicate which errors should result in the behavior specified here.



689 8.2.  BGP Wide Community Container Error Handling

691   If any Atom in a BGP Wide Community container's Exclude Targets TLV
692   is unrecognized, that Wide Community MUST NOT be considered a match
693   and no actions for that community should be processed.  While the
694   Targets TLV is meant to be inclusive, the Exclude Targets TLV is
695   meant to be proscriptive of applying the action.

[major] §4.4.1 talks about what to do if the Atom is "undefined for the
community", but there's no indication of what to do in the unrecognized
case.  Please add text for that there.


[major] In the Targets TLV case, if an unrecognized Atom is ignored (?),
there may be routes that are not matched -- leaving them without action.
The action (and not the logic of the match) is what is important here; for
example, a Target Atom may leave out routes that should have been
announced, or announced ones that shouldn't have.  The same result is
possible based on the Exclude Targets Atom.



697 9.  Example

[] Nice example!  Consider moving it to an Appendix.



699 9.1.  Example Type 1 Wide Community Definition
...
707   AS 64496 has established a registered set of values to use for its
708   User-defined Class:

[nit] s/established a registered set/established a set



...
725      The Targets TLV MUST contain at least one entry.

[major] This is an example, don't use Normative language: s/MUST/must



727      The Exclude Targets TLV MAY contain entries of the above supported
728      Atoms.

[major] This is an example, don't use Normative language: s/MAY/may



...
739 9.2.  Example Type 1 BGP Wide Community Encoding

741   In this example, the BGP Wide Community defined above is used by a
742   BGP Speaker to AS_PATH prepend BGP routes containing this community
743   AS_PATH prepend 4 TIMES when this route is to be distribute to any of
744   AS 2424, AS 8888, to peers marked as User Class Amsterdam (100) or to
745   peers marked User Class Moscow (104).  However, such prepending would
746   not be done to peers that have been configured in the User Class of,
747   but not to peers of User Class New York (101), regardless of their AS
748   number.

[minor] s/AS_PATH prepend BGP routes containing this community AS_PATH
prepend 4 TIMES/AS_PATH prepend BGP routes containing this community 4
times


[minor] s/when this route is to be distribute to any of AS 2424, AS 8888,
to peers marked as/when advertising to AS 2424 or AS 8888, and to peers
marked as


[minor] s/been configured in the User Class of, but not to peers of User
Class New York (101)/been configured in the User Class New York (101)



...
753      0                   1                   2                   3
754      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
755     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
756     |         Type (1, Wide)        |    Flags  |0|0|  Reserved(0)  |
757     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
758     | Length: 53                    |
759     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[] If I counted correctly, the length should be 57.

760     | Community: LOCAL PREPEND ACTION CATEGORY                    1 |
761     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
762     | Source AS 64496                                               |
763     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
764     | Context AS 64496                                              |
765     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
766     | Target TLV (1)|   Length: 18                  |
767     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[] If I counted correctly, the length should be 22.

768     | ASN List (1)  |   Length: 8                   |
769     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
770     | Target ASN# 2424                                              |
771     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
772     | Target ASN# 8888                                              |
773     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
774     |  User List(7) |   Length: 8                   |
775     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
776     | Amsterdam (100)                                               |
777     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
778     | Moscow (104)                                                  |
779     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
780     |ExcTargetTLV(2)|   Length: 5                   |
781     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[] If I counted correctly, the length should be 7.

782     |  User List(7) |   Length: 4                   |
783     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
784     | New York (101)                                                |
785     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
786     | Param TLV (3) |   Length: 5                   |
787     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

[] If I counted correctly, the length should be 7.

788     | Integer32 (4) |   Length: 4                   |
789     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
790     | Prepend # 4                                                   |
791     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+



...
796 10.1.  BGP Community Container Security Considerations

798   Transitive BGP Community Container communities could unintentionally
799   spread far from their origin.  If a router receives many routes from
800   multiple sources on the Internet with different communities, it could
801   cause significant memory usage.  To prevent excessive memory usage,
802   routers should be configured to strip unexpected communities from
803   received routes.

[] The "far from their origin" doesn't seem to directly result in "cause
significant memory usage".  Maybe a better justification may be the
potential impact (in terms of actions) that could result from receiving and
parsing.


[major] "routers should be configured to strip unexpected communities"

This sounds like a good place to be Normative: MUST/SHOULD?  I think that
RECOMMENDED might be best.

Also, given that we're talking about the Container, there may be other
types which are not communities.  Please generalize.



805   All the security considerations for BGP Communities [RFC1997], BGP
806   Extended Communities [RFC4360], and BGP Large Communities [RFC8092]
807   apply to BGP Community Containers.

[minor] s/All the security considerations/The security considerations


[minor] Put this paragraph first.



[] Other work that used UTF-8 has been required to include text like this:

   This document uses UTF-8 encoding for the UTF-8 String Atom.
   There are a number of security issues with Unicode. Implementers
   and operators are advised to review Unicode Technical Report #36
   [UTR36] to learn about these issues. UTF-8 "Shortest Form" encoding
   is REQUIRED to guard against the technical issues outlined in [UTR36].

An Informative reference to UTR36 should be added as follows:

   [UTR36]  Davis, M., Ed. and M. Suignard, Ed., "Unicode Security
            Considerations", Unicode Technical Report #36, August
            2010, <http://unicode.org/reports/tr36/>.



809 10.2.  BGP Wide Community Security Considerations

811   For BGP Wide Communities, the Community Value the Source AS may
812   provide sufficient context to strip unwanted or unexpected
813   communities.

[minor] s/Community Value the Source AS/Community Value and the Source AS
Number


[] Stripping communities ("removing" may be a better word) was also
mentioned in the previous section.  Repeating the recommendation made there
would be appropriate.



815   Given the flexibility and power offered by BGP Wide communities, it
816   is important to consider the additional possibilities allowed by
817   their definition.  In particular, for locally defined BGP Wide
818   Communities, it may be wise to restrict the range of parameters.  For
819   registered BGP Wide Communities, the security considerations of the
820   document defining them MUST address issues specific to those newly
821   defined Communities.

[minor] "it may be wise"

Can we turn this into a Normative recommendation?



823 11.  IANA Considerations

[major] This section requests the creation of new registries, but it
doesn't indicate where they should be located.  Other community-related
documents have created a separate group (from the general bgp-parameters
group [1]), for example: [2] and [3].

I suggest that you request a new group here too.  You can then make a
general statement that all the new requested registries should be located
there (instead of doing so for each individual one).

Also, please indicate that all the Registration Policies are per rfc8126,
and add a Normative reference to it.

[1] https://www.iana.org/assignments/bgp-parameters/
[2] https://www.iana.org/assignments/bgp-well-known-communities/
[3] https://www.iana.org/assignments/bgp-extended-communities/



...
832 11.2.  BGP Community Container Atoms Types
...
838   Registration procedures:

840            0x00: Reserved.
841       0x01-0x08: Defined in this document.
842       0x09-0xFE: IETF Review.
843            0xFF: Reserved.

[minor] Please number all the tables in this section.


[major] This table, and others in this section,  mixes the registration
policies and the assignments.  Please create separate tables.  The table
indicating the Registration Policies should indicate the whole range:
 0x01-0xFE: IETF Review.



...
848       Name                             Type Value
849       ----                             ----------
850       Autonomous System Number List      0x01
851       IPv4 Prefix list                   0x02
852       IPv6 Prefix list                   0x03
853       Unsigned Integer32 list            0x04
854       IEEE Floating Point Number list    0x05
855       Neighbor Class list                0x06
856       User-defined Class list            0x07
857       UTF-8 string                       0x08

[major] This table, and others in this section, should have an extra column
for references.  For the values defined here, use "[this document]".



...
881 11.4.  BGP Community Container Types
...
890                      0x0000 : Reserved.
891                      0x0001 : BGP Wide Community (defined in this
892                               document).
893               0x0002-0x0004 : Reserved.
894               0x0005-0x00FF : IETF Review.
895               0x0100-0xFF00 : First Come, First Served.
896               0xFF01-0xFFFE : Experimental.
897                      0xFFFF : Reserved.

[] s/First Come, First Served/First Come First Served


[] s/Experimental/Experimental Use


[major] This table mixes the registration policies and the assignments.
Please create separate tables.

The table indicating the Registration Policies should indicate the whole
range:  0x0001-0x00FF: IETF Review.

A different table should then include any allocations made in this
document, including a column that indicates the reference (to "[this
document]").  In this case 0x0001-0x0004.  BTW, why are 0x0002-0x0004
reserved?  Should they be marked as deprecated?



899 11.5.  Registered Type 1 BGP Wide Communities Community Types

[major] §4.1 calls this field "Community Value".



...
907                  0x00000000 : Reserved.
908       0x00000001-0x7FFFFFFF : Available for private/local use.
909                  0x80000000 : Reserved.
910       0x80000001-0xFFFFFEFF : First Come, First Served for
911                               registered use.
912       0xFFFFFF00-0xFFFFFFFE : Experimental.
913                  0xFFFFFFFF : Reserved.

[major] The Registration Policies, per rfc8126, should be:

"Available for private/local use." = Private Use

"First Come, First Served for registered use" = First Come First Served


[] Note that Private Use and Experimental Use are similar in that IANA
doesn't keep track of them and there's no restriction on their use.  This
means that, unless specified in this document (§4.1), a Private Use
Community Values cannot be constrained to local use.

OTOH, there's an opportunity to clarify the expectations of an experiment.
>From rfc8126:

   4.2.  Experimental Use

   Experimental Use is similar to Private Use, but with the purpose
   being to facilitate experimentation.  See [RFC3692] for details.
   IANA does not record assignments from registries or ranges with this
   policy (and therefore there is no need for IANA to review them) and
   assignments are not generally useful for broad interoperability.
   Unless the registry explicitly allows it, it is not appropriate for
   documents to select explicit values from registries or ranges with
   this policy.  Specific experiments will select a value to use during
   the experiment.

   When code points are set aside for Experimental Use, it's important
   to make clear any expected restrictions on experimental scope.  For
   example, say whether it's acceptable to run experiments using those
   code points over the open Internet or whether such experiments should
   be confined to more closed environments.  See [RFC6994] for an
   example of such considerations.



915 11.6.  Registered Type 1 BGP Wide Community Optional Sub-Types
...
931       Name                             Type Value
932       ----                             ----------
933       Targets                              1
934       Exclude Targets                      2
935       Parameters                           3

[major] The specification in §4.4.* uses an extended name: BGP Wide
Community Target(s) TLV, for example.  Please be consistent.

[EoR -08]