Subject: [Idr] Review for draft-ietf-idr-bgp-gr-notification-01

From: Jan Henke <[email-masked]>
Date: Mon, 05 Aug 2013 18:53:58 +0200

Hi,

below you find my review of draft-ietf-idr-bgp-gr-notification-01. It is
my first review for an IETF draft, therefore I am happy to receive
feedback on the review itself in a separate message.

The draft is overall well written and understandable, however I suggest
the following improvements.

1. Introduction
---------------
The introduction states that NOTIFICATION messages were excluded from the
graceful restart defined in RFC4724. It does not mention why these
message class was excluded in RFC4724. It would be nice for the reader
to get a summary of the rationale for the original exclusion and the
motivation to change that with this document.

2. Modification BGP Graceful Restart Capability
-----------------------------------------------
While reading this section I was irritated by the "Flags for Address Family"
paragraph. I found out that the "N" bit was defined in the -00 revision of
the draft, but I did not find any RFC modifying it. Therefore I suggest to
remove the paragraph from the document, as there is no change to RFC4724.

3.  BGP Hard Reset Subcode
--------------------------
While reading this section I was hoping to get some information about the
appropriate usage of the Hard Reset Subcode. Providing some
typical use cases would improve this section and make it easier for future
implementations to have a similar semantics in respect to this code.
-- 
Best regards
Jan Henke

---

From: "Chris Hall" <[email-masked]>
Date: Tue, 6 Aug 2013 12:37:20 +0100

Jan Henke wrote (on Mon 05-Aug-2013 at 17:54 +0100):
....
> 1. Introduction
> ---------------
> The introduction states that NOTIFICATION messages were excluded
> from the graceful restart defined in RFC4724. It does not mention
> why these message class was excluded in RFC4724. It would be nice
> for the reader to get a summary of the rationale for the original
> exclusion and the motivation to change that with this document.

Amen to that... it is considerable puzzle to me that to (say) add a new address family to a session and exploit Graceful Restart, it is necessary to (somehow) *avoid* sending a NOTIFICATION message :-(

> While reading this section I was irritated by the "Flags for
> Address Family" paragraph. I found out that the "N" bit was
> defined in the -00 revision of the draft, but I did not find
> any RFC modifying it. Therefore I suggest to remove the
> paragraph from the document, as there is no change to RFC4724.

Having the 'N' qualification per AFI/SAFI does seem de trop.

It seems there are two cases of session termination:

  (1) with Graceful Restart, as previously advertised

  (2) without Graceful Restart, previous advertisement
      notwithstanding

And RFC4724 says:

   * without NOTIFICATION => (1),
   * with NOTIFICATION => (2).

And the problem is: to arrange a Graceful Restart the restarting end has to *avoid* sending a NOTIFICATION, and any information that a NOTIFICATION might have carried is lost.

This draft seems to say: 

   * without NOTIFICATION => (1)
   * with any NOTIFICATION (except "Hard Reset") => (1), 
   * "Hard Reset" => (2).

Now, the logic is inverted: if the sender does want to inhibit Graceful Restart, it needs to send a "Hard Reset", and any information that a more specific NOTIFICATION might have carried is lost.

The key paragraph of the draft is:

   When a BGP Speaker receives a BGP NOTIFICATION message, it
   SHOULD follow the standard rules of the receiving speaker
   mentioned in [RFC4724] for all AFI/SAFIs for which it has
   announced the BGP Graceful Notification flag.  The BGP
   speaker generating a BGP NOTIFICATION message SHOULD follow
   the standard rules of the receiving Speaker in [RFC4724]
   for all AFI/SAFIs that were announced with the BGP Graceful
   Notification flag.

By "standard rules", we mean section 4.2 of RFC4724... or all the rules of RFC4724 excluding the one which says that NOTIFICATION <=> inhibit Graceful Restart.

The first sentence seems to say that the receiving speak will, unilaterally, no longer follow RFC4724, but will do Graceful Restart even if the sender sends a NOTIFICATION.  I suppose this tells the sender that if it does want case (2), they had better send a "Hard Reset" -- which may be a bit of a blow if the sender doesn't understand the 'N' bit.

I assume the second sentence is s/receiving/restarting/ ?  I assume "were announced" means announced by the speaker to whom the NOTIFICATION is being sent ?

If there is no significant need for (2) then the receiver doesn't really need permission to not inhibit Graceful Restart on receipt of a NOTIFICATION.  So the real meaning of the 'N' qualification is: having received 'N', a restarting speaker: (a) if it wants Graceful Restart, it can still send a (helpful) NOTIFICATION (which would currently have to be suppressed); (b) must send a "Hard Reset" if Graceful Restart needs to be suppressed; (c) must carry out the restarting speaker procedure if it does not send "Hard Reset".

The draft also says:

   A BGP speaker MAY continue to operate in the Graceful Restart
   mode even if it receives a Graceful Restart capability without
   a Graceful Notification Flag. 

When receiving a NOTIFICATION the BGP speaker is not required to take any notice of the incoming 'N' qualification.  This seems to reiterate that... which suggests that the need for (2) is discounted.  If the other end does not announce 'N', then sending a NOTIFICATION and expecting it to do Graceful Restart would be to assume that the other end is not, in fact, RFC4724 compliant ?

If there is a significant need for both cases of session termination, then it seems to me that the NOTIFICATION itself needs to carry that extra bit of information, somewhere.  Perhaps, a new "NOTIFICATION_GRACEFUL" message could be implemented, exactly as the current but meaning "please *do* Graceful Restart, as previously advertised".  It's possible that such a message could be sent even if the far end does not understand it -- *provided* it logs the event and then does a Graceful Restart !

Of course, if the sender wants to inhibit Graceful Restart, it can always send the 'F' bit as zero when the session comes back up... but that is not guaranteed to happen immediately, so perhaps that's not good enough ?

As it stands, the draft lumps all "crash" NOTIFICATIONs into one.  I'm not sure of the utility of doing a Graceful Restart on receiving a "Cease/Administrative Shutdown" or "Cease/Peer De-configured".  For many other NOTIFICATIONs Graceful Restart seems entirely the right thing to do.  I'm not sure about the OPEN Message errors.

> While reading this section I was hoping to get some information
> about the appropriate usage of the Hard Reset Subcode. Providing
> some typical use cases would improve this section and make it
> easier for future implementations to have a similar semantics 
> in respect to this code.

Indeed.

Chris