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