From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=Rg/3J7Os; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 004125A0272 for ; Tue, 21 Apr 2026 06:42:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1776746539; bh=xcldcFw2dmpWcuBvB0na/aC4xAfuCz0eMJprsYVcLhY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Rg/3J7OsRq/Pb9HmZ2TmvL1zdz29FlPNTpNxejHWFNOESaSaJTkUe4VgLjK1/nCMl HpcmMlDVyYl2njiaTfzANFOqSdw4Rnwa524hwo3HqtTSrYUZIAYMLBqt22etBi2i61 YCeE4uDOwg6ABLOgsQtCvbV8Hn/zgRR/r14df7hFBO+5UpfCHsgUOX3RhBTZIN2m79 0qoDKw6UKuk9mVrPhnMdP5kzVacR/pMy2EBLjX2j3Pqo7BTL7BVG3Pc++1ZuV10LOa lV3/aoDTMCf67eA1LAQB+4GYo1GBxllrH/JmY0m2mPrGf8AV5SqJwBQ72bcm/pKoIu /xxYgoQwS6+GQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4g08pg3cd0z4wJ2; Tue, 21 Apr 2026 14:42:19 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v4 04/17] fwd_rule: Move conflict checking back within fwd_rule_add() Date: Tue, 21 Apr 2026 14:42:04 +1000 Message-ID: <20260421044217.2500314-5-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260421044217.2500314-1-david@gibson.dropbear.id.au> References: <20260421044217.2500314-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: DH3CYZYFYPVNV5YJP22FUWYLSAWL2FLD X-Message-ID-Hash: DH3CYZYFYPVNV5YJP22FUWYLSAWL2FLD X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson , Laurent Vivier X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 2bffb631d31e ("fwd_rule: Move rule conflict checking from fwd_rule_add() to caller") moved rule conflict checking out of fwd_rule_add(). This seemed like a good idea at the time, but turns out to be kind of awkward: it means we're now checking for conflicts *before* we've checked the rule for internal consistency (including first <= last), which leaves an awkward assert() which might fire in unexpected places. While it's true that it's not really necessary to include this in order to safely add a rule, the benefits from skipping it are pretty marginal. So, for simplicity, fold this check back into fwd_rule_add(), making it non-fatal. If we ever have cases with enough rules that the O(n^2) nature of the check matters, we might need to revisit. Signed-off-by: David Gibson Reviewed-by: Laurent Vivier --- fwd_rule.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/fwd_rule.c b/fwd_rule.c index cd3dec04..1413584f 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -195,29 +195,6 @@ static bool fwd_rule_conflicts(const struct fwd_rule *a, const struct fwd_rule * 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 - */ -static 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))); - } -} - /** * fwd_rule_add() - Validate and add a rule to a forwarding table * @fwd: Table to add to @@ -230,7 +207,7 @@ static int 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 port; + unsigned port, i; if (new->first > new->last) { warn("Rule has invalid port range %u-%u", @@ -293,6 +270,18 @@ static int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) return -EINVAL; } + for (i = 0; i < fwd->count; i++) { + char newstr[FWD_RULE_STRLEN], rulestr[FWD_RULE_STRLEN]; + + if (!fwd_rule_conflicts(new, &fwd->rules[i])) + continue; + + warn("Forwarding configuration conflict: %s versus %s", + fwd_rule_fmt(new, newstr, sizeof(newstr)), + fwd_rule_fmt(&fwd->rules[i], rulestr, sizeof(rulestr))); + return -EEXIST; + } + if (fwd->count >= ARRAY_SIZE(fwd->rules)) { warn("Too many rules (maximum %u)", ARRAY_SIZE(fwd->rules)); return -ENOSPC; @@ -435,7 +424,6 @@ static void fwd_rule_range_except(struct fwd_table *fwd, uint8_t proto, rule.last = i - 1; rule.to = base + delta; - fwd_rule_conflict_check(&rule, fwd->rules, fwd->count); if (fwd_rule_add(fwd, &rule) < 0) goto fail; -- 2.53.0