[Idr] Re: AD Review of draft-ietf-idr-bgp-sendholdtimer-10

[Yingzhen Qu <[e-mail]>]

Hi Roman,

Thanks for the review and comments. I've uploaded version -11 to address
your comments. Please see detailed answers inline below.

Thanks,
Yingzhen

On Tue, Jun 25, 2024 at 8:48 AM Roman Danyliw <[e-mail]> wrote:

> Hi!
>
> I'm swapping with John and stepping in as the responsible AD for
> draft-ietf-idr-bgp-sendholdtimer.  I performed an AD review on
> draft-ietf-idr-bgp-sendholdtimer-10.  Thanks for this document.  My
> feedback is as follows:
>
> ** idnits reports:
>
>   -- The document seems to lack a disclaimer for pre-RFC5378 work, but may
>      have content which was first submitted before 10 November 2008.  If
> you
>      have contacted all the original authors and they are all willing to
> grant
>      the BCP78 rights to the IETF Trust, then this is fine, and you can
> ignore
>      this comment.  If not, you may need to add the pre-RFC5378
> disclaimer.
>      (See the Legal Provisions document at
>      https://trustee.ietf.org/license-info for more information.)
>
> Have the original authors been contact or should the alternative
> boilerplate be used?
>

[Yingzhen]: updated to use "pre5378Trust200902".

>
> ** Section 3.1
>    The following optional session attributes for each connection are
>    added to Section 8, before "The state session attribute indicates the
>    current state of the BGP FSM":
>
> The placement of this (14) and (15) doesn’t seem accurate.  The (1) – (8)
> list preceding the sentence “The state session attribute indicates the
> current state of the BGP FSM" in Section 8 of RFC4271 lists mandatory
> attributes (which the attribute described in this document is not).
>
> It seems like the (14) and (15) from this document should be added to the
> end of the “(1) – (13) list” that occurs after the text “The optional
> Session attributes are listed below”.
>
> [Yingzhen]: Thanks for catching this. Fixed.


> ** Section 3.3
>       -  logs an error message in the local system with the BGP Error
>          Code "Send Hold Timer Expired",
>
> Is this step mandatory?
>
> [Yingzhen]: yes, otherwise it'd be marked with "(optionally)".


> ** Section 3.4
>    Section 10 of [RFC4271] summarizes BGP Timers.  This document adds
>    another BGP timer: SendHoldTimer.
>
> This text, unlike prior text, isn’t explicit in saying where the new text
> is being inserted in Section 10 of RFC4271.
>

[Yingzhen]: The exact insertion place doesn't matter for Section 10.


>
> ** Section 6.
>    This documents suggests that an attempt to send a
>    NOTIFICATION message with the "Send Hold Timer Expired" error code is
>    still made,
>
> What does “suggests” mean?
>

[Yingzhen]: "suggests" means it's optional. It's up to an implementation to
decide whether or not to send the NOTIFICATION. Theoretically when the send
hold timer expires, a NOTIFICATION won't be delivered to the peer. However,
BGP may not have accurate knowledge about the TCP connection, and the
NOTIFICATION might be able to reach the peer. By sending the NOTIFICATION,
there will be something on the wire and a record in the BGP speaker, and
this will help to debug the reason for the session reset.


> ** Section 7
>    This specification does not change BGP's security characteristics.
>
Doesn’t it improve the resilience of the BGP model by allowing consistent
> termination of peers (i.e., improved availability of the global network)?
>

[Yingzhen]: I added "Implementing the BGP SendHoldTimer as specified in
this document will enhance network resilience by terminating connections
with malfunctioning or overwhelmed remote peers."

>
> Regards,
> Roman
> _______________________________________________
> Idr mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>