From: claudio <claudio@openbsd.org>
Date: Wed, 18 Jan 2023 17:40:17 +0000 (+0000)
Subject: Use the vstate of the filterstate struct instead of passing an extra copy
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c85bce7bf6bb0ca6a05de7b3c4119694739b2ba2;p=openbsd

Use the vstate of the filterstate struct instead of passing an extra copy
to the various prefix update functions.
While there fix a filterstate leak in up_generate_updates().
With and OK tb@
---

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index cc186250d94..6ca595f5ce7 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.588 2023/01/18 13:20:00 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.589 2023/01/18 17:40:17 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -1744,7 +1744,7 @@ rde_update_update(struct rde_peer *peer, uint32_t path_id,
 
 	/* add original path to the Adj-RIB-In */
 	if (prefix_update(rib_byid(RIB_ADJ_IN), peer, path_id, path_id_tx,
-	    in, prefix, prefixlen, in->vstate) == 1)
+	    in, prefix, prefixlen) == 1)
 		peer->prefix_cnt++;
 
 	/* max prefix checker */
@@ -1772,7 +1772,7 @@ rde_update_update(struct rde_peer *peer, uint32_t path_id,
 			    &state.nexthop->exit_nexthop, prefix,
 			    prefixlen);
 			prefix_update(rib, peer, path_id, path_id_tx, &state,
-			    prefix, prefixlen, in->vstate);
+			    prefix, prefixlen);
 		} else if (prefix_withdraw(rib, peer, path_id, prefix,
 		    prefixlen)) {
 			rde_update_log(wmsg, i, peer,
@@ -3847,8 +3847,7 @@ rde_softreconfig_in(struct rib_entry *re, void *bula)
 				/* update Local-RIB */
 				prefix_update(rib, peer, p->path_id,
 				    p->path_id_tx, &state,
-				    &prefix, pt->prefixlen,
-				    prefix_roa_vstate(p));
+				    &prefix, pt->prefixlen);
 			} else if (action == ACTION_DENY) {
 				/* remove from Local-RIB */
 				prefix_withdraw(rib, peer, p->path_id, &prefix,
@@ -3986,8 +3985,7 @@ rde_roa_softreload(struct rib_entry *re, void *bula)
 				/* update Local-RIB */
 				prefix_update(rib, peer, p->path_id,
 				    p->path_id_tx, &state,
-				    &prefix, pt->prefixlen,
-				    prefix_roa_vstate(p));
+				    &prefix, pt->prefixlen);
 			} else if (action == ACTION_DENY) {
 				/* remove from Local-RIB */
 				prefix_withdraw(rib, peer, p->path_id, &prefix,
@@ -4187,7 +4185,6 @@ network_add(struct network_config *nc, struct filterstate *state)
 	struct filter_set_head	*vpnset = NULL;
 	struct in_addr		 prefix4;
 	struct in6_addr		 prefix6;
-	uint8_t			 vstate;
 	uint16_t		 i;
 	uint32_t		 path_id_tx;
 
@@ -4249,14 +4246,16 @@ network_add(struct network_config *nc, struct filterstate *state)
 		rde_apply_set(vpnset, peerself, peerself, state,
 		    nc->prefix.aid);
 
+	path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen);
+
 #if NOTYET
-	state.aspath.aspa_state = ASPA_NEVER_KNOWN;
+	state->aspath.aspa_state = ASPA_NEVER_KNOWN;
 #endif
-	vstate = rde_roa_validity(&rde_roa, &nc->prefix,
+	state->vstate = rde_roa_validity(&rde_roa, &nc->prefix,
 	    nc->prefixlen, aspath_origin(state->aspath.aspath));
-	path_id_tx = pathid_assign(peerself, 0, &nc->prefix, nc->prefixlen);
+
 	if (prefix_update(rib_byid(RIB_ADJ_IN), peerself, 0, path_id_tx,
-	    state, &nc->prefix, nc->prefixlen, vstate) == 1)
+	    state, &nc->prefix, nc->prefixlen) == 1)
 		peerself->prefix_cnt++;
 	for (i = RIB_LOC_START; i < rib_size; i++) {
 		struct rib *rib = rib_byid(i);
@@ -4266,7 +4265,7 @@ network_add(struct network_config *nc, struct filterstate *state)
 		    state->nexthop ? &state->nexthop->exit_nexthop : NULL,
 		    &nc->prefix, nc->prefixlen);
 		prefix_update(rib, peerself, 0, path_id_tx, state, &nc->prefix,
-		    nc->prefixlen, vstate);
+		    nc->prefixlen);
 	}
 	filterset_free(&nc->attrset);
 }
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index c6a192d68fe..c14b2dba9ca 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.279 2023/01/17 16:09:01 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.280 2023/01/18 17:40:17 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -624,14 +624,12 @@ struct prefix	*prefix_adjout_lookup(struct rde_peer *, struct bgpd_addr *,
 		    int);
 struct prefix	*prefix_adjout_next(struct rde_peer *, struct prefix *);
 int		 prefix_update(struct rib *, struct rde_peer *, uint32_t,
-		    uint32_t, struct filterstate *, struct bgpd_addr *,
-		    int, uint8_t);
+		    uint32_t, struct filterstate *, struct bgpd_addr *, int);
 int		 prefix_withdraw(struct rib *, struct rde_peer *, uint32_t,
 		    struct bgpd_addr *, int);
 void		 prefix_add_eor(struct rde_peer *, uint8_t);
 void		 prefix_adjout_update(struct prefix *, struct rde_peer *,
-		    struct filterstate *, struct bgpd_addr *, int,
-		    uint32_t, uint8_t);
+		    struct filterstate *, struct bgpd_addr *, int, uint32_t);
 void		 prefix_adjout_withdraw(struct prefix *);
 void		 prefix_adjout_destroy(struct prefix *);
 void		 prefix_adjout_dump(struct rde_peer *, void *,
diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c
index 4325451cd8a..1bf575ca15e 100644
--- a/usr.sbin/bgpd/rde_rib.c
+++ b/usr.sbin/bgpd/rde_rib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_rib.c,v 1.251 2022/12/28 21:30:16 jmc Exp $ */
+/*	$OpenBSD: rde_rib.c,v 1.252 2023/01/18 17:40:17 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -955,7 +955,7 @@ prefix_adjout_match(struct rde_peer *peer, struct bgpd_addr *addr)
 int
 prefix_update(struct rib *rib, struct rde_peer *peer, uint32_t path_id,
     uint32_t path_id_tx, struct filterstate *state, struct bgpd_addr *prefix,
-    int prefixlen, uint8_t vstate)
+    int prefixlen)
 {
 	struct rde_aspath	*asp, *nasp = &state->aspath;
 	struct rde_community	*comm, *ncomm = &state->communities;
@@ -973,7 +973,7 @@ prefix_update(struct rib *rib, struct rde_peer *peer, uint32_t path_id,
 		    path_compare(nasp, prefix_aspath(p)) == 0) {
 			/* no change, update last change */
 			p->lastchange = getmonotime();
-			p->validation_state = vstate;
+			p->validation_state = state->vstate;
 			return (0);
 		}
 	}
@@ -997,11 +997,11 @@ prefix_update(struct rib *rib, struct rde_peer *peer, uint32_t path_id,
 	/* If the prefix was found move it else add it to the RIB. */
 	if (p != NULL)
 		return (prefix_move(p, peer, asp, comm, state->nexthop,
-		    state->nhflags, vstate));
+		    state->nhflags, state->vstate));
 	else
 		return (prefix_add(prefix, prefixlen, rib, peer, path_id,
 		    path_id_tx, asp, comm, state->nexthop, state->nhflags,
-		    vstate));
+		    state->vstate));
 }
 
 /*
@@ -1123,7 +1123,7 @@ prefix_add_eor(struct rde_peer *peer, uint8_t aid)
 void
 prefix_adjout_update(struct prefix *p, struct rde_peer *peer,
     struct filterstate *state, struct bgpd_addr *prefix, int prefixlen,
-    uint32_t path_id_tx, uint8_t vstate)
+    uint32_t path_id_tx)
 {
 	struct rde_aspath *asp;
 	struct rde_community *comm;
@@ -1160,7 +1160,7 @@ prefix_adjout_update(struct prefix *p, struct rde_peer *peer,
 		    prefix_communities(p)) &&
 		    path_compare(&state->aspath, prefix_aspath(p)) == 0) {
 			/* nothing changed */
