On Mon, Jun 25, 2018 at 8:23 AM, Roopa Prabhu <ro...@cumulusnetworks.com> wrote:
> On Sat, Jun 23, 2018 at 8:46 AM, Jason A. Donenfeld <ja...@zx2c4.com> wrote:
>> Hey Roopa,
>>
>> On a kernel with a minimal networking config,
>> CONFIG_IP_MULTIPLE_TABLES appears to be broken for certain rules after
>> f9d4b0c1e9695e3de7af3768205bacc27312320c.
>>
>> Try, for example, running:
>>
>> $ ip -4 rule add table main suppress_prefixlength 0
>>
>> It returns with EEXIST.
>>
>> Perhaps the reason is that the new rule_find function does not match
>> on suppress_prefixlength? However, rule_exist from before didn't do
>> that either. I'll keep playing and see if I can track it down myself,
>> but thought I should let you know first.
>
> I am surprised at that also. I cannot find prior rule_exist looking at
> suppress_prefixlength.
> I will dig deeper also today. But your patch LGTM with a small change
> I commented on it.
>
So the previous rule_exists code did not check for attribute matches correctly.
It would ignore a rule at the first non-existent attribute mis-match.
eg in your case, it would
return at pref mismatch.
$ip -4 rule add table main suppress_prefixlength 0
$ip -4 rule add table main suppress_prefixlength 0
$ip -4 rule add table main suppress_prefixlength 0
$ip rule show
0: from all lookup local
32763: from all lookup main suppress_prefixlength 0
32764: from all lookup main suppress_prefixlength 0
32765: from all lookup main suppress_prefixlength 0
32766: from all lookup main
32767: from all lookup default
With your patch (with my proposed fixes), you should get proper EXISTS check
$ git diff HEAD
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 126ffc5..de4c171 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -428,6 +428,14 @@ static struct fib_rule *rule_find(struct
fib_rules_ops *ops,
if (rule->l3mdev && r->l3mdev != rule->l3mdev)
continue;
+ if (rule->suppress_ifgroup != -1 &&
+ (r->suppress_ifgroup != rule->suppress_ifgroup))
+ continue;
+
+ if (rule->suppress_prefixlen != -1 &&
+ (r->suppress_prefixlen != rule->suppress_prefixlen))
+ continue;
+
if (uid_range_set(&rule->uid_range) &&
(!uid_eq(r->uid_range.start, rule->uid_range.start) ||
!uid_eq(r->uid_range.end, rule->uid_range.end)))
$ ip -4 rule add table main suppress_prefixlength 0
$ ip -4 rule add table main suppress_prefixlength 0
RTNETLINK answers: File exists
