From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: factor filter_set imsg handling into own functions
To: tech@openbsd.org
Date: Wed, 3 Dec 2025 11:10:32 +0100

On Wed, Dec 03, 2025 at 10:48:14AM +0100, Claudio Jeker wrote:
> The handling of filter_set in the RDE is not optimal. It is time to
> optimise rde_apply_set() and this is the first step.
> 
> Having filterset_send and recv will allow to use a different struct inside
> the RDE and drop a lot of fat from that struct.
> This is the first trivial step.

ok, some grumbling below.

> Index: rde_filter.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
> diff -u -p -r1.136 rde_filter.c
> --- rde_filter.c	9 May 2023 13:11:19 -0000	1.136
> +++ rde_filter.c	3 Dec 2025 09:35:20 -0000
> @@ -743,6 +743,37 @@ filterset_name(enum action_types type)
>  	fatalx("filterset_name: got lost");
>  }
>  
> +int
> +filterset_send(struct imsgbuf *i, struct filter_set_head *set)

I understand that this just moves code around, but i really isn't my
preferred name for a struct imsgbuf. Not sure I like the ibuf* in the
callers better, though :)

> +{
> +	struct filter_set	*s;
> +
> +	TAILQ_FOREACH(s, set, entry)
> +		if (imsg_compose(i, IMSG_FILTER_SET, 0, 0, -1, s,
> +		    sizeof(struct filter_set)) == -1)

can this use sizeof(*s) like in filterset_recv?

> +			return (-1);
> +	return (0);
> +}
> +
> +void
> +filterset_recv(struct imsg *imsg, struct filter_set_head *set)
> +{
> +	struct filter_set	*s;
> +
> +	if ((s = malloc(sizeof(*s))) == NULL)
> +		fatal(NULL);
> +	if (imsg_get_data(imsg, s, sizeof(*s)) == -1) {
> +		log_warnx("rde_dispatch: wrong imsg len");
> +		free(s);
> +		return;
> +	}
> +	if (s->type == ACTION_SET_NEXTHOP) {
> +		s->action.nh_ref = nexthop_get(&s->action.nexthop);
> +		s->type = ACTION_SET_NEXTHOP_REF;
> +	}
> +	TAILQ_INSERT_TAIL(set, s, entry);
> +}
> +
>  /*
>   * Copyright (c) 2001 Daniel Hartmeier
>   * All rights reserved.
>