-			p->validation_state = vstate;
+			p->validation_state = state->vstate;
 			p->lastchange = getmonotime();
 			p->flags &= ~PREFIX_FLAG_STALE;
 			return;
@@ -1205,7 +1205,7 @@ prefix_adjout_update(struct prefix *p, struct rde_peer *peer,
 	}
 
 	prefix_link(p, NULL, p->pt, peer, 0, p->path_id_tx, asp, comm,
-	    state->nexthop, state->nhflags, vstate);
+	    state->nexthop, state->nhflags, state->vstate);
 	peer->prefix_out_cnt++;
 
 	if (p->flags & PREFIX_FLAG_MASK)
diff --git a/usr.sbin/bgpd/rde_update.c b/usr.sbin/bgpd/rde_update.c
index 5af1473fb2c..c1e5a5aee4d 100644
--- a/usr.sbin/bgpd/rde_update.c
+++ b/usr.sbin/bgpd/rde_update.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_update.c,v 1.151 2023/01/12 17:35:51 claudio Exp $ */
+/*	$OpenBSD: rde_update.c,v 1.152 2023/01/18 17:40:17 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -199,15 +199,16 @@ up_generate_updates(struct filter_head *rules, struct rde_peer *peer,
 		}
 
 		/* check if this was actually a withdraw */
-		if (need_withdraw)
+		if (need_withdraw) {
+			rde_filterstate_clean(&state);
 			break;
+		}
 
 		/* from here on we know this is an update */
 
 		up_prep_adjout(peer, &state, addr.aid);
 		prefix_adjout_update(p, peer, &state, &addr,
-		    new->pt->prefixlen, new->path_id_tx,
-		    prefix_roa_vstate(new));
+		    new->pt->prefixlen, new->path_id_tx);
 		rde_filterstate_clean(&state);
 
 		/* max prefix checker outbound */
@@ -337,8 +338,7 @@ up_generate_addpath(struct filter_head *rules, struct rde_peer *peer,
 
 		up_prep_adjout(peer, &state, addr.aid);
 		prefix_adjout_update(p, peer, &state, &addr,
-		    new->pt->prefixlen, new->path_id_tx,
-		    prefix_roa_vstate(new));
+		    new->pt->prefixlen, new->path_id_tx);
 		rde_filterstate_clean(&state);
 
 		/* max prefix checker outbound */
@@ -441,7 +441,7 @@ up_generate_addpath_all(struct filter_head *rules, struct rde_peer *peer,
 
 		up_prep_adjout(peer, &state, addr.aid);
 		prefix_adjout_update(p, peer, &state, &addr,
-		    prefixlen, new->path_id_tx, prefix_roa_vstate(new));
+		    prefixlen, new->path_id_tx);
 		rde_filterstate_clean(&state);
 
 		/* max prefix checker outbound */
@@ -509,7 +509,7 @@ up_generate_default(struct filter_head *rules, struct rde_peer *peer,
 	}
 
 	up_prep_adjout(peer, &state, addr.aid);
-	prefix_adjout_update(p, peer, &state, &addr, 0, 0, ROA_NOTFOUND);
+	prefix_adjout_update(p, peer, &state, &addr, 0, 0);
 	rde_filterstate_clean(&state);
 
 	/* max prefix checker outbound */
@@ -1028,7 +1028,6 @@ up_dump_attrnlri(u_char *buf, int len, struct rde_peer *peer)
 		goto done;
 
 	rde_filterstate_prep(&state, p);
-
 	r = up_generate_attr(buf + 2, len - 2, peer, &state, AID_INET);
 	rde_filterstate_clean(&state);
 	if (r == -1) {