From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: bgpd: various little cleanup and prep steps
To: Theo Buehler <tb@theobuehler.org>
Cc: tech@openbsd.org
Date: Tue, 16 Dec 2025 13:06:00 +0100

On Tue, Dec 16, 2025 at 12:58:36PM +0100, Theo Buehler wrote:
> On Tue, Dec 16, 2025 at 11:49:46AM +0100, Claudio Jeker wrote:
> > Cleanup a few things:
> > 
> > - use for loops in rde_dump_ctx_new() instead of a while loop.
> > - in insert_community() use sizeof(*new) instead of sizeof(struct ...)
> > - add a struct pt_entry *pte argument to adjout_prefix_next() and
> >   adjout_prefix_withdraw()
> > - extend the prefix / pt API with pt_first, pt_next and pt_get_next.
> >   The first to functions can be used to build iterators and pt_get_next
> >   is like pt_get but uses RB_NFIND instead of RB_FIND and therefor finds
> >   the next matching address. This will be needed to implement the new
> >   adj-rib-out which hangs of pt_entry structs.
> > 
> > I will split up this diff before commit.
> > This is from a much bigger diff that I try to split up into more managable
> > chunks.
> 
> The first hunk feels like a net negative (it was hard to decipher before,
> now it is pretty unreadable). But sure,

I need to refactor that code at some point and to reduce the level of indent
by 6 feet and cover the rest with dirt.
 
