Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: remove some ACTION_SET types which are no longer needed
To:
tech@openbsd.org
Date:
Wed, 4 Feb 2026 13:32:52 +0100

Download raw body.

Thread
With the filter_set & rde_filter_set_elm split there is no more need
to split ACTION_SET_NEXTHOP and ACTION_SET_NEXTHOP_REF,
ACTION_PFTABLE and ACTION_PFTABLE_ID, and
ACTION_RTLABEL and ACTION_RTLABEL_ID.

This was needed because in the RDE the code was using references and ids
and not the address / name. Now that the objects are split up the
filter_set no longer needs to hold nh_ref and id and rde_filter_set_elm
only uses id and nh_ref. rde_filterset_conv() takes care of the
conversion.

Removes a lot of code that just ensures that no unexpected types sneak
through.

-- 
:wq Claudio

Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.531 bgpd.h
--- bgpd.h	4 Feb 2026 11:41:11 -0000	1.531
+++ bgpd.h	4 Feb 2026 11:51:33 -0000
@@ -1290,7 +1290,6 @@ enum action_types {
 	ACTION_SET_PREPEND_PEER,
 	ACTION_SET_AS_OVERRIDE,
 	ACTION_SET_NEXTHOP,
-	ACTION_SET_NEXTHOP_REF,
 	ACTION_SET_NEXTHOP_REJECT,
 	ACTION_SET_NEXTHOP_BLACKHOLE,
 	ACTION_SET_NEXTHOP_NOMODIFY,
@@ -1298,23 +1297,18 @@ enum action_types {
 	ACTION_DEL_COMMUNITY,
 	ACTION_SET_COMMUNITY,
 	ACTION_PFTABLE,
-	ACTION_PFTABLE_ID,
 	ACTION_RTLABEL,
-	ACTION_RTLABEL_ID,
 	ACTION_SET_ORIGIN
 };
 
-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 nexthop			*nh_ref;
 		struct community		 community;
 		struct bgpd_addr		 nexthop;
 		char				 pftable[PFTABLE_LEN];
Index: bgpd_imsg.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd_imsg.c,v
diff -u -p -r1.1 bgpd_imsg.c
--- bgpd_imsg.c	4 Feb 2026 11:41:11 -0000	1.1
+++ bgpd_imsg.c	4 Feb 2026 11:43:30 -0000
@@ -93,10 +93,6 @@ imsg_send_filterset(struct imsgbuf *imsg
 			if (ibuf_add_n8(msg, s->action.origin) == -1)
 				goto fail;
 			break;
-		case ACTION_SET_NEXTHOP_REF:
-		case ACTION_RTLABEL_ID:
-		case ACTION_PFTABLE_ID:
-			goto fail;
 		}
 	}
 
@@ -176,11 +172,6 @@ ibuf_recv_one_filterset(struct ibuf *ibu
 		if (ibuf_get_n8(ibuf, &set->action.origin) == -1)
 			return -1;
 		break;
-	case ACTION_SET_NEXTHOP_REF:
-	case ACTION_RTLABEL_ID:
-	case ACTION_PFTABLE_ID:
-		errno = EBADMSG;
-		return -1;
 	}
 	return 0;
 }
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
diff -u -p -r1.183 printconf.c
--- printconf.c	4 Nov 2025 11:07:16 -0000	1.183
+++ printconf.c	4 Feb 2026 11:43:46 -0000
@@ -365,12 +365,6 @@ print_set(struct filter_set_head *set)
 			printf("origin ");
 			print_origin(s->action.origin);
 			break;
-		case ACTION_RTLABEL_ID:
-		case ACTION_PFTABLE_ID:
-		case ACTION_SET_NEXTHOP_REF:
-			/* not possible */
-			printf("king bula saiz: config broken");
-			break;
 		}
 	}
 	printf("}");
Index: rde_filter.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_filter.c,v
diff -u -p -r1.141 rde_filter.c
--- rde_filter.c	4 Feb 2026 11:41:11 -0000	1.141
+++ rde_filter.c	4 Feb 2026 11:47:24 -0000
@@ -163,7 +163,7 @@ rde_apply_set(const struct rde_filter_se
 			state->aspath.aspath = aspath_get(np, nl);
 			free(np);
 			break;
-		case ACTION_SET_NEXTHOP_REF:
+		case ACTION_SET_NEXTHOP:
 		case ACTION_SET_NEXTHOP_REJECT:
 		case ACTION_SET_NEXTHOP_BLACKHOLE:
 		case ACTION_SET_NEXTHOP_NOMODIFY:
@@ -179,21 +179,17 @@ rde_apply_set(const struct rde_filter_se
 			community_delete(&state->communities,
 			    &set->action.community, peer);
 			break;
-		case ACTION_PFTABLE_ID:
+		case ACTION_PFTABLE:
 			pftable_unref(state->aspath.pftableid);
 			state->aspath.pftableid = pftable_ref(set->action.id);
 			break;
-		case ACTION_RTLABEL_ID:
+		case ACTION_RTLABEL:
 			rtlabel_unref(state->aspath.rtlabelid);
 			state->aspath.rtlabelid = rtlabel_ref(set->action.id);
 			break;
 		case ACTION_SET_ORIGIN:
 			state->aspath.origin = set->action.origin;
 			break;
-		case ACTION_SET_NEXTHOP:
-		case ACTION_PFTABLE:
-		case ACTION_RTLABEL:
-			fatalx("unexpected filter action in RDE");
 		}
 	}
 }
