Major cleanup in rde_update.c. Merge equal code used in different places

into own functions. Move up_dump_upcall() into rde.c and rename it
rde_up_dump_upcall(). This is needed for the next step.
up_test_update() tests if an update or withdraw is needed and up_generate()
creates the updates.  "get it in" henning@ (he is eager on softreconfig out)

3 files changed, 174 insertions, 269 deletions

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index 07077ca53d0..d2dcd3559ca 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.175 2005/11/01 10:58:29 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.176 2005/11/01 14:37:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -65,6 +65,7 @@ void		 rde_dump_upcall(struct pt_entry *, void *);
 void		 rde_dump_as(struct filter_as *, pid_t);
 void		 rde_dump_prefix_upcall(struct pt_entry *, void *);
 void		 rde_dump_prefix(struct ctl_show_rib_prefix *, pid_t);
+void		 rde_up_dump_upcall(struct pt_entry *, void *);
 void		 rde_update_queue_runner(void);
 void		 rde_update6_queue_runner(void);
 
@@ -1776,6 +1777,16 @@ rde_send_nexthop(struct bgpd_addr *next, int valid)
 u_char	queue_buf[4096];
 
 void
+rde_up_dump_upcall(struct pt_entry *pt, void *ptr)
+{
+	struct rde_peer		*peer = ptr;
+
+	if (pt->active == NULL)
+		return;
+	up_generate_updates(peer, pt->active, NULL);
+}
+
+void
 rde_generate_updates(struct prefix *new, struct prefix *old)
 {
 	struct rde_peer			*peer;
@@ -2141,7 +2152,7 @@ peer_dump(u_int32_t id, u_int16_t afi, u_int8_t safi)
 			    ANNOUNCE_DEFAULT_ROUTE)
 				up_generate_default(peer, AF_INET);
 			else
-				pt_dump(up_dump_upcall, peer, AF_INET);
+				pt_dump(rde_up_dump_upcall, peer, AF_INET);
 		}
 	if (afi == AFI_ALL || afi == AFI_IPv6)
 		if (safi == SAFI_ALL || safi == SAFI_UNICAST ||
@@ -2150,7 +2161,7 @@ peer_dump(u_int32_t id, u_int16_t afi, u_int8_t safi)
 			    ANNOUNCE_DEFAULT_ROUTE)
 				up_generate_default(peer, AF_INET6);
 			else
-				pt_dump(up_dump_upcall, peer, AF_INET6);
+				pt_dump(rde_up_dump_upcall, peer, AF_INET6);
 		}
 }
 
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 1bef56b5e49..b9fd7b8a41a 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.70 2005/08/10 08:34:06 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.71 2005/11/01 14:37:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -324,9 +324,13 @@ int		 nexthop_compare(struct nexthop *, struct nexthop *);
 
 /* rde_decide.c */
 void		 prefix_evaluate(struct prefix *, struct pt_entry *);
+
+/* rde_update.c */
 void		 up_init(struct rde_peer *);
 void		 up_down(struct rde_peer *);
-void		 up_dump_upcall(struct pt_entry *, void *);
+int		 up_test_update(struct rde_peer *, struct prefix *);
+int		 up_generate(struct rde_peer *, struct rde_aspath *,
+		     struct bgpd_addr *, u_int8_t);
 void		 up_generate_updates(struct rde_peer *,
 		     struct prefix *, struct prefix *);
 void		 up_generate_default(struct rde_peer *, sa_family_t);
diff --git a/usr.sbin/bgpd/rde_update.c b/usr.sbin/bgpd/rde_update.c
index 6229004f05b..86b21cb8b16 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.41 2005/11/01 10:58:29 claudio Exp $ */
+/*	$OpenBSD: rde_update.c,v 1.42 2005/11/01 14:37:16 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -253,113 +253,165 @@ up_add(struct rde_peer *peer, struct update_prefix *p, struct update_attr *a)
 	return (0);
 }
 
-void
-up_generate_updates(struct rde_peer *peer,
-    struct prefix *new, struct prefix *old)
+int
+up_test_update(struct rde_peer *peer, struct prefix *p)
 {
-	struct update_attr		*a;
-	struct update_prefix		*p;
-	struct attr			*attr;
-	struct rde_aspath		*fasp;
-	struct bgpd_addr		 addr;
+	struct bgpd_addr	 addr;
+	struct attr		*attr;
 
 	if (peer->state != PEER_UP)
-		return;
+		return (-1);
 
-	if (new == NULL || (new->aspath->nexthop != NULL &&
-	    new->aspath->nexthop->state != NEXTHOP_REACH)) {
-		if (old == NULL)
-			/* new prefix got filtered and no old prefix avail */
-			return;
+	if (p == NULL)
+		/* no prefix available */
+		return (0);
 
