From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: rework the rde side of filter_sets
To: tech@openbsd.org
Date: Wed, 4 Feb 2026 11:39:16 +0100

> Changed this to:
> int
> ibuf_recv_filterset_count(struct ibuf *ibuf, uint16_t *count)
> {
>         return ibuf_get_n16(ibuf, count);
> }
> 
> I want this dumb wrapper to correctly decouple the message parsing from
> the rest.

I figured that was the motivation. I like this approach a lot better.

> > > +	case ACTION_SET_RELATIVE_WEIGHT:
> > > +		if (ibuf_get_n32(ibuf, &set->action.relative) == -1)
> > 
> > I'm a bit surprised that gcc doesn't complain about int32_t * vs uint32_t *.
> 
> Did not try this on sparc64.
> Let me add a helper variable here.

I did and it didn't whine (also with egcc).

> > >  				continue;
> > >  			break;
> > >  		case ACTION_SET_NEXTHOP_BLACKHOLE:
> > >  		case ACTION_SET_NEXTHOP_REJECT:
> > >  		case ACTION_SET_NEXTHOP_NOMODIFY:
> > >  		case ACTION_SET_NEXTHOP_SELF:
> > 
> > I don't follow why these ACTION_SET_NEXTHOP_* are ignored wile the
> > ACTION_SET_NEXTHOP_REF is an error.
> 
> These actions do not include any extra data. The type is enough.
> On the other hand ACTION_SET_NEXTHOP_REF is not an allowed type in
> struct filter_set. Similar to ACTION_PFTABLE_ID and ACTION_RTLABEL_ID.
> For rde_filter_set on the other hand ACTION_SET_NEXTHOP, ACTION_RTLABEL
> and ACTION_PFTABLE are not allowed.
> 
> The filter_set needs to pass the un-referenced object (bgpd_addr or
> string) and then the RDE converts them in rde_filterset_conv().

This matches what I thought. Thanks for spelling it out explicitly.

> I could probably change this now to use the same types for the id/ref
> since we now have two distinct objects.
> I should do that cleanup as next.

Makes sense.

> > > +	if ((rfs = calloc(1, sizeof(*rfs) + count * sizeof(*rfse))) == NULL)
> > 
> > Should this check for overflow?
> > 
> > 	if (count > (SIZE_MAX - sizeof(*rfs)) / sizeof(*rfse))
> 
> I think this could be a CTASSERT if at all since the count is limited to
> UINT16_MAX.  sizeof(*rfse) is not that big that it can overflow.

I kind of agree. With count now changed to uint16_t, the overflow check
makes less sense.

ok tb