@@ -563,12 +559,6 @@ filterset_free(struct filter_set_head *s
 
 	while ((s = TAILQ_FIRST(sh)) != NULL) {
 		TAILQ_REMOVE(sh, s, entry);
-		if (s->type == ACTION_RTLABEL_ID)
-			rtlabel_unref(s->action.id);
-		else if (s->type == ACTION_PFTABLE_ID)
-			pftable_unref(s->action.id);
-		else if (s->type == ACTION_SET_NEXTHOP_REF)
-			nexthop_unref(s->action.nh_ref);
 		free(s);
 	}
 }
@@ -593,7 +583,6 @@ filterset_name(enum action_types type)
 	case ACTION_SET_AS_OVERRIDE:
 		return ("as-override");
 	case ACTION_SET_NEXTHOP:
-	case ACTION_SET_NEXTHOP_REF:
 	case ACTION_SET_NEXTHOP_REJECT:
 	case ACTION_SET_NEXTHOP_BLACKHOLE:
 	case ACTION_SET_NEXTHOP_NOMODIFY:
@@ -604,10 +593,8 @@ filterset_name(enum action_types type)
 	case ACTION_DEL_COMMUNITY:
 		return ("community delete");
 	case ACTION_PFTABLE:
-	case ACTION_PFTABLE_ID:
 		return ("pftable");
 	case ACTION_RTLABEL:
-	case ACTION_RTLABEL_ID:
 		return ("rtlabel");
 	case ACTION_SET_ORIGIN:
 		return ("origin");
@@ -678,13 +665,7 @@ filterset_copy(const struct filter_set_h
 	TAILQ_FOREACH(s, source, entry) {
 		if ((t = malloc(sizeof(struct filter_set))) == NULL)
 			fatal(NULL);
-		memcpy(t, s, sizeof(struct filter_set));
-		if (t->type == ACTION_RTLABEL_ID)
-			rtlabel_ref(t->action.id);
-		else if (t->type == ACTION_PFTABLE_ID)
-			pftable_ref(t->action.id);
-		else if (t->type == ACTION_SET_NEXTHOP_REF)
-			nexthop_ref(t->action.nh_ref);
+		*t = *s;
 		TAILQ_INSERT_TAIL(dest, t, entry);
 	}
 }
@@ -725,7 +706,7 @@ rde_filterset_equal(const struct rde_fil
 			if (a->action.relative == b->action.relative)
 				continue;
 			break;
-		case ACTION_SET_NEXTHOP_REF:
+		case ACTION_SET_NEXTHOP:
 			if (a->action.nh_ref == b->action.nh_ref)
 				continue;
 			break;
@@ -740,8 +721,8 @@ rde_filterset_equal(const struct rde_fil
 			    sizeof(a->action.community)) == 0)
 				continue;
 			break;
-		case ACTION_RTLABEL_ID:
-		case ACTION_PFTABLE_ID:
+		case ACTION_RTLABEL:
+		case ACTION_PFTABLE:
 			if (a->action.id == b->action.id)
 				continue;
 			break;
@@ -749,10 +730,6 @@ rde_filterset_equal(const struct rde_fil
 			if (a->action.origin == b->action.origin)
 				continue;
 			break;
-		case ACTION_SET_NEXTHOP:
-		case ACTION_RTLABEL:
-		case ACTION_PFTABLE:
-			fatalx("unexpected filter action in RDE");
 		}
 		/* compare failed */
 		return 0;
@@ -793,11 +770,11 @@ rde_filterset_free(struct rde_filter_set
 
 	rfse = rfs->set;
 	for (i = 0; i < rfs->len; i++, rfse++) {
-		if (rfse->type == ACTION_RTLABEL_ID)
+		if (rfse->type == ACTION_RTLABEL)
 			rtlabel_unref(rfse->action.id);
-		else if (rfse->type == ACTION_PFTABLE_ID)
+		else if (rfse->type == ACTION_PFTABLE)
 			pftable_unref(rfse->action.id);
-		else if (rfse->type == ACTION_SET_NEXTHOP_REF)
+		else if (rfse->type == ACTION_SET_NEXTHOP)
 			nexthop_unref(rfse->action.nh_ref);
 	}
 	free(rfs);
@@ -857,20 +834,13 @@ rde_filterset_conv(const struct filter_s
 		break;
 	case ACTION_SET_NEXTHOP:
 		rfse->action.nh_ref = nexthop_get(&set->action.nexthop);
-		rfse->type = ACTION_SET_NEXTHOP_REF;
 		break;
 	case ACTION_RTLABEL:
 		rfse->action.id = rtlabel_name2id(set->action.rtlabel);
-		rfse->type = ACTION_RTLABEL_ID;
 		break;
 	case ACTION_PFTABLE:
 		rfse->action.id = pftable_name2id(set->action.pftable);
-		rfse->type = ACTION_PFTABLE_ID;
 		break;
-	case ACTION_SET_NEXTHOP_REF:
-	case ACTION_RTLABEL_ID:
-	case ACTION_PFTABLE_ID:
-		fatalx("unexpected filter action in RDE");
 	}
 }
 
Index: rde_rib.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
diff -u -p -r1.288 rde_rib.c
--- rde_rib.c	3 Feb 2026 10:10:35 -0000	1.288
+++ rde_rib.c	4 Feb 2026 11:44:33 -0000
@@ -1328,7 +1328,7 @@ nexthop_modify(struct nexthop *setnh, en
 	case ACTION_SET_NEXTHOP_SELF:
 		*flags = NEXTHOP_SELF;
 		break;
-	case ACTION_SET_NEXTHOP_REF:
+	case ACTION_SET_NEXTHOP:
 		/*
 		 * it is possible that a prefix matches but has the wrong
 		 * address family for the set nexthop. In this case ignore it.