From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix idle hold for failed peers
To: tech@openbsd.org
Date: Tue, 14 Jul 2026 09:43:49 +0200

On Tue, Jul 14, 2026 at 09:32:16AM +0200, Claudio Jeker wrote:
> Noticed with a bad peer that has a broken bgp implementation which sends
> OTC updates with bad flags. It seems to constantly reconnect ignoring the
> IdleHold timer of 30min+.
> 
> The culprit is the fast reopen in session_accept() which should only trigger
> on first error (peer->errcnt < 2) but errcnt is never increased when bgpd
> sends the notification. It is only done when a notification is received.
> 
> Add a errcnt++ in session_notification() similar to parse_notification().

ok

> On top of this adjust the exponential backoff of IdleHoldTime so that it
> actually reaches MAX_IDLE_HOLD. Right now the * 2 game gets us to ~31min
> and then stops since MAX_IDLE_HOLD is 1h. Just use the proper way of
> clamping the number down to MAX_IDLE_HOLD.

also ok

Feels like two separate commits.

> 
> -- 
> :wq Claudio
> 
> Index: session_bgp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session_bgp.c,v
> diff -u -p -r1.9 session_bgp.c
> --- session_bgp.c	8 May 2026 12:03:50 -0000	1.9
> +++ session_bgp.c	14 Jul 2026 06:58:34 -0000
> @@ -512,6 +512,7 @@ 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;
>  }
> @@ -1858,9 +1859,10 @@ change_state(struct peer *peer, enum ses
>  			} else if (event != EVNT_STOP) {
>  				timer_set(&peer->timers, Timer_IdleHold,
>  				    peer->IdleHoldTime);
> -				if (event != EVNT_NONE &&
> -				    peer->IdleHoldTime < MAX_IDLE_HOLD / 2)
> +				if (event != EVNT_NONE)
>  					peer->IdleHoldTime *= 2;
> +				if (peer->IdleHoldTime > MAX_IDLE_HOLD)
> +					peer->IdleHoldTime = MAX_IDLE_HOLD;
>  				session_down(peer);
>  			} else {
>  				session_down(peer);
>