From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: resize communities with reallocarray
To: tech@openbsd.org
Date: Thu, 13 Nov 2025 16:12:41 +0100

On Thu, Nov 13, 2025 at 08:06:08AM -0700, Theo de Raadt wrote:
> I'm very surprised recallocarray() was being used to always create a
> new object, resulting in large amount of munmap and mmap.  That was
> a strange choice.

Yes, resizing an array precisely is rarely the right thing to do. The
prohibitive cost of recallocarray() is largely amortized if you resort
to a doubling scheme (where realloc normally has to copy anyway).

> Is the + 8 growth sufficent, or can you pre-allocate even more to avoid
> repeat hits?

claudio and I also discussed this. Let's do this in two steps.

> 
> > insert_community() is part of the hot path especially when people use
> > communities to tag internally for their filters.
> > Using recallocarray() there is a bit problematic since it has a much
> > higher overhead than any realloc verision.
> > Use reallocarray() and memset() instead.
> > 
> > In one of my IXP test setups insert_community is called 224 million times
> > during initial sync.
> > -- 
> > :wq Claudio
> > 
> > Index: rde_community.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/bgpd/rde_community.c,v
> > diff -u -p -r1.18 rde_community.c
> > --- rde_community.c	4 Nov 2025 14:43:22 -0000	1.18
> > +++ rde_community.c	13 Nov 2025 14:28:42 -0000
> > @@ -227,9 +227,10 @@ insert_community(struct rde_community *c
> >  		struct community *new;
> >  		int newsize = comm->size + 8;
> >  
> > -		if ((new = recallocarray(comm->communities, comm->size,
> > -		    newsize, sizeof(struct community))) == NULL)
> > +		if ((new = reallocarray(comm->communities, newsize,
> > +		    sizeof(struct community))) == NULL)
> >  			fatal(__func__);
> > +		memset(&new[comm->size], 0, sizeof(struct community) * 8);
> >  		comm->communities = new;
> >  		comm->size = newsize;
> >  	}
> > 
>