From 595dd55b18676ded0e1285a129a745fae9b2bef6 Mon Sep 17 00:00:00 2001
From: claudio <claudio@openbsd.org>
Date: Thu, 2 Aug 2018 14:41:42 +0000
Subject: [PATCH] Split out the rule skipping logic into own function and by
 doing so ensure that both filter lists are treated the same way. This fixes
 an inconsistency with ibgp and ebgp filters as used in the example config. OK
 benno@ sthen@

---
 usr.sbin/bgpd/rde_filter.c | 68 +++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/usr.sbin/bgpd/rde_filter.c b/usr.sbin/bgpd/rde_filter.c
index 4ba3129d5d7..c67a19d476c 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.97 2018/07/22 16:59:08 claudio Exp $ */
+/*	$OpenBSD: rde_filter.c,v 1.98 2018/08/02 14:41:42 claudio Exp $ */
 
 /*
  * Copyright (c) 2004 Claudio Jeker <claudio@openbsd.org>
@@ -549,6 +549,37 @@ rde_prefix_match(struct filter_prefix *fp, struct prefix *p)
 	return (0); /* should not be reached */
 }
 
+/* return true when the rule f can never match for this peer */
+static int
+rde_filter_skip_rule(struct rde_peer *peer, struct filter_rule *f)
+{
+	/* if any of the two is unset then rule can't be skipped */
+	if (peer == NULL || f == NULL)
+		return (0);
+
+	if (f->peer.groupid != 0 &&
+	    f->peer.groupid != peer->conf.groupid)
+		return (1);
+
+	if (f->peer.peerid != 0 &&
+	    f->peer.peerid != peer->conf.id)
+		return (1);
+
+	if (f->peer.remote_as != 0 &&
+	    f->peer.remote_as != peer->conf.remote_as)
+		return (1);
+
+	if (f->peer.ebgp != 0 &&
+	    f->peer.ebgp != peer->conf.ebgp)
+		return (1);
+
+	if (f->peer.ibgp != 0 &&
+	    f->peer.ibgp != !peer->conf.ebgp)
+		return (1);
+
+	return (0);
+}
+
 int
 rde_filter_equal(struct filter_head *a, struct filter_head *b,
     struct rde_peer *peer, struct prefixset_head *psh)
@@ -561,46 +592,15 @@ rde_filter_equal(struct filter_head *a, struct filter_head *b,
 
 	while (fa != NULL || fb != NULL) {
 		/* skip all rules with wrong peer */
-		if (peer != NULL && fa != NULL && fa->peer.groupid != 0 &&
-		    fa->peer.groupid != peer->conf.groupid) {
+		if (rde_filter_skip_rule(peer, fa)) {
 			fa = TAILQ_NEXT(fa, entry);
 			continue;
 		}
-		if (peer != NULL && fa != NULL && fa->peer.peerid != 0 &&
-		    fa->peer.peerid != peer->conf.id) {
-			fa = TAILQ_NEXT(fa, entry);
-			continue;
-		}
-
-		if (peer != NULL && fb != NULL && fb->peer.groupid != 0 &&
-		    fb->peer.groupid != peer->conf.groupid) {
-			fb = TAILQ_NEXT(fb, entry);
-			continue;
-		}
-		if (peer != NULL && fb != NULL && fb->peer.peerid != 0 &&
-		    fb->peer.peerid != peer->conf.id) {
+		if (rde_filter_skip_rule(peer, fb)) {
 			fb = TAILQ_NEXT(fb, entry);
 			continue;
 		}
 
-		if (peer != NULL && fa != NULL && fa->peer.remote_as != 0 &&
-		    fa->peer.remote_as != peer->conf.remote_as) {
-			fa = TAILQ_NEXT(fa, entry);
-			continue;
-		}
-
-		if (peer != NULL && fa != NULL && fa->peer.ebgp != 0 &&
-		    fa->peer.ebgp != peer->conf.ebgp) {
-			fa = TAILQ_NEXT(fa, entry);
-			continue;
-		}
-
-		if (peer != NULL && fa != NULL && fa->peer.ibgp != 0 &&
-		    fa->peer.ibgp != !peer->conf.ebgp) {
-			fa = TAILQ_NEXT(fa, entry);
-			continue;
-		}
-
 		/* compare the two rules */
 		if ((fa == NULL && fb != NULL) || (fa != NULL && fb == NULL))
 			/* new rule added or removed */
-- 
2.20.1