Index | Thread | Search

From:
Tom Smyth <tom.smyth@wirelessconnect.eu>
Subject:
Re: bgpd: allow quick open on startup
To:
tech@openbsd.org
Date:
Thu, 23 Jul 2026 11:21:30 +0100

Download raw body.

Thread
Hi Claudio,

pardon my ignorance does the changes around  does this mean that when
starting up after a reboot bgp sessions will restart quicker  ?

I see a delay of about 2 -3 minutes wait  when patching boxes before the
BGP sessions restart after the box has booted?

Thanks

Tom Smyth



On Thu, 23 Jul 2026 at 11:14, Claudio Jeker <cjeker@diehard.n-r-g.com>
wrote:

> With the change to the IdleHoldTimer I did a few days ago the regress
> tests starterd to blow up. I remember I tested them but I probably screwed
> up something on my side.
>
> The problem is that on startup we keep seesion in Idle for
> SESSION_CLEAR_DELAY seconds. In that state no connection is accepted.
> So this applies the same logic and moves the peer directly to Active
> and adds an extra hack into the bgp_fsm EVNT_START handling to reduce
> the Timer_ConnectRetry to SESSION_CLEAR_DELAY.
>
> In init_peer() instead of passing via the timer just call the bgp_fsm().
> Also there is no need to stop a timer that is not running.
>
> The initial delay I added here was to ensure that the async bits of the
> initial config load have finished in the RDE. This just prevents extra
> work on startup for setups with big configs and many peers. Maybe the RDE
> should signal the SE when finished so we could use that signal to start all
> sessions on startup.
>
> With the diff below regress is passing again.
> --
> :wq Claudio
>
> Index: session.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
> diff -u -p -r1.538 session.c
> --- session.c   21 Jul 2026 08:23:44 -0000      1.538
> +++ session.c   23 Jul 2026 09:14:32 -0000
> @@ -593,10 +593,8 @@ init_peer(struct peer *p, struct bgpd_co
>         peer_cnt++;
>
>         change_state(p, STATE_IDLE, EVNT_NONE);
> -       if (p->conf.down)
> -               timer_stop(&p->timers, Timer_IdleHold); /* no autostart */
> -       else
> -               timer_set(&p->timers, Timer_IdleHold, SESSION_CLEAR_DELAY);
> +       if (!p->conf.down)
> +               bgp_fsm(p, EVNT_START, NULL);
>
>         p->stats.last_updown = getmonotime();
>
> Index: session_bgp.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/session_bgp.c,v
> diff -u -p -r1.11 session_bgp.c
> --- session_bgp.c       21 Jul 2026 08:23:44 -0000      1.11
> +++ session_bgp.c       23 Jul 2026 09:10:01 -0000
> @@ -1554,16 +1554,19 @@ bgp_fsm(struct peer *peer, enum session_
>                         timer_stop(&peer->timers, Timer_Keepalive);
>                         timer_stop(&peer->timers, Timer_IdleHold);
>
> -                       if (!peer->depend_ok)
> +                       if (!peer->depend_ok) {
>                                 timer_stop(&peer->timers,
> Timer_ConnectRetry);
> -                       else if (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) {
> +                       } else if (peer->prev_state == STATE_NONE ||
> +                           peer->IdleHoldTime ==
> INTERVAL_IDLE_HOLD_INITIAL) {
> +                               u_int holdtime =
> INTERVAL_IDLE_HOLD_INITIAL;
> +                               if (peer->prev_state == STATE_NONE)
> +                                       holdtime = SESSION_CLEAR_DELAY;
>                                 change_state(peer, STATE_ACTIVE, event);
>                                 timer_set(&peer->timers,
> Timer_ConnectRetry,
> -                                   INTERVAL_IDLE_HOLD_INITIAL);
> +                                   holdtime);
>                         } else {
>                                 change_state(peer, STATE_CONNECT, event);
>                                 timer_set(&peer->timers,
> Timer_ConnectRetry,
>
>

-- 
Kindest regards,
Tom Smyth.