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=Kpcw4Zin; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 0C3995A065C for ; Fri, 17 Apr 2026 07:05:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1776402332; bh=OCdq5CmMN4zDQTX+T1cPGxvHYlG8GkToRCNJ1yw7EZM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Kpcw4ZinVWXY3BQGkvvuoGI1mcDiu5Mw+x8LxLp0CWfOT6dkFjp0hskeyCv7smQxU 1yaGiogzUZxLCyGb4K68AkIHDa1QJFkhNw/UvYGlvZ+xbRK/X7xhp67bdugpMmp7sd VdkEpOJj64qhm2NGJ4bOBFJefaIL0l18xSP0OeibwoDo2kgn3ZrixH8rFVKmcMWgp/ gNDIyVqF2lqT4p4XeGedPo4oROjD2e4YSjKBQY4LT8RdbeSPEAet7GvRLMLdBs4vSN VRGSvome+lyBcQfdfm6eSBvadkrUfEuo/NM6rPPwxWPGQxYIfUl0ir2vyx8VCDm8Yc rze9e0SoxTAeQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fxjWJ0zmQz4wSy; Fri, 17 Apr 2026 15:05:32 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH v3 10/11] fwd_rule: Move conflict checking back within fwd_rule_add() Date: Fri, 17 Apr 2026 15:05:19 +1000 Message-ID: <20260417050520.102247-11-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260417050520.102247-1-david@gibson.dropbear.id.au> References: <20260417050520.102247-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: LAYZUO4GHAZXEIFLRTKFPNVVLMKX4ULC X-Message-ID-Hash: LAYZUO4GHAZXEIFLRTKFPNVVLMKX4ULC 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 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 --- fwd_rule.c | 38 +++++++++++++------------------------- 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/fwd_rule.c b/fwd_rule.c index 7bba2602..cb462401 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", @@ -292,6 +269,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; @@ -436,7 +425,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