From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: fix softreload bug in the RDE
To: tech@openbsd.org
Date: Tue, 28 Apr 2026 15:40:17 +0200

On Tue, Apr 28, 2026 at 10:34:23AM +0200, Claudio Jeker wrote:
> In rde_reload_done() the code handling the peer->reconf_rib case has a
> continue which skips the code path that actually reapplies the outbound
> filters. This is a bug since the code needs to reapply the outbound
> filters for every peer.
> 
> Removing the continue changes the way peer->reconf_rib and
> peer->reconf_out behave. Before only one of them was possible at any time
> but now in the reconf_rib case reconf_out is most probably also set.
> Because of this switch the order of checks in rde_softreconfig_in_done()
> so that reconf_rib has precedence.

ok tb

> 
> Noticed while reworking the filters.
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> diff -u -p -r1.694 rde.c
> --- rde.c	27 Apr 2026 15:52:20 -0000	1.694
> +++ rde.c	28 Apr 2026 08:28:17 -0000
> @@ -4003,7 +4003,6 @@ rde_reload_done(void)
>  				fatal("%s: prefix_dump_new", __func__);
>  			log_peer_info(&peer->conf, "flushing Adj-RIB-Out");
>  			softreconfig++;	/* account for the running flush */
> -			continue;
>  		}
>  
>  		/* reapply outbound filters for this peer */
> @@ -4213,7 +4212,13 @@ rde_softreconfig_in_done(void *arg, uint
>  	RB_FOREACH(peer, peer_tree, &peertable) {
>  		uint8_t aid;
>  
> -		if (peer->reconf_out) {
> +		if (peer->reconf_rib) {
> +			/* dump the full table to neighbors that changed rib */
> +			for (aid = AID_MIN; aid < AID_MAX; aid++) {
> +				if (peer->capa.mp[aid])
> +					peer_dump(peer, aid);
> +			}
> +		} else if (peer->reconf_out) {
>  			if (peer->export_type == EXPORT_NONE) {
>  				/* nothing to do here */
>  				peer->reconf_out = 0;
> @@ -4227,12 +4232,6 @@ rde_softreconfig_in_done(void *arg, uint
>  			} else
>  				rib_byid(peer->loc_rib_id)->state =
>  				    RECONF_RELOAD;
> -		} else if (peer->reconf_rib) {
> -			/* dump the full table to neighbors that changed rib */
> -			for (aid = AID_MIN; aid < AID_MAX; aid++) {
> -				if (peer->capa.mp[aid])
> -					peer_dump(peer, aid);
> -			}
>  		}
>  	}
>  
>