A BGP Cease NOTIFICATION Subcode for Bidirectional Forwarding Detection (BFD)
draft-ietf-idr-bfd-subcode-05

Thanks for this short, useful, readable document. I have just a few
comments and nits.

## Comments

### Section 1, hold time of zero doesn't mean zero

                                                                    As
   per Section 4.2 of [RFC4271], the minimum Hold Time interval
   supported by the protocol must be either zero, or at least three
   seconds.   

Since zero is a distinguished value that disables keepalive processing (RFC
4271 §4.4), maybe something like this instead?

                                                                     As
   per Section 4.2 and Section 4.4 of [RFC4271], the minimum Hold Time 
   interval is at least three seconds, unless KEEPALIVE processing has
   been disabled by negotiating the distinguished Hold Time of zero.

### Section 2, SHOULD v. MUST

   When a BGP connection is terminated due to a BFD session going into
   the Down state, the BGP speaker SHOULD send a NOTIFICATION message
   with the Error Code Cease and the Error Subcode "BFD Down".

Why isn't it a MUST? If the qualm is over the fact you might not always be
able to send a notification message, perhaps rewrite something like "the
NOTIFICATION message sent by the BGP speaker MUST use Error Code Cease and
Error Subcode 'BFD Down'"? Or "MUST attempt to send", "MUST send if
possible", etc.

### Section 3, this clause no verb

The first clause here is missing a verb:

   When the procedures in [RFC8538] for sending a NOTIFICATION message
   with a Cease Code and Hard Reset Subcode, and the BGP connection is
   being terminated because BFD has gone Down, the BFD Down Subcode
   SHOULD be encapsulated in the Hard Reset's data portion of the
   NOTIFICATION message.

Maybe adding "are in use" would fix it? As in 

   When the procedures in [RFC8538] for sending a NOTIFICATION message
   with a Cease Code and Hard Reset Subcode are in use, and the BGP ...
   
### Section 2, SHOULD v. MUST again

In the text quoted in the previous, there's another SHOULD, and again I
wonder why it's not a MUST.

### Section 4, no new security considerations

While I agree that

   This document introduces no additional BGP security considerations.

it might be considerate to explain to the reader why they should believe 
that. Up to you, but I might have said something like,

   The new functionality introduced by this document is limited to indicating
   the reason a session is being reset. It is difficult to imagine a way an
   attacker could make use of that information in a harmful way, considering
   that by definition the BGP session in question has already been closed. 

## Nits

### Section 1

                                 This is typically used by the clients
   to quickly trigger closure of their connections than the native
   protocol timers might allow.
   
Should be "to trigger closure... more quickly than the native". (Add a
"more" and move the "quickly".)

### Section 1

   If a BGP speaker desires to have its connections terminate faster
   than the negotiated BGP Hold Timer can accommodate upon loss of
   connectivity with a neighbor, the BGP speakers can rely upon BFD is
   used to supply that faster detection.
   
I think that last clause should remove the words "the BGP speakers can 
rely upon".

### Section 1

                                          When the BFD session state
   changes to Down, the BGP speaker terminates the connection with a
   NOTIFICATION message sent to the neighbor, if possible, and then
   close the TCP connection for the connection.
   
s/close/closes/

Thanks for a simple, straightforward document.

I concur with Eric's remark about the shepherd writeup.

The SHOULD in Section 2 is peculiar.  Why might I not do what it says, which SHOULD allows?  The document should provide some guidance here.  I have the same concern about the SHOULD at the end of Section 3.

The first sentence of the last paragraph of Section 1 and of Section 3 appear to have been mangled.  Please review.  Also, in the final sentence of Section 1, "close" should be "closes".

Thank you to Melinda Shore for the SECDIR review.  I concur with both of her points:

-- The Security Considerations would benefit from making a declarative statement that no additional considerations applying beyond those already stated for BFD in RFC5880.

-- Noting that this is an option sub-code in the BFD ecosystem.

# Éric Vyncke, INT AD, comments for draft-ietf-idr-bfd-subcode-04

CC @evyncke

Thank you for the work put into this document: it is clear and the intent is useful.

Please find below one nit.

Special thanks to Keyur Patel for the shepherd's detailed write-up including the WG consensus *but* missing the justification of the intended status. 

I hope that this review helps to improve the document,

Regards,

-éric

## NITS

### Section 7.2

As detected by the id-nits tools, reference to RFC4486 does not seem to be used.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments