Graceful Restart Mechanism for BGP
draft-ietf-idr-restart-13

[Ballot comment]
First, as mentioned, tcpmd5 is adequate protection.  However there are
a lot of environments where this is not available.  I think another
solution would be to provide configuration of the new TCP behavior on
a per-peer basis.  I realize that this capability is not all that
useful without the new TCP behavior.  I'm willing to look at other
proposed mechanisms for protecting against this attack once the
security consideration is updated to describe the attack in sufficient
detail that I can analyze the risk.

[Ballot discuss]
In current BGP, when a new connection is received from a peer but the
old connection exists, the new connection is closed with a
notification.  This spec changes the behavior of BGP so that the new
connection replaces the old.  The security considerations section
states that this creates a potential for DOS attack.  I think it is
worse than that: I think that if an attacker can replace the
connection and send data over it, a new avenue for route injection or
modification is created.  This attack must be documented in the
security considerations section.  I realize this is not the only way
to change BGP routing and that in many environments the features
provided by this specification will be desirable.  I think this spec must also provide adequate protection for this attack; see the non-blocking comment below for my ideas on what that means.

[Ballot comment]
Comments from the Ops directorate by Simon Leinen:

This is a BGP extension that allows forwarding to continue while a BGP
speaker is restarted, and BGP to orderly resume when the session comes
up again.  The changes to BGP are fairly minimal, and well motivated.
The document is well written and looks like an excellent basis for
interoperable implementations.  It also covers possible operational
issues.  I believe this is a useful addition to our toolset.

[Ballot discuss]
I found several places in this document where the RFC2119 language is not as strong as it probably should be. Several SHOULDs would really be better off as MUSTs, and MAYs as SHOULDs. I detail these below:

In section 4, Graceful Restart Capability

>          The most significant bit is defined as the Restart State (R)
>          bit which can be used to avoid possible deadlock caused by
>          waiting for the End-of-RIB marker when multiple BGP speakers
>          peering with each other restart. When set (value 1), this bit
>          indicates that the BGP speaker has restarted, and its peer
>          SHOULD NOT wait for the End-of-RIB marker from the speaker
>          before advertising routing information to the speaker.

If the possibility of deadlock in this case is real, then I believe this needs to be a "MUST NOT wait" rather than "SHOULD NOT wait".

>          The remaining bits are reserved, and SHOULD be set to zero by
>          the sender and ignored by the receiver.

In multiple places, reserved values have a SHOULD for setting to zero as in the above quoted text. I believe a MUST here is better in order to underscore being strict in what is sent on the wire.

>    A BGP speaker SHOULD NOT include more than one instance of the
>    Graceful Restart Capability in the capability advertisement [BGP-
>    CAP].  If more than one instance of the Graceful Restart Capability
>    is carried in the capability advertisement, the receiver of the
>    advertisement SHOULD ignore all but the last instance of the Graceful
>    Restart Capability.

Is there any reason an implementation might want to send multiple instances? If not, make it a MUST NOT. Same for how to handle the case where this rule is violated - e.g., MUST ignore all but the last...

>    A BGP speaker MAY advertise the Graceful Restart Capability for an
>    address family to its peer if it has the ability to preserve its
>    forwarding state for the address family when BGP restarts. In
>    addition, even if the speaker does not have the ability to preserve
>    its forwarding state for any address family during BGP restart, it is
>    still recommended that the speaker advertise the Graceful Restart
>    Capability to its peer (as mentioned before this is done by not
>    including any  in the advertised capability). There are
>    two reasons for doing this. First, to indicate its intention of
>    generating the End-of-RIB marker upon the completion of its initial
>    routing updates, as doing this would be useful for routing
>    convergence in general. Second, to indicate its support for a peer
>    which wishes to perform a graceful restart.

This also sounds like it needs to be at least a SHOULD rather than a MAY,
the paragraph makes it fairly clear that it is not something that is
"truly optional" for this mechanism to work properly.

>    The End-of-RIB marker SHOULD be sent by a BGP speaker to its peer
>    once it completes the initial routing update (including the case when
>    there is no update to send) for an address family after the BGP
>    session is established.

