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