public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller
Date: Tue,  7 Apr 2026 13:16:22 +1000	[thread overview]
Message-ID: <20260407031630.2457081-11-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20260407031630.2457081-1-david@gibson.dropbear.id.au>

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>
---
 conf.c     |  5 +++++
 fwd.c      | 26 +-------------------------
 fwd_rule.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 fwd_rule.h |  2 ++
 4 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/conf.c b/conf.c
index c9ee8c59..a93837cc 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 c05107d1..c9637525 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 abe9dfbf..4d5048f9 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -87,3 +87,47 @@ 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 with errir if 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 e92efb6d..f852be39 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 */
-- 
2.53.0


  parent reply	other threads:[~2026-04-07  3:16 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07  3:16 [PATCH 00/18] Rework forwarding option parsing David Gibson
2026-04-07  3:16 ` [PATCH 01/18] conf: Split parsing of port specifiers from the rest of -[tuTU] parsing David Gibson
2026-04-07  3:16 ` [PATCH 02/18] conf: Simplify handling of default forwarding mode David Gibson
2026-04-07 23:14   ` Stefano Brivio
2026-04-08  1:10     ` David Gibson
2026-04-07  3:16 ` [PATCH 03/18] conf: Move first pass handling of -[TU] next to handling of -[tu] David Gibson
2026-04-07  3:16 ` [PATCH 04/18] doc: Consolidate -[tu] option descriptions for passt and pasta David Gibson
2026-04-07 23:14   ` Stefano Brivio
2026-04-08  1:23     ` David Gibson
2026-04-07  3:16 ` [PATCH 05/18] conf: Permit -[tTuU] all in pasta mode David Gibson
2026-04-07  3:16 ` [PATCH 06/18] fwd: Better split forwarding rule specification from associated sockets David Gibson
2026-04-07 23:14   ` Stefano Brivio
2026-04-08  1:30     ` David Gibson
2026-04-08 21:39       ` Stefano Brivio
2026-04-09  0:47         ` David Gibson
2026-04-07  3:16 ` [PATCH 07/18] fwd_rule: Move forwarding rule formatting David Gibson
2026-04-07  3:16 ` [PATCH 08/18] conf: Pass protocol explicitly to conf_ports_range_except() David Gibson
2026-04-07  3:16 ` [PATCH 09/18] fwd: Split rule building from rule adding David Gibson
2026-04-07  3:16 ` David Gibson [this message]
2026-04-07 23:14   ` [PATCH 10/18] fwd_rule: Move rule conflict checking from fwd_rule_add() to caller Stefano Brivio
2026-04-08  1:37     ` David Gibson
2026-04-08  4:42       ` David Gibson
2026-04-07  3:16 ` [PATCH 11/18] fwd: Improve error handling in fwd_rule_add() David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-09  0:10     ` David Gibson
2026-04-07  3:16 ` [PATCH 12/18] conf: Don't be strict about exclusivity of forwarding mode David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-09  0:12     ` David Gibson
2026-04-07  3:16 ` [PATCH 13/18] conf: Rework stepping through chunks of port specifiers David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-09  0:13     ` David Gibson
2026-04-07  3:16 ` [PATCH 14/18] conf: Rework checking for garbage after a range David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-09  0:15     ` David Gibson
2026-04-07  3:16 ` [PATCH 15/18] conf: Move "all" handling to port specifier David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-07  3:16 ` [PATCH 16/18] conf: Allow user-specified auto-scanned port forwarding ranges David Gibson
2026-04-08 21:40   ` Stefano Brivio
2026-04-07  3:16 ` [PATCH 17/18] conf: Move SO_BINDTODEVICE workaround to conf_ports() David Gibson
2026-04-07  3:16 ` [PATCH 18/18] conf: Don't pass raw commandline argument to conf_ports_spec() David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260407031630.2457081-11-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).