[Idr] AD Review of draft-ietf-idr-rpd-15

[Alvaro Retana <[e-mail]>]

Dear Authors, Shepherd, Chairs, WG:

This draft has been in my queue for a while—apologies for the delay in
getting to it, and thank you for your patience.


I have many comments/concerns about the specification and the
application.  Significant work is still needed!  When reviewing a
document (any document), I read it and then check the mailing lists.
My concerns with the application align with the objections brought up
during the first WGLC [1].  While they were not repeated with the same
energy during the second WGLC [2] [3], those concerns have still not
been addressed!

Addressing objections to this solution doesn't necessarily imply
abandoning the work.  No solution is perfect!  Where possible, I
expect changes in the text to clarify the behavior and mention issues
and potential mitigations.  The fact that implementations and
deployments exist or that some of the questions have been responded to
on the mailing list doesn't mean that the draft is clear or the issues
have been addressed.  The focus is on what the document says!  One
example of this point is related to security -- unfortunately, the
Security Considerations section says this (and only this):

   Protocol extensions defined in this document do not affect BGP
   security other than as discussed in the Security Considerations
   section of [RFC8955].

But this work is not related to rfc8955!  <sigh>  As I said,
significant work is needed!


This document mixes the specification of Wide Community Atoms and
their use in the Routing Policy Distribution (RPD) application.  The
Atoms seem (mostly) in line with the Wide Community specification [4].
The application is similar to the AS_PATH prepending example [5].  A
problem arises when the specification of the Atoms is made to (only)
fit the application, even if their use can be generalized.  Therefore,
the document should be clear about the separation between Wide
Community Atoms (which can be reused elsewhere, etc.) and the
application itself (i.e., the new NLRI).  It might be a good idea to
separate the work into two documents.  [See specific comments in §4.]

Other high-level issues:

(1) The document attempts to separate RPD from other policies, insisting that
    an "RPD route is not a configuration".  However, it does not provide any
    comments or guidance on which mechanism to select or their co-existence.

(1a) How does RPD interact/co-exist with other policies?  I'm thinking of
     manual configuration, PCEP, rfc9067 (YANG Data Model for Routing Policy)
     + draft-ietf-idr-bgp-model, and even Wide Communities tagging specific
     routes [5].  Consider the guidance in §2/rfc5706 [6].

(1b) The mailing list contained several statements related to the suitability
     of RPD with respect to other solutions.  Please include some of the
     considerations in the document.  The goal here is not to put other
     solutions down but to highlight why an operator may want to consider this
     one.

(2) The Problem Statement mentions Inbound and Outbound Traffic Control, but
    the RPD mechanism only applies to the inbound direction.  Please trim the
    justification to include what the document addresses.

(3) §4.2.1 defines the RouteAttr Atom with several TLVs used to describe a
    route.  Given that the Wide Communities draft includes other Atoms that can
    be used similarly, why aren't the TLVs defined as Atoms?  Unfortunately,
    the intent of draft-ietf-idr-wide-bgp-communities is not followed here,
    making it harder to reuse the defined mechanisms.

(4) The document could greatly benefit from an Operational Considerations
    section.  Please scan the text for several places (at least 5!) where I
    suggest the same.


Before moving this document forward, I'll wait for a revised ID and
for this review to be addressed.  If it becomes necessary to talk live
about a specific point, please feel free to look for time in my
calendar:  https://fantastical.app/aretana-tJ8Q/open-time

Thanks!

Alvaro.



[1] https://mailarchive.ietf.org/arch/msg/idr/0gfjwluhnCcbRto7LowK5Xy0vLE/
[2] https://mailarchive.ietf.org/arch/msg/idr/MLkALnz0PbBF9gcLnnR2XU9dTnw
[3] https://mailarchive.ietf.org/arch/msg/idr/TcQ9nYJCC4v3iX-_W3cgivDQ4FM
[4] https://datatracker.ietf.org/doc/draft-ietf-idr-wide-bgp-communities/
[5] https://datatracker.ietf.org/doc/html/draft-ietf-idr-wide-bgp-communities-08#name-example
[6] https://datatracker.ietf.org/doc/html/rfc5706#autoid-8



[Line numbers from idnits.]

2	Network Working Group                                              Z. Li
3	Internet-Draft                                                    Huawei
4	Intended status: Standards Track                                   L. Ou
5	Expires: 29 July 2022                                             Y. Luo
6	                                                  China Telcom Co., Ltd.
7	                                                                   S. Lu
8	                                                                 Tencent
9	                                                               G. Mishra
10	                                                            Verizon Inc.
11	                                                                 H. Chen
12	                                                               Futurewei
13	                                                               S. Zhuang
14	                                                                 H. Wang
15	                                                                  Huawei
16	                                                         25 January 2022

[major] There are 8 authors listed.  Per the guidance from rfc7322,
there should not be more than 5.  Please reduce the list to 5 (or
less), or identify en Editor (or 2).  The rest of the authors can be
listed in the Contributors section.



...
21	Abstract

23	   It is hard to adjust traffic and optimize traffic paths in a
24	   traditional IP network from time to time through manual
25	   configurations.  It is desirable to have a mechanism for setting up
26	   routing policies, which adjusts traffic and optimizes traffic paths
27	   automatically.  This document describes BGP Extensions for Routing
28	   Policy Distribution (BGP RPD) to support this.

[minor] It is not clear from the text above what "this" (last
sentence) is.  The extensions can be used "for setting up routing
policies", but it would be out of scope to claim that the mechanism
"adjusts traffic and optimizes traffic paths automatically".



...
98	1.  Introduction

100	   It is difficult to optimize traffic paths in a traditional IP network
101	   because of the following:

[] I think there's some background missing to quality what a
"traditional IP network" is and separate it from "non-traditional"
(?).  Also, what type of optimizations are considered?



103	   *  Complex.  Traffic can only be adjusted device by device.  The
104	      configurations on all the routers that the traffic traverses need
105	      to be changed or added.  There are already lots of policies
106	      configured on the routers in an operational network.  There are
107	      different types of policies, which include security, management
108	      and control policies.  These policies are relatively stable.
109	      However, the policies for adjusting traffic are dynamic.  Whenever
110	      the traffic through a route is not expected, the policies to
111	      adjust the traffic for that route are configured on the related
112	      routers.  It is complex to dynamically add or change the policies
113	      to the existing policies on the special routers to adjust the
114	      traffic.  Some people would like to separate the stable route
115	      policies from the dynamic ones even though they have configuration
116	      automation systems (including YANG models).

[nit] "traffic through a route"

This phrase doesn't sound right to me.  Maybe traffic through a path
(in the network), or even through a specific router...


[nit] "It is complex to dynamically add or change the policies to the
existing policies on the special routers to adjust the traffic."

You're explaining why the optimization is complex by saying that it is
complex...  It sounds like a circular definition: Why is it complex?
Because it is complex.

Also, "change the policies to the existing policies" doesn't mean
anything to me.


[minor] "Some people would like to separate the stable route policies
from the dynamic ones even though they have configuration automation
systems (including YANG models)."

Is there relevance to this sentence?  How does it contribute to
explaining complexity?



118	   *  Difficult maintenance.  The routing policies used to adjust
119	      network traffic are dynamic, posing difficulties to subsequent
120	      maintenance.  High maintenance skills are required.

[minor] What in the dynamic nature of the policies makes them
difficult to maintain?  Again, the justification is circular: it is
difficult because it is difficult...


[minor] "High maintenance skills are required."

What are "High maintenance skills"?


[] Are the justifications for "complex" and "difficult" the same?



122	   *  Slow.  Adding or changing some route policies on some routers
123	      through a configuration automation system for adjusting some
124	      traffic to avoid congestions may be slow.

[minor] Again...it is slow because it is slow...  Only that in this
case it "may be slow", which begs the question: can it also be fast
and easy? ;-)



126	   It is desirable to have an automatic mechanism for setting up routing
127	   policies, which can simplify routing policy configuration and be
128	   fast.  This document describes extensions to BGP for Routing Policy
129	   Distribution to resolve these issues.

[] As mentioned in the Abstract, the automatic part is not really
achieve by defining some extensions (which could be triggered
manually!).

Also, the text above doesn't present a compelling reason to want to
make things simpler and faster... :-(



131	2.  Terminology

[] Only NLRI and RPD are used in the text.  Both can be expanded on
first use.  This section is not necessary.


133	   The following terminology is used in this document.

135	   *  ACL: Access Control List

[nit] Not used.


137	   *  BGP: Border Gateway Protocol [RFC4271]

[minor] BGP is a well-known term.


139	   *  FS: Flow Specification

[nit] Not used.


141	   *  NLRI: Network Layer Reachability Information [RFC4271]

142	   *  PBR: Policy-Based Routing

[nit] Not used.

144	   *  RPD: Routing Policy Distribution

146	   *  VPN: Virtual Private Network

[nit] Not used.



148	3.  Problem Statement

150	   Providers have the requirement to adjust their business traffic
151	   routing policies from time to time because of the following:

153	   *  Business development or network failure introduces link congestion
154	      and overload.

156	   *  Business changes or network additions produce unused resources
157	      such as idle links.

159	   *  Network transmission quality is decreased as the result of delay,
160	      loss and they need to adjust traffic to other paths.

162	   *  To control OPEX and CPEX, they may prefer the transit provider
163	      with lower price.

[] There is a significant disconnect between this problem statement
and the justification presented in the Introduction!  In short, there
is no clear path from the business problem described here to the
problems with traffic optimization (from the Introduction).  For
example, there's no speed component mentioned here...

Instead of adding more, it would be better to unify the
justification/background.



165	3.1.  Inbound Traffic Control

167	   In Figure 1, for the reasons above, the provider P of AS100 may wish
168	   the inbound traffic from AS200 to enter AS100 through link L3 instead
169	   of the others.  Since P doesn't have any administrative control over
170	   AS200, there is no way for P to directly modify the route selection
171	   criteria inside AS200.

[major] Mechanisms do exist!  Besides using MED, inter-AS agreements
for community-to-local-preference translation seems popular.  As
mentioned at the start, please indicate (in an operational
considerations section) why/when RPD should be considered over other
mechanisms.



...
201	3.2.  Outbound Traffic Control

203	   In Figure 2, the provider P of AS100 prefers link L3 for the traffic
204	   to the destination Prefix2 among multiple exits and links to AS200.
205	   This preference can be dynamic and might change frequently because of
206	   the reasons above.  So, provider P expects an efficient and
207	   convenient solution.

[minor] Again, the mechanisms exist!  For example, configuring
local-pref on the IGW2 router (single point, one line configuration)
sounds efficient and convenient to me.

Also, the focus of the rest of the draft seems to be on the Inbound
direction.  This section (or mention of outbound control) may not be
needed.



...
236	4.  Protocol Extensions

238	   This document specifies a solution using a new AFI and SAFI with the
239	   BGP Wide Community for encoding a routing policy.

[minor] Extend AFI and SAFI on first use.


[major] Please add a reference to rfc4760 and
draft-ietf-idr-wide-bgp-communities above.


[major] Note that rfc4760 uses the "<AFI, SAFI>" notation instead of
"AFI and SAFI".



241	4.1.  Using a New AFI and SAFI

[major] Given that the new <AFI,SAFI> basically creates a
"configuration session", should there be a recommendation about
keeping the session independent of routing information?  See the text
in rfc7752bis:

https://datatracker.ietf.org/doc/html/draft-ietf-idr-rfc7752bis#autoid-68



...
265	   NLRI Length:  1 octet represents the length of NLRI.  If the Length
266	      is anything other than 9 or 21, the NLRI is corrupt and the
267	      enclosing UPDATE message MUST be ignored.

[major] The NLRI, as defined in rfc4760, includes the "NLRI Length"
field, so defining this field as "the length of NLRI" is incorrect, as
the size would be 10 or 22 octets.


[major] Please specify that the length is in octets.



269	   Policy Type:  1 octet indicates the type of a policy.  1 is for
270	      Export policy. 2 is for Import policy.  If the Policy Type is any
271	      other value, the NLRI is corrupt and the enclosing UPDATE message
272	      MUST be ignored.

[major] The Peer IP indicates which peers "a BGP speaker will apply
the policy in the message to".  This sounds as an Export policy to me.
There seems to be no use case (or valid setting) of the Peer IP with
an Import policy.



274	   Distinguisher:  4 octet unsigned integer that uniquely identifies the
275	      content/policy.  It is used to sort/order the polices from the
276	      lower to higher distinguisher.  They are applied in ascending
277	      order.  A policy with a lower/smaller distinguisher is applied
278	      before the policies with a higher/larger distinguisher.

[minor] Please be consistent on the naming: Distinguisher vs distinguisher.


[major] Who sets this value?  The originator?  What are the issues
with multiple originators?


[major] "applied in ascending order"

Even if using a reliable transport, ordered delivery of BGP UPDATES is
not deterministic.  Also, more than one source may originate UPDATES.
This means that a policy may be applied before all its components are
present.  How can this be mitigated?  What is the effect of transient
or incomplete policies?


[major] Multiple BGP speakers may generate an UPDATE for the same Peer
IP with the same Distinguisher.  Which UPDATE should be applied?



280	   Peer IP:  4/16 octet value indicates IPv4/IPv6 peers.  Its default
281	      value is 0, which indicates that when receiving a BGP UPDATE
282	      message with the NLRI, a BGP speaker will apply the policy in the
283	      message to all its IPv4/IPv6 peers.

[] Use this part to describe the field, not the operation (which
should be described later).


[major] What should the receiver do if the value is not a valid IP
address (and not 0)?


[major] "BGP speaker will apply the policy"

Should this statement be Normative?  SHOULD/MUST?


[major] Are the policies applied equally to internal and external peers?


[minor] I assume that the Targets TLV may also provide some additional
details of where a policy should be applied, specially in the case of
0, right?


[major] The Peer IP field (whether indicating a specific address or
not) applies to existing peers only.  What I mean is that if a BGP
session is not up then there is no peer, right?  This means that we
have a chicken and egg scenario where the policy would apply to an
existing peer, but the peer would have to already be up for the policy
to apply: when a session goes up, UPDATES are sent....but in this case
the policy UPDATES may not have been received...and when they are the
advertisements may change.  What is the impact of these transient
advertisements?  Should the policies be also applied when a session
goes up?

rfc8212 talks about not advertising routes until an Export Policy has
been applied for an eBGP peer.  However, Export Policy is defined as
"a local policy".  This document repeatedly separates this new
mechanism from "configuration" (which is what I would characterize as
a "local policy").



285	   Under RPD AFI/SAFI, the RPD routes are stored and ordered according
286	   to the keys (Policy type, Distinguisher, Peer IP).  Under IPv4/IPv6
287	   Unicast AFI/SAFI, there are IPv4/IPv6 unicast routes learned and
288	   various static policies configured.  In addition, there are dynamic
289	   RPD policies from the RPD AFI/SAFI when RPD is enabled.

[major] "RPD routes are stored and ordered according to the keys
(Policy type, Distinguisher, Peer IP)"

rfc4271 has a conceptual model for route storage, but ordering is not
considered.  How does this specification interact with the RIB model
from rfc4271?

What is the significance of the ordering?  The description of the
Distinguisher says that it is "used to sort/order the
polices...[which]...are applied in ascending order", but this text
includes other keys.  IOW, the Distinguisher description implies that
the application depends (only!) on its value and not on the policy
type.  The text is contradictory!


[minor] I'm not sure what purpose the last two sentences serve in the
context of defining the new NLRI.  It seems to me that the text is not
needed, and can cause confusion.



291	   Before advertising an IPv4/IPv6 Unicast AFI/SAFI route, the
292	   configured policies are applied to it first, and then the RPD Export
293	   policies are applied.

[major] Should this text be Normative?


[major] rfc4271 doesn't account for different types of policies (for
example, configured or not).  What are "configured policies"?  Is
Routing Policy provisioned using rfc9067+draft-ietf-idr-bgp-model
considered a "configured policy" or an "RPD export policy"?



