mirrors/bird
mirrors/bird
0
0
Fork 0
mirror of https://gitlab.nic.cz/labs/bird.git synced 2024-12-31 06:01:52 +00:00
Code Releases Activity

BGP: Improve handling of hold and keepalive timers

Browse Source 
The effective keepalive time now scales relative to the negotiated
hold time, to maintain proportion between the keepalive time and the
hold time. This avoids issues when both keepalive and hold times
were configured, the hold time was negotiated to a smaller value,
but the keepalive time stayed the same.

Add new options 'min hold time' and 'min keepalive time', which reject
session attempts with too small hold time.

Improve validation of config options an their documentation.

Thanks to Alexander Zubkov and Sergei Goriunov for suggestions.
This commit is contained in:
Ondrej Zajicek 2022-12-09 05:51:30 +01:00
parent e80156d936
commit 3859e4efc1
5 changed files with 58 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/bird.sgml
View File

@ -2750,9 +2750,16 @@ using the following configuration parameters:
<tag><label id="bgp-hold-time">hold time <m/number/</tag>
Time in seconds to wait for a Keepalive message from the other side
before considering the connection stale. Default: depends on agreement
with the neighboring router, we prefer 240 seconds if the other side is
willing to accept it.
before considering the connection stale. The effective value is
negotiated during session establishment and it is a minimum of this
configured value and the value proposed by the peer. The zero value has
a special meaning, signifying that no keepalives are used. Default: 240
seconds.
<tag><label id="bgp-min-hold-time">min hold time <m/number/</tag>
Minimum value of the hold time that is accepted during session negotiation.
If the peer proposes a lower value, the session is rejected with error.
Default: none.
<tag><label id="bgp-startup-hold-time">startup hold time <m/number/</tag>
Value of the hold timer used before the routers have a chance to exchange
@ -2760,8 +2767,15 @@ using the following configuration parameters:
<tag><label id="bgp-keepalive-time">keepalive time <m/number/</tag>
Delay in seconds between sending of two consecutive Keepalive messages.
The effective value depends on the negotiated hold time, as it is scaled
to maintain proportion between the keepalive time and the hold time.
Default: One third of the hold time.
<tag><label id="bgp-min-keepalive-time">min keepalive time <m/number/</tag>
Minimum value of the keepalive time that is accepted during session
negotiation. If the proposed hold time would lead to a lower value of
the keepalive time, the session is rejected with error. Default: none.
<tag><label id="bgp-connect-delay-time">connect delay time <m/number/</tag>
Delay in seconds between protocol startup and the first attempt to
connect. Default: 5 seconds.

15
proto/bgp/bgp.c
View File

@ -2007,6 +2007,21 @@ bgp_postconfig(struct proto_config *CF)
if (internal && cf->enforce_first_as)
cf_error("Enforce first AS check is requires EBGP sessions");
if (cf->keepalive_time > cf->hold_time)
cf_error("Keepalive time must be at most hold time");
if (cf->keepalive_time > (cf->hold_time / 2))
log(L_WARN "Keepalive time should be at most 1/2 of hold time");
if (cf->min_hold_time > cf->hold_time)
cf_error("Min hold time (%u) exceeds hold time (%u)",
cf->min_hold_time, cf->hold_time);
uint keepalive_time = cf->keepalive_time ?: cf->hold_time / 3;
if (cf->min_keepalive_time > keepalive_time)
cf_error("Min keepalive time (%u) exceeds keepalive time (%u)",
cf->min_keepalive_time, keepalive_time);
struct bgp_channel_config *cc;
BGP_CF_WALK_CHANNELS(cf, cc)

5
proto/bgp/bgp.h
View File

@ -120,8 +120,11 @@ struct bgp_config {
unsigned llgr_time; /* Long-lived graceful restart stale time */
unsigned connect_delay_time; /* Minimum delay between connect attempts */
unsigned connect_retry_time; /* Timeout for connect attempts */
unsigned hold_time, initial_hold_time;
unsigned hold_time;
unsigned min_hold_time; /* Minimum accepted hold time */
unsigned initial_hold_time;
unsigned keepalive_time;
unsigned min_keepalive_time; /* Minimum accepted keepalive time */
unsigned error_amnesia_time; /* Errors are forgotten after */
unsigned error_delay_time_min; /* Time to wait after an error is detected */
unsigned error_delay_time_max;

6
proto/bgp/config.Y
View File

@ -153,7 +153,8 @@ bgp_proto:
| bgp_proto RS CLIENT bool ';' { BGP_CFG->rs_client = $4; }
| bgp_proto CONFEDERATION expr ';' { BGP_CFG->confederation = $3; }
| bgp_proto CONFEDERATION MEMBER bool ';' { BGP_CFG->confederation_member = $4; }
| bgp_proto HOLD TIME expr ';' { BGP_CFG->hold_time = $4; }
| bgp_proto HOLD TIME expr ';' { BGP_CFG->hold_time = $4; if (($4 && $4<3) || ($4>65535)) cf_error("Hold time must be in range 3-65535 or zero"); }
| bgp_proto MIN HOLD TIME expr ';' { BGP_CFG->min_hold_time = $5; }
| bgp_proto STARTUP HOLD TIME expr ';' { BGP_CFG->initial_hold_time = $5; }
| bgp_proto DIRECT ';' { BGP_CFG->multihop = 0; }
| bgp_proto MULTIHOP ';' { BGP_CFG->multihop = 64; }
@ -177,7 +178,8 @@ bgp_proto:
| bgp_proto START DELAY TIME expr ';' { BGP_CFG->connect_delay_time = $5; log(L_WARN "%s: Start delay time option is deprecated, use connect delay time", this_proto->name); }
| bgp_proto CONNECT DELAY TIME expr ';' { BGP_CFG->connect_delay_time = $5; }
| bgp_proto CONNECT RETRY TIME expr ';' { BGP_CFG->connect_retry_time = $5; }
| bgp_proto KEEPALIVE TIME expr ';' { BGP_CFG->keepalive_time = $4; }
| bgp_proto KEEPALIVE TIME expr ';' { BGP_CFG->keepalive_time = $4; if (($4<1) || ($4>65535)) cf_error("Keepalive time must be in range 1-65535"); }
| bgp_proto MIN KEEPALIVE TIME expr ';' { BGP_CFG->min_keepalive_time = $5; }
| bgp_proto ERROR FORGET TIME expr ';' { BGP_CFG->error_amnesia_time = $5; }
| bgp_proto ERROR WAIT TIME expr ',' expr ';' { BGP_CFG->error_delay_time_min = $5; BGP_CFG->error_delay_time_max = $7; }
| bgp_proto DISABLE AFTER ERROR bool ';' { BGP_CFG->disable_after_error = $5; }

20
proto/bgp/packets.c
View File

@ -847,9 +847,25 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
if (bgp_read_options(conn, pkt+29, pkt[28], len-29) < 0)
return;
/* RFC 4271 4.2 - hold time must be either 0 or at least 3 */
if (hold > 0 && hold < 3)
{ bgp_error(conn, 2, 6, pkt+22, 2); return; }
/* Compute effective hold and keepalive times */
uint hold_time = MIN(hold, p->cf->hold_time);
uint keepalive_time = p->cf->keepalive_time ?
(p->cf->keepalive_time * hold_time / p->cf->hold_time) :
hold_time / 3;
/* Keepalive time might be rounded down to zero */
if (hold_time && !keepalive_time)
keepalive_time = 1;
/* Check effective values against configured minimums */
if ((hold_time < p->cf->min_hold_time) ||
(keepalive_time < p->cf->min_keepalive_time))
{ bgp_error(conn, 2, 6, pkt+22, 2); return; }
/* RFC 6286 2.2 - router ID is nonzero and AS-wide unique */
if (!id || (p->is_internal && id == p->local_id))
{ bgp_error(conn, 2, 3, pkt+24, -4); return; }
@ -947,8 +963,8 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
}
/* Update our local variables */
conn->hold_time = MIN(hold, p->cf->hold_time);
conn->keepalive_time = p->cf->keepalive_time ? : conn->hold_time / 3;
conn->hold_time = hold_time;
conn->keepalive_time = keepalive_time;
conn->as4_session = conn->local_caps->as4_support && caps->as4_support;
conn->ext_messages = conn->local_caps->ext_messages && caps->ext_messages;
p->remote_id = id;