Re: [PATCH] fib_rules: match rules based on suppress_* properties too

[Roopa Prabhu]

On Sat, Jun 23, 2018 at 8:59 AM, Jason A. Donenfeld  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 
> 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

Re: [PATCH] fib_rules: match rules based on suppress_* properties too

[David Miller]

From: "Jason A. Donenfeld" 
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 
> 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.

[PATCH] fib_rules: match rules based on suppress_* properties too

[Jason A. Donenfeld]

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 
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;
 
-- 
2.17.1