Subject: [GROW] review of draft-ietf-grow-bmp-local-rib-07 (preparing for sheperd write-up)

From: Job Snijders <[email-masked]>
Date: Tue, 3 Nov 2020 06:13:27 +0000

Dear group, authors

As part of the sheperd write-up I am reviewing the
draft-ietf-grow-bmp-local-rib-07 Internet-Draft. Overall the document
looks good to me.

Please consider these notes as input from an individual working group
participant. The suggestions are editorial in nature, my focus on
readability and clarity.

Thank you for your consideration!

Kind regards,

Job

### note 1

Suggested rewording of Abstract:

NEW Abstract:
The BGP Monitoring Protocol (BMP) defines access to various Routing
Information Bases (RIBs). This document updates BMP (RFC 8671) by
adding access to the Local Routing Information Base (Loc-RIB), as
defined in RFC 4271. The Loc-RIB contains the routes that have been
selected by the local BGP speaker's Decision Process. 

### note 2

Throughout the document I would suggest changing the phrase
"Local-RIB" to "Loc-RIB".

### note 3

Perhaps the first sentence of the introduction reads better if changed
to the following:

NEW:
	This document defines a mechanism to monitor the BGP Loc-RIB state
	of remote BGP instances without the need to establish BGP peering
	sessions.

### note 4

I have trouble understanding the following:

	The BGP Monitoring Protocol (BMP) suggests that locally originated
	routes are locally sourced routes, such as redistributed or otherwise
	added routes to the BGP instance by the local router. It does not
	specify routes that are in the BGP instance Loc-RIB, such as routes
	after best-path selection.

### note 5

OLD:
Adj-RIBs-In Post-Policy may still contain hundreds of thousands of
routes per-peer but only a handful are selected and installed in
the Loc-RIB as part of the best-path selection.

NEW:
The Adj-RIB-In for a given peer Post-Policy may contain hundreds of
thousands of routes, with only a handful of routes selected and installed
in the Loc-RIB after best-path selection.

### note 6

Section 1.1. "Current method to Monitor Loc-RIB" probably needs to be
"Alternative method to monitor Loc-RIB"

s/current/alternative/

### note 7

In section 3, the following change hopefully clarifies that the Loc-RIB
as observed through BMP is a composite of potentially-to-be-redistributed-into-BGP-routes
and routes received from other peers. 

OLD:
It is further defined that the routes selected include locally originated
and routes from all peers.

NEW:
Note that the Loc-RIB state as monitored through BMP might also contain routes
imported from other routing protocols such as an IGP, or local static routes.

### note 8

section 5.3

Curious: why ASCII and not UTF-8 (of which ASCII is a subset)?

### note 9

Section 6.1 states "several methods to implement Loc-RIB efficiently"

is this the implementation of Loc-RIB in a BGP-4 speaker? Or the implementation
of BMP Loc-RIB monitoring?

---

From: Jeffrey Haas <[email-masked]>
Date: Tue, 3 Nov 2020 19:52:10 -0500

Job, 

I agree that utf-8 is more appropriate, but would remind you that a great deal more text about handling it would be needed. See prior discussion on RFC 8203. 

Jeff

> On Nov 3, 2020, at 1:29 AM, Job Snijders
> 
> ### note 8
> 
> section 5.3
>

---

From: "Tim Evens (tievens)" <[email-masked]>
Date: Wed, 4 Nov 2020 17:49:48 +0000

I really appreciate your review and comments. These are great. 

See responses inline marked [tievens].

You can see the "pending" changes via: 
* https://github.com/TimEvens/draft-ietf-grow-bmp-loc-rib/pull/19/files
* https://github.com/TimEvens/draft-ietf-grow-bmp-loc-rib/blob/rev-08/draft-ietf-grow-bmp-local-rib.txt

On 11/2/20, 10:14 PM, "GROW on behalf of Job Snijders" <[email-masked] on behalf of [email-masked]> wrote:

    Dear group, authors

    Kind regards,

    Job

    ### note 1

    NEW Abstract:

[tievens] Changes look good, but this draft doesn’t update 8671, it still updates 7854. 

    ### note 2

[tievens] Agree. The draft itself has local in the name.  Does it make sense to change
the draft name or keep it as is? 

    ### note 3

    NEW:

[tievens] Updated.

    ### note 4

[tievens] I can see that.  Changed to: 
    
	"BMP [RFC7854] does not define a method to send the BGP
	instance Loc-RIB.  It does define in section 8.2 of [RFC7854] locally
	originated routes, but these routes are defined as the routes
	originated into BGP.  For example, locally sourced routes that are
	redistributed."

    ### note 5

    OLD:

    NEW:

[tievens] Changed.

    ### note 6

    s/current/alternative/

[tievens] That works, but it's really the only method right now... hence current... it becomes
	alternative with this draft. (  Changed to alternative. 

    ### note 7

    OLD:

    NEW:

[tievens] Nice. changed. 

    ### note 8

    section 5.3

[tievens] To be honest, ascii so that it's printable and works with iproute2/etc...  Although, 
   from a BMP perspective UTF-8 is more flexible and wouldn't restrict system implementations.
   Changed to UTF-8. VRF names will be system specific in terms of syntax/data-type, but from 
   a BMP receiver standpoint, we can easily, and should, support UTF-8.  

    ### note 9

[tievens] It is the speaker implementation.  I've changed it to 
	"There are several methods for a BGP speaker to implement Loc-RIB efficiently."
	Is this more clear? 

    _______________________________________________
    GROW mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/grow

---

From: "Tim Evens (tievens)" <[email-masked]>
Date: Wed, 4 Nov 2020 18:01:14 +0000

Jeff, 

I've updated this to UTF-8. From a receiver point of view, we handle utf-8 generically without any special need to deserialize it.  I suggest we do not attempt to define structure and/or constraints around the VRF names as that would severely impact system implementations that are out-of-scope of this draft. 

How about if we add:

"The structure, syntax, and constraints for a VRF name are router specific."

??

For example:

	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.  The structure, syntax,
		and constraints for a VRF name are router specific.

Thanks,
Tim

On 11/3/20, 4:52 PM, "GROW on behalf of Jeffrey Haas" <[email-masked] on behalf of [email-masked]> wrote:

    Job, 

    Jeff

---

From: Job Snijders <[email-masked]>
Date: Fri, 13 Nov 2020 10:34:33 +0000

Dear Tim,

Hi Tim,

On Wed, Nov 04, 2020 at 05:49:48PM +0000, Tim Evens (tievens) wrote:
> [tievens] Agree. The draft itself has local in the name.  Does it make
> sense to change the draft name or keep it as is? 

The filename is not relevant, I'd leave it as-is. Ultimately the
document will have an RFC number as filename anyway.

> 	alternative with this draft. (  Changed to alternative. 

Thanks. At this stage of the document's lifecycle it sometimes is
helpful to assume a writing style 'as if the document has been
published'. :-)

Thank you for the changes!

Can you upload a new revision next week?

Kind regards,

Job

---

From: "Tim Evens (tievens)" <[email-masked]>
Date: Sat, 14 Nov 2020 02:30:29 +0000

Thanks.  I will update the draft as soon as it’s open again.

Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Job Snijders <[email-masked]>
Sent: Friday, November 13, 2020 2:34:33 AM
To: Tim Evens (tievens) <[email-masked]>
Cc: [email-masked] <[email-masked]>
Subject: Re: [GROW] review of draft-ietf-grow-bmp-local-rib-07 (preparing for sheperd write-up)

Dear Tim,

Hi Tim,

Kind regards,

Job