-		if (peer == old->peer)
-			/* Do not send routes back to sender */
-			return;
+	if (peer == p->peer)
+		/* Do not send routes back to sender */
+		return (0);
 
-		pt_getaddr(old->prefix, &addr);
-		switch (addr.af) {
-		case AF_INET:
-			if (peer->capa_announced.mp_v4 == SAFI_NONE &&
-			    peer->capa_received.mp_v6 != SAFI_NONE)
-				return;
-			break;
-		case AF_INET6:
-			if (peer->capa_announced.mp_v6 == SAFI_NONE)
-				return;
-			break;
-		}
+	pt_getaddr(p->prefix, &addr);
+	switch (addr.af) {
+	case AF_INET:
+		if (peer->capa_announced.mp_v4 == SAFI_NONE &&
+		    peer->capa_received.mp_v6 != SAFI_NONE)
+			return (-1);
+		break;
+	case AF_INET6:
+		if (peer->capa_announced.mp_v6 == SAFI_NONE)
+			return (-1);
+		break;
+	}
 
-		if (peer->conf.ebgp &&
-		    !aspath_loopfree(old->aspath->aspath,
-		    peer->conf.remote_as))
-			/*
-			 * Do not send routes back to sender which would
-			 * cause a aspath loop.
-			 */
-			return;
+	if (peer->conf.ebgp && !aspath_loopfree(p->aspath->aspath, 
+	    peer->conf.remote_as)) {
+		/*
+		 * Do not send routes back to sender which would
+		 * cause an aspath loop.
+		 */
+		return (0);
+	}
 
-		if (old->peer->conf.ebgp == 0 && peer->conf.ebgp == 0) {
-			/*
-			 * route reflector redistribution rules:
-			 * 1. if announce is set		-> announce
-			 * 2. old non-client, new non-client	-> no
-			 * 3. old client, new non-client	-> yes
-			 * 4. old non-client, new client	-> yes
-			 * 5. old client, new client		-> yes
-			 */
-			if (old->peer->conf.reflector_client == 0 &&
-			    peer->conf.reflector_client == 0 &&
-			    (old->aspath->flags & F_PREFIX_ANNOUNCED) == 0)
-				/* Do not redistribute updates to ibgp peers */
-				return;
-		}
+	if (p->peer->conf.ebgp == 0 && peer->conf.ebgp == 0) {
+		/*
+		 * route reflector redistribution rules:
+		 * 1. if announce is set                -> announce
+		 * 2. old non-client, new non-client    -> no
+		 * 3. old client, new non-client        -> yes
+		 * 4. old non-client, new client        -> yes
+		 * 5. old client, new client            -> yes
+		 */
+		if (p->peer->conf.reflector_client == 0 &&
+		    peer->conf.reflector_client == 0 &&
+		    (p->aspath->flags & F_PREFIX_ANNOUNCED) == 0)
+			/* Do not redistribute updates to ibgp peers */
+			return (0);
+	}
 
