| Message ID | 20180623155930.25983-1-Jason@zx2c4.com |
|---|---|
| State | New |
| Headers | show |
| Series | fib_rules: match rules based on suppress_* properties too | expand |
From: "Jason A. Donenfeld" <Jason@zx2c4.com> Date: Sat, 23 Jun 2018 17:59:30 +0200 > Two rules with different values of suppress_prefix or suppress_ifgroup > are not the same. This fixes an -EEXIST when running: > > $ ip -4 rule add table main suppress_prefixlength 0 > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Fixes: f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs into fib_nl2rule") But the old rule_find() code didn't check this key either, so I can't see how the behavior in this area changed. I think the behavior changed for a different reason. The commit mentioned in your Fixes: tag changed newrule semantics wrt. defaults or "any" values. The original code matched on pure values of the keys, whereas the new code only compares the keys when the new rule is not specifying an "any" value. - if (r->table != rule->table) + if (rule->table && r->table != rule->table) continue; And I think these changes are what makes your test case fail after the commit. Some other key didn't match previous due to the handling of "any" values.
On Sat, Jun 23, 2018 at 8:59 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > Two rules with different values of suppress_prefix or suppress_ifgroup > are not the same. This fixes an -EEXIST when running: > > $ ip -4 rule add table main suppress_prefixlength 0 > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Fixes: f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs into fib_nl2rule") > --- > net/core/fib_rules.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c > index 126ffc5bc630..665799311b98 100644 > --- a/net/core/fib_rules.c > +++ b/net/core/fib_rules.c > @@ -416,6 +416,12 @@ static struct fib_rule *rule_find(struct fib_rules_ops *ops, > if (rule->mark && r->mark != rule->mark) > continue; > > + if (r->suppress_ifgroup != rule->suppress_ifgroup) > + continue; > + > + if (r->suppress_prefixlen != rule->suppress_prefixlen) > + continue; > + > if (rule->mark_mask && r->mark_mask != rule->mark_mask) > continue; > Can you please change the check to compare only if the new rule has the attributes set ? eg: if (rule->suppress_ifgroup != -1 && (r->suppress_ifgroup != rule->suppress_ifgroup)) same thing for suppress_prefixlen
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 126ffc5bc630..665799311b98 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -416,6 +416,12 @@ static struct fib_rule *rule_find(struct fib_rules_ops *ops, if (rule->mark && r->mark != rule->mark) continue; + if (r->suppress_ifgroup != rule->suppress_ifgroup) + continue; + + if (r->suppress_prefixlen != rule->suppress_prefixlen) + continue; + if (rule->mark_mask && r->mark_mask != rule->mark_mask) continue;
Two rules with different values of suppress_prefix or suppress_ifgroup are not the same. This fixes an -EEXIST when running: $ ip -4 rule add table main suppress_prefixlength 0 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Fixes: f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs into fib_nl2rule") --- net/core/fib_rules.c | 6 ++++++ 1 file changed, 6 insertions(+) -- 2.17.1