[GROW] Benjamin Kaduk's No Objection on draft-ietf-grow-bmp-local-rib-10: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-grow-bmp-local-rib-10: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-grow-bmp-local-rib/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I was going to call out a couple of my remarks, but the particularly important
ones seem to already be covered by Alvaro's Discuss, which I consequently support.

I also proposed some editorial suggestions in a github PR:
https://github.com/TimEvens/draft-ietf-grow-bmp-loc-rib/pull/23

Section 1

Please expand SPF and CSPF on first use
(https://www.rfc-editor.org/materials/abbrev.expansion.txt seems to only
mark "OSPF" as well-known and not in need of expansion).

      For example, the received Adj-RIB-In will be different if add-
      paths is not enabled or if maximum number of equal paths are
      different from Loc-RIB to routes advertised.

Where is "add-paths" defined?

Section 1.1

   monitored.  As shown in Figure 3, the target router Loc-RIB is
   advertised via Adj-RIB-Out to the BMP router over a standard BGP

Where is "the BMP router" defined/indicated?  From context I assume it
is "a router inserted into the BGP network for the (sole?) purpose of
re-exporting via BMP a data stream".  Perhaps it is simpler to avoid the
new term and just say "to another BGP-speaking router" and "That router
then forwards"?

      version information).  In order to derive a Loc-RIB to a router,
      the router name or other system information is needed.  The BMP

(nit): is there a missing word here around "to a router"?  ("entry"?
s/to/for/?)
I'm having a hard time parsing this sentence so I can't just make a
suggestion in my editorial PR.

Section 3

   *  Post-Policy Adj-RIB-Out: The result of applying outbound policy to
      an Adj-RIB-Out. This MUST be what is actually sent to the peer.

My memory may be failing me, but I think I remember some discussion of
this (MUST-level) requirement on Post-Policy Adj-RIB-Out some years ago,
presumably in the context of a different document.  Is this a
preexisting requirement that we could refer to, or a new requirement
being imposed here?

Section 4.1

   A new peer type is defined for Loc-RIB to distinguish that it
   represents Loc-RIB with or without RD and local instances.

I think I'm confused by the line about "with or without RD and local
instances".  Is there a choice made by the sender?  How would the
recipient know which choice was made?
Continuing on to read Section 5.1, it seems that the "peer
distinguisher" field in the per-peer header can be used by the recipient
to differentiate the cases, so perhaps it is more clear to say here
"[...] represents Loc-RIB.  Its representation can differentiate between
instances with and without RD, and use a unique local value to indicate
local instances."?

Section 4.2

   In section 4.2 of [RFC7854], the "locally sourced routes" comment
   under the L flag description is removed.  If locally sourced routes

We might want to mention this focused update to RFC 7854 in the
introduction alongisde of the mention that Section 8.2 of RFC 7854 is
completely replaced.

   are communicated using BMP, they MUST be conveyed using the Loc-RIB
   instance peer type.

It seems like this might be a breaking change during incremental
deployment, as a BMP receiver might lose a data stream it was otherwise
receiving until it gets a software update.  It seems like at a minimum
this should get called out in the "other considerations" section, and
maybe a more graceful transition procedure defined/allowed.

Section 5.x

We don't cover the Initiation/Termination messages from RFC 7854, but do
seem to have at least brief mentions of the others.  This is probably
fine, but I just wanted to confirm that the processing of those messages
is unchanged for the Loc-RIB case compared to the existing cases.

Section 5.1

   *  Peer Address: Zero-filled.  Remote peer address is not applicable.
      The V flag is not applicable with Loc-RIB Instance peer type
      considering addresses are zero-filed.

Since we split the flags off into being per-peer-type, there no longer
is a V flag defined and we may not need to have text about its
irrelevance.

Section 5.2.1

   *  Type = 3: VRF/Table Name.  The Information field contains a UTF-8
      string whose value MUST be equal to the value of the VRF or table

It is hopefully implicit, but I'd suggest repeating the language from RFC
7854 that "[t]here is no requirement to terminate the string with a null
(or any other particular) character" to avoid any doubt.  (Likewise in
Section 5.3.)

Section 5.3

   Peer down notification MUST use reason code 6.  Following the reason

Should we forward-reference Section 8.4 where we allocate that value?

Section 7

   The same considerations as in section 11 of [RFC7854] apply to this
   document.  Implementations of this protocol SHOULD require to

(side note) I think the current security considerations text is
appropriate for this document, but noticed that RFC 7854 recommended
IPsec in tunnel mode (vs. transport mode), which was a little surprising
to me.  That said, changing the recommendation now just for Loc-RIB
would be disruptive for negligible benefit, so this is just a side note
and no change is recommended.

Section 8.2

I suspect that we want to create a new sub-registry for the peer-type-3
flags.

   This document defines a new flag (Section 4.2) and proposes that peer
   flags are specific to the peer type:

I'd suggest not using the word "proposes" anymore; we're basically at
the approval stage and it's more like a "going to happen" thing than a
"proposal".

Section 8.3

Is there actually an existing registry for PEER UP informational message
TLV types?  I don't see anything listed at
https://www.iana.org/assignments/bmp-parameters/bmp-parameters.xhtml
that matches up to that very well.