Re: [Idr] Opsdir early review of draft-ietf-idr-wide-bgp-communities-07

["Joe Clarke (jclarke)" <[e-mail]>]

Thanks, Jeff.  See below.

From: Jeffrey Haas <[e-mail]>
Date: Friday, July 8, 2022 at 16:09
To: Joe Clarke (jclarke) <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Re: [Idr] Opsdir early review of draft-ietf-idr-wide-bgp-communities-07
Joe,

On Fri, Jun 03, 2022 at 06:49:43AM -0700, Joe Clarke via Datatracker wrote:
> Reviewer: Joe Clarke
> Review result: Has Issues
>
> Sorry for the late review.  I have been tasked to review this document on
> behalf of the Ops Directorate.  This document defines a new BGP path attribute
> called the BGP Community Container as well as a Wide Community type.  That
> said, it is not immediately clear from the abstract what this document really
> does.  It isn't until Section 2 where it becomes clearer.

After some consideration, I didn't have any specific text to offer here to
address this comment.  Hopefully the other authors have suggestions.

[JMC] I was looking for more of the text from Section 2 to concretely specify what I should find in this document.  The Abstract says it defines a new encoding whereas Section 2 starts by saying, “This specification defines a new BGP Path Attribute, the BGP”.  Both are correct, but I found myself digging more through the new path attribute and the wide container.

[JMC] Oh, and it looks like you dropped a period in the new abstract:

s/advertise an operator's parameters for execution It also provides an extensible platform for any future community encoding requirements./advertise an operator's parameters for execution.  It also provides an extensible platform for any future community encoding requirements./


> While I appreciate Section 7 on operational considerations, it struck me that
> the statement, "in supporting both standard and BGP Wide Communities, allow for
> their easy inbound and outbound processing" was not terribly instructive.
> Perhaps an example of what the authors would consider "easy" in this case would
> be useful.

In the pending -08, I've addressed this text.  The update will be posted
this weekend, but you can see a preview if you're eager here:
https://github.com/jhaas-pfrc/bgp-wide-community/pull/7

[JMC] Thanks.  I’ve looked at both.  The new language is better.  While a strong MUST would be appreciated, I can see how that may be too strong in this case.


> I have the same comment here as in Section 4.4.2.  If the parameters are empty
> how exactly should the receiving router behave?  Why not just make these a
> strong MUST NOT encode if they are empty to remove any ambiguity?

To a large extent, to follow Postel's Maxim.  If you encode an empty TLV
that is meaningless, do you want it to be an error if you insert it but
don't populate it?  The authors' choice is to not make it an error.

[JMC] Okay.

>
> Section 9.2
>
> s/AS_PATH prepend 4 TIMES TO/AS_PATH prepend 4 times/
>
> Not _sure_ this is correct, but I couldn't quite grok the sentence as written.

I've updated the example's explanatory text.  Please see if it addresses
your concerns.

[JMC] Reads better to me, thanks.  Actually, it’s much clearer except for the last part of the sentence pertaining to Use Case New York.  I get what you’re trying to say, but it took me a few times to digest that the “of” applies to New York.  Maybe double the “New York”:

“…in the User Class of New York (101), but not to peers of the User Class New York (101)”

Joe