-		/* announce type handling */
-		switch (peer->conf.announce_type) {
-		case ANNOUNCE_UNDEF:
-		case ANNOUNCE_NONE:
-		case ANNOUNCE_DEFAULT_ROUTE:
-			return;
-		case ANNOUNCE_ALL:
-			break;
-		case ANNOUNCE_SELF:
-			/*
-			 * pass only prefix that have a aspath count
-			 * of zero this is equal to the ^$ regex.
-			 */
-			if (old->aspath->aspath->ascnt != 0)
-				return;
-			break;
+	/* announce type handling */
+	switch (peer->conf.announce_type) {
+	case ANNOUNCE_UNDEF:
+	case ANNOUNCE_NONE:
+	case ANNOUNCE_DEFAULT_ROUTE:
+		/*
+		 * no need to withdraw old prefix as this will be
+		 * filtered out as well.
+		 */
+		return (-1);
+	case ANNOUNCE_ALL:
+		break;
+	case ANNOUNCE_SELF:
+		/*
+		 * pass only prefix that have a aspath count
+		 * of zero this is equal to the ^$ regex.
+		 */
+		if (p->aspath->aspath->ascnt != 0)
+			return (0);
+		break;
+	}
+
+	/* well known communities */
+	if (rde_filter_community(p->aspath,
+	    COMMUNITY_WELLKNOWN, COMMUNITY_NO_ADVERTISE))
+		return (0);
+	if (peer->conf.ebgp && rde_filter_community(p->aspath,
+	    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPORT))
+		return (0);
+	if (peer->conf.ebgp && rde_filter_community(p->aspath,
+	    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPSUBCONFED))
+		return (0);
+
+	/*
+	 * Don't send messages back to originator
+	 * this is not specified in the RFC but seems logical.
+	 */
+	if ((attr = attr_optget(p->aspath, ATTR_ORIGINATOR_ID)) != NULL) {
+		if (memcmp(attr->data, &peer->remote_bgpid,
+		    sizeof(peer->remote_bgpid)) == 0) {
+			/* would cause loop don't send */
+			return (-1);
 		}
+	}
 
-		/* well known communities */
-		if (rde_filter_community(old->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_ADVERTISE))
-			return;
-		if (peer->conf.ebgp && rde_filter_community(old->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPORT))
-			return;
-		if (peer->conf.ebgp && rde_filter_community(old->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPSUBCONFED))
-			return;
+	return (1);
+}
+
+int
+up_generate(struct rde_peer *peer, struct rde_aspath *asp,
+    struct bgpd_addr *addr, u_int8_t prefixlen)
+{
+	struct update_attr		*ua = NULL;
+	struct update_prefix		*up;
+
+	if (asp) {
+		ua = calloc(1, sizeof(struct update_attr));
+		if (ua == NULL)
+			fatal("up_generate_updates");
 
+		if (up_generate_attr(peer, ua, asp, addr->af) == -1) {
+			log_warnx("generation of bgp path attributes failed");
+			free(ua);
+			return (-1);
+		}
 		/*
-		 * Don't send messages back to originator
-		 * this is not specified in the RFC but seems logical.
+		 * use aspath_hash as attr_hash, this may be unoptimal
+		 * but currently I don't care.
 		 */
-		if ((attr = attr_optget(old->aspath,
-		    ATTR_ORIGINATOR_ID)) != NULL) {
-			if (memcmp(attr->data, &peer->remote_bgpid,
-			    sizeof(peer->remote_bgpid)) == 0)
-				/* would cause loop don't send */
-				return;
-		}
+		ua->attr_hash = aspath_hash(asp->aspath->data,
+		    asp->aspath->len);
+	}
+
+	up = calloc(1, sizeof(struct update_prefix));
+	if (up == NULL)
+		fatal("up_generate_updates");
+	up->prefix = *addr;
+	up->prefixlen = prefixlen;
+
+	if (up_add(peer, up, ua) == -1)
+		return (-1);
+
+	return (0);
+}
+
+void
+up_generate_updates(struct rde_peer *peer,
+    struct prefix *new, struct prefix *old)
+{
+	struct rde_aspath		*fasp;
+	struct bgpd_addr		 addr;
+
+	if (peer->state != PEER_UP)
+		return;
+
+	if (new == NULL || (new->aspath->nexthop != NULL &&
+	    new->aspath->nexthop->state != NEXTHOP_REACH)) {
+		if (up_test_update(peer, old) != 1)
+			return;
 
 		/* copy attributes for output filter */
 		fasp = path_copy(old->aspath);
 
-		/* default override not needed here as this is a withdraw */
-
+		pt_getaddr(old->prefix, &addr);
 		if (rde_filter(peer, fasp, &addr, old->prefix->prefixlen,
 		    old->peer, DIR_OUT) == ACTION_DENY) {
 			path_put(fasp);
@@ -368,107 +420,22 @@ up_generate_updates(struct rde_peer *peer,
 		path_put(fasp);
 
 		/* withdraw prefix */
-		p = calloc(1, sizeof(struct update_prefix));
-		if (p == NULL)
-			fatal("up_generate_updates");
-
-		p->prefix = addr;
-		p->prefixlen = old->prefix->prefixlen;
-		if (up_add(peer, p, NULL) == -1)
-			log_warnx("queuing withdraw failed.");
+		up_generate(peer, NULL, &addr, old->prefix->prefixlen);
 	} else {
-		if (peer == new->peer) {
-			/* Do not send routes back to sender */
-			up_generate_updates(peer, NULL, old);
-			return;
-		}
-
-		pt_getaddr(new->prefix, &addr);
-		switch (addr.af) {
-		case AF_INET:
-			if (peer->capa_announced.mp_v4 == SAFI_NONE &&
-			    peer->capa_received.mp_v6 != SAFI_NONE)
-				return;
-			break;
-		case AF_INET6:
-			if (peer->capa_announced.mp_v6 == SAFI_NONE)
-				return;
-			break;
-		}
-
-		if (peer->conf.ebgp &&
-		    !aspath_loopfree(new->aspath->aspath,
-		    peer->conf.remote_as)) {
-			/*
-			 * Do not send routes back to sender which would
-			 * cause a aspath loop.
-			 */
-			up_generate_updates(peer, NULL, old);
-			return;
-		}
-
-		if (new->peer->conf.ebgp == 0 && peer->conf.ebgp == 0) {
-			/*
-			 * route reflector redistribution rules:
-			 * 1. if announce is set		-> announce
-			 * 2. old non-client, new non-client	-> no
-			 * 3. old client, new non-client	-> yes
-			 * 4. old non-client, new client	-> yes
-			 * 5. old client, new client		-> yes
-			 */
-			if (new->peer->conf.reflector_client == 0 &&
-			    peer->conf.reflector_client == 0 &&
-			    (new->aspath->flags & F_PREFIX_ANNOUNCED) == 0) {
-				/* Do not redistribute updates to ibgp peers */
-				up_generate_updates(peer, NULL, old);
-				return;
-			}
-		}
-
-		/* announce type handling */
-		switch (peer->conf.announce_type) {
-		case ANNOUNCE_UNDEF:
-		case ANNOUNCE_NONE:
-		case ANNOUNCE_DEFAULT_ROUTE:
-			/*
-			 * no need to withdraw old prefix as this will be
-			 * filtered out as well.
-			 */
-			return;
-		case ANNOUNCE_ALL:
+		switch (up_test_update(peer, new)) {
+		case 1:
 			break;
-		case ANNOUNCE_SELF:
-			/*
-			 * pass only prefix that have a aspath count
-			 * of zero this is equal to the ^$ regex.
-			 */
-			if (new->aspath->aspath->ascnt != 0) {
-				up_generate_updates(peer, NULL, old);
-				return;
-			}
-			break;
-		}
-
-		/* well known communities */
-		if (rde_filter_community(new->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_ADVERTISE)) {
-			up_generate_updates(peer, NULL, old);
-			return;
-		}
-		if (peer->conf.ebgp && rde_filter_community(new->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPORT)) {
+		case 0:
 			up_generate_updates(peer, NULL, old);
 			return;
-		}
-		if (peer->conf.ebgp && rde_filter_community(new->aspath,
-		    COMMUNITY_WELLKNOWN, COMMUNITY_NO_EXPSUBCONFED)) {
-			up_generate_updates(peer, NULL, old);
+		case -1:
 			return;
 		}
 
 		/* copy attributes for output filter */
 		fasp = path_copy(new->aspath);
 
+		pt_getaddr(new->prefix, &addr);
 		if (rde_filter(peer, fasp, &addr, new->prefix->prefixlen,
 		    new->peer, DIR_OUT) == ACTION_DENY) {
 			path_put(fasp);
@@ -476,50 +443,11 @@ up_generate_updates(struct rde_peer *peer,
 			return;
 		}
 
-		/*
-		 * Don't send messages back to originator
-		 * this is not specified in the RFC but seems logical.
-		 */
-		if ((attr = attr_optget(new->aspath,
-		    ATTR_ORIGINATOR_ID)) != NULL) {
-			if (memcmp(attr->data, &peer->remote_bgpid,
-			    sizeof(peer->remote_bgpid)) == 0) {
-				/* would cause loop don't send */
-				path_put(fasp);
-				return;
-			}
-		}
-
 		/* generate update */
-		p = calloc(1, sizeof(struct update_prefix));
-		if (p == NULL)
-			fatal("up_generate_updates");
-
-		a = calloc(1, sizeof(struct update_attr));
-		if (a == NULL)
-			fatal("up_generate_updates");
-
-		if (up_generate_attr(peer, a, fasp, addr.af) == -1) {
-			log_warnx("generation of bgp path attributes failed");
-			free(a);
-			free(p);
-			return;
-		}
-
-		/*
-		 * use aspath_hash as attr_hash, this may be unoptimal
-		 * but currently I don't care.
-		 */
-		a->attr_hash = aspath_hash(fasp->aspath->data,
-		    fasp->aspath->len);
-		p->prefix = addr;
-		p->prefixlen = new->prefix->prefixlen;
+		up_generate(peer, fasp, &addr, new->prefix->prefixlen);
 
 		/* no longer needed */
 		path_put(fasp);
-
-		if (up_add(peer, p, a) == -1)
-			log_warnx("queuing update failed.");
 	}
 }
 
@@ -527,8 +455,6 @@ up_generate_updates(struct rde_peer *peer,
 void
 up_generate_default(struct rde_peer *peer, sa_family_t af)
 {
-	struct update_attr	*a;
-	struct update_prefix	*p;
 	struct rde_aspath	*asp;
 	struct bgpd_addr	 addr;
 
@@ -550,59 +476,23 @@ up_generate_default(struct rde_peer *peer, sa_family_t af)
 	 * XXX apply default overrides. Not yet possible, mainly a parse.y
 	 * problem.
 	 */
+	/* rde_apply_set(asp, set, af, NULL ???, DIR_IN); */
 
 	/* filter as usual */
 	bzero(&addr, sizeof(addr));
 	addr.af = af;
-	/*
-	 * XXX we should pass peerself here. This will be fixed with the
-	 * apply default overrides.
-	 */
-	if (rde_filter(peer, asp, &addr, 0, NULL, DIR_OUT) == ACTION_DENY) {
+
+	if (rde_filter(peer, asp, &addr, 0, NULL, DIR_OUT) ==
+	    ACTION_DENY) {
 		path_put(asp);
 		return;
 	}
 
 	/* generate update */
-	p = calloc(1, sizeof(struct update_prefix));
-	if (p == NULL)
-		fatal("up_generate_default");
-
-	a = calloc(1, sizeof(struct update_attr));
-	if (a == NULL)
-		fatal("up_generate_default");
-
-	if (up_generate_attr(peer, a, asp, af) == -1) {
-		log_warnx("generation of bgp path attributes failed");
-		free(a);
-		free(p);
-		return;
-	}
-
-	/*
-	 * use aspath_hash as attr_hash, this may be unoptimal
-	 * but currently I don't care.
-	 */
-	a->attr_hash = aspath_hash(asp->aspath->data, asp->aspath->len);
-	p->prefix = addr;
-	p->prefixlen = 0; /* default route */
+	up_generate(peer, asp, &addr, 0);
 
 	/* no longer needed */
 	path_put(asp);
-
-	if (up_add(peer, p, a) == -1)
-		log_warnx("queuing update failed.");
-
-}
-
-void
-up_dump_upcall(struct pt_entry *pt, void *ptr)
-{
-	struct rde_peer	*peer = ptr;
-
-	if (pt->active == NULL)
-		return;
-	up_generate_updates(peer, pt->active, NULL);
 }
 
 u_char	up_attr_buf[4096];