From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: factor filter_set imsg handling into own functions
To: tech@openbsd.org
Date: Wed, 3 Dec 2025 10:48:14 +0100

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.
-- 
:wq Claudio

Index: bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
diff -u -p -r1.285 bgpd.c
--- bgpd.c	4 Nov 2025 10:47:25 -0000	1.285
+++ bgpd.c	2 Dec 2025 15:49:25 -0000
@@ -45,7 +45,6 @@ void		sighdlr(int);
 __dead void	usage(void);
 int		main(int, char *[]);
 pid_t		start_child(enum bgpd_process, char *, int, int, int);
-int		send_filterset(struct imsgbuf *, struct filter_set_head *);
 int		reconfigure(const char *, struct bgpd_config *);
 int		send_config(struct bgpd_config *);
 int		dispatch_imsg(struct imsgbuf *, int, struct bgpd_config *);
@@ -565,18 +564,6 @@ start_child(enum bgpd_process p, char *a
 }
 
 int
-send_filterset(struct imsgbuf *i, struct filter_set_head *set)
-{
-	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)
-			return (-1);
-	return (0);
-}
-
-int
 reconfigure(const char *conffile, struct bgpd_config *conf)
 {
 	struct bgpd_config	*new_conf;
@@ -685,7 +672,7 @@ send_config(struct bgpd_config *conf)
 			if (imsg_compose(ibuf_rde, IMSG_FLOWSPEC_ADD, 0, 0, -1,
 			    f->flow, FLOWSPEC_SIZE + f->flow->len) == -1)
 				return (-1);
-			if (send_filterset(ibuf_rde, &f->attrset) == -1)
+			if (filterset_send(ibuf_rde, &f->attrset) == -1)
 				return (-1);
 			if (imsg_compose(ibuf_rde, IMSG_FLOWSPEC_DONE, 0, 0, -1,
 			    NULL, 0) == -1)
@@ -794,7 +781,7 @@ send_config(struct bgpd_config *conf)
 	/* filters for the RDE */
 	while ((r = TAILQ_FIRST(conf->filters)) != NULL) {
 		TAILQ_REMOVE(conf->filters, r, entry);
-		if (send_filterset(ibuf_rde, &r->set) == -1)
+		if (filterset_send(ibuf_rde, &r->set) == -1)
 			return (-1);
 		if (imsg_compose(ibuf_rde, IMSG_RECONF_FILTER, 0, 0, -1,
 		    r, sizeof(struct filter_rule)) == -1)
@@ -819,7 +806,7 @@ send_config(struct bgpd_config *conf)
 			return (-1);
 
 		/* export targets */
-		if (send_filterset(ibuf_rde, &vpn->export) == -1)
+		if (filterset_send(ibuf_rde, &vpn->export) == -1)
 			return (-1);
 		if (imsg_compose(ibuf_rde, IMSG_RECONF_VPN_EXPORT, 0, 0,
 		    -1, NULL, 0) == -1)
@@ -827,7 +814,7 @@ send_config(struct bgpd_config *conf)
 		filterset_free(&vpn->export);
 
 		/* import targets */
-		if (send_filterset(ibuf_rde, &vpn->import) == -1)
+		if (filterset_send(ibuf_rde, &vpn->import) == -1)
 			return (-1);
 		if (imsg_compose(ibuf_rde, IMSG_RECONF_VPN_IMPORT, 0, 0,
 		    -1, NULL, 0) == -1)
@@ -1183,7 +1170,7 @@ send_network(int type, struct network_co
 	/* networks that get deleted don't need to send the filter set */
 	if (type == IMSG_NETWORK_REMOVE)
 		return (0);
-	if (send_filterset(ibuf_rde, h) == -1)
+	if (filterset_send(ibuf_rde, h) == -1)
 		return (-1);
 	if (imsg_compose(ibuf_rde, IMSG_NETWORK_DONE, 0, 0, -1, NULL, 0) == -1)
 		return (-1);
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.523 bgpd.h
--- bgpd.h	2 Dec 2025 10:50:19 -0000	1.523
+++ bgpd.h	3 Dec 2025 09:35:35 -0000
@@ -1299,19 +1299,19 @@ enum action_types {
 struct nexthop;
 struct filter_set {
 	TAILQ_ENTRY(filter_set)		entry;
+	enum action_types		type;
 	union {
 		uint8_t				 prepend;
+		uint8_t				 origin;
 		uint16_t			 id;
 		uint32_t			 metric;
 		int32_t				 relative;
-		struct bgpd_addr		 nexthop;
 		struct nexthop			*nh_ref;
 		struct community		 community;
+		struct bgpd_addr		 nexthop;
 		char				 pftable[PFTABLE_LEN];
 		char				 rtlabel[ROUTELABEL_LEN];
-		uint8_t				 origin;
 	}				action;
-	enum action_types		type;
 };
 
 struct roa_set {
@@ -1577,6 +1577,8 @@ int	filterset_cmp(struct filter_set *, s
 void	filterset_move(struct filter_set_head *, struct filter_set_head *);
 void	filterset_copy(struct filter_set_head *, struct filter_set_head *);
 const char	*filterset_name(enum action_types);
+int	filterset_send(struct imsgbuf *, struct filter_set_head *);
+void	filterset_recv(struct imsg *, struct filter_set_head *);
 
 /* rde_sets.c */
 struct as_set	*as_sets_lookup(struct as_set_head *, const char *);
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.672 rde.c
--- rde.c	2 Dec 2025 13:03:35 -0000	1.672
+++ rde.c	2 Dec 2025 16:47:14 -0000
@@ -412,7 +412,6 @@ rde_dispatch_imsg_session(struct imsgbuf
 	struct peer_config	 pconf;
 	struct rde_peer		*peer;
 	struct rde_aspath	*asp;
-	struct filter_set	*s;
 	struct as_set		*aset;
 	struct rde_prefixset	*pset;
 	ssize_t			 n;
@@ -698,20 +697,7 @@ badnetdel:
 			    flowspec_flush_upcall, NULL);
 			break;
 		case IMSG_FILTER_SET:
-			if ((s = malloc(sizeof(struct filter_set))) == NULL)
-				fatal(NULL);
-			if (imsg_get_data(&imsg, s, sizeof(struct filter_set))
-			    == -1) {
-				log_warnx("rde_dispatch: wrong imsg len");
-				free(s);
-				break;
-			}
-			if (s->type == ACTION_SET_NEXTHOP) {
-				s->action.nh_ref =
-				    nexthop_get(&s->action.nexthop);
-				s->type = ACTION_SET_NEXTHOP_REF;
-			}
-			TAILQ_INSERT_TAIL(&session_set, s, entry);
+			filterset_recv(&imsg, &session_set);
 			break;
 		case IMSG_CTL_SHOW_NETWORK:
 		case IMSG_CTL_SHOW_RIB:
@@ -863,7 +849,6 @@ rde_dispatch_imsg_parent(struct imsgbuf 
 	struct imsgbuf		*i;
 	struct filter_head	*nr;
 	struct filter_rule	*r;
-	struct filter_set	*s;
 	struct rib		*rib;
 	struct rde_prefixset	*ps;
 	struct rde_aspath	*asp;
@@ -1231,16 +1216,7 @@ rde_dispatch_imsg_parent(struct imsgbuf 
 			nexthop_update(&knext);
 			break;
 		case IMSG_FILTER_SET:
-			if ((s = malloc(sizeof(*s))) == NULL)
-				fatal(NULL);
-			if (imsg_get_data(&imsg, s, sizeof(*s)) == -1)
-				fatalx("IMSG_FILTER_SET bad len");
-			if (s->type == ACTION_SET_NEXTHOP) {
-				s->action.nh_ref =
-				    nexthop_get(&s->action.nexthop);
-				s->type = ACTION_SET_NEXTHOP_REF;
-			}
-			TAILQ_INSERT_TAIL(&parent_set, s, entry);
+			filterset_recv(&imsg, &parent_set);
 			break;
 		case IMSG_MRT_OPEN:
 		case IMSG_MRT_REOPEN:
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)
+{
+	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)
+			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.