New function filterset_cmp() used two compare two struct filter_set for

equality. This function is a bit more complicated than a memcmp() because there
are types that need to be considered equal e.g. ACTION_SET_MED and
ACTION_SET_RELATIVE_MED. Also ACTION_SET_COMMUNITY and ACTION_SET_NEXTHOP
need some special care.  OK henning@

3 files changed, 66 insertions, 23 deletions

diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h
index 14666a29e19..5bea6ba98ed 100644
--- a/usr.sbin/bgpd/bgpd.h
+++ b/usr.sbin/bgpd/bgpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpd.h,v 1.174 2005/07/01 13:38:14 claudio Exp $ */
+/*	$OpenBSD: bgpd.h,v 1.175 2005/07/04 09:37:24 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -577,13 +577,32 @@ enum action_types {
 	ACTION_SET_NEXTHOP_BLACKHOLE,
 	ACTION_SET_NEXTHOP_NOMODIFY,
 	ACTION_SET_COMMUNITY,
-/*	ACTION_SCRUB_COMMUNITY, */
 	ACTION_PFTABLE,
 	ACTION_PFTABLE_ID,
 	ACTION_RTLABEL,
 	ACTION_RTLABEL_ID
 };
 
+static const char * const filterset_names[] = {
+	"localpref",
+	"localpref",
+	"metric",
+	"metric",
+	"weight",
+	"weight",
+	"prepend-self",
+	"prepend-peer",
+	"nexthop",
+	"nexthop",
+	"nexthop",
+	"nexthop",
+	"community",
+	"pftable",
+	"pftable",
+	"rtlabel",
+	"rtlabel"
+};
+
 struct filter_set {
 	TAILQ_ENTRY(filter_set)		entry;
 	union {
@@ -715,7 +734,8 @@ void		 pftable_ref(u_int16_t);
 
 
 /* rde_filter.c */
-void			 filterset_free(struct filter_set_head *);
+void		 filterset_free(struct filter_set_head *);
+int		 filterset_cmp(struct filter_set *, struct filter_set *);
 
 
 #endif /* __BGPD_H__ */
diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y
index 1aebab4c54b..222ab74af37 100644
--- a/usr.sbin/bgpd/parse.y
+++ b/usr.sbin/bgpd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.167 2005/07/04 09:31:35 claudio Exp $ */
+/*	$OpenBSD: parse.y,v 1.168 2005/07/04 09:37:24 claudio Exp $ */
 
 /*
  * Copyright (c) 2002, 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -2247,24 +2247,12 @@ merge_filterset(struct filter_set_head *sh, struct filter_set *s)
 	struct filter_set	*t;
 
 	TAILQ_FOREACH(t, sh, entry) {
-		if (s->type != t->type)
-			continue;
-
-		switch (s->type) {
-		case ACTION_SET_COMMUNITY:
-			if (s->action.community.as == t->action.community.as &&
-			    s->action.community.type ==
-			    t->action.community.type) {
+		if (filterset_cmp(s, t) == 0) {
+			if (s->type == ACTION_SET_COMMUNITY)
 				yyerror("community is already set");
-				return (-1);
-			}
-			break;
-		case ACTION_SET_NEXTHOP:
-			if (s->action.nexthop.af != t->action.nexthop.af)
-				break;
-			/* FALLTHROUGH */
-		default:
-			yyerror("redefining set parameters is not fluffy");
+			else
+				yyerror("redefining set parameter %s",
+				    filterset_names[s->type]);
 			return (-1);
 		}
 	}
@@ -2272,7 +2260,6 @@ merge_filterset(struct filter_set_head *sh, struct filter_set *s)
 	return (0);
 }
 
-
 void
 copy_filterset(struct filter_set_head *source, struct filter_set_head *dest)
 {
diff --git a/usr.sbin/bgpd/rde_filter.c b/usr.sbin/bgpd/rde_filter.c
index b73b8edb6fd..c22226fc23d 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.32 2005/07/01 22:04:37 claudio Exp $ */
+/*	$OpenBSD: rde_filter.c,v 1.33 2005/07/04 09:37:24 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -295,6 +295,7 @@ rde_filter_community(struct rde_aspath *asp, int as, int type)
 	return (community_match(a->data, a->len, as, type));
 }
 
+/* free a filterset and take care of possible name2id references */
 void
 filterset_free(struct filter_set_head *sh)
 {
@@ -310,3 +311,38 @@ filterset_free(struct filter_set_head *sh)
 	}
 }
 
+/*
+ * this function is a bit more complicated than a memcmp() because there are
+ * types that need to be considered equal e.g. ACTION_SET_MED and
+ * ACTION_SET_RELATIVE_MED. Also ACTION_SET_COMMUNITY and ACTION_SET_NEXTHOP
+ * need some special care.
+ */
+int
+filterset_cmp(struct filter_set *a, struct filter_set *b)
+{
+	if (strcmp(filterset_names[a->type], filterset_names[b->type]))
+		return (a->type - b->type);
+
+	if (a->type == ACTION_SET_COMMUNITY) {	/* a->type == b->type */
+		/* compare community */
+		if (a->action.community.as - b->action.community.as != 0)
+			return (a->action.community.as -
+			    b->action.community.as);
+		return (a->action.community.type - b->action.community.type);
+	}
+
+	if (a->type == ACTION_SET_NEXTHOP && b->type == ACTION_SET_NEXTHOP) {
+		/*
+		 * This is the only intresting case, all others are considered
+		 * equal. It does not make sense to e.g. set a nexthop and
+		 * reject it at the same time. Allow one IPv4 and one IPv6
+		 * per filter set or only one of the other nexthop modifiers.
+		 */
+		return (a->action.nexthop.af - b->action.nexthop.af);
+	}
+
+	/* equal */
+	return (0);
+}
+
+