295	   The NLRI containing the Routing Policy is carried in MP_REACH_NLRI
296	   and MP_UNREACH_NLRI path attributes in a BGP UPDATE message, which
297	   MUST also contain the BGP mandatory attributes and MAY contain some
298	   BGP optional attributes.

[major] Besides the NLRI, the MP_REACH_NLRI attribute contains a Next
Hop.  How should it be set?


[minor] Please move this mention of where the NLRI is carried to the
top of this section, and reference rfc4760.


[major] "...which MUST also contain the BGP mandatory attributes and
MAY contain some BGP optional attributes."

This behavior is specified elsewhere, and there's no need to repeat it
here again.  Please remove this last part.



300	   When receiving a BGP UPDATE message with routing policy, a BGP
301	   speaker processes it as follows:

[nit] The text uses "routing policy" and "RPD policy" to refer to the
same thing.  Please pick one and be consistent about its use.


[major] Should any of this behavior be Normative?


303	   *  If the peer IP in the NLRI is 0, then apply the routing policy to
304	      all the remote peers of this BGP speaker.

[nit] s/peer IP/Peer IP/g


[major] The behavior is also specified above when the Peer IP field
was described.  Please specify things only once!  Suggestion: use the
description to describe the fields and specify the behavior elsewhere.


[minor] This text uses "remote peer" -- when is a peer not remote?


306	   *  If the peer IP in the NLRI is non-zero, then the IP address
307	      indicates a remote peer of this BGP speaker and the routing policy
308	      will be applied to it.

[major] s/then the IP address/if the IP address


[major] Just to confirm... If the policy applies to some (eBGP peers,
for example), but not all, then the sender would have to originate one
UPDATE per peer, right?  The Withdraw case would be similar.

If so, can the ASN List or IPv4 Prefix List Atom (or even the Neighbor
Class List) be used in the Targets TLV to narrow down the peers to use
(while using 0 as the Peer IP field)?

Operationally, when would it be better to use a specific Peer IP?
Please include this type of information in an Operational
Considerations section.



...
312	4.2.  BGP Wide Community and Atoms

314	   The BGP wide community attribute is defined in
315	   [I-D.ietf-idr-wide-bgp-communities].  This document specifies how two
316	   wide communities associate the routing policy NLRI to Routing Policy
317	   NLRI (section 4.1) to distribute routing policy to BGP peers.  The
318	   wide communities which define routing policy are:

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

Keep the capitalization in line with I-D.ietf-idr-wide-bgp-communities.


[minor] "BGP wide community attribute" -- it is not an attribute (just
take that word out).


[minor] "associate the routing policy NLRI to Routing Policy NLRI"

s/
   This document specifies how two wide communities associate the routing
   policy NLRI to Routing Policy NLRI (section 4.1) to distribute routing
   policy to BGP peers.  The wide communities which define routing policy
   are:
/
   This document specifies two BGP Wide Communities to distribute routing
   policy to BGP peers, as follows:


320	   *  MATCH AND SET ATTR (TBD1)

322	   *  MATCH and NOT ADVERTISE (TBD2)

324	   These wide communities are passed in the BGP wide community container
325	   in the wide community attribute.  These communities support three of
326	   the optional TLVs: Target TLV, Exclude Target TLV, and Parameter TLV.
327	   The value of each of these TLVs comprises a series of Atoms, each of
328	   which is a TLV (or sub-TLV).

[minor] The first sentence (which uses outdated terminology) is not
needed because it restates what I-D.ietf-idr-wide-bgp-communities
does.  Same for the last one.  Please keep the terminology in sync.

Suggestion>
   These BGP Wide Communities support all three TLVs defined in
   [I-D.ietf-idr-wide-bgp-communities]: Targets TLV, Exclude
   Targets TLV, and Parameters TLV.


[major] I-D.ietf-idr-wide-bgp-communities requires that new Wide
Community definitions specify which TLVs and Atoms area applicable.
Please do so.



330	   A new wide community Atom is defined for BGP Wide Community Target(s)
331	   TLV (RouteAttr), and two new Atoms are defined for BGP Wide Community
332	   Parameter(s) TLV.  For your reference, the format of the TLV is
333	   illustrated below:

[minor] The format is not necessary here as it is repeated later.
Also, please point at the corrresponding sections.

Suggestion>
   A new Atom is defined for the Targets TLV (Section xxx), and
   two new Atoms for the Parameters TLV (Section yyy).



[] Please reorder the specification sections so that the Community
Values are defined first, then then the Atoms.  For example:

   4.2.1.    BGP Wide Communities
   4.2.2.    Atoms
   4.2.2.1.  RouteAttr
   4.2.2.2.  MED Change
   4.2.2.3.  AS-Path Change

Note also that this section (4.2) is just an introduction to the
subsections (4.2.*).  You could also move the text to §4 and eliminate
one of the levels:

   4.2.    BGP Wide Communities
   4.3.    Atoms
   4.3.1.  RouteAttr
   4.3.2.  MED Change
   4.3.3.  AS-Path Change

[See the comments later on about the RouteAttr TLVs and how they
should be defined as Atoms too.]



...
344	4.2.1.  RouteAttr atom Sub-TLV

[major] Please don't mix Atoms and Sub-TLVs.  While Atoms are carried
in TLVs, I-D.ietf-idr-wide-bgp-communities is clear to call them Atoms
(and not Sub-TLVs).

s/RouteAttr atom Sub-TLV/Atom Type TBD3, The Route Attributes (RouteAttr)

...to be in line with I-D.ietf-idr-wide-bgp-communities.  Please use a
similar title for the other sections defining Atoms.



346	   A RouteAttr Atom sub-TLV (or RouteAttr sub-TLV for short) is defined
347	   and may be included in a Target TLV.  It has the following format.

[major] "is defined and may be included in a Target TLV."

There should be two parts to the specification: a definition of what
the Atom is (this section), and a statement on its applicability to
specific Wide Communities.  This last part should be specified in the
description of the Wide Communities (not in the description of the
Atom or the TLV).

IOW, the specification of the Atom should be general while the Wide
Community species whether it is defined to be used with it, and how.
Please indicate what the RouteAttr is used for (to describe the
attributes of a route or group of routes).



349	      0                   1                   2                   3
350	      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
351	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
352	     |  Type (TBD3)  |        Length (variable)        |
353	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
354	     |                         sub-sub-TLVs                          ~
355	     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

357	                 Figure 5: Format of RouteAttr Atom sub-TLV

[minor] For consistency, please use the same format as in
§5/I-D.ietf-idr-wide-bgp-communities.



359	   The Type for RouteAttr atom is TBD3.  In RouteAttr sub-TLV, four sub-
360	   sub-TLVs are defined: IPv4 Prefix, IPv6 Prefix, AS-Path, and
361	   Community sub-sub-TLV.

[minor] Please separate the second sentence into its own paragraph.


[minor] Because RouteAttr is not a sub-TLV, please use sub-TLV
(instead of sub-sub-TLV) -- or just TLV.


[minor] Consider describing each sub-TLV in a separate section.



363	   An IP prefix sub-sub-TLV gives matching criteria on IPv4 prefixes.
364	   Its format is illustrated below:

[major] "matching criteria"

When used with the Wide Communities specified in this document, this
sub-TLV in the RouteAttr Atom is used to match.  However, the general
use is to describe the IPv4 prefixes associated with a route or group
of routes.

Note that the use (in the context of this document) should be
described when the Wide Community is defined.

[] Suggestion>
   The IPv4 Prefix sub-TLV describes the IPv4 prefixes associated with
   a route (or group of routes).


[major] RouteAttr is the first Atom to have sub-TLVs.  Please indicate
the general format before defining the first one.



...
386	   Length:  N x 8, where N is the number of tuples <M-Type, Flags, IPv4
387	      Address, Mask, GeMask, LeMask>.  If Length is not a multiple of 8,
388	      the Atom is corrupt and the enclosing UPDATE message MUST be
389	      ignored.

[minor] Move the tuple description to the definition of this sub-TLV
to indicate that it carries a list of "IPv4 prefix descriptions".


[major] This sub-TLV contains a list of route descriptions.  What does
it mean to have more than one?  Note that
I-D.ietf-idr-wide-bgp-communities defines the semantics for Atoms of
List type (which is why I'm suggesting that you describe this sub-TLV
as a list) -- does that apply?



391	   M-Type:  4-bit field specifying match type.  The following four
392	      values are defined.  IPaddress is the IP address in the sub-sub-
393	      TLV while IProute is the IP route being matched.

[nit] s/specifying match type/specifying the match type


[major] "IProute is the IP route being matched"

In general, this sub-TLV describes a route.  The description can then
be used (with the Wide Communities defined here) to compare (and
match) according to the function.  Please specify the sub-TLV in terms
of what it describes -- the Wide Community specification should define
how each TLV, Atom, etc. is used with it.

Suggestion>
   M-Type:  4-bit field specifying the route format type.  The values are
   specified below.

[See other suggestions below.]


[] For some reason rfc8955 is mentioned in the Security Considerations
section.  This reminded me that rfc8955 already defines a numeric-op
that could have been reused in this case (without having to redefine
the logic).  Was this considered?


[] Move the definition of the M-types to *after* all the fields have
been introduced, and before the example.


395	      M-Type = 0:  Exact match with the Mask length IP address prefix.
396	         GeMask and LeMask MUST be sent as zero and ignored on receipt.

[] Suggestion>
   M-Type = 0:  The route described corresponds to the IP Address
      with the specified Mask length.  GeMask and LeMask MUST be
      sent as zero and ignored on receipt.


[] Type 0 is equivalent to using the IP Prefix List Atoms.


398	      M-Type = 1:  Matches if the Mask number of prefix bits exactly
399	         match between IPaddress and IProute and the actual prefix
400	         length of IProute is greater than or equal to GeMask.  LeMask
401	         MUST be sent as zero and ignored on receipt.

[] Suggestion>
   M-Type = 1:  Describes a set of routes that correspond to the
      prefix represented by the IP Address/Mask combination and a
      mask greater than or equal to GeMask.  LeMask MUST be sent
      as zero and ignored on receipt.


403	      M-Type = 2:  Matches if the Mask number of prefix bits exactly
404	         match between IPaddress and IProute and the actual prefix
405	         length of IProute is less than or equal to LeMask.  GeMask MUST
406	         be sent as zero and ignored on receipt.

[] Suggestion>
   M-Type = 2:  Describes a set of routes that correspond to the
      prefix represented by the IP Address/Mask combination and a
      mask less than or equal to LeMask.  GeMask MUST be sent
      as zero and ignored on receipt.


408	      M-Type = 3:  Matches if the Mask number of prefix bits exactly
409	         match between IPaddress and IProute and the actual prefix
410	         length of IProute is less than or equal to LeMask and greater
411	         than or equal to GeMask.

[] Suggestion>
   M-Type = 3:  Describes a set of routes that correspond to the
      prefix represented by the IP Address/Mask combination and a
      mask greater than or equal to GeMask and less than or equal
      to LeMask.



413	   Flags:  4 bits.  No flags are currently defined.  They MUST be sent
414	      as zero and ignored on receipt.

[major] Do you expect IANA to manage the assignments of any future
flags?  Or would a better name for this field be "Reserved"?



416	   IPv4 Address:  4 octets for an IPv4 address.

[] See the comment below about the NLRI encoding.

Suggestion>
  IPv4 Address:  4 octets that describe an IPv4 prefix.  This field,
     together with the Mask follow the same semantics as the NLRI
     encoding from [rfc4271], except that the trailing bits in the
     IP Address fill the 4-octet field.



418	   Mask:  1 octet for the IP address prefix length that needs to exactly
419	      match between the IP address in the sub-sub-TLV and the route.

[] It would be better (an in line with rfc4271 and
I-D.ietf-idr-wide-bgp-communities) if the Mask was replaced with
"Prefix Length" (just as in the NLRI encoding).  Then the explanations
can be simplifier.

This adds the ability to indicate "all IP addresses" with a Mask of 0.

Suggestion>
   Mask:  1 octet field that represents the Prefix Length of the
      IP Address, as specified in §4.3/rfc4271.



421	   GeMask:  1 octet for route prefix length match range's lower bound,
422	      MUST not be less than Mask or be 0.

[major] "MUST not" is not an rfc2119 keyword


[] Suggestion>
   GeMask:  1-octet field that represents the lowest bound of
      the IP Address' mask.  This field MUST be greater than,
      or equal to, the Mask, or be 0.


[major] What should a receiver do if the value is not set correctly?



424	   LeMask:  1 octet for route prefix length match range's upper bound,
425	      MUST be greater than Mask or be 0.

[] Suggestion>
   LeMask:  1-octet field that represents the upper bound of
      the IP Address' mask.  This field MUST be greater than,
      or equal to, the Mask, or be 0.


[major] What should a receiver do if the value is not set correctly?


[major] In the examples, use the documentation addresses from rfc6890.

427	   For example, tuple <M-Type=0, Flags=0, IPv4 Address = 1.1.0.0, Mask =
428	   22, GeMask = 0, LeMask = 0> represents an exact IP prefix match for
429	   1.1.0.0/22.

[minor] s/represents an exact IP prefix match for
1.1.0.0/22./represents 1.1.0.0/22.


431	   <M-Type=1, Flags=0, IPv4 Address = 16.1.0.0, Mask = 24, GeMask = 24,
432	   LeMask = 0> represents match IP prefix 16.1.0.0/24 greater-equal 24
433	   (i.e., route matches if route's first Mask=24 bits match 16.1.0 and
434	   24 =< route's prefix length =< 32).

[minor] s/represents.../represents the set of routes that include
16.1.0.0/24 with a prefix length greater than, or equal to, 24 bits
(up to and including 32 bits).


436	   <M-Type=2, Flags=0, IPv4 Address = 17.1.0.0, Mask = 24, GeMask = 0,
437	   LeMask = 26> represents match IP prefix 17.1.0.0/24 less-equal 26
438	   (i.e., route matches if route's first Mask=24 bits match 17.1.0 and
439	   24 =< route's prefix length <= 26).

[minor] s/represents.../represents the set of routes that include
17.1.0.0/24 with a prefix length less than, or equal to, 26 bits (up
to and including 24 bits).


441	   <M-Type=3, Flags=0, IPv4 Address = 18.1.0.0, Mask = 24, GeMask = 24,
442	   LeMask = 30> represents match IP prefix 18.1.0.0/24 greater-equal 24
443	   and less-equal 30 (i.e., route matches if route's first Mask=24 bits
444	   match 18.1.0 and 24 =< route's prefix length <= 30).

[minor] s/represents.../represents the set of routes that include
18.1.0.0/24 with a prefix length greater, or equal to, 24 bits, and
less than, or equal to, 30 bits.


[] As mentioned before, consider describing each sub-TLV (Atom) in a
separate section.



446	   Similarly, an IPv6 Prefix sub-sub-TLV represents match criteria on
447	   IPv6 prefixes.  Its format is illustrated below:
...
465	                Figure 7: Format of IPv6 Prefix sub-sub-TLV

[] Add: The fields are described as in Section xxx.


[] It would be very nice if a corresponding example for IPv6 was presented.



...
486	   AS-Path Regex String:  AS-Path regular expression string.

[major] Where are the regular expressions defined?  What format are
they represented in?



488	   A community sub-sub-TLV represents a list of communities to be
489	   matched all.  Its format is illustrated below:

[major] "a list of communities"

Please be clear (and include a reference) about which type of
communities you're talking about.  Why only those?



...
514	4.2.2.  Sub-TLVs of the Parameters TLV

516	   This document introduces 2 community values:

[major] The specification of the new Community Values is hidden back
here. :-(  As suggested above, put this in its own section.


518	   MATCH AND SET ATTR (TBD1):  If the IPv4/IPv6 unicast routes to a
519	      remote peer match the specific conditions defined in the routing
520	      policy extracted from the RPD route, then the attributes of the
521	      IPv4/IPv6 unicast routes will be modified when sending to the
522	      remote peer per the actions defined in the RPD route.

[] Suggestion>
   MATCH AND SET ATTR (TBD1):  This BGP Wide Community is used to
   advertise a set of policy conditions and attribute modifications.
   If routes match the conditions, their attributes are modified.


524	   MATCH AND NOT ADVERTISE (TBD2):  If the IPv4/IPv6 unicast routes to a
525	      remote peer match the specific conditions defined in the routing
526	      policy extracted from the RPD route, then the IPv4/IPv6 unicast
527	      routes will not be advertised to the remote peer.

[] Suggestion>
   MATCH AND NOT ADVERTISE (TBD2):  This BGP Wide Community is
   used to advertise a set of policy conditions that result in
   the advertisement suppression.  If routes match the conditions,
   they will not be advertised.


[major] In general, the text above is just a description, not a
specification of the new Wide Communities.  Among other things, a
specification should indicate how the operation is achieved -- as
required by [I-D.ietf-idr-wide-bgp-communities], a statement
indicating which TLVs and Atoms can be used and their semantics.



529	   For the Parameter(s) TLV, two action sub-TLVs are defined: MED change
530	   sub-TLV and AS-Path change sub-TLV.  When the community in the
531	   container is MATCH AND SET ATTR, the Parameter(s) TLV can include
532	   these sub-TLVs.  When the community is MATCH AND NOT ADVERTISE, the
533	   Parameter(s) TLV's value is empty.

[major] "For the Parameter(s) TLV, two action sub-TLVs are defined"

There should be two parts to the specification: a definition of what
the Atoms are (this section), and a statement on its applicability to
specific Wide Communities.  This last part should be specified in the
description of the Wide Communities (not in the description of the
Atom or the TLV).

IOW, the specification of the Atom should be general while the Wide
Community species whether it is defined to be used with it, and how.



535	   A MED change sub-TLV indicates an action to change the MED.  Its
536	   format is illustrated below:

[minor] Expand MED on first use.  Note that the attribute is called
MULTI_EXIT_DISC in rfc4271 -- please use existing terminology.


[major] Note that this Atom has a specific action in mind for the
application defined in this document, but it can be generalized to an
Atom that adds/subtracts a value.  A Wide Community can then specify
how to use it in its context.

[In fact, the Unsinged Integer32 List Atom could be used for the same
purpose -- with Wide Community-specific semantics.  The first value
indicates addition, the second subtraction, one must be 0, etc...]



538	     0                   1                   2                   3
539	     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
540	    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
541	    |  Type  1      |          Length (5)           |      OP       |
542	    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
543	    |                           Value                               |
544	    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

546	                  Figure 10: Format of MED Change sub-TLV

[major] Please represent this new Atom according to the format in
§5/draft-ietf-idr-wide-bgp-communities.



548	   Type:  1 for MED Change.

[major] You are defining a new Atom.  This value needs to come from
the Atom Types registry, and not from the registry defined in the IANA
section.



...
553	   OP:  1 octet.  Three are defined:

[major] Does "OP" have a meaning?  Is it a bit-field or a field
carrying a number?


[minor] s/Three are defined/Three values are defined


[minor] Consider moving the description of the values to *after* all
the fields are defined.  As it is now, the Value is used in the
operation of OP before it is defined.


555	      OP = 0:  assign the Value to the existing MED.

[major] "assign the Value to the existing MED"

If the MED exists, the action is to replace.  What if the MED
attribute doesn't exist?


557	      OP = 1:  add the Value to the existing MED.  If the sum is greater
558	         than the maximum value for MED, assign the maximum value to
559	         MED.

[major] "add the Value to the existing MED"

Again, what if the MED attribute doesn't exist?


[minor] s/If the sum is greater than the maximum value for MED, assign
the maximum value to MED./If the sum is greater than the maximum
allowed value, use that maximum value instead.


561	      OP = 2:  subtract the Value from the existing MED.  If the
562	         existing MED minus the Value is less than 0, assign 0 to MED.

[major] "subtract the Value from the existing MED"

Again, what if the MED attribute doesn't exist?


[minor] s/If the existing MED minus the Value is less than 0, assign 0
to MED./If the result is less than 0, use 0 instead.


564	      If OP is any other value, the sub-TLV is ignored.

[major] Other error conditions above resulted in discarding the whole
UPDATE.  Why is this different?  What is the effect of ignoring vs
discarding?



566	   Value:  4 octets.

[major] What is the format of this information?



568	   An AS-Path change sub-TLV indicates an action to change the AS-Path.
569	   Its format is illustrated below:

[major] rfc4271 uses AS_PATH to refer to the attribute.  Please be consistent.



...
589	   Type:  2 for AS-Path Change.

[major] You are defining a new Atom.  This value needs to come from
the Atom Types registry, and not from the registry defined in the IANA
section.



591	   Length:  n x 5.  If Length is not a multiple of 5, the sub-TLV is
592	      corrupt and the enclosing UPDATE MUST be ignored.

[major] This Atom contains a list of ASN + count.  What does it mean
to have more than one?  Note that I-D.ietf-idr-wide-bgp-communities
defines the semantics for Atoms of List type -- does that apply?



594	   ASi:  4 octet.  An AS number.

596	   Counti:  1 octet.  ASi repeats Counti times.

[minor] Please desribe this Atom as a list of ASN/Count units and then
define one of those units -- using AS/Count (not ASi/Counti).



598	   The sequence of AS numbers are added to the existing AS Path.

[major] "sequence of AS numbers are added to the existing AS Path"

This Atom seems to want to include a set of ASNs to prepend the
AS_PATH with.  Besides the local ASN, why would other ASNs be
prepended to a route?  In general, prepending an ASN other than the
local one seems to be an act directly related to forging the AS_PATH.

How would adding ASNs other than the local one work with BGPSec?

What if the route doesn't have an AS_PATH?


[] The example in I-D.ietf-idr-wide-bgp-communities manages to prepend
the local ASN without the need for an extra Atom.  Why is a new
mechanism needed?



600	4.3.  Capability Negotiation

[major] rfc4760 already defines Capability Negotiation for multiple
<AFI,SAFI> pairs.  Why isn't that enough?



...
627	   Send/Receive: This field indicates whether the sender is (a) willing
628	   to receive Routing Policies from its peer (value 1), (b) would like
629	   to send Routing Policies to its peer (value 2), or (c) both (value 3)
630	   for the <AFI, SAFI>.  If Send/Receive is any other value, that tuple
631	   is ignored but any other tuples present are still used.

[major] As mentioned before, rfc4760 already defines Capability
Negotiation for this case, including support for unidirectional
advertisement (when only one side advertises the capability).  Why
isn't that enough?



...
638	5.1.  Application Scenario

[] I think that this section would fit better closer to the Problem
Statement to show the way this extension can be used.



...
662	   The controller connects the RR through a BGP session.  There is a BGP
663	   session between the RR and each of routers A, B and C in AS1, which
664	   is shown in the figure.  Other sessions in AS1 are not shown in the
665	   figure.

[nit] s/connects the RR through a BGP session/peers with the RR using
a BGP session


[nit] s/is shown/are shown



667	   There is router X in AS2.  There is a BGP session between X and each
668	   of routers A, B and C in AS1.

[nit] s/There is router X in AS2./Router X is in AS2.



670	   There is router Y in AS3.  There is a BGP session between Y and
671	   router C in AS1.

[nit] s/There is router Y in AS3./Router Y is in AS3.



673	   The controller sends a RPD route to the RR.  After receiving the RPD
674	   route from the controller, the RR reflects the RPD route to routers
675	   A, B and C.  After receiving the RPD route from the RR, routers A, B
676	   and C extract the routing policy from the RPD route.  If the peer IP
677	   in the NLRI of the RPD route is 0, then apply the routing policy to
678	   all the remote peers of routers A, B and C.  If the peer IP in the
679	   NLRI of the RPD route is non-zero, then the IP address indicates a
680	   remote peer of routers A, B and C and such routing policy is applied
681	   to the specific remote peer.  The IPv4/IPv6 unicast routes towards
682	   router X in AS2 and router Y in AS3 will be adjusted based on the
683	   routing policy sent by the controller via a RPD route.

[nit] s/to the RR.  After receiving the RPD route from the controller,
the RR reflects the RPD route to routers A, B and C./to the RR which
reflects it to routers A, B and C.


[minor] "If the peer IP in the NLRI..."

Because this is an example, it would be nice if it demonstrated how
the routes could be "adjusted based on the routing policy sent by the
controller via a RPD route", instead of just saying so.  What are the
possible applications?

For example, look at the example in
draft-ietf-idr-wide-bgp-communities; it presents what an operator may
want to do and then shows how.



685	   The controller uses the RT extend community to notify a router
686	   whether to receive a RPD policy.  For example, if there is not any
687	   adjustment on router B, the controller sends RPD routes with the RTs
688	   for A and C.  B will not receive the routes.

[minor] Using the RT is not specifically part of this specification --
and in this context part of the example.  Even so, it feels out of
place.  This type of operational consideration should go into it's own
section in the form of a recommendation to operators to use a
mechanism to allow the selective application of policy (when not
applying it directly -- as in this case with a RR).  The RT mechanism
would be an example -- it needs an Informative reference.

Also, the Peer IP (if not 0) should provide some type of filtering as
well: if the Peer IP address doesn't represent any of the local peers
then the policy doesn't apply to the local router...



690	   The process of adjusting traffic in a network is a close loop.  The
691	   loop starts from the controller with some traffic expectations on a
692	   set of routes.  The controller obtains the information about traffic
693	   flows for the related routes.  It analyzes the traffic and checks
694	   whether the current traffic flows meet the expectations.  If the
695	   expectations are not met, the controller adjusts the traffic.  And
696	   then the loop goes to the starter of the loop (The controller obtains
697	   the information about traffic ...).

[] Nice, but not part of this specification.  If you want to leave it
as part of an operational considerations section, please illustrate
with examples how the monitoring can be done, etc..



699	5.2.  About Failure

701	   This section describes some details about handling a failure related
702	   to a RPD route being applied.

[nit] s/handling a failure/handling failure

I haven't read the rest of this section yet -- but I'm hopping it
talks about failure in general.



704	   A RPD route is not a configuration.  When it is sent to a router from
705	   a controller, no ack is needed from the router.  The existing BGP
706	   mechanisms are re-used for delivering a RPD route.  After the route
707	   is delivered to a router, it will be successful.  This is guaranteed
708	   by the BGP protocols.

[minor] I'm not sure what not being "a configuration" (??) has to do
with a reliable transport.


[major] "After the route is delivered to a router, it will be successful."

What do you mean by the route being "successful"?  If you're talking
about the transport, then there's no need for most of this paragraph
as everyone working with BGP knows that it is reliable.

But if you're referring to the application of the policy being
successful, that is a very different thing!  How would the sender know
this?  In fact, how does the sender know the initial (or current)
policy state?  Among other things, it is possible for the routes to
have changed by the time the RPD policy arrives...

>From the text above about monitoring, my guess is that the current
policy is inferred from the observation of the traffic.  Is that
right?  If so, prepending an already prepended route (for example) may
not have any effect on the traffic.  How can the sender tell the
difference between a failure in the application of the policy vs an
ineffective policy?



710	   If there is a failure for the router to install the route locally,
711	   this failure is a bug of the router.  The bug needs to be fixed.

[major] "install the route locally"

In the context of RPD, what does this mean?



713	   For the errors mentioned in [RFC7606], they are handled according to
714	   [RFC7606].  These errors are bugs, which need to be resolved.

[major] rfc7606 applies to all BGP UPDATES, so it is not necessary to
mention it here unless there are exceptions.  Or if you want to add
other processing.  In this case, you should point to the additional
error handling in draft-ietf-idr-wide-bgp-communities.   Even if it
also assumed.


[major] "These errors are bugs, which need to be resolved."

While most of the errors may be bugs, rfc7606 doesn't explicitly
categorize them that way.  Let's not do it here either.



716	   When the controller fails while a RPD route is being applied such as
717	   on the way to the router, some existing mechanisms such BGP Graceful
718	   Restart (GR) [RFC4724] and BGP Long-lived Graceful Restart (LLGR) can
719	   be used to let the router keep the routes from the controller for
720	   some time.

722	   With support of "Long-lived Graceful Restart Capability"
723	   [I-D.ietf-idr-long-lived-gr], the routes can be retained for a longer
724	   time after the controller fails.

726	   After the controller recovers from its failure, the router will have
727	   all the routes (including the RPD route being applied) from the
728	   controller.

[minor] For these 3 paragraphs...are you recommending that LLGR (or at
least GR) be used?  Should it be Normative?


[minor] This text should limit itself to describe what the normal
operation is (controller fails, then the route is withdrawn), and
maybe suggest mitigation.  This is in line with operational
considerations.


[major] BGP's operation is such that routes are withdrawn when a
session goes down -- the same would happen here, resulting in the
removal of policy.  The mechanisms mentioned above need to be
configured everywhere in the network to even start a conversation
about persistence.  What is the effect of non-persistent policies?
Either because of normal BGP operation, mis-configuration or partial
deployment of the the features mentioned above, filtering...  More for
an Operational Considerations section.



730	   In the worst case, the controller fails and the RPD routes for
731	   adjusting the traffic are withdrawn.  The traffic adjusted/redirected
732	   may take its old path.  This should be acceptable.

[] "This should be acceptable."

..??


[major] This draft specifies a way to match/identify routes and then
as-path-prepend and/or change the MED to influence incoming traffic.
It is well known that while these mechanisms may work, the unknown
nature of the neighboring AS's policy renders them useless in many
situations.  Specifically, if LOCAL_PREF is used then none of this is
useful.  See rfc1998 and draft-ietf-grow-as-path-prepending.  Please
include some text (maybe in an operational considerations section)
about the issues.

[BTW, I know there was a discussion about not referencing
draft-ietf-grow-as-path-prepending.  That is fine.  But the point that
this document recommends prepending and that there are issues with it
remains.]



734	6.  Contributors

[minor] Please see §4/rfc7322 on the recommended order of the sections
in an RFC and reorder acoordingly.



736	   The following people have substantially contributed to the definition
737	   of the BGP-FS RPD and to the editing of this document:

[] "BGP-FS RPD"   ???



...
743	7.  Security Considerations

745	   Protocol extensions defined in this document do not affect BGP
746	   security other than as discussed in the Security Considerations
747	   section of [RFC8955].

[major] What?!

What is the relationship between this work and rfc8955?


[major] In writing a proper Security Consideration section you should
consider which other considerations apply.  In this case,
draft-ietf-idr-wide-bgp-communities is a clear requirement.


[major] You should also consider the effect of the the new
functionality.  In this case, from changing the attributes in existing
routes, to attempting to influence the routing policy in neighboring
ASes, etc..   Some of the effects may be pre-existing -- at least show
that you through about them!!


[major] Using the mechanisms in this document, any router that can
successfully negotiate the new AFI/SAFI can change the policies.  In
the sample scenarios presented, that would be all the routers in the
network.  What prevents a (rogue) router from advertising an
incorrect/undesired policy change?  What are the potential effects of
that?  Or major concern is the use of the MATCH AND NOT ADVERTISE Wide
Community.



...
756	9.  IANA Considerations

758	9.1.  Existing Assignments

760	   IANA has assigned an AFI of value 16398 from the registry "Address
761	   Family Numbers" for Routing Policy.

[major] This section uses "Routing Policy", the registry and §4.1 use
"Routing Policy AFI".  Which one do you want to use?  Please be
consistent!


763	   IANA has assigned a SAFI of value 75 from the registry "Subsequent
764	   Address Family Identifiers (SAFI) Parameters" for Routing Policy.

[major] The registry says "Routing Policy SAFI", this section "Routing
Policy", and §4.1 nothing.   Please be consistent!

766	   IANA has assigned a Code Point of value 72 from the registry
767	   "Capability Codes" for Routing Policy Distribution.



769	9.2.  Registered IANA Wide Communities

771	   IANA Should assign from the Registered Wide Community Values" the
772	   following values:

[major] The name of the registry is "Registered Type 1 BGP Wide
Community Community Types".  But please keep an eye on
draft-ietf-idr-wide-bgp-communities to make sure this is the final
name.



...
782	9.3.  RouteAttr Atom Type

784	   IANA is requested to assign a code-point from the registry "BGP
785	   Community Container Atom Types" as follows:

787	    +---------------------+------------------------------+-------------+
788	    | Atom Code Point     | Description                  | Reference   |
789	    +---------------------+------------------------------+-------------+
790	    | TBD3 (48 suggested) | RouteAttr Atom               |This document|
791	    +---------------------+------------------------------+-------------+

[major] The values in this registry are expressed in hex. Do you mean 0x48?



793	9.4.  Route Attributes Sub-sub-TLV Registry

[] As mentioned before, these sub-TLVs should be defined as Atoms.

795	   IANA is requested to create a registry called "Route Attributes Sub-
796	   sub-TLV" under RouteAttr Atom Sub-TLV.  The allocation policy of this
797	   registry is "First Come First Served (FCFS)".

[major] Where should this registry be located?


[major] Please add a Normative reference to rfc8126 when referring to
Registration Policies.



...
817	9.5.  Attribute Change Sub-TLV Registry

819	   IANA is requested to create a registry called "Attribute Change Sub-
820	   TLV" under Parameter(s) TLV.  The allocation policy of this registry
821	   is "First Come First Served (FCFS)".

[major] This registry is not needed.  See comments above.



...
877	10.2.  Informative References
...
886	   [I-D.ietf-idr-registered-wide-bgp-communities]
887	              Raszuk, R. and J. Haas, "Registered Wide BGP Community
888	              Values", Work in Progress, Internet-Draft, draft-ietf-idr-
889	              registered-wide-bgp-communities-02, 31 May 2016,
890	              <https://www.ietf.org/archive/id/draft-ietf-idr-
891	              registered-wide-bgp-communities-02.txt>.

== Unused Reference: 'I-D.ietf-idr-registered-wide-bgp-communities' is
   defined on line 886, but no explicit reference was found in the text


...
898	   [RFC7606]  Chen, E., Ed., Scudder, J., Ed., Mohapatra, P., and K.
899	              Patel, "Revised Error Handling for BGP UPDATE Messages",
900	              RFC 7606, DOI 10.17487/RFC7606, August 2015,
901	              <https://www.rfc-editor.org/info/rfc7606>.

[major] This reference should be Normative.

[EoR -15]