From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: more cleanup for rib_dump_free()
To: tech@openbsd.org
Date: Tue, 18 Nov 2025 17:56:15 +0100

On Tue, Nov 18, 2025 at 05:52:00PM +0100, Claudio Jeker wrote:
> Introduce two helpers rib_dump_cleanup() and prefix_adjout_dump_cleanup()
> which can be called by rib_dump_free() to properly remove the re or prefix
> reference held by the context.
> 
> This allows to remove two duplicated static inline functions from rde_rib.c

ok tb

> -- 
> :wq Claudio
> 
> ? obj
> Index: rde.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> diff -u -p -r1.319 rde.h
> --- rde.h	18 Nov 2025 16:39:36 -0000	1.319
> +++ rde.h	18 Nov 2025 16:48:33 -0000
> @@ -731,6 +731,7 @@ void		 prefix_adjout_withdraw(struct pre
>  void		 prefix_adjout_destroy(struct prefix *);
>  void		 prefix_adjout_flush_pending(struct rde_peer *);
>  int		 prefix_adjout_reaper(struct rde_peer *);
> +void		 prefix_adjout_dump_cleanup(struct prefix *);
>  void		 prefix_adjout_dump_r(struct rib_context *);
>  int		 prefix_adjout_dump_new(struct rde_peer *, uint8_t,
>  		    unsigned int, void *, void (*)(struct prefix *, void *),
> Index: rde_adjout.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
> diff -u -p -r1.1 rde_adjout.c
> --- rde_adjout.c	18 Nov 2025 16:39:36 -0000	1.1
> +++ rde_adjout.c	18 Nov 2025 16:48:34 -0000
> @@ -447,6 +447,13 @@ prefix_restart(struct rib_context *ctx)
>  }
>  
>  void
> +prefix_adjout_dump_cleanup(struct prefix *p)
> +{
> +	if (prefix_is_dead(prefix_unlock(p)))
> +		prefix_adjout_destroy(p);
> +}
> +
> +void
>  prefix_adjout_dump_r(struct rib_context *ctx)
>  {
>  	struct prefix *p, *next;
> Index: rde_rib.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
> diff -u -p -r1.276 rde_rib.c
> --- rde_rib.c	18 Nov 2025 16:39:36 -0000	1.276
> +++ rde_rib.c	18 Nov 2025 16:48:34 -0000
> @@ -77,21 +77,6 @@ re_is_locked(struct rib_entry *re)
>  	return (re->lock != 0);
>  }
>  
> -static inline struct prefix *
> -prefix_unlock(struct prefix *p)
> -{
> -	if ((p->flags & PREFIX_FLAG_LOCKED) == 0)
> -		fatalx("%s: unlocking unlocked prefix", __func__);
> -	p->flags &= ~PREFIX_FLAG_LOCKED;
> -	return p;
> -}
> -
> -static inline int
> -prefix_is_dead(struct prefix *p)
> -{
> -	return (p->flags & PREFIX_FLAG_DEAD) != 0;
> -}
> -
>  static inline struct rib_tree *
>  rib_tree(struct rib *rib)
>  {
> @@ -466,14 +451,20 @@ rib_dump_runner(void)
>  }
>  
>  static void
> +rib_dump_cleanup(struct rib_entry *re)
> +{
> +	if (rib_empty(re_unlock(re)))
> +		rib_remove(re);
> +}
> +static void
>  rib_dump_free(struct rib_context *ctx)
>  {
>  	if (ctx->ctx_done)
>  		ctx->ctx_done(ctx->ctx_arg, ctx->ctx_aid);
> -	if (ctx->ctx_re && rib_empty(re_unlock(ctx->ctx_re)))
> -		rib_remove(ctx->ctx_re);
> -	if (ctx->ctx_p && prefix_is_dead(prefix_unlock(ctx->ctx_p)))
> -		prefix_adjout_destroy(ctx->ctx_p);
> +	if (ctx->ctx_re)
> +		rib_dump_cleanup(ctx->ctx_re);
> +	if (ctx->ctx_p)
> +		prefix_adjout_dump_cleanup(ctx->ctx_p);
>  	LIST_REMOVE(ctx, entry);
>  	free(ctx);
>  }
>