fwd_rule: Move rule conflict checking from fwd_rule_add() to caller

Amongst other checks, fwd_rule_add() checks that the newly added rule
doesn't conflict with any existing rules.  However, unlike the other things
we verify, this isn't really required for safe operation.  Rule conflicts
are a useful thing for the user to know about, but the forwarding logic
is perfectly sound with conflicting rules (the first one will win).

In order to support dynamic rule updates, we want fwd_rule_add() to become
a more low-level function, only checking the things it really needs to.
So, move rule conflict checking to its caller via new helpers in
fwd_rule.c.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

4 files changed, 53 insertions, 25 deletions

diff --git a/conf.c b/conf.c
index 3b5cf61..1411eea 100644
--- a/conf.c
+++ b/conf.c
@@ -205,13 +205,18 @@ static void conf_ports_range_except(const struct ctx *c, char optname,
 
 			if (c->ifi4) {
 				rulev.addr = inany_loopback4;
+				fwd_rule_conflict_check(&rulev,
+							fwd->rules, fwd->count);
 				fwd_rule_add(fwd, &rulev);
 			}
 			if (c->ifi6) {
 				rulev.addr = inany_loopback6;
+				fwd_rule_conflict_check(&rulev,
+							fwd->rules, fwd->count);
 				fwd_rule_add(fwd, &rulev);
 			}
 		} else {
+			fwd_rule_conflict_check(&rule, fwd->rules, fwd->count);
 			fwd_rule_add(fwd, &rule);
 		}
 		base = i - 1;
diff --git a/fwd.c b/fwd.c
index c05107d..c963752 100644
--- a/fwd.c
+++ b/fwd.c
@@ -341,7 +341,7 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 	/* Flags which can be set from the caller */
 	const uint8_t allowed_flags = FWD_WEAK | FWD_SCAN | FWD_DUAL_STACK_ANY;
 	unsigned num = (unsigned)new->last - new->first + 1;
-	unsigned i, port;
+	unsigned port;
 
 	assert(!(new->flags & ~allowed_flags));
 	/* Passing a non-wildcard address with DUAL_STACK_ANY is a bug */
@@ -354,30 +354,6 @@ void fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new)
 	if ((fwd->sock_count + num) > ARRAY_SIZE(fwd->socks))
 		die("Too many listening sockets");
 
-	/* Check for any conflicting entries */
-	for (i = 0; i < fwd->count; i++) {
-		char newstr[INANY_ADDRSTRLEN], rulestr[INANY_ADDRSTRLEN];
-		const struct fwd_rule *rule = &fwd->rules[i];
-
-		if (new->proto != rule->proto)
-			/* Non-conflicting protocols */
-			continue;
-
-		if (!inany_matches(fwd_rule_addr(new), fwd_rule_addr(rule)))
-			/* Non-conflicting addresses */
-			continue;
-
-		if (new->last < rule->first || rule->last < new->first)
-			/* Port ranges don't overlap */
-			continue;
-
-		die("Forwarding configuration conflict: %s/%u-%u versus %s/%u-%u",
-		    inany_ntop(fwd_rule_addr(new), newstr, sizeof(newstr)),
-		    new->first, new->last,
-		    inany_ntop(fwd_rule_addr(rule), rulestr, sizeof(rulestr)),
-		    rule->first, rule->last);
-	}
-
 	fwd->rulesocks[fwd->count] = &fwd->socks[fwd->sock_count];
 	for (port = new->first; port <= new->last; port++)
 		fwd->rulesocks[fwd->count][port - new->first] = -1;
diff --git a/fwd_rule.c b/fwd_rule.c
index a034d5d..5bc94ef 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -93,3 +93,48 @@ void fwd_rules_info(const struct fwd_rule *rules, size_t count)
 		info("    %s", fwd_rule_fmt(&rules[i], buf, sizeof(buf)));
 	}
 }
+
+/**
+ * fwd_rule_conflicts() - Test if two rules conflict with each other
+ * @a, @b:	Rules to test
+ */
+static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule *b)
+{
+	if (a->proto != b->proto)
+		/* Non-conflicting protocols */
+		return false;
+
+	if (!inany_matches(fwd_rule_addr(a), fwd_rule_addr(b)))
+		/* Non-conflicting addresses */
+		return false;
+
+	assert(a->first <= a->last && b->first <= b->last);
+	if (a->last < b->first || b->last < a->first)
+		/* Port ranges don't overlap */
+		return false;
+
+	return true;
+}
+
+/**
+ * fwd_rule_conflict_check() - Die if given rule conflicts with any in list
+ * @new:	New rule
+ * @rules:	Existing rules against which to test
+ * @count:	Number of rules in @rules
+ */
+void fwd_rule_conflict_check(const struct fwd_rule *new,
+			     const struct fwd_rule *rules, size_t count)
+{
+	unsigned i;
+
+	for (i = 0; i < count; i++) {
+		char newstr[FWD_RULE_STRLEN], rulestr[FWD_RULE_STRLEN];
+
+		if (!fwd_rule_conflicts(new, &rules[i]))
+			continue;
+
+		die("Forwarding configuration conflict: %s versus %s",
+		    fwd_rule_fmt(new, newstr, sizeof(newstr)),
+		    fwd_rule_fmt(&rules[i], rulestr, sizeof(rulestr)));
+	}
+}
diff --git a/fwd_rule.h b/fwd_rule.h
index e92efb6..f852be3 100644
--- a/fwd_rule.h
+++ b/fwd_rule.h
@@ -52,5 +52,7 @@ struct fwd_rule {
 
 const union inany_addr *fwd_rule_addr(const struct fwd_rule *rule);
 void fwd_rules_info(const struct fwd_rule *rules, size_t count);
+void fwd_rule_conflict_check(const struct fwd_rule *new,
+			     const struct fwd_rule *rules, size_t count);
 
 #endif /* FWD_RULE_H */