Download raw body.
bgpd: rework IdleHold part of the bgp fsm
I realized while fixing the errcnt issue, that it actually is not needed
at all. Here is a diff that solves the same quick reconnect in a slightly
different way.
Instead of handling this specially in session_accept() just put the session
into a state that allows the connection to be accepted but then only
initiate the connection after INTERVAL_IDLE_HOLD_INITIAL.
So at start (and on neighbor clear/up) we set IdleHoldTime to 0. In that
case the IdleHoldTimer fires immediatly and puts the session into active
state. Doing that allows us to remove the session_accept() special for
STATE_IDLE. As usual nothing is free so instead the bgp_fsm() needs
to handle the case where the IdleHoldTime was 0 (actually
INTERVAL_IDLE_HOLD_INITIAL since the code in STATE_IDLE adjust
IdleHoldTime after arming the timer).
In that case use the ConnectRetryTimer to delay the local connect by
INTERVAL_IDLE_HOLD_INITIAL since we don't want to immediatly reopen the
session but allow peer to reconnect.
On top of this add code to STATE_ACTIVE / EVNT_START to also bring up a
session immediately. This allows a 'bgpctl nei X up' to work also for
active sessions (something that bothered me for a long time). It also
allows passive peers to open up connections that way. This last part is
certainly up for discussion. I myself am not sure if we really want that.
Finally there is a big reshuffle of code in change_state() STATE_IDLE so
that IdleHoldTime is only updated at the end. It also simplifies the
EVNT_NONE handling which is only triggered by init_peer().
This seems to behave like I want but this certainly needs some testing
from others as well to see if I missed some other crazy edge case.
At least the regress tests are happy with this, and those do some funky
business when it comes to cutting IdleHoldTime down.
--
:wq Claudio
Index: control.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/control.c,v
diff -u -p -r1.140 control.c
--- control.c 24 Jun 2026 06:01:13 -0000 1.140
+++ control.c 14 Jul 2026 08:48:16 -0000
@@ -389,9 +389,7 @@ control_dispatch_msg(struct pollfd *pfd,
bgp_fsm(p, EVNT_START, NULL);
p->conf.down = 0;
p->conf.reason[0] = '\0';
- p->IdleHoldTime =
- INTERVAL_IDLE_HOLD_INITIAL;
- p->errcnt = 0;
+ p->IdleHoldTime = 0;
control_result(c, CTL_RES_OK);
break;
case IMSG_CTL_NEIGHBOR_DOWN:
@@ -405,9 +403,7 @@ control_dispatch_msg(struct pollfd *pfd,
case IMSG_CTL_NEIGHBOR_CLEAR:
neighbor.reason[
sizeof(neighbor.reason) - 1] = '\0';
- p->IdleHoldTime =
- INTERVAL_IDLE_HOLD_INITIAL;
- p->errcnt = 0;
+ p->IdleHoldTime = 0;
if (!p->conf.down) {
session_stop(p,
ERR_CEASE_ADMIN_RESET,
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
diff -u -p -r1.537 session.c
--- session.c 14 May 2026 12:26:44 -0000 1.537
+++ session.c 14 Jul 2026 14:27:33 -0000
@@ -362,9 +362,7 @@ session_main(int debug, int verbose)
bgp_fsm(p, EVNT_START, NULL);
break;
case Timer_IdleHoldReset:
- p->IdleHoldTime =
- INTERVAL_IDLE_HOLD_INITIAL;
- p->errcnt = 0;
+ p->IdleHoldTime = 0;
timer_stop(&p->timers,
Timer_IdleHoldReset);
break;
@@ -714,15 +712,6 @@ session_accept(int listenfd)
}
p = getpeerbyip(conf, (struct sockaddr *)&cliaddr);
-
- if (p != NULL && p->state == STATE_IDLE && p->errcnt < 2) {
- if (timer_running(&p->timers, Timer_IdleHold, NULL)) {
- /* fast reconnect after clear */
- p->passive = 1;
- bgp_fsm(p, EVNT_START, NULL);
- }
- }
-
if (p != NULL &&
(p->state == STATE_CONNECT || p->state == STATE_ACTIVE)) {
if (p->fd != -1) {
Index: session.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.h,v
diff -u -p -r1.198 session.h
--- session.h 24 Jun 2026 06:01:13 -0000 1.198
+++ session.h 14 Jul 2026 08:49:21 -0000
@@ -140,7 +140,6 @@ struct peer {
struct peer *template;
int fd;
int lasterr;
- u_int errcnt;
u_int IdleHoldTime;
unsigned int if_scope; /* interface scope for IPv6 */
uint32_t local_bgpid;
@@ -155,7 +154,6 @@ struct peer {
uint16_t remote_port;
uint8_t depend_ok;
uint8_t demoted;
- uint8_t passive;
uint8_t throttled;
uint8_t rpending;
uint8_t rdesession;
Index: session_bgp.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session_bgp.c,v
diff -u -p -r1.10 session_bgp.c
--- session_bgp.c 14 Jul 2026 07:58:05 -0000 1.10
+++ session_bgp.c 14 Jul 2026 14:31:03 -0000
@@ -512,7 +512,6 @@ session_notification(struct peer *p, uin
session_sendmsg(buf, p, BGP_NOTIFICATION);
p->stats.msg_sent_notification++;
- p->errcnt++;
p->stats.last_sent_errcode = errcode;
p->stats.last_sent_suberr = subcode;
}
@@ -1187,7 +1186,6 @@ parse_notification(struct peer *peer, st
}
}
- peer->errcnt++;
peer->stats.last_rcvd_errcode = errcode;
peer->stats.last_rcvd_suberr = subcode;
@@ -1558,17 +1556,20 @@ bgp_fsm(struct peer *peer, enum session_
if (!peer->depend_ok)
timer_stop(&peer->timers, Timer_ConnectRetry);
- else if (peer->passive || peer->conf.passive ||
- peer->conf.template) {
+ else if (peer->conf.passive || peer->conf.template) {
change_state(peer, STATE_ACTIVE, event);
timer_stop(&peer->timers, Timer_ConnectRetry);
+ } else if (peer->IdleHoldTime ==
+ INTERVAL_IDLE_HOLD_INITIAL) {
+ change_state(peer, STATE_ACTIVE, event);
+ timer_set(&peer->timers, Timer_ConnectRetry,
+ INTERVAL_IDLE_HOLD_INITIAL);
} else {
change_state(peer, STATE_CONNECT, event);
timer_set(&peer->timers, Timer_ConnectRetry,
peer->conf.connectretry);
session_connect(peer);
}
- peer->passive = 0;
break;
case EVNT_STOP:
timer_stop(&peer->timers, Timer_IdleHold);
@@ -1610,7 +1611,12 @@ bgp_fsm(struct peer *peer, enum session_
case STATE_ACTIVE:
switch (event) {
case EVNT_START:
- /* ignore */
+ if (!peer->depend_ok || peer->conf.template)
+ break;
+ timer_set(&peer->timers, Timer_ConnectRetry,
+ peer->holdtime);
+ change_state(peer, STATE_CONNECT, event);
+ session_connect(peer);
break;
case EVNT_CON_OPEN:
session_tcp_established(peer);
@@ -1830,8 +1836,6 @@ change_state(struct peer *peer, enum ses
* session was not established successfully before, the
* starttimerinterval needs to be exponentially increased
*/
- if (peer->IdleHoldTime == 0)
- peer->IdleHoldTime = INTERVAL_IDLE_HOLD_INITIAL;
peer->holdtime = INTERVAL_HOLD_INITIAL;
timer_stop(&peer->timers, Timer_ConnectRetry);
timer_stop(&peer->timers, Timer_Keepalive);
@@ -1845,7 +1849,22 @@ change_state(struct peer *peer, enum ses
memset(&peer->capa.peer, 0, sizeof(peer->capa.peer));
session_md5_reload(peer);
+ if (peer->prev_state == STATE_NONE ||
+ peer->prev_state == STATE_ESTABLISHED) {
+ /* initialize capability negotiation structures */
+ memcpy(&peer->capa.ann, &peer->conf.capabilities,
+ sizeof(peer->capa.ann));
+ }
+
+ /* called from init_peer for basic setup. */
+ if (peer->prev_state == STATE_NONE)
+ break;
+
if (peer->prev_state == STATE_ESTABLISHED) {
+ if (event == EVNT_STOP) {
+ session_down(peer);
+ break;
+ }
if (peer->capa.neg.grestart.restart == 2 &&
(event == EVNT_CON_CLOSED ||
event == EVNT_CON_FATAL ||
@@ -1856,30 +1875,22 @@ change_state(struct peer *peer, enum ses
/* don't punish graceful restart */
timer_set(&peer->timers, Timer_IdleHold, 0);
session_graceful_restart(peer);
- } else if (event != EVNT_STOP) {
+ break;
+ } else {
timer_set(&peer->timers, Timer_IdleHold,
peer->IdleHoldTime);
- if (event != EVNT_NONE &&
- peer->IdleHoldTime < MAX_IDLE_HOLD / 2)
- peer->IdleHoldTime *= 2;
- session_down(peer);
- } else {
session_down(peer);
}
} else if (event != EVNT_STOP) {
timer_set(&peer->timers, Timer_IdleHold,
peer->IdleHoldTime);
- if (event != EVNT_NONE &&
- peer->IdleHoldTime < MAX_IDLE_HOLD / 2)
- peer->IdleHoldTime *= 2;
}
- if (peer->prev_state == STATE_NONE ||
- peer->prev_state == STATE_ESTABLISHED) {
- /* initialize capability negotiation structures */
- memcpy(&peer->capa.ann, &peer->conf.capabilities,
- sizeof(peer->capa.ann));
- }
+ peer->IdleHoldTime *= 2;
+ if (peer->IdleHoldTime == 0)
+ peer->IdleHoldTime = INTERVAL_IDLE_HOLD_INITIAL;
+ if (peer->IdleHoldTime > MAX_IDLE_HOLD)
+ peer->IdleHoldTime = MAX_IDLE_HOLD;
break;
case STATE_CONNECT:
if (peer->prev_state == STATE_ESTABLISHED &&
bgpd: rework IdleHold part of the bgp fsm