> ok tb
> 
> > -- 
> > :wq Claudio
> > 
> > 
> > Index: rde.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> > diff -u -p -r1.675 rde.c
> > --- rde.c	12 Dec 2025 21:42:58 -0000	1.675
> > +++ rde.c	16 Dec 2025 10:25:36 -0000
> > @@ -3256,13 +3256,13 @@ rde_dump_ctx_new(struct ctl_show_rib_req
> >  						if (pte == NULL)
> >  							continue;
> >  						/* dump all matching paths */
> > -						p = adjout_prefix_first(peer,
> > -						    pte);
> > -						while (p != NULL) {
> > +						for (p = adjout_prefix_first(
> > +						    peer, pte);
> > +						    p != NULL;
> > +						    p = adjout_prefix_next(
> > +						    peer, pte, p)) {
> >  							rde_dump_adjout_upcall(
> >  							    peer, pte, p, ctx);
> > -							p = adjout_prefix_next(
> > -							    peer, p);
> >  						}
> >  					}
> >  					continue;
> > @@ -3276,10 +3276,11 @@ rde_dump_ctx_new(struct ctl_show_rib_req
> >  					continue;
> >  
> >  				/* dump all matching paths */
> > -				p = adjout_prefix_first(peer, pte);
> > -				while (p != NULL) {
> > -					rde_dump_adjout_upcall(peer, pte, p, ctx);
> > -					p = adjout_prefix_next(peer, p);
> > +				for (p = adjout_prefix_first(peer, pte);
> > +				    p != NULL;
> > +				    p = adjout_prefix_next(peer, pte, p)) {
> > +					rde_dump_adjout_upcall(peer, pte, p,
> > +					    ctx);
> >  				}
> >  			} while ((peer = peer_match(&req->neighbor,
> >  			    peer->conf.id)));
> > @@ -3545,7 +3546,7 @@ static void
> >  rde_up_flush_upcall(struct rde_peer *peer, struct pt_entry *pte,
> >      struct adjout_prefix *p, void *ptr)
> >  {
> > -	adjout_prefix_withdraw(peer, p);
> > +	adjout_prefix_withdraw(peer, pte, p);
> >  }
> >  
> >  int
> > Index: rde.h
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
> > diff -u -p -r1.333 rde.h
> > --- rde.h	15 Dec 2025 12:16:19 -0000	1.333
> > +++ rde.h	16 Dec 2025 10:28:38 -0000
> > @@ -558,9 +558,12 @@ void	 pt_getaddr(struct pt_entry *, stru
> >  int	 pt_getflowspec(struct pt_entry *, uint8_t **);
> >  struct pt_entry	*pt_fill(struct bgpd_addr *, int);
> >  struct pt_entry	*pt_get(struct bgpd_addr *, int);
> > -struct pt_entry *pt_add(struct bgpd_addr *, int);
> > +struct pt_entry	*pt_get_next(struct bgpd_addr *, int);
> > +struct pt_entry	*pt_add(struct bgpd_addr *, int);
> >  struct pt_entry	*pt_get_flow(struct flowspec *);
> >  struct pt_entry	*pt_add_flow(struct flowspec *);
> > +struct pt_entry	*pt_first(uint8_t);
> > +struct pt_entry	*pt_next(struct pt_entry *);
> >  void	 pt_remove(struct pt_entry *);
> >  struct pt_entry	*pt_lookup(struct bgpd_addr *);
> >  int	 pt_prefix_cmp(const struct pt_entry *, const struct pt_entry *);
> > @@ -738,12 +741,12 @@ struct adjout_prefix	*adjout_prefix_get(
> >  struct adjout_prefix	*adjout_prefix_first(struct rde_peer *,
> >  			    struct pt_entry *);
> >  struct adjout_prefix	*adjout_prefix_next(struct rde_peer *,
> > -			    struct adjout_prefix *);
> > +			    struct pt_entry *, struct adjout_prefix *);
> >  
> >  void		 prefix_add_eor(struct rde_peer *, uint8_t);
> >  void		 adjout_prefix_update(struct adjout_prefix *, struct rde_peer *,
> >  		    struct filterstate *, struct pt_entry *, uint32_t);
> > -void		 adjout_prefix_withdraw(struct rde_peer *,
> > +void		 adjout_prefix_withdraw(struct rde_peer *, struct pt_entry *,
> >  		    struct adjout_prefix *);
> >  void		 adjout_prefix_destroy(struct rde_peer *,
> >  		    struct adjout_prefix *);
> > Index: rde_adjout.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_adjout.c,v
> > diff -u -p -r1.13 rde_adjout.c
> > --- rde_adjout.c	15 Dec 2025 12:16:19 -0000	1.13
> > +++ rde_adjout.c	16 Dec 2025 10:30:28 -0000
> > @@ -480,7 +480,8 @@ adjout_prefix_first(struct rde_peer *pee
> >   * Return next prefix after a lookup that is actually an update.
> >   */
> >  struct adjout_prefix *
> > -adjout_prefix_next(struct rde_peer *peer, struct adjout_prefix *p)
> > +adjout_prefix_next(struct rde_peer *peer, struct pt_entry *pte,
> > +    struct adjout_prefix *p)
> >  {
> >  	struct adjout_prefix *np;
> >  
> > @@ -553,10 +554,11 @@ adjout_prefix_update(struct adjout_prefi
> >   * the prefix in the RIB linked to the peer withdraw list.
> >   */
> >  void
> > -adjout_prefix_withdraw(struct rde_peer *peer, struct adjout_prefix *p)
> > +adjout_prefix_withdraw(struct rde_peer *peer, struct pt_entry *pte,
> > +    struct adjout_prefix *p)
> >  {
> >  	if (peer_is_up(peer))
> > -		pend_prefix_add(peer, NULL, p->pt, p->path_id_tx);
> > +		pend_prefix_add(peer, NULL, pte, p->path_id_tx);
> >  
> >  	adjout_prefix_destroy(peer, p);
> >  }
> > Index: rde_community.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v
> > diff -u -p -r1.19 rde_community.c
> > --- rde_community.c	13 Nov 2025 15:52:47 -0000	1.19
> > +++ rde_community.c	16 Dec 2025 10:25:36 -0000
> > @@ -228,9 +228,9 @@ insert_community(struct rde_community *c
> >  		int newsize = comm->size + 8;
> >  
> >  		if ((new = reallocarray(comm->communities, newsize,
> > -		    sizeof(struct community))) == NULL)
> > +		    sizeof(*new))) == NULL)
> >  			fatal(__func__);
> > -		memset(&new[comm->size], 0, sizeof(struct community) * 8);
> > +		memset(&new[comm->size], 0, sizeof(*new) * 8);
> >  		comm->communities = new;
> >  		comm->size = newsize;
> >  	}
> > Index: rde_prefix.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_prefix.c,v
> > diff -u -p -r1.58 rde_prefix.c
> > --- rde_prefix.c	27 Feb 2025 14:03:32 -0000	1.58
> > +++ rde_prefix.c	16 Dec 2025 10:25:36 -0000
> > @@ -329,6 +329,15 @@ pt_get(struct bgpd_addr *prefix, int pre
> >  }
> >  
> >  struct pt_entry *
> > +pt_get_next(struct bgpd_addr *prefix, int prefixlen)
> > +{
> > +	struct pt_entry	*pte;
> > +
> > +	pte = pt_fill(prefix, prefixlen);
> > +	return RB_NFIND(pt_tree, &pttable, pte);
> > +}
> > +
> > +struct pt_entry *
> >  pt_add(struct bgpd_addr *prefix, int prefixlen)
> >  {
> >  	struct pt_entry		*p = NULL;
> > @@ -382,6 +391,25 @@ pt_add_flow(struct flowspec *f)
> >  		fatalx("pt_add: insert failed");
> >  
> >  	return (p);
> > +}
> > +
> > +struct pt_entry *
> > +pt_first(uint8_t aid)
> > +{
> > +	struct pt_entry	*pte;
> > +	struct bgpd_addr addr = { .aid = aid };
> > +
> > +	if (aid == AID_UNSPEC)
> > +		return RB_MIN(pt_tree, &pttable);
> > +
> > +	pte = pt_fill(&addr, 0);
> > +	return RB_NFIND(pt_tree, &pttable, pte);
> > +}
> > +
> > +struct pt_entry *
> > +pt_next(struct pt_entry *pte)
> > +{
> > +	return RB_NEXT(pt_tree, &pttable, pte);
> >  }
> >  
> >  void
> > Index: rde_update.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
> > diff -u -p -r1.190 rde_update.c
> > --- rde_update.c	15 Dec 2025 12:16:19 -0000	1.190
> > +++ rde_update.c	16 Dec 2025 10:25:36 -0000
> > @@ -244,7 +244,7 @@ up_generate_updates(struct rde_peer *pee
> >  done:
> >  	/* withdraw prefix */
> >  	if (p != NULL)
> > -		adjout_prefix_withdraw(peer, p);
> > +		adjout_prefix_withdraw(peer, re->prefix, p);
> >  }
> >  
> >  /*
> > @@ -266,7 +266,7 @@ up_generate_addpath(struct rde_peer *pee
> >  
> >  	/* collect all current paths */
> >  	head = adjout_prefix_first(peer, re->prefix);
> > -	for (p = head; p != NULL; p = adjout_prefix_next(peer, p)) {
> > +	for (p = head; p != NULL; p = adjout_prefix_next(peer, re->prefix, p)) {
> >  		addpath_prefix_list[pidx++] = p->path_id_tx;
> >  		if (pidx >= nitems(addpath_prefix_list))
> >  			fatalx("too many addpath paths to select from");
> > @@ -347,7 +347,7 @@ up_generate_addpath(struct rde_peer *pee
> >  			p = adjout_prefix_get(peer, addpath_prefix_list[i],
> >  			    re->prefix);
> >  			if (p != NULL)
> > -				adjout_prefix_withdraw(peer, p);
> > +				adjout_prefix_withdraw(peer, re->prefix, p);
> >  		}
> >  	}
> >  }
> > @@ -397,7 +397,7 @@ up_generate_addpath_all(struct rde_peer 
> >  		/* withdraw old path */
> >  		p = adjout_prefix_get(peer, old_pathid_tx, re->prefix);
> >  		if (p != NULL)
> > -			adjout_prefix_withdraw(peer, p);
> > +			adjout_prefix_withdraw(peer, re->prefix, p);
> >  	}
> >  }
> >  
> > 
> 

-- 
:wq Claudio