The attributes cache broke the set community filterset because community_set()

modified the attribute data directly and corrupted the cache by doing it.
It is no longer allowed to modify attributes via attr_optget() -> change
attr->data. Instead remove the old attribute from the aspath and then add
a new modifed one again. Included in this change is the removal of a "feature"
that allowed only one community per AS. If you had problems to add multiple
communities via filters then this was the problem. Looks good Henning.

3 files changed, 57 insertions, 29 deletions

diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 8e4a67dfcb8..ae049bdee7a 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.80 2006/01/05 16:00:07 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.81 2006/01/10 16:11:12 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -283,7 +283,7 @@ int		 aspath_compare(struct aspath *, struct aspath *);
 struct aspath	*aspath_prepend(struct aspath *, u_int16_t, int);
 int		 aspath_match(struct aspath *, enum as_spec, u_int16_t);
 int		 community_match(void *, u_int16_t, int, int);
-int		 community_set(struct attr *, int, int);
+int		 community_set(struct rde_aspath *, int, int);
 
 /* rde_rib.c */
 void		 path_init(u_int32_t);
diff --git a/usr.sbin/bgpd/rde_attr.c b/usr.sbin/bgpd/rde_attr.c
index 59c0d57661c..1139ee49208 100644
--- a/usr.sbin/bgpd/rde_attr.c
+++ b/usr.sbin/bgpd/rde_attr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_attr.c,v 1.57 2006/01/10 16:03:11 claudio Exp $ */
+/*	$OpenBSD: rde_attr.c,v 1.58 2006/01/10 16:11:12 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -29,6 +29,8 @@
 #include "bgpd.h"
 #include "rde.h"
 
+void	attr_free(struct rde_aspath *, struct attr *);
+
 int
 attr_write(void *p, u_int16_t p_len, u_int8_t flags, u_int8_t type,
     void *data, u_int16_t data_len)
@@ -240,6 +242,24 @@ attr_compare(struct rde_aspath *a, struct rde_aspath *b)
 }
 
 void
+attr_free(struct rde_aspath *asp, struct attr *attr)
+{
+	u_int8_t	l;
+
+	for (l = 0; l < asp->others_len; l++)
+		if (asp->others[l] == attr) {
+			attr_put(asp->others[l]);
+			asp->others[l] = NULL;
+			--asp->others_len;
+			for (; l < asp->others_len; l++)
+				asp->others[l] = asp->others[l + 1];
+			return;
+		}
+
+	/* no realloc() others because the slot will be reused soon */
+}
+
+void
 attr_freeall(struct rde_aspath *asp)
 {
 	u_int8_t	l;
@@ -702,35 +722,51 @@ community_match(void *data, u_int16_t len, int as, int type)
 }
 
 int
-community_set(struct attr *attr, int as, int type)
+community_set(struct rde_aspath *asp, int as, int type)
 {
-	u_int8_t *p = attr->data;
-	unsigned int i, ncommunities = attr->len;
+	struct attr	*attr;
+	u_int8_t	*p = NULL;
+	unsigned int	 i, ncommunities = 0;
+	u_int8_t	 f = ATTR_OPTIONAL|ATTR_TRANSITIVE;
+	u_int8_t	 t = ATTR_COMMUNITIES;
+
+	attr = attr_optget(asp, ATTR_COMMUNITIES);
 
-	ncommunities >>= 2; /* divide by four */
+	if (attr != NULL) {
+		p = attr->data;
+		ncommunities = attr->len >> 2; /* divide by four */
+	}
 
+	/* first check if the community is not already set */
 	for (i = 0; i < ncommunities; i++) {
-		if (as >> 8 == p[0] && (as & 0xff) == p[1])
-			break;
+		if (as >> 8 == p[0] && (as & 0xff) == p[1] &&
+		    type >> 8 == p[2] && (type & 0xff) == p[3])
+			/* already present, nothing todo */
+			return (1);
 		p += 4;
 	}
 
-	if (i >= ncommunities) {
-		if (attr->len > 0xffff - 4) /* overflow */
-			return (0);
-		i = attr->len + 4;
-		if ((p = realloc(attr->data, i)) == NULL)
-			return (0);
+	if (ncommunities++ >= 0x3fff)
+		/* overflow */
+		return (0);
+
+	if ((p = malloc(ncommunities << 2)) == NULL)
+		fatal("community_set");
 
-		attr->data = p;
-		attr->len = i;
-		p = attr->data + attr->len - 4;
-	}
 	p[0] = as >> 8;
 	p[1] = as & 0xff;
 	p[2] = type >> 8;
 	p[3] = type & 0xff;
 
+	if (attr != NULL) {
+		memcpy(p + 4, attr->data, attr->len);
+		f = attr->flags;
+		t = attr->type;
+		attr_free(asp, attr);
+	}
+
+	attr_optadd(asp, f, t, p, ncommunities << 2);
+
 	return (1);
 }
 
diff --git a/usr.sbin/bgpd/rde_filter.c b/usr.sbin/bgpd/rde_filter.c
index 1b2cde3dbc3..1225307b196 100644
--- a/usr.sbin/bgpd/rde_filter.c
+++ b/usr.sbin/bgpd/rde_filter.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_filter.c,v 1.39 2005/12/08 17:17:36 claudio Exp $ */
+/*	$OpenBSD: rde_filter.c,v 1.40 2006/01/10 16:11:12 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -65,7 +65,6 @@ rde_apply_set(struct rde_aspath *asp, struct filter_set_head *sh,
 {
 	struct filter_set	*set;
 	struct aspath		*new;
-	struct attr		*a;
 	u_int16_t		 as;
 	u_int8_t		 prepend;
 
@@ -154,14 +153,7 @@ rde_apply_set(struct rde_aspath *asp, struct filter_set_head *sh,
 			    af);
 			break;
 		case ACTION_SET_COMMUNITY:
-			if ((a = attr_optget(asp, ATTR_COMMUNITIES)) == NULL) {
-				attr_optadd(asp, ATTR_OPTIONAL|ATTR_TRANSITIVE,
-				    ATTR_COMMUNITIES, NULL, 0);
-				if ((a = attr_optget(asp,
-				    ATTR_COMMUNITIES)) == NULL)
-					fatalx("internal community bug");
-			}
-			community_set(a, set->action.community.as,
+			community_set(asp, set->action.community.as,
 			    set->action.community.type);
 			break;
 		case ACTION_PFTABLE: