Subject: [Idr] AD Review of draft-ietf-idr-bgp-gr-notification-13

From: Alvaro Retana <[email-masked]>
Date: Thu, 7 Dec 2017 13:11:59 -0800

Dear authors:

I just finished reading this document — I have some comments, please see
the list below.

I understand the intent of this document: instead of resetting a BGP
session when a NOTIFICATION is received, use Graceful Restart; if the
session is going to *really* be reset, then use the new Hard Reset
sub-code.  That makes sense to me…but, is that the only code/sub-code for
which it makes sense to do a hard reset?  The NOTIFICATION has always had
the “stigma” of being something bad, so much that we (idr/IETF) have even
worked on ways to reduce its use (rfc7606, for example).  I want to ask the
WG to consider whether other code/sub-code NOTIFICATION combinations should
also result in a hard reset.  I think there are several cases, for example:

(1) rfc4486 (Subcodes for BGP Cease Notification Message)
defines “Administrative Shutdown” ("a BGP speaker decides to
administratively shut down its peering with a neighbor”).  It seems to me
that the sender of this NOTIFICATION would not want to "follow the rules
for the Receiving Speaker” (as specified in Section 4).

(2) rfc4486 also defines "Administrative Reset” ("a BGP speaker decides to
administratively reset the peering with a neighbor”); no more details are
provided, but that sounds like a hard reset to me.

(3) …there’s probably others...

Having said all that, I note that Section 3.1. (Sending a Hard Reset)
specifies the “encapsulation” (for lack of a better word) of the real
reason for the Hard Reset.  If the consensus is to go forward with that,
and not call out other exceptions, then I think that the text in 3.1 should
expand more on the encapsulation operation and the rationale for doing it
this way, and the document should also address other recent work that
recommends the use of Administrative Shutdown, for example
draft-ietf-grow-bgp-session-culling (a BCP currently in the RFC Editor’s
Queue).

[After I wrote the text above…] I found that some of the points have been
discussed on the list already — please include some of that
discussion/analysis in the document.

I’ll wait until the issue above and ones marked Major (below) are addressed
before starting the IETF LC.

Thanks!

Alvaro.

Major:

M1. Unfortunately, rfc4724 failed to setup a registry for the Restat Flags
(or the Flags for Address Family), which means that anyone is able to use
the bits in there (assuming the receiver looks at them, of course).  Given
that there are only a few bits, and to prevent conflicts, I would really
like to see a registry set up.  This document is already tagged to Update
rfc4724, so it seems like a good place to establish the registries.  [If
for some reason you rather not include that information here, then we can
take care of it elsewhere.  IOW, this request is not a requirement.]

M2. Section 4.1. (Rules for the Receiving Speaker) has me a little
confused.  Are the proposed changes contingent to setting the N bit?
The text starts by saying: "As part of this extension, routes from the
peer previously marked as stale MUST NOT be deleted, until and unless
the optional timer…expires…”…does that mean that the timer is no
longer optional?   Then you also say: “...if the Graceful Notification
("N") bit is not set in the newly received Graceful Restart
Capability, no new actions are triggered on the Receiving Speaker --
in particular, a clear "N” bit does not trigger deletion of stale
routes.”  If I understand rfc4724 correctly, stale routes could be
deleted — the text indicates changes in the behavior even if the N bit
is not set, right?  If you are Updating this section of rfc4724, what
would make it crystal clear is an “OLD/NEW” notation of the text (as
in, this is the OLD text…and this is the NEW text…).

M3. Security Considerations:  Maybe not a security issue, but
something to think about.  Section 4.1 says that “routes...previously
marked as stale MUST NOT be deleted, until and unless the optional
timer...expires, or unless a Hard Reset is performed.  This supersedes
the “consecutive restarts” requirement…”.  Not deleting the stale
routes and not making the timer mandatory could result in stale routes
that live forever if an attacker manages to create consecutive
restarts (by simply sending NOTIFICATIONS before EoR) — stale routes
are ok in the short term, but may point in the wrong direction
eventually.  Is this an issue?  I think that it would be mitigated if
the timer was made mandatory (with a nice default).

Minor:

P1. Section 4: “...receive and send BGP NOTIFICATION messages in
Graceful mode...” What is “Graceful mode”?

Nits:

N1. In Section 2, please indicate that the first figure corresponds to the
GR Capability from rfc4724.

N2. s/subcode is defined known as/subcode is defined as

N3. s/Graceful Notification flag/Graceful Notification bit   (For
consistency)

N4. The operation is obviously per-AF; maybe it’s worth saying that
somewhere just for completeness.