In other places in the document, this marker "SHALL" be sent, why only "SHOULD"  here?

Beginning in section 5.1, I noticed a shift from using "MUST" to using "SHALL" in various places. Noting that MUST==SHALL according to RFC2119, that's OK, but it makes the document more difficult to read in my opinion. I would suggest normalizing on one or the other, rather than using them interchangeably in the same document.

>    "Acting accordingly" in this context means that the previous TCP
>    session SHOULD be closed, and the new one retained.  Note that this
>    behavior differs from the default behavior, as specified in [BGP-4]
>    section 6.8.  Since the previous connection is considered to be
>    reset, no NOTIFICATION message should be sent -- the previous TCP
>    session is simply closed.

Is there any reason an implementation might keep the old TCP session around? If not, make it a MUST to close it. I can't imagine why latent, unused, TCP sessions lying about would be a good thing.

[Ballot comment]
INTRODUCTION, paragraph 0:


        Pekka Savola raised some concerns on June 14 on the tcpm list
        (where the TCP Simple AuthenticationOption was discussed). Are
        these valid? (Also, what is " GR helper mode?" The draft doesn't
        mention it.)

        From Pekka'e email:
        Enabling Graceful Restart can cause black holes and loops under
        valid and rather common scenarios (e.g., power loss, forwarding
        plane crash).  It has been designed in such a fashion that it
        cannot be enabled by default due to these bad effects, the users
        must know what they are doing. [1] As such, major router vendors
        do not enable graceful restart by default.  It is not clear to me
        whether major router vendors enable graceful restart helper-mode by
        default.  Helper mode is required for GR to be used.
        [1] I made an IETF Last call comment about this.  The text was
        improved slightly in -11 but as the applicability and usefulness
        of GR seemed to be felt to be out of scope from interoperability
        perspective, these issues were not further discussed.


INTRODUCTION, paragraph 12:

>    This document describes a mechanism for BGP that would help minimize
>    the negative effects on routing caused by BGP restart. An End-of-RIB
>    marker is specified and can be used to convey routing convergence
>    information.  A new BGP capability, termed "Graceful Restart
>    Capability", is defined which would allow a BGP speaker to express
>    its ability to preserve forwarding state during BGP restart. Finally,
>    procedures are outlined for temporarily retaining routing information
>    across a TCP transport reset.

        The term "reset" has a specific meaning in TCP (sending a segment
        with the RST flag set). I think GR is also applicable to TCP
        connections that terminate without an RST. Wording should be changed
        to this effect; also elsewhere in the text.


Section 3., paragraph 1:

>    An UPDATE message with no reachable NLRI and empty withdrawn NLRI is
>    specified as the End-Of-RIB Marker that can be used by a BGP speaker
>    to indicate to its peer the completion of the initial routing update
>    after the session is established. For IPv4 unicast address family,
>    the End-Of-RIB Marker is an UPDATE message with the minimum length
>    [BGP-4].  For any other address family, it is an UPDATE message that
>    contains only the MP_UNREACH_NLRI attribute [BGP-MP] with no
>    withdrawn routes for that .

        Nit: Expand NLRI, RIB, AFI, SAFI, etc.
        (Or point to document that defines terminology.)

[Ballot discuss]
Section 5.2:

""Acting accordingly" in this context means that the previous TCP
  session SHOULD be closed, and the new one retained.  Note that this
  behavior differs from the default behavior, as specified in [BGP-4]
  section 6.8.  Since the previous connection is considered to be
  reset, no NOTIFICATION message should be sent -- the previous TCP
  session is simply closed."

Why is the above statement about closing the old one only at SHOULD strength. Is it clear for all how to handle the sitatuation when one is disregarding from the should?

Also how does the should interact with the following from section 6:

        - drops the TCP connection associated with the ESTABLISHED
        session,

Please clarify as this seem to be an contradiction.

[Ballot comment]
Section 4:

Capability value: Consists of the "Restart Flags" field, "Restart
      Time" field, and zero or more of the tuples  as follows:

As the Capability value field is restricted to at maximum 255 octets. I think one should specify what the maximum number of  triplets that the capability option can hold. I assume the number of normally used triplets is small so that the limit of 63 triplets is not a problem. I would suggest changing the above value definition to:

Capability value: Consists of the "Restart Flags" field, "Restart
      Time" field, and zero to 63 of the tuples  as follows:

[Ballot comment]
Section 4:

Capability value: Consists of the "Restart Flags" field, "Restart
      Time" field, and zero or more of the tuples  as follows:

As the Capability value field is restricted to at maximum 255 octets. I think one should specify what the maximum number of  triplets that the capability option can hold. I assume the number of normally used triplets is small so that the limit of 63 triplets is not a problem. I would suggest changing the above value definition to:

Capability value: Consists of the "Restart Flags" field, "Restart
      Time" field, and zero to 63 of the tuples  as follows:


Section 5.2:

""Acting accordingly" in this context means that the previous TCP
  session SHOULD be closed, and the new one retained.  Note that this
  behavior differs from the default behavior, as specified in [BGP-4]
  section 6.8.  Since the previous connection is considered to be
  reset, no NOTIFICATION message should be sent -- the previous TCP
  session is simply closed."

Why is the above statement about closing the old one only at SHOULD strength. Is it clear for all how to handle the sitatuation when one is disregarding from the should?

Last Call Comments not sent.

Upon approval of this document the IANA will update the reference for the BGP Capability -
Graceful Restart Capability. The Capability Code for Graceful Restart Capability is 64.
This registration is located at the following:
http://www.iana.org/assignments/capability-codes

We understand this to be the only IANA Action.

[to not have to search for it next time:]
From: Yakov Rekhter
Subject: BGP Graceful Restart to Proposed Standard
Date: Thu, 01 Jul 2004 19:38:57 -0700
To: zinin@psg.com, fenner@research.att.com
Cc: skh@nexthop.com, idr@ietf.org, iesg-secretary@ietf.org,
    yakov@juniper.net

Alex and Bill,

The IDR WG would like to ask the IESG to to advance
draft-ietf-idr-restart-10.txt to a Proposed Standard.

The implementation report is in draft-ietf-idr-bgp-gr-survey-01.txt.

Yakov.

Subject: AD Review comments on draft-ietf-idr-restart
Date: Fri, 12 Aug 2005 11:13:49 -0400
To: srihari@procket.com,yakov@juniper.net,rex@procket.com,jgs@cisco.c
    om,enke@redback.com
Cc: idr@ietf.org

I'm sorry that these are so late, especially since I found only a couple of
concerns.

1. Very minor - the document needs an IANA Considerations section,
probably just one sentence asking to change the reference for
64 from [Chen] to this document.

2. There's a new timer introduced (upper bound on the amount of time a
router defers its route selection) which MUST be supported but there's
no guidance on a default value or whether it should be used at all
by default.  (It may be easiest to just say that it SHOULD not be
configured to be used by default, and the upper bound is left to be
chosen by configuration by the user; if there's some guidance as to
how to set it that'd be even better)

3. In the description of the state machine modification, I'm concerned
that the literal interpretation of

      If the Graceful Restart capability with one or more AFI/SAFI has
      been received for the session, then TcpConnectionConfirmed (Event
      17) is treated as TcpConnectionFails (Event 18).

is that the new connection is torn down -- in Established state,
the FSM says that receiving a TcpConnectionFails means

              - sets the ConnectRetryTimer to zero,
              - deletes all routes associated with this connection,
              - releases all the BGP resources,
              - drops the TCP connection,
              - increments the ConnectRetryCounter by 1,
              - changes its state to Idle.

and clearly in graceful restart you don't want to do most of those things.

In fact, you don't know that it's a restarting connection until you've
gotten the OPEN on the new connection, right?  You probably want to
be modifying the "If a valid OPEN message (BGPOpen (Event 19)) is received"
condition in the Established state of the FSM?

  Bill