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

[John Scudder via Datatracker <[e-mail]>]

John Scudder 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 agree with Alvaro’s comments. Some additional of my own:

1. I appreciate that section 1.1, "Alternative Method to Monitor Loc-RIB” was
necessary to guide WG discussion (not to mention as a way of illustrating the
problem to shortsighted authors of the base specification) but in the RFC, it
will be clutter for the implementor to skip over. IMO it would be kind of you
to move it to an appendix (or even remove it entirely, if you wish).

2. "BGP Instance: refers to an instance of an instance of BGP-4”. Did you mean
the repetition of “instance of”? Or is this an instance of the

Paris in the
the spring

problem? (I assume it is.)

3. In this definition:

   *  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.

The MUST seems inappropriate, unless you are imposing a new requirement — and
if so, what are you imposing the requirement on? The concept of a “Post-Policy
Adj-RIB-Out” (which implies the existence of a Pre-Policy Adj-RIB-Out”, as you
also define) doesn’t even exist in RFC 4271 — the Adj-RIB-Out is post-policy by
definition.

If you were to remove section 1.1, you could also remove the definitions of
Adj-RIB-Out, and this whole problem would go away as if by magic. :-)

4. 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.

"Distinguish whether”?

5. 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
   are communicated using BMP, they MUST be conveyed using the Loc-RIB
   instance peer type.

Given that you aren’t bumping the version number or otherwise signaling that
this spec is in use, how is the collector expected to know this? A priori by
configuration? It seems as though a prudent collector implementation would
still have to support the RFC 7854 encoding as well. At first glance that seems
OK — the collector could interpret the L-bit and also the Loc-RIB Instance Peer
type. It would be nice to have some words about this.

Having written the above, I think the text I’ve quoted, per se, is fine — you
can mandate that the new encoding always be used. (And too bad for old
collectors that don’t understand the format, I guess.)

6. Section 5, nit:

   other protocols into BGP or otherwise locally originated (ie.
   aggregate routes).

If you mean “for example” it’s “e.g.” (but it’s clearer IMO to write “for
example”). If you mean “in other words”, it’s “i.e.” (with two periods, but
it’s clearer IMO to write “in other words”).

7. Section 5.1, nit:

   All peer messages that include a per-peer header section 4.2 of
   [RFC7854] MUST use the following values:

The way this text has rendered, the reference needs parentheses around it. Same
for this one:

   *  Peer BGP ID: Set to the BGP instance global or RD (e.g.  VRF)
      specific router-id section 1.1 of [RFC7854].

8. 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
      name (e.g.  RD instance name) being conveyed.  The string size
      MUST be within the range of 1 to 255 bytes.

I take it the empty string isn’t allowed, by design.

9. Section 5.2.1:

   Multiple TLVs of the same type can be repeated as part of the same
   message, for example to convey a filtered view of a VRF.  A BMP
   receiver should append multiple TLVs of the same type to a set in
   order to support alternate or additional names for the same peer.  If
   multiple strings are included, their ordering MUST be preserved when
   they are reported.

The way this is written, it’s not clear if you’re modifying the base spec to
permit multiple TLVs of the same arbitrary type, informatively stating a
property of the base spec, or saying this is how the VRF/Table Name TLV is to
be handled. Looking back over RFC 7854, I think it is probably the latter — you
want to say only how the VRF/Table Name TLV is used. In that case, I think you
should change “same type” to “this type”. The indent of the paragraph seems
wrong, too — shouldn’t it be part of the bullet text?

Also — “a set” of what?

10. Section 5.3:

   Peer down notification MUST use reason code 6.  Following the reason
   is data in TLV format.  The following peer Down information TLV type
   is defined:

“Peer Down” (so capitalized).

Also, if multiple names were provided with the Peer Up, do they have to be
provided identically with the Peer Down? As you’ve specced it right now, only a
single name may be carried with the Peer Down (or at least you don’t explicitly
permit multiple).

11. Section 5.4.2, “Granularity”:

You say the speaker SHOULD do state compression. The requirement is fine in the
abstract (in other words, I hope BMP implementations will do this) but it seems
out of place in a spec that’s specifically about Loc-RIB. Surely this
requirement is for BMP in general?

Speaking of requirements, you give this as a SHOULD. Under what circumstances
would you contemplate an implementation might not do this, and (assuming you
keep the section at all) can you be specific? An ideal way of doing this is to
add a MAY clause of the form “… although an implementation MAY forgo doing
state compression on February 29 of any year.” (This problem goes away if you
eliminate section 5.4.2 of course.)

12. Section 6.1.1.

I had a hard time following this section. Let me break it down:

   There MUST be multiple emulated peers for each Loc-RIB instance,

The plain English of this seems to be telling me that each Loc-RIB instance,
regardless of its attributes, MUST have at least two emulated peers
(“multiple”). That doesn’t make much sense to me.

   such
   as with VRFs.

I can’t discern what “such as with VRFs” means in this sentence.

   The BMP receiver identifies the Loc-RIB by the peer
   header distinguisher and BGP ID.  The BMP receiver uses the VRF/
   Table Name from the PEER UP information to associate a name to the
   Loc-RIB.

This part is saying that the table name is just for pretty printing. OK.

   In some implementations, it might be required to have more than one
   emulated peer for Loc-RIB to convey different address families for
   the same Loc-RIB.

OK. Presumably the unsaid part is that other implementations will convey all
address families using a single emulated peer?

By the way, what is the receiver supposed to do if the different emulated peers
in this situation, advertise different VRF/Table Names? You’ve already said
that the VRF/Table Name is a non-key value, so presumably even if the string is
different, they still map to the same Loc-RIB — but what is the Loc-RIB’s name?

   In this case, the peer distinguisher and BGP ID
   should be the same since it represents the same Loc-RIB instance.
   Each emulated peer instance MUST send a PEER UP with the OPEN message
   indicating the address family capabilities.

OK.

   A BMP receiver MUST
   process these capabilities to know which peer belongs to which
   address family.

Wouldn’t this information also be present in the (encapsulated) UPDATE
messages, which will each have an AFI/SAFI embedded?

13. Section 6.1.2:

   filtered.  If multiple filters are associated to the same Loc-RIB, a
   Table Name MUST be used in order to allow a BMP receiver to make the
   right associations.

I’m not sure what the implementor is supposed to do with the MUST. Is the point
here that if the router is supplying, for example, EBGP routes to one
monitoring station and IBGP routes to another, it should have a table name like
“EBGP-routes” associated with one and “IBGP-routes” with the other? That would
be better than nothing of course but absent additional configuration on both
ends, doesn’t do anything to allow the receiver to automatically “make the
right associations”, it’s just a string — do you intend that it be possible to
programmatically derive the string and/or derive meaning from the string? I’m
guessing not, and that you’ve deliberately left the formation of the string up
to the user. Do you intend for the implementation to enforce uniqueness of the
string (you don’t say this explicitly)?

14. Section 8.2:

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

I think you will need to give IANA more detailed instructions, since you are
asking them to reorganize the registry. Take a look at the existing Peer Flags
registry. It doesn’t have a column for peer type. How do you want them to
reorganize it to reflect your wishes?

15. Section 8, all subsections:

There are some other problems with the IANA section, but since you’ve already
gotten early allocations from IANA for all these code points, probably the most
straightforward way to fix them is to look at what’s actually registered, and
per subsection, say something like “IANA has temporarily registered 'VRF/Table
Name’ with value 3 in the 'BMP Initiation Message TLVs’ registry. IANA is
requested to make this registration permanent and update the reference